summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDan Williams <dcbw@redhat.com>2014-10-15 21:17:45 -0500
committerDan Williams <dcbw@redhat.com>2015-05-15 09:48:29 -0500
commit0d672e8389232c75d435e58ce845c040f0eeec4f (patch)
treed30f27f988795c915e024400f0edf263eec9b62e
parentab6159d831261ac8ac54e42fed39423a6e7f8387 (diff)
downloadNetworkManager-dcbw/devices-for-all-1-squashed.tar.gz
core: allow multiple devices with the same interface namedcbw/devices-for-all-1-squashed
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 a8c6ecda63..a0a8e3ee1c 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -2073,6 +2073,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
*
@@ -2639,6 +2651,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 034caffa92..6172d5da9a 100644
--- a/src/devices/nm-device.h
+++ b/src/devices/nm-device.h
@@ -382,6 +382,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);
@@ -412,6 +413,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 9ff3940ad0..bf04ac1c93 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 */
@@ -2175,7 +2205,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,
@@ -2427,8 +2457,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);
@@ -2440,72 +2472,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 */
@@ -2582,8 +2568,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));
}
@@ -2592,6 +2578,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);
@@ -2812,6 +2811,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,
@@ -2886,11 +2902,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,
@@ -2906,11 +2922,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;
@@ -3187,14 +3211,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 */