On Tue, 2011-10-18 at 13:48 +0200, Thomas Graf wrote:
> A bonding device is like a virtual ethernet device. We therefore reuse
> nm-device-ethernet and add some special handling to detect bonding
> connections.

Does rtnl_link_get_type() really return a "char *"?  Any reason that's
'const char *'?  I'd expect callers not to be able to modify the
returned string, and the code you've got here dupes it before returning.
Especially when constants are assigned to io_name in say
bonding.c/vlan.c/dummy.c too...

Just something to think about for libnl4 I guess, since the libnl3 API
probably can't be changed now.  I've caught quite a few errors and
memory leaks over the years by making sure accessors return 'const'
stuff.

Dan

> Changes v2:
>  - Fixed memory leak
> 
> Signed-off-by: Thomas Graf <[email protected]>
> ---
>  src/nm-device-ethernet.c   |   35 ++++++++++++++++++++++++++++++++---
>  src/nm-device-ethernet.h   |    2 ++
>  src/nm-device.c            |    1 -
>  src/nm-system.c            |   32 ++++++++++++++++++++++++++++++++
>  src/nm-system.h            |    2 ++
>  src/nm-udev-manager.c      |   14 ++++++++++++--
>  src/settings/nm-settings.c |   13 +++++++++++--
>  7 files changed, 91 insertions(+), 8 deletions(-)
> 
> diff --git a/src/nm-device-ethernet.c b/src/nm-device-ethernet.c
> index 8556c5b..6e1471f 100644
> --- a/src/nm-device-ethernet.c
> +++ b/src/nm-device-ethernet.c
> @@ -55,6 +55,7 @@
>  #include "nm-setting-wired.h"
>  #include "nm-setting-8021x.h"
>  #include "nm-setting-pppoe.h"
> +#include "nm-setting-bond.h"
>  #include "ppp-manager/nm-ppp-manager.h"
>  #include "nm-logging.h"
>  #include "nm-properties-changed-signal.h"
> @@ -599,6 +600,22 @@ nm_device_ethernet_get_address (NMDeviceEthernet *self, 
> struct ether_addr *addr)
>       memcpy (addr, &priv->hw_addr, sizeof (priv->hw_addr));
>  }
>  
> +gboolean
> +nm_device_bond_connection_matches(NMDevice *device, NMConnection *connection)
> +{
> +     NMSettingBond *s_bond;
> +     const char *devname;
> +
> +     devname = nm_device_get_iface (device);
> +     g_assert(devname);
> +
> +     s_bond = nm_connection_get_setting_bond (connection);
> +     if (s_bond && !strcmp (devname, nm_setting_bond_get_interface_name 
> (s_bond)))
> +             return TRUE;
> +
> +     return FALSE;
> +}
> +
>  /* Returns speed in Mb/s */
>  static guint32
>  nm_device_ethernet_get_speed (NMDeviceEthernet *self)
> @@ -890,6 +907,14 @@ real_get_best_auto_connection (NMDevice *dev,
>               g_assert (s_con);
>  
>               connection_type = nm_setting_connection_get_connection_type 
> (s_con);
> +
> +             if (!strcmp (connection_type, NM_SETTING_BOND_SETTING_NAME)) {
> +                     if (nm_device_bond_connection_matches (dev, connection))
> +                             return connection;
> +
> +                     continue;
> +             }
> +     
>               if (!strcmp (connection_type, NM_SETTING_PPPOE_SETTING_NAME))
>                       is_pppoe = TRUE;
>  
> @@ -1625,7 +1650,7 @@ real_check_connection_compatible (NMDevice *device,
>       NMSettingConnection *s_con;
>       NMSettingWired *s_wired;
>       const char *connection_type;
> -     gboolean is_pppoe = FALSE;
> +     gboolean is_pppoe = FALSE, is_bond = FALSE;
>       const GByteArray *mac;
>       gboolean try_mac = TRUE;
>       const GSList *mac_blacklist, *mac_blacklist_iter;
> @@ -1635,19 +1660,23 @@ real_check_connection_compatible (NMDevice *device,
>  
>       connection_type = nm_setting_connection_get_connection_type (s_con);
>       if (   strcmp (connection_type, NM_SETTING_WIRED_SETTING_NAME)
> +         && strcmp (connection_type, NM_SETTING_BOND_SETTING_NAME)
>           && strcmp (connection_type, NM_SETTING_PPPOE_SETTING_NAME)) {
>               g_set_error (error,
>                            NM_ETHERNET_ERROR, 
> NM_ETHERNET_ERROR_CONNECTION_NOT_WIRED,
> -                          "The connection was not a wired or PPPoE 
> connection.");
> +                          "The connection was not a wired, bond, or PPPoE 
> connection.");
>               return FALSE;
>       }
>  
>       if (!strcmp (connection_type, NM_SETTING_PPPOE_SETTING_NAME))
>               is_pppoe = TRUE;
>  
> +     if (!strcmp (connection_type, NM_SETTING_BOND_SETTING_NAME))
> +             is_bond = TRUE;
> +
>       s_wired = (NMSettingWired *) nm_connection_get_setting (connection, 
> NM_TYPE_SETTING_WIRED);
>       /* Wired setting is optional for PPPoE */
> -     if (!is_pppoe && !s_wired) {
> +     if (!is_pppoe && !s_wired && !is_bond) {
>               g_set_error (error,
>                            NM_ETHERNET_ERROR, 
> NM_ETHERNET_ERROR_CONNECTION_INVALID,
>                            "The connection was not a valid wired 
> connection.");
> diff --git a/src/nm-device-ethernet.h b/src/nm-device-ethernet.h
> index b9e2afd..46db79e 100644
> --- a/src/nm-device-ethernet.h
> +++ b/src/nm-device-ethernet.h
> @@ -63,6 +63,8 @@ NMDevice *nm_device_ethernet_new (const char *udi,
>  void nm_device_ethernet_get_address (NMDeviceEthernet *dev,
>                                       struct ether_addr *addr);
>  
> +gboolean nm_device_bond_connection_matches(NMDevice *device, NMConnection 
> *connection);
> +
>  G_END_DECLS
>  
>  #endif       /* NM_DEVICE_ETHERNET_H */
> diff --git a/src/nm-device.c b/src/nm-device.c
> index a4cd3e9..89b3385 100644
> --- a/src/nm-device.c
> +++ b/src/nm-device.c
> @@ -4060,4 +4060,3 @@ nm_device_clear_autoconnect_inhibit (NMDevice *device)
>       g_return_if_fail (priv);
>       priv->autoconnect_inhibit = FALSE;
>  }
> -
> diff --git a/src/nm-system.c b/src/nm-system.c
> index 27344ed..6c861fa 100644
> --- a/src/nm-system.c
> +++ b/src/nm-system.c
> @@ -1206,3 +1206,35 @@ nm_system_add_bonding_master(NMSettingBond *setting)
>  
>       return TRUE;
>  }
> +
> +/**
> + * nm_system_get_link_type:
> + * @name: name of link
> + *
> + * Lookup virtual link type. The returned string is allocated and needs
> + * to be freed after usage.
> + *
> + * Returns: Name of virtual link type or NULL if not a virtual link.
> + **/
> +char *
> +nm_system_get_link_type(const char *name)
> +{
> +     struct rtnl_link *result;
> +     struct nl_sock *nlh;
> +     char *type;
> +
> +     nlh = nm_netlink_get_default_handle ();
> +     if (!nlh)
> +             return NULL;
> +
> +     if (rtnl_link_get_kernel (nlh, 0, name, &result) < 0)
> +             return NULL;
> +
> +     if ((type = rtnl_link_get_type (result)))
> +             type = g_strdup(type);
> +
> +     rtnl_link_put (result);
> +
> +     return type;
> +}
> +
> diff --git a/src/nm-system.h b/src/nm-system.h
> index a606503..1fe91f6 100644
> --- a/src/nm-system.h
> +++ b/src/nm-system.h
> @@ -92,4 +92,6 @@ gboolean            nm_system_iface_set_mac                 
> (int ifindex, const struct eth
>  
>  gboolean             nm_system_add_bonding_master    (NMSettingBond 
> *setting);
>  
> +char *                       nm_system_get_link_type         (const char 
> *name);
> +
>  #endif
> diff --git a/src/nm-udev-manager.c b/src/nm-udev-manager.c
> index e8c6b82..ede39bb 100644
> --- a/src/nm-udev-manager.c
> +++ b/src/nm-udev-manager.c
> @@ -41,6 +41,7 @@
>  #if WITH_WIMAX
>  #include "nm-device-wimax.h"
>  #endif
> +#include "nm-system.h"
>  
>  typedef struct {
>       GUdevClient *client;
> @@ -430,9 +431,18 @@ device_creator (NMUdevManager *manager,
>       }
>  
>       if (!driver) {
> -             if (g_str_has_prefix (ifname, "easytether")) {
> +             char *type;
> +
> +             type = nm_system_get_link_type (ifname);
> +             if (type) {
> +                     if (g_strcmp0 (type, "bond") == 0)
> +                             driver = "bonding";
> +                     g_free (type);
> +             } else if (g_str_has_prefix (ifname, "easytether")) {
>                       driver = "easytether";
> -             } else {
> +             }
> +             
> +             if (!driver) {
>                       nm_log_warn (LOGD_HW, "%s: couldn't determine device 
> driver; ignoring...", path);
>                       goto out;
>               }
> diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c
> index f0bfc16..7cf930a 100644
> --- a/src/settings/nm-settings.c
> +++ b/src/settings/nm-settings.c
> @@ -52,6 +52,7 @@
>  #include <nm-setting-wired.h>
>  #include <nm-setting-wireless.h>
>  #include <nm-setting-wireless-security.h>
> +#include <nm-setting-bond.h>
>  
>  #include "../nm-device-ethernet.h"
>  #include "nm-dbus-glib-types.h"
> @@ -1172,7 +1173,7 @@ impl_settings_save_hostname (NMSettings *self,
>  }
>  
>  static gboolean
> -have_connection_for_device (NMSettings *self, GByteArray *mac)
> +have_connection_for_device (NMSettings *self, GByteArray *mac, NMDevice 
> *device)
>  {
>       NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self);
>       GHashTableIter iter;
> @@ -1194,6 +1195,14 @@ have_connection_for_device (NMSettings *self, 
> GByteArray *mac)
>               s_con = nm_connection_get_setting_connection (connection);
>               ctype = nm_setting_connection_get_connection_type (s_con);
>  
> +             if (!strcmp (ctype, NM_SETTING_BOND_SETTING_NAME)) {
> +                     if (nm_device_bond_connection_matches (device, 
> connection)) {
> +                             ret = TRUE;
> +                             break;
> +                     } else
> +                             continue;
> +             }
> +
>               if (   strcmp (ctype, NM_SETTING_WIRED_SETTING_NAME)
>                   && strcmp (ctype, NM_SETTING_PPPOE_SETTING_NAME))
>                       continue;
> @@ -1445,7 +1454,7 @@ nm_settings_device_added (NMSettings *self, NMDevice 
> *device)
>       mac = g_byte_array_sized_new (ETH_ALEN);
>       g_byte_array_append (mac, tmp.ether_addr_octet, ETH_ALEN);
>  
> -     if (   have_connection_for_device (self, mac)
> +     if (   have_connection_for_device (self, mac, device)
>           || is_mac_auto_wired_blacklisted (self, mac))
>               goto ignore;
>  


_______________________________________________
networkmanager-list mailing list
[email protected]
http://mail.gnome.org/mailman/listinfo/networkmanager-list

Reply via email to