Re: [PATCH] Make ifcfg-eth* NM_CONTROLLED=no work without HWADDR by matching on device name
- From: Dan Williams <dcbw redhat com>
- To: Mads Kiilerich <mads kiilerich com>
- Cc: networkmanager-list gnome org
- Subject: Re: [PATCH] Make ifcfg-eth* NM_CONTROLLED=no work without HWADDR by matching on device name
- Date: Tue, 26 May 2009 10:29:33 -0400
On Tue, 2009-05-26 at 11:54 +0200, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <mads kiilerich com>
> # Date 1243331661 -7200
> # Node ID e3665ad78dd3eceea2939becadc7f8441c033764
> # Parent eac10857612988004e00d61330d1f465c2176a84
> Make ifcfg-eth* NM_CONTROLLED=no work without HWADDR by matching on device name
>
> IMHO this should work because the "old" network system uses the mac addresses
> to ensure stable device names anyway. The existing match on mac addresses could
> perhaps be removed instead.
>
> The patch applies to the 0.7.1 branch.
MAC addresses are used because there are no other unique identifiers.
MAC is the only unique identifier, and therefore that's what must be
used to match devices.
Device names change, and therefore are not stable, and therefore cannot
be used to provide any type of stable naming. Yes, udev rules can be
used to work around some of this problem, but they certainly don't fix
everything, and sometimes get it wrong, because there is no possible way
for them to get it right 100% of the time. Besides, they use MAC
addresses anyway, which means the MAC must be valid for a stable device
name provided by udev to exist.
If the ifcfg does not have an HWADDR, then there is no possible way to
match it up with a real hardware device. Use udev rules to match on the
device's serial number or some other combination of bus IDs to provide a
fake MAC if the device doesn't provide one itself. Then you can use
that MAC in the HWADDR field.
Dan
> This fixes https://bugzilla.redhat.com/show_bug.cgi?id=502142 for me.
>
> diff -r eac108576129 -r e3665ad78dd3 libnm-util/libnm-util.ver
> --- a/libnm-util/libnm-util.ver Mon Apr 13 10:18:31 2009 -0400
> +++ b/libnm-util/libnm-util.ver Tue May 26 11:54:21 2009 +0200
> @@ -198,6 +198,7 @@
> nm_setting_wired_get_speed;
> nm_setting_wired_get_duplex;
> nm_setting_wired_get_auto_negotiate;
> + nm_setting_wired_get_device_name;
> nm_setting_wired_get_mac_address;
> nm_setting_wired_get_mtu;
> nm_setting_wireless_ap_security_compatible;
> diff -r eac108576129 -r e3665ad78dd3 libnm-util/nm-setting-wired.c
> --- a/libnm-util/nm-setting-wired.c Mon Apr 13 10:18:31 2009 -0400
> +++ b/libnm-util/nm-setting-wired.c Tue May 26 11:54:21 2009 +0200
> @@ -75,6 +75,7 @@
> gboolean auto_negotiate;
> GByteArray *mac_address;
> guint32 mtu;
> + char *device_name;
> } NMSettingWiredPrivate;
>
> enum {
> @@ -85,6 +86,7 @@
> PROP_AUTO_NEGOTIATE,
> PROP_MAC_ADDRESS,
> PROP_MTU,
> + PROP_DEVICE_NAME,
>
> LAST_PROP
> };
> @@ -143,6 +145,14 @@
> return NM_SETTING_WIRED_GET_PRIVATE (setting)->mtu;
> }
>
> +const char *
> +nm_setting_wired_get_device_name (NMSettingWired *setting)
> +{
> + g_return_val_if_fail (NM_IS_SETTING_WIRED (setting), NULL);
> +
> + return NM_SETTING_WIRED_GET_PRIVATE (setting)->device_name;
> +}
> +
> static gboolean
> verify (NMSetting *setting, GSList *all_settings, GError **error)
> {
> @@ -194,6 +204,8 @@
> if (priv->mac_address)
> g_byte_array_free (priv->mac_address, TRUE);
>
> + g_free (priv->device_name);
> +
> G_OBJECT_CLASS (nm_setting_wired_parent_class)->finalize (object);
> }
>
> @@ -226,6 +238,10 @@
> case PROP_MTU:
> priv->mtu = g_value_get_uint (value);
> break;
> + case PROP_DEVICE_NAME:
> + g_free (priv->device_name);
> + priv->device_name = g_value_dup_string (value);
> + break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
> break;
> @@ -257,6 +273,9 @@
> case PROP_MTU:
> g_value_set_uint (value, nm_setting_wired_get_mtu (setting));
> break;
> + case PROP_DEVICE_NAME:
> + g_value_set_string (value, nm_setting_wired_get_device_name (setting));
> + break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
> break;
> @@ -314,7 +333,7 @@
> (object_class, PROP_MAC_ADDRESS,
> _nm_param_spec_specialized (NM_SETTING_WIRED_MAC_ADDRESS,
> "MAC Address",
> - "Harware address",
> + "Hardware address",
> DBUS_TYPE_G_UCHAR_ARRAY,
> G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE));
>
> @@ -325,5 +344,13 @@
> "MTU",
> 0, G_MAXUINT32, 0,
> G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE | NM_SETTING_PARAM_FUZZY_IGNORE));
> +
> + g_object_class_install_property
> + (object_class, PROP_DEVICE_NAME,
> + g_param_spec_string (NM_SETTING_WIRED_DEVICE_NAME,
> + "DeviceName",
> + "Device Name",
> + NULL,
> + G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE));
> +
> }
> -
> diff -r eac108576129 -r e3665ad78dd3 libnm-util/nm-setting-wired.h
> --- a/libnm-util/nm-setting-wired.h Mon Apr 13 10:18:31 2009 -0400
> +++ b/libnm-util/nm-setting-wired.h Tue May 26 11:54:21 2009 +0200
> @@ -46,7 +46,7 @@
> NM_SETTING_WIRED_ERROR_MISSING_PROPERTY
> } NMSettingWiredError;
>
> -#define NM_TYPE_SETTING_WIRED_ERROR (nm_setting_wired_error_get_type ())
> +#define NM_TYPE_SETTING_WIRED_ERROR (nm_setting_wired_error_get_type ())
> GType nm_setting_wired_error_get_type (void);
>
> #define NM_SETTING_WIRED_ERROR nm_setting_wired_error_quark ()
> @@ -58,6 +58,7 @@
> #define NM_SETTING_WIRED_AUTO_NEGOTIATE "auto-negotiate"
> #define NM_SETTING_WIRED_MAC_ADDRESS "mac-address"
> #define NM_SETTING_WIRED_MTU "mtu"
> +#define NM_SETTING_WIRED_DEVICE_NAME "device-name"
>
> typedef struct {
> NMSetting parent;
> @@ -76,6 +77,7 @@
> gboolean nm_setting_wired_get_auto_negotiate (NMSettingWired *setting);
> const GByteArray *nm_setting_wired_get_mac_address (NMSettingWired *setting);
> guint32 nm_setting_wired_get_mtu (NMSettingWired *setting);
> +const char *nm_setting_wired_get_device_name (NMSettingWired *setting);
>
> G_END_DECLS
>
> diff -r eac108576129 -r e3665ad78dd3 system-settings/plugins/ifcfg-rh/nm-ifcfg-connection.c
> --- a/system-settings/plugins/ifcfg-rh/nm-ifcfg-connection.c Mon Apr 13 10:18:31 2009 -0400
> +++ b/system-settings/plugins/ifcfg-rh/nm-ifcfg-connection.c Tue May 26 11:54:21 2009 +0200
> @@ -79,16 +79,43 @@
> static guint signals[LAST_SIGNAL] = { 0 };
>
> static char *
> -get_ether_device_udi (DBusGConnection *g_connection, const GByteArray *mac, GSList *devices)
> +get_ether_device_udi (DBusGConnection *g_connection, const char* device_name, const GByteArray *mac, GSList *devices)
> {
> GError *error = NULL;
> GSList *iter;
> char *udi = NULL;
>
> - if (!g_connection || !mac)
> + if (!g_connection)
> return NULL;
>
> - for (iter = devices; !udi && iter; iter = g_slist_next (iter)) {
> + /* First try to match on device name (eth0 etc) by reading hal net.interface */
> + for (iter = devices; device_name && !udi && iter; iter = g_slist_next (iter)) {
> + DBusGProxy *dev_proxy;
> + char *hal_net_interface = NULL;
> +
> + dev_proxy = dbus_g_proxy_new_for_name (g_connection,
> + "org.freedesktop.Hal",
> + iter->data,
> + "org.freedesktop.Hal.Device");
> + if (!dev_proxy)
> + continue;
> +
> + if (dbus_g_proxy_call_with_timeout (dev_proxy,
> + "GetPropertyString", 10000, &error,
> + G_TYPE_STRING, "net.interface", G_TYPE_INVALID,
> + G_TYPE_STRING, &hal_net_interface, G_TYPE_INVALID)) {
> + if (!strcmp (hal_net_interface, device_name))
> + udi = g_strdup (iter->data);
> + } else {
> + g_error_free (error);
> + error = NULL;
> + }
> + g_free (hal_net_interface);
> + g_object_unref (dev_proxy);
> + }
> +
> + /* If no match then try to match on MAC address (can this ever happen?) */
> + for (iter = devices; mac && !udi && iter; iter = g_slist_next (iter)) {
> DBusGProxy *dev_proxy;
> char *address = NULL;
>
> @@ -102,7 +129,7 @@
> if (dbus_g_proxy_call_with_timeout (dev_proxy,
> "GetPropertyString", 10000, &error,
> G_TYPE_STRING, "net.address", G_TYPE_INVALID,
> - G_TYPE_STRING, &address, G_TYPE_INVALID)) {
> + G_TYPE_STRING, &address, G_TYPE_INVALID)) {
> struct ether_addr *dev_mac;
>
> if (address && strlen (address)) {
> @@ -171,7 +198,7 @@
> s_wired = (NMSettingWired *) nm_connection_get_setting (connection, NM_TYPE_SETTING_WIRED);
> if (s_wired) {
> devices = nm_system_config_hal_manager_get_devices_of_type (hal_mgr, NM_DEVICE_TYPE_ETHERNET);
> - udi = get_ether_device_udi (g_connection, nm_setting_wired_get_mac_address (s_wired), devices);
> + udi = get_ether_device_udi (g_connection, nm_setting_wired_get_device_name (s_wired), nm_setting_wired_get_mac_address (s_wired), devices);
> }
> break;
>
> @@ -179,7 +206,7 @@
> s_wireless = (NMSettingWireless *) nm_connection_get_setting (connection, NM_TYPE_SETTING_WIRELESS);
> if (s_wireless) {
> devices = nm_system_config_hal_manager_get_devices_of_type (hal_mgr, NM_DEVICE_TYPE_WIFI);
> - udi = get_ether_device_udi (g_connection, nm_setting_wireless_get_mac_address (s_wireless), devices);
> + udi = get_ether_device_udi (g_connection, NULL, nm_setting_wireless_get_mac_address (s_wireless), devices);
> }
> break;
>
> diff -r eac108576129 -r e3665ad78dd3 system-settings/plugins/ifcfg-rh/reader.c
> --- a/system-settings/plugins/ifcfg-rh/reader.c Mon Apr 13 10:18:31 2009 -0400
> +++ b/system-settings/plugins/ifcfg-rh/reader.c Tue May 26 11:54:21 2009 +0200
> @@ -194,7 +194,7 @@
> PLUGIN_WARN (IFCFG_PLUGIN_NAME, " warning: duplicate DNS server %s", tag); \
> } \
> }
> -
> +
>
> static NMSetting *
> make_ip4_setting (shvarFile *ifcfg, const char *network_file, GError **error)
> @@ -1139,6 +1139,15 @@
> g_object_set (s_wired, NM_SETTING_WIRED_MAC_ADDRESS, mac, NULL);
> g_byte_array_free (mac, TRUE);
> } else {
> + PLUGIN_WARN (IFCFG_PLUGIN_NAME, " warning: invalid MAC adress");
> + }
> +
> + value = svGetValue (ifcfg, "DEVICE", FALSE);
> + if (value && strlen (value)) {
> + g_object_set (s_wired, NM_SETTING_WIRED_DEVICE_NAME, value, NULL);
> + g_free (value);
> + } else {
> + PLUGIN_WARN (IFCFG_PLUGIN_NAME, " warning: no DEVICE specified");
> g_object_unref (s_wired);
> s_wired = NULL;
> }
> _______________________________________________
> NetworkManager-list mailing list
> NetworkManager-list gnome org
> http://mail.gnome.org/mailman/listinfo/networkmanager-list
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
