summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDan Williams <dcbw@redhat.com>2014-10-15 21:17:45 -0500
committerDan Williams <dcbw@redhat.com>2015-06-09 11:27:34 -0500
commita897dfd5bbda6fb09f7225fa933536f3dac54647 (patch)
tree18e1112b3e2c557000a01f84c2dde514d8ac788a
parent0990f6654e8ff8bd62c196d819d2a58581714838 (diff)
downloadNetworkManager-dcbw/devices-for-all-2.tar.gz
core: allow multiple devices with the same interface namedcbw/devices-for-all-2
But, of course, only one realized device can have the same interface name at a time. This commit effectively reverts most of: 1b37cd03409fa3c0fb26fb6e8093b54b2b7c5d8d core: allow ActiveConnections to be created without a device But it's not easy to do a separate revert of that code due to interdependencies with nm-manager.c. Creating devices when they are defined by a connection also makes makes it possible to require the NMDevice to be present when activating it, which means we can remove a bunch of code from NMManager that had to handle software devices not existing yet at the time of the activation request. But it also means we must be more careful when finding master interfaces during slave activation, since we cannot simply match by interface name alone. Instead we must find the master which matches both the interface name and can control slaves of the type which is being activated.
-rw-r--r--src/devices/nm-device.c40
-rw-r--r--src/devices/nm-device.h2
-rw-r--r--src/nm-activation-request.c6
-rw-r--r--src/nm-active-connection.c15
-rw-r--r--src/nm-active-connection.h2
-rw-r--r--src/nm-manager.c260
6 files changed, 192 insertions, 133 deletions
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 39590893eb..ed9d45456f 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -2133,6 +2133,18 @@ nm_device_master_release_slaves (NMDevice *self)
}
/**
+ * nm_device_is_master:
+ * @self: the device
+ *
+ * Returns: %TRUE is the device can have slaves
+ */
+gboolean
+nm_device_is_master (NMDevice *self)
+{
+ return NM_DEVICE_GET_PRIVATE (self)->is_master;
+}
+
+/**
* nm_device_get_master:
* @self: the device
*
@@ -2713,6 +2725,34 @@ nm_device_check_connection_compatible (NMDevice *self, NMConnection *connection)
return NM_DEVICE_GET_CLASS (self)->check_connection_compatible (self, connection);
}
+gboolean
+nm_device_check_slave_connection_compatible (NMDevice *self, NMConnection *slave)
+{
+ NMDevicePrivate *priv;
+ NMSettingConnection *s_con;
+ const char *connection_type, *slave_type;
+
+ g_return_val_if_fail (NM_IS_DEVICE (self), FALSE);
+ g_return_val_if_fail (NM_IS_CONNECTION (slave), FALSE);
+
+ priv = NM_DEVICE_GET_PRIVATE (self);
+
+ if (!priv->is_master)
+ return FALSE;
+
+ /* All masters should have connection type set */
+ connection_type = NM_DEVICE_GET_CLASS (self)->connection_type;
+ g_return_val_if_fail (connection_type, FALSE);
+
+ s_con = nm_connection_get_setting_connection (slave);
+ g_assert (s_con);
+ slave_type = nm_setting_connection_get_slave_type (s_con);
+ if (!slave_type)
+ return FALSE;
+
+ return strcmp (connection_type, slave_type) == 0;
+}
+
/**
* nm_device_can_assume_connections:
* @self: #NMDevice instance
diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
index 53e97bf33c..6f273854c6 100644
--- a/src/devices/nm-device.h
+++ b/src/devices/nm-device.h
@@ -385,6 +385,7 @@ void nm_device_capture_initial_config (NMDevice *dev);
/* Master */
GSList * nm_device_master_get_slaves (NMDevice *dev);
+gboolean nm_device_is_master (NMDevice *dev);
/* Slave */
NMDevice * nm_device_get_master (NMDevice *dev);
@@ -415,6 +416,7 @@ gboolean nm_device_complete_connection (NMDevice *device,
GError **error);
gboolean nm_device_check_connection_compatible (NMDevice *device, NMConnection *connection);
+gboolean nm_device_check_slave_connection_compatible (NMDevice *device, NMConnection *connection);
gboolean nm_device_uses_assumed_connection (NMDevice *device);
diff --git a/src/nm-activation-request.c b/src/nm-activation-request.c
index 2c084b874e..6e531b6305 100644
--- a/src/nm-activation-request.c
+++ b/src/nm-activation-request.c
@@ -396,9 +396,7 @@ master_failed (NMActiveConnection *self)
* @specific_object: the object path of the specific object (ie, WiFi access point,
* etc) that will be used to activate @connection and @device
* @subject: the #NMAuthSubject representing the requestor of the activation
- * @device: the device/interface to configure according to @connection; or %NULL
- * if the connection describes a software device which will be created during
- * connection activation
+ * @device: the device/interface to configure according to @connection
*
* Creates a new device-based activation request.
*
@@ -411,7 +409,7 @@ nm_act_request_new (NMConnection *connection,
NMDevice *device)
{
g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL);
- g_return_val_if_fail (!device || NM_IS_DEVICE (device), NULL);
+ g_return_val_if_fail (NM_IS_DEVICE (device), NULL);
g_return_val_if_fail (NM_IS_AUTH_SUBJECT (subject), NULL);
return (NMActRequest *) g_object_new (NM_TYPE_ACT_REQUEST,
diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c
index 2a8f53ace1..7a3fe185d4 100644
--- a/src/nm-active-connection.c
+++ b/src/nm-active-connection.c
@@ -432,8 +432,13 @@ nm_active_connection_set_device (NMActiveConnection *self, NMDevice *device)
priv->pending_activation_id = g_strdup_printf ("activation::%p", (void *)self);
nm_device_add_pending_action (device, priv->pending_activation_id, TRUE);
}
- } else
+ } else {
+ /* The ActiveConnection's device can only be cleared after the
+ * connection is activated.
+ */
+ g_warn_if_fail (priv->state > NM_ACTIVE_CONNECTION_STATE_UNKNOWN);
priv->device = NULL;
+ }
g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_INT_DEVICE);
g_signal_emit (self, signals[DEVICE_CHANGED], 0, priv->device, old_device);
@@ -553,10 +558,7 @@ nm_active_connection_set_master (NMActiveConnection *self, NMActiveConnection *m
/* Master is write-once, and must be set before exporting the object */
g_return_if_fail (priv->master == NULL);
g_return_if_fail (priv->path == NULL);
- if (priv->device) {
- /* Note, the master ActiveConnection may not yet have a device */
- g_return_if_fail (priv->device != nm_active_connection_get_device (master));
- }
+ g_return_if_fail (priv->device != nm_active_connection_get_device (master));
nm_log_dbg (LOGD_DEVICE, "(%p): master ActiveConnection is [%p] %s",
self, master, nm_active_connection_get_id (master));
@@ -720,6 +722,7 @@ set_property (GObject *object, guint prop_id,
priv->connection = g_value_dup_object (value);
break;
case PROP_INT_DEVICE:
+ g_return_if_fail (priv->device == NULL);
nm_active_connection_set_device (NM_ACTIVE_CONNECTION (object), g_value_get_object (value));
break;
case PROP_INT_SUBJECT:
@@ -1012,7 +1015,7 @@ nm_active_connection_class_init (NMActiveConnectionClass *ac_class)
(object_class, PROP_INT_DEVICE,
g_param_spec_object (NM_ACTIVE_CONNECTION_INT_DEVICE, "", "",
NM_TYPE_DEVICE,
- G_PARAM_READWRITE |
+ G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY |
G_PARAM_STATIC_STRINGS));
g_object_class_install_property
diff --git a/src/nm-active-connection.h b/src/nm-active-connection.h
index 710cfeed9f..5adc664778 100644
--- a/src/nm-active-connection.h
+++ b/src/nm-active-connection.h
@@ -49,7 +49,7 @@
#define NM_ACTIVE_CONNECTION_VPN "vpn"
#define NM_ACTIVE_CONNECTION_MASTER "master"
-/* Internal non-exported properties */
+/* Internal non-exported construct-time properties */
#define NM_ACTIVE_CONNECTION_INT_CONNECTION "int-connection"
#define NM_ACTIVE_CONNECTION_INT_DEVICE "int-device"
#define NM_ACTIVE_CONNECTION_INT_SUBJECT "int-subject"
diff --git a/src/nm-manager.c b/src/nm-manager.c
index b354608c1f..5097e4d5d6 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -535,16 +535,53 @@ find_device_by_ip_iface (NMManager *self, const gchar *iface)
return NULL;
}
+/**
+ * find_device_by_iface:
+ * @self: the #NMManager
+ * @iface: the device interface to find
+ * @connection: a connection to ensure the returned device is compatible with
+ * @slave: a slave connection to ensure a master is compatible with
+ *
+ * Finds a device by interface name, preferring realized devices. If @slave
+ * is given, this function will only return master devices and will ensure
+ * @slave, when activated, can be a slave of the returned master device. If
+ * @connection is given, this function will only consider devices that are
+ * compatible with @connection.
+ *
+ * Returns: the matching #NMDevice
+ */
static NMDevice *
-find_device_by_iface (NMManager *self, const gchar *iface)
+find_device_by_iface (NMManager *self,
+ const char *iface,
+ NMConnection *connection,
+ NMConnection *slave)
{
+ NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
+ NMDevice *fallback = NULL;
GSList *iter;
- for (iter = NM_MANAGER_GET_PRIVATE (self)->devices; iter; iter = g_slist_next (iter)) {
- if (g_strcmp0 (nm_device_get_iface (NM_DEVICE (iter->data)), iface) == 0)
- return NM_DEVICE (iter->data);
+ g_return_val_if_fail (iface != NULL, NULL);
+
+ for (iter = priv->devices; iter; iter = iter->next) {
+ NMDevice *candidate = iter->data;
+
+ if (strcmp (nm_device_get_iface (candidate), iface))
+ continue;
+ if (connection && !nm_device_check_connection_compatible (candidate, connection))
+ continue;
+ if (slave) {
+ if (!nm_device_is_master (candidate))
+ continue;
+ if (!nm_device_check_slave_connection_compatible (candidate, slave))
+ continue;
+ }
+
+ if (nm_device_is_real (candidate))
+ return candidate;
+ else if (!fallback)
+ fallback = candidate;
}
- return NULL;
+ return fallback;
}
static gboolean
@@ -860,8 +897,8 @@ find_parent_device_for_connection (NMManager *self, NMConnection *connection)
if (!parent_name)
return NULL;
- /* Try as an interface name */
- parent = find_device_by_iface (self, parent_name);
+ /* Try as an interface name of a parent device */
+ parent = find_device_by_iface (self, parent_name, NULL, NULL);
if (parent)
return parent;
@@ -988,18 +1025,16 @@ system_create_virtual_device (NMManager *self, NMConnection *connection, GError
if (!iface)
return NULL;
- /* Make sure we didn't create a device for this connection already */
+ /* If some other device is already compatible with this connection,
+ * don't create a new device for it.
+ */
for (iter = priv->devices; iter; iter = g_slist_next (iter)) {
NMDevice *candidate = iter->data;
if ( g_strcmp0 (nm_device_get_iface (candidate), iface) == 0
- || nm_device_check_connection_compatible (candidate, connection)) {
+ && nm_device_check_connection_compatible (candidate, connection)) {
nm_log_dbg (LOGD_DEVICE, "(%s) already created virtual interface name %s",
nm_connection_get_id (connection), iface);
- g_set_error (error,
- NM_MANAGER_ERROR,
- NM_MANAGER_ERROR_FAILED,
- "interface name '%s' already created", iface);
return NULL;
}
}
@@ -1660,7 +1695,8 @@ device_ip_iface_changed (NMDevice *device,
NMDevice *candidate = NM_DEVICE (iter->data);
if ( candidate != device
- && g_strcmp0 (nm_device_get_iface (candidate), ip_iface) == 0) {
+ && g_strcmp0 (nm_device_get_iface (candidate), ip_iface) == 0
+ && nm_device_is_real (candidate)) {
remove_device (self, candidate, FALSE, FALSE);
break;
}
@@ -1728,14 +1764,6 @@ add_device (NMManager *self, NMDevice *device)
const GSList *unmanaged_specs;
RfKillType rtype;
GSList *iter, *remove = NULL;
- int ifindex;
-
- /* No duplicates */
- ifindex = nm_device_get_ifindex (device);
- if (ifindex > 0 && nm_manager_get_device_by_ifindex (self, ifindex))
- return;
- if (find_device_by_iface (self, nm_device_get_iface (device)))
- return;
/* Remove existing devices owned by the new device; eg remove ethernet
* ports that are owned by a WWAN modem, since udev may announce them
@@ -1745,9 +1773,11 @@ add_device (NMManager *self, NMDevice *device)
* the child NMDevice entirely
*/
for (iter = priv->devices; iter; iter = iter->next) {
- iface = nm_device_get_ip_iface (iter->data);
- if (nm_device_owns_iface (device, iface))
- remove = g_slist_prepend (remove, iter->data);
+ NMDevice *candidate = iter->data;
+
+ iface = nm_device_get_ip_iface (candidate);
+ if (nm_device_is_real (candidate) && nm_device_owns_iface (device, iface))
+ remove = g_slist_prepend (remove, candidate);
}
for (iter = remove; iter; iter = iter->next)
remove_device (self, NM_DEVICE (iter->data), FALSE, FALSE);
@@ -1868,37 +1898,37 @@ platform_link_added (NMManager *self,
NMDevice *device = NULL;
GError *error = NULL;
gboolean nm_plugin_missing = FALSE;
+ GSList *iter;
g_return_if_fail (ifindex > 0);
if (nm_manager_get_device_by_ifindex (self, ifindex))
return;
- device = find_device_by_iface (self, plink->name);
- if (device) {
- gboolean compatible = FALSE;
+ /* Let unrealized devices try to realize themselves with the link */
+ for (iter = NM_MANAGER_GET_PRIVATE (self)->devices; iter; iter = iter->next) {
+ NMDevice *candidate = iter->data;
+ gboolean compatible = TRUE;
- if (nm_device_is_real (device))
- return;
+ if (strcmp (nm_device_get_iface (candidate), plink->name))
+ continue;
- if (nm_device_realize (device, plink, &compatible, &error)) {
+ if (nm_device_is_real (candidate)) {
+ /* Ignore the link added event since there's already a realized
+ * device with the link's name.
+ */
+ return;
+ } else if (nm_device_realize (candidate, plink, &compatible, &error)) {
/* Success */
- nm_device_setup_finish (device, plink);
+ nm_device_setup_finish (candidate, plink);
return;
}
- nm_log_warn (LOGD_DEVICE, "(%s): %s", plink->name, error->message);
- remove_device (self, device, FALSE, FALSE);
+ nm_log_dbg (LOGD_DEVICE, "(%s): failed to realize from plink: '%s'",
+ plink->name, error->message);
g_clear_error (&error);
- if (compatible) {
- /* Device compatible with platform link, but some other fatal error
- * happened during realization.
- */
- return;
- }
-
- /* Fall through and create new compatible device for the link */
+ /* Try next unrealized device */
}
/* Try registered device factories */
@@ -2191,7 +2221,7 @@ find_master (NMManager *self,
return TRUE; /* success, but no master */
/* Try as an interface name first */
- master_device = find_device_by_iface (self, master);
+ master_device = find_device_by_iface (self, master, NULL, connection);
if (master_device) {
if (master_device == device) {
g_set_error_literal (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_DEPENDENCY_FAILED,
@@ -2443,8 +2473,10 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError *
NMDevice *device, *existing, *master_device = NULL;
NMConnection *connection;
NMConnection *master_connection = NULL;
+ NMConnection *existing_connection = NULL;
NMActiveConnection *master_ac = NULL;
- GError *local_err = NULL;
+ NMAuthSubject *subject;
+ char *error_desc = NULL;
g_return_val_if_fail (NM_IS_MANAGER (self), FALSE);
g_return_val_if_fail (NM_IS_ACTIVE_CONNECTION (active), FALSE);
@@ -2456,72 +2488,26 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError *
g_assert (connection);
device = nm_active_connection_get_device (active);
- if (!device) {
- if (!nm_connection_is_virtual (connection)) {
- NMSettingConnection *s_con = nm_connection_get_setting_connection (connection);
+ g_return_val_if_fail (device != NULL, FALSE);
- g_assert (s_con);
- g_set_error (error,
- NM_MANAGER_ERROR,
- NM_MANAGER_ERROR_UNKNOWN_DEVICE,
- "Unsupported virtual interface type '%s'",
- nm_setting_connection_get_connection_type (s_con));
- return FALSE;
- }
-
- device = system_create_virtual_device (self, connection, &local_err);
- if (!device) {
- g_set_error (error,
- NM_MANAGER_ERROR,
- NM_MANAGER_ERROR_UNKNOWN_DEVICE,
- "Failed to create virtual interface: %s",
- local_err ? local_err->message : "(unknown)");
- g_clear_error (&local_err);
- return FALSE;
- }
-
- if (!nm_active_connection_set_device (active, device)) {
- g_set_error_literal (error,
- NM_MANAGER_ERROR,
- NM_MANAGER_ERROR_UNKNOWN_DEVICE,
- "The device could not be activated with this connection");
- return FALSE;
- }
-
- /* A newly created device, if allowed to be managed by NM, will be
- * in the UNAVAILABLE state here. To ensure it can be activated
- * immediately, we transition it to DISCONNECTED.
- */
- if ( nm_device_is_available (device, NM_DEVICE_CHECK_DEV_AVAILABLE_NONE)
- && (nm_device_get_state (device) == NM_DEVICE_STATE_UNAVAILABLE)) {
- nm_device_state_changed (device,
- NM_DEVICE_STATE_DISCONNECTED,
- NM_DEVICE_STATE_REASON_NONE);
- }
- } else {
- NMConnection *existing_connection = NULL;
- NMAuthSubject *subject;
- char *error_desc = NULL;
-
- /* If the device is active and its connection is not visible to the
- * user that's requesting this new activation, fail, since other users
- * should not be allowed to implicitly deactivate private connections
- * by activating a connection of their own.
- */
- existing_connection = nm_device_get_connection (device);
- subject = nm_active_connection_get_subject (active);
- if (existing_connection &&
- !nm_auth_is_subject_in_acl (existing_connection,
- subject,
- &error_desc)) {
- g_set_error (error,
- NM_MANAGER_ERROR,
- NM_MANAGER_ERROR_PERMISSION_DENIED,
- "Private connection already active on the device: %s",
- error_desc);
- g_free (error_desc);
- return FALSE;
- }
+ /* If the device is active and its connection is not visible to the
+ * user that's requesting this new activation, fail, since other users
+ * should not be allowed to implicitly deactivate private connections
+ * by activating a connection of their own.
+ */
+ existing_connection = nm_device_get_connection (device);
+ subject = nm_active_connection_get_subject (active);
+ if (existing_connection &&
+ !nm_auth_is_subject_in_acl (existing_connection,
+ subject,
+ &error_desc)) {
+ g_set_error (error,
+ NM_MANAGER_ERROR,
+ NM_MANAGER_ERROR_PERMISSION_DENIED,
+ "Private connection already active on the device: %s",
+ error_desc);
+ g_free (error_desc);
+ return FALSE;
}
/* Final connection must be available on device */
@@ -2598,8 +2584,8 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError *
}
nm_active_connection_set_master (active, master_ac);
- nm_log_dbg (LOGD_CORE, "Activation of '%s' depends on active connection %s",
- nm_connection_get_id (connection),
+ nm_log_dbg (LOGD_CORE, "Activation of '%s' depends on active connection %p %s",
+ nm_connection_get_id (connection), master_ac,
nm_active_connection_get_path (master_ac));
}
@@ -2608,6 +2594,19 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError *
if (existing)
nm_device_steal_connection (existing, connection);
+ if (nm_device_get_state (device) == NM_DEVICE_STATE_UNMANAGED) {
+ nm_device_state_changed (device,
+ NM_DEVICE_STATE_UNAVAILABLE,
+ NM_DEVICE_STATE_REASON_USER_REQUESTED);
+ }
+
+ if ( nm_device_is_available (device, NM_DEVICE_CHECK_DEV_AVAILABLE_NONE)
+ && (nm_device_get_state (device) == NM_DEVICE_STATE_UNAVAILABLE)) {
+ nm_device_state_changed (device,
+ NM_DEVICE_STATE_DISCONNECTED,
+ NM_DEVICE_STATE_REASON_USER_REQUESTED);
+ }
+
/* Export the new ActiveConnection to clients and start it on the device */
nm_active_connection_export (active);
g_object_notify (G_OBJECT (self), NM_MANAGER_ACTIVE_CONNECTIONS);
@@ -2828,6 +2827,23 @@ nm_manager_activate_connection (NMManager *self,
return active;
}
+/**
+ * validate_activation_request:
+ * @self: the #NMManager
+ * @context: the D-Bus context of the requestor
+ * @connection: the partial or complete #NMConnection to be activated
+ * @device_path: the object path of the device to be activated, or "/"
+ * @out_device: on successful reutrn, the #NMDevice to be activated with @connection
+ * @out_vpn: on successful return, %TRUE if @connection is a VPN connection
+ * @error: location to store an error on failure
+ *
+ * Performs basic validation on an activation request, including ensuring that
+ * the requestor is a valid Unix process, is not disallowed in @connection
+ * permissions, and that a device exists that can activate @connection.
+ *
+ * Returns: on success, the #NMAuthSubject representing the requestor, or
+ * %NULL on error
+ */
static NMAuthSubject *
validate_activation_request (NMManager *self,
DBusGMethodInvocation *context,
@@ -2902,11 +2918,11 @@ validate_activation_request (NMManager *self,
} else
device = nm_manager_get_best_device_for_connection (self, connection);
- if (!device) {
+ if (!device && !vpn) {
gboolean is_software = nm_connection_is_virtual (connection);
/* VPN and software-device connections don't need a device yet */
- if (!vpn && !is_software) {
+ if (!is_software) {
g_set_error_literal (error,
NM_MANAGER_ERROR,
NM_MANAGER_ERROR_UNKNOWN_DEVICE,
@@ -2922,11 +2938,19 @@ validate_activation_request (NMManager *self,
if (!iface)
goto error;
- device = find_device_by_iface (self, iface);
+ device = find_device_by_iface (self, iface, connection, NULL);
g_free (iface);
}
}
+ if ((!vpn || device_path) && !device) {
+ g_set_error_literal (error,
+ NM_MANAGER_ERROR,
+ NM_MANAGER_ERROR_UNKNOWN_DEVICE,
+ "Failed to find a compatible device for this connection");
+ goto error;
+ }
+
*out_device = device;
*out_vpn = vpn;
return subject;
@@ -3203,14 +3227,6 @@ impl_manager_add_and_activate_connection (NMManager *self,
if (!subject)
goto error;
- /* AddAndActivate() requires a device to complete the connection with */
- if (!device) {
- error = g_error_new_literal (NM_MANAGER_ERROR,
- NM_MANAGER_ERROR_UNKNOWN_DEVICE,
- "This connection requires an existing device.");
- goto error;
- }
-
all_connections = nm_settings_get_connections (priv->settings);
if (vpn) {
/* Try to fill the VPN's connection setting and name at least */