vlan: don't fail if parent isn't found at construct time for existing devices
authorDan Williams <dcbw@redhat.com>
Mon, 13 Apr 2015 16:43:12 +0000 (11:43 -0500)
committerDan Williams <dcbw@redhat.com>
Wed, 6 May 2015 21:14:24 +0000 (16:14 -0500)
For existing devices, depending on the order that netlink sends interfaces to
us, the parent may be found after the VLAN interface and not be available when
the VLAN interface is constructed.  Instead of failing construction, when a
NMDeviceVlan has no parent keep it unavailable for activation.  Then have
the Manager notify existing devices when a new device is found, and let
NMDeviceVlan find the parent later and become available via that mechanism.

This doesn't apply to VLANs created by NM itself, because the kernel requires
a parent ifindex when creating a VLAN device.  Thus this fix only applies to
VLANs created outside NetworkManager, or existing when NM starts up.

introspection/nm-device.xml
libnm-core/nm-dbus-interface.h
libnm-util/NetworkManager.h
src/devices/nm-device-vlan.c
src/devices/nm-device.h
src/nm-manager.c

index 7aef2f3..71b830f 100644 (file)
           A new connection activation was enqueued.
         </tp:docstring>
       </tp:enumvalue>
+      <tp:enumvalue suffix="PARENT_CHANGED" value="61">
+        <tp:docstring>
+          The device's parent changed.
+        </tp:docstring>
+      </tp:enumvalue>
+      <tp:enumvalue suffix="PARENT_MANAGED_CHANGED" value="62">
+        <tp:docstring>
+          The device parent's management changed.
+        </tp:docstring>
+      </tp:enumvalue>
     </tp:enum>
 
     <tp:struct name="NM_DEVICE_STATE_REASON_STRUCT">
index 9bab80f..ccc3b3a 100644 (file)
@@ -467,6 +467,8 @@ typedef enum {
  * @NM_DEVICE_STATE_REASON_MODEM_AVAILABLE: Modem now ready and available
  * @NM_DEVICE_STATE_REASON_SIM_PIN_INCORRECT: SIM PIN was incorrect
  * @NM_DEVICE_STATE_REASON_NEW_ACTIVATION: New connection activation was enqueued
+ * @NM_DEVICE_STATE_REASON_PARENT_CHANGED: the device's parent changed
+ * @NM_DEVICE_STATE_REASON_PARENT_MANAGED_CHANGED: the device parent's management changed
  *
  * Device state change reason codes
  *
@@ -534,6 +536,8 @@ typedef enum {
        NM_DEVICE_STATE_REASON_MODEM_AVAILABLE = 58,
        NM_DEVICE_STATE_REASON_SIM_PIN_INCORRECT = 59,
        NM_DEVICE_STATE_REASON_NEW_ACTIVATION = 60,
+       NM_DEVICE_STATE_REASON_PARENT_CHANGED = 61,
+       NM_DEVICE_STATE_REASON_PARENT_MANAGED_CHANGED = 62,
 } NMDeviceStateReason;
 
 
index 5a98b8e..4fa861e 100644 (file)
@@ -472,6 +472,8 @@ typedef enum {
  * @NM_DEVICE_STATE_REASON_MODEM_AVAILABLE: Modem now ready and available
  * @NM_DEVICE_STATE_REASON_SIM_PIN_INCORRECT: SIM PIN was incorrect
  * @NM_DEVICE_STATE_REASON_NEW_ACTIVATION: New connection activation was enqueued
+ * @NM_DEVICE_STATE_REASON_PARENT_CHANGED: the device's parent changed
+ * @NM_DEVICE_STATE_REASON_PARENT_MANAGED_CHANGED: the device parent's management changed
  *
  * Device state change reason codes
  *
@@ -539,6 +541,8 @@ typedef enum {
        NM_DEVICE_STATE_REASON_MODEM_AVAILABLE = 58,
        NM_DEVICE_STATE_REASON_SIM_PIN_INCORRECT = 59,
        NM_DEVICE_STATE_REASON_NEW_ACTIVATION = 60,
+       NM_DEVICE_STATE_REASON_PARENT_CHANGED = 61,
+       NM_DEVICE_STATE_REASON_PARENT_MANAGED_CHANGED = 62,
 
        NM_DEVICE_STATE_REASON_LAST = 0xFFFF
 } NMDeviceStateReason;
index b2a2fbf..5513d73 100644 (file)
@@ -107,6 +107,15 @@ bring_up (NMDevice *dev, gboolean *no_firmware)
 
 /******************************************************************/
 
+static gboolean
+is_available (NMDevice *device, NMDeviceCheckDevAvailableFlags flags)
+{
+       if (!NM_DEVICE_VLAN_GET_PRIVATE (device)->parent)
+               return FALSE;
+
+       return NM_DEVICE_CLASS (nm_device_vlan_parent_class)->is_available (device, flags);
+}
+
 static void
 parent_state_changed (NMDevice *parent,
                       NMDeviceState new_state,
@@ -124,9 +133,13 @@ parent_state_changed (NMDevice *parent,
 }
 
 static void
-nm_device_vlan_set_parent (NMDeviceVlan *device, NMDevice *parent)
+nm_device_vlan_set_parent (NMDeviceVlan *self, NMDevice *parent, gboolean construct)
 {
-       NMDeviceVlanPrivate *priv = NM_DEVICE_VLAN_GET_PRIVATE (device);
+       NMDeviceVlanPrivate *priv = NM_DEVICE_VLAN_GET_PRIVATE (self);
+       NMDevice *device = NM_DEVICE (self);
+
+       if (parent == priv->parent)
+               return;
 
        if (priv->parent_state_id) {
                g_signal_handler_disconnect (priv->parent, priv->parent_state_id);
@@ -140,10 +153,56 @@ nm_device_vlan_set_parent (NMDeviceVlan *device, NMDevice *parent)
                                                          "state-changed",
                                                          G_CALLBACK (parent_state_changed),
                                                          device);
+
+               /* Set parent-dependent unmanaged flag */
+               if (construct) {
+                       nm_device_set_initial_unmanaged_flag (device,
+                                                             NM_UNMANAGED_PARENT,
+                                                             !nm_device_get_managed (parent));
+               } else {
+                       nm_device_set_unmanaged (device,
+                                                NM_UNMANAGED_PARENT,
+                                                !nm_device_get_managed (parent),
+                                                NM_DEVICE_STATE_REASON_PARENT_MANAGED_CHANGED);
+               }
        }
+
+       /* Recheck availability now that the parent has changed */
+       nm_device_queue_recheck_available (self,
+                                          NM_DEVICE_STATE_REASON_PARENT_CHANGED,
+                                          NM_DEVICE_STATE_REASON_PARENT_CHANGED);
        g_object_notify (G_OBJECT (device), NM_DEVICE_VLAN_PARENT);
 }
 
+static gboolean
+component_added (NMDevice *device, GObject *component)
+{
+       NMDeviceVlan *self = NM_DEVICE_VLAN (device);
+       NMDeviceVlanPrivate *priv = NM_DEVICE_VLAN_GET_PRIVATE (self);
+       NMDevice *added_device;
+       int parent_ifindex = -1;
+
+       if (priv->parent)
+               return FALSE;
+
+       if (!NM_IS_DEVICE (component))
+               return FALSE;
+       added_device = NM_DEVICE (component);
+
+       if (!nm_platform_vlan_get_info (NM_PLATFORM_GET, nm_device_get_ifindex (device), &parent_ifindex, NULL)) {
+               _LOGW (LOGD_VLAN, "failed to get VLAN interface info while checking added component.");
+               return FALSE;
+       }
+
+       if (nm_device_get_ifindex (added_device) != parent_ifindex)
+               return FALSE;
+
+       nm_device_vlan_set_parent (self, added_device, FALSE);
+
+       /* Don't claim parent exclusively */
+       return FALSE;
+}
+
 /******************************************************************/
 
 static gboolean
@@ -153,6 +212,9 @@ match_parent (NMDeviceVlan *self, const char *parent)
 
        g_return_val_if_fail (parent != NULL, FALSE);
 
+       if (!priv->parent)
+               return FALSE;
+
        if (nm_utils_is_uuid (parent)) {
                NMActRequest *parent_req;
                NMConnection *parent_connection;
@@ -309,8 +371,7 @@ update_connection (NMDevice *device, NMConnection *connection)
 
        parent = nm_manager_get_device_by_ifindex (nm_manager_get (), parent_ifindex);
        g_assert (parent);
-       if (priv->parent != parent)
-               nm_device_vlan_set_parent (NM_DEVICE_VLAN (device), parent);
+       nm_device_vlan_set_parent (NM_DEVICE_VLAN (device), parent, FALSE);
 
        /* Update parent in the connection; default to parent's interface name */
        new_parent = nm_device_get_iface (parent);
@@ -426,12 +487,6 @@ constructed (GObject *object)
        if (G_OBJECT_CLASS (nm_device_vlan_parent_class)->constructed)
                G_OBJECT_CLASS (nm_device_vlan_parent_class)->constructed (object);
 
-       if (!priv->parent) {
-               _LOGE (LOGD_VLAN, "no parent specified.");
-               priv->invalid = TRUE;
-               return;
-       }
-
        itype = nm_platform_link_get_type (NM_PLATFORM_GET, ifindex);
        if (itype != NM_LINK_TYPE_VLAN) {
                _LOGE (LOGD_VLAN, "failed to get VLAN interface type.");
@@ -445,18 +500,27 @@ constructed (GObject *object)
                return;
        }
 
-       if (   parent_ifindex < 0
-           || parent_ifindex != nm_device_get_ip_ifindex (priv->parent)
-           || vlan_id < 0) {
+       if (parent_ifindex < 0 || vlan_id < 0) {
                _LOGW (LOGD_VLAN, "VLAN parent ifindex (%d) or VLAN ID (%d) invalid.",
                       parent_ifindex, priv->vlan_id);
                priv->invalid = TRUE;
                return;
        }
 
+       if (priv->parent && parent_ifindex != nm_device_get_ip_ifindex (priv->parent)) {
+               _LOGW (LOGD_VLAN, "VLAN parent %s (%d) and parent ifindex %d don't match.",
+                      nm_device_get_iface (priv->parent),
+                      nm_device_get_ifindex (priv->parent),
+                      parent_ifindex);
+               priv->invalid = TRUE;
+               return;
+       }
+
        priv->vlan_id = vlan_id;
-       _LOGI (LOGD_HW | LOGD_VLAN, "VLAN ID %d with parent %s",
-              priv->vlan_id, nm_device_get_iface (priv->parent));
+       _LOGI (LOGD_HW | LOGD_VLAN, "VLAN ID %d with parent %s (%d)",
+              priv->vlan_id,
+              priv->parent ? nm_device_get_iface (priv->parent) : "unknown",
+              parent_ifindex);
 }
 
 static void
@@ -489,7 +553,7 @@ set_property (GObject *object, guint prop_id,
 
        switch (prop_id) {
        case PROP_INT_PARENT_DEVICE:
-               nm_device_vlan_set_parent (NM_DEVICE_VLAN (object), g_value_get_object (value));
+               nm_device_vlan_set_parent (NM_DEVICE_VLAN (object), g_value_get_object (value), TRUE);
                break;
        case PROP_VLAN_ID:
                priv->vlan_id = g_value_get_uint (value);
@@ -512,7 +576,7 @@ dispose (GObject *object)
        }
        priv->disposed = TRUE;
 
-       nm_device_vlan_set_parent (self, NULL);
+       nm_device_vlan_set_parent (self, NULL, FALSE);
 
        G_OBJECT_CLASS (nm_device_vlan_parent_class)->dispose (object);
 }
@@ -551,6 +615,8 @@ nm_device_vlan_class_init (NMDeviceVlanClass *klass)
        parent_class->act_stage1_prepare = act_stage1_prepare;
        parent_class->ip4_config_pre_commit = ip4_config_pre_commit;
        parent_class->deactivate = deactivate;
+       parent_class->is_available = is_available;
+       parent_class->component_added = component_added;
 
        parent_class->check_connection_compatible = check_connection_compatible;
        parent_class->complete_connection = complete_connection;
@@ -626,10 +692,6 @@ new_link (NMDeviceFactory *factory, NMPlatformLink *plink, GError **error)
                device = NULL;
        }
 
-       /* Set initial parent-dependent unmanaged flag */
-       if (device)
-               nm_device_set_initial_unmanaged_flag (device, NM_UNMANAGED_PARENT, !nm_device_get_managed (parent));
-
        return device;
 }
 
@@ -682,10 +744,6 @@ create_virtual_device_for_connection (NMDeviceFactory *factory,
                device = NULL;
        }
 
-       /* Set initial parent-dependent unmanaged flag */
-       if (device)
-               nm_device_set_initial_unmanaged_flag (device, NM_UNMANAGED_PARENT, !nm_device_get_managed (parent));
-
        return device;
 }
 
index f7ba2a5..e3daa9b 100644 (file)
@@ -237,6 +237,21 @@ typedef struct {
        gboolean        (* have_any_ready_slaves) (NMDevice *self,
                                                   const GSList *slaves);
 
+       /**
+        * component_added:
+        * @self: the #NMDevice
+        * @component: the component (device, modem, etc) which was added
+        *
+        * Notifies @self that a new component was added to the Manager.  This
+        * may include any kind of %GObject subclass, and the device is expected
+        * to match only specific components they care about, like %NMModem objects
+        * or %NMDevice objects.
+        *
+        * Returns: %TRUE if the component was claimed exclusively and no further
+        * devices should be notified of the new component.  %FALSE to indicate
+        * that the component was not exclusively claimed and other devices should
+        * be notified.
+        */
        gboolean        (* component_added) (NMDevice *self, GObject *component);
 
        gboolean        (* owns_iface) (NMDevice *self, const char *iface);
index fe80156..755b1f1 100644 (file)
@@ -1776,6 +1776,18 @@ device_ip_iface_changed (NMDevice *device,
        }
 }
 
+static gboolean
+notify_component_added (NMManager *self, GObject *component)
+{
+       GSList *iter;
+
+       for (iter = NM_MANAGER_GET_PRIVATE (self)->devices; iter; iter = iter->next) {
+               if (nm_device_notify_component_added (NM_DEVICE (iter->data), component))
+                       return TRUE;
+       }
+       return FALSE;
+}
+
 /**
  * add_device:
  * @self: the #NMManager
@@ -1893,6 +1905,8 @@ add_device (NMManager *self, NMDevice *device, gboolean try_assume)
        g_signal_emit (self, signals[DEVICE_ADDED], 0, device);
        g_object_notify (G_OBJECT (self), NM_MANAGER_DEVICES);
 
+       notify_component_added (self, G_OBJECT (device));
+
        /* New devices might be master interfaces for virtual interfaces; so we may
         * need to create new virtual interfaces now.
         */
@@ -1929,14 +1943,7 @@ factory_component_added_cb (NMDeviceFactory *factory,
                             GObject *component,
                             gpointer user_data)
 {
-       NMManager *self = NM_MANAGER (user_data);
-       GSList *iter;
-
-       for (iter = NM_MANAGER_GET_PRIVATE (self)->devices; iter; iter = iter->next) {
-               if (nm_device_notify_component_added (NM_DEVICE (iter->data), component))
-                       return TRUE;
-       }
-       return FALSE;
+       return notify_component_added (NM_MANAGER (user_data), component);
 }
 
 #define PLUGIN_PREFIX "libnm-device-plugin-"