From aeb2426e88e9af7dc3734b311b607ff817d69550 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 25 Oct 2021 10:45:03 +0200 Subject: [PATCH] libnm: change default value for "dcb.app-fcoe-mode" property String properties in libnm's NMSetting really should have NULL as a default value. The only property that didn't, was "dcb.app-fcoe-mode". Change the default so that it is also NULL. Changing a default value is an API change, but in this case probably no issue. For one, DCB is little used. But also, it's not clear who would care and notice the change. Also, because previously verify() would reject a NULL value as invalid. That means, there are no existing, valid profiles that have this value set to NULL. We just make NULL the default, and define that it means the same as "fabric". Note that when we convert integer properties to D-Bus/GVariant, we often omit the default value. For string properties, they are serialized as "s" variant type. As such, NULL cannot be expressed as "s" type, so we represent NULL by omitting the property. That makes especially sense if the default value is also NULL. Otherwise, it's rather odd. We change that, and we will now always express non-NULL value on D-Bus and let NULL be encoded by omitting the property. --- src/core/nm-dcb.c | 3 ++- .../network-scripts/ifcfg-dcb-test.cexpected | 1 - src/libnm-core-impl/nm-setting-dcb.c | 23 ++++++++----------- src/libnm-core-impl/tests/test-setting.c | 21 ++++++----------- src/libnmc-setting/settings-docs.h.in | 2 +- .../generate-docs-nm-settings-nmcli.xml.in | 2 +- 6 files changed, 20 insertions(+), 32 deletions(-) diff --git a/src/core/nm-dcb.c b/src/core/nm-dcb.c index 0b995ebf7..f048d9994 100644 --- a/src/core/nm-dcb.c +++ b/src/core/nm-dcb.c @@ -258,7 +258,8 @@ _fcoe_setup(const char * iface, flags = nm_setting_dcb_get_app_fcoe_flags(s_dcb); if (flags & NM_SETTING_DCB_FLAG_ENABLE) { - const char *mode = nm_setting_dcb_get_app_fcoe_mode(s_dcb); + const char *mode = + nm_setting_dcb_get_app_fcoe_mode(s_dcb) ?: NM_SETTING_DCB_FCOE_MODE_FABRIC; if (!do_helper(NULL, FCOEADM, run_func, user_data, error, "-m %s -c %s", mode, iface)) return FALSE; diff --git a/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-dcb-test.cexpected b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-dcb-test.cexpected index 56e233cc6..fef36f2fa 100644 --- a/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-dcb-test.cexpected +++ b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-dcb-test.cexpected @@ -4,7 +4,6 @@ DCB_APP_FCOE_ENABLE=yes DCB_APP_FCOE_ADVERTISE=yes DCB_APP_FCOE_WILLING=yes DCB_APP_FCOE_PRIORITY=5 -DCB_APP_FCOE_MODE=fabric DCB_APP_ISCSI_ENABLE=yes DCB_APP_ISCSI_ADVERTISE=yes DCB_APP_ISCSI_WILLING=yes diff --git a/src/libnm-core-impl/nm-setting-dcb.c b/src/libnm-core-impl/nm-setting-dcb.c index 45a42bcfe..e71bdd8ae 100644 --- a/src/libnm-core-impl/nm-setting-dcb.c +++ b/src/libnm-core-impl/nm-setting-dcb.c @@ -607,17 +607,10 @@ verify(NMSetting *setting, NMConnection *connection, GError **error) error)) return FALSE; - if (!priv->app_fcoe_mode) { - g_set_error_literal(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_MISSING_PROPERTY, - _("property missing")); - g_prefix_error(error, "%s.%s: ", NM_SETTING_DCB_SETTING_NAME, NM_SETTING_DCB_APP_FCOE_MODE); - return FALSE; - } - - if (strcmp(priv->app_fcoe_mode, NM_SETTING_DCB_FCOE_MODE_FABRIC) - && strcmp(priv->app_fcoe_mode, NM_SETTING_DCB_FCOE_MODE_VN2VN)) { + if (!NM_IN_STRSET(priv->app_fcoe_mode, + NULL, + NM_SETTING_DCB_FCOE_MODE_FABRIC, + NM_SETTING_DCB_FCOE_MODE_VN2VN)) { g_set_error_literal(error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, @@ -888,7 +881,6 @@ nm_setting_dcb_init(NMSettingDcb *self) { NMSettingDcbPrivate *priv = NM_SETTING_DCB_GET_PRIVATE(self); - priv->app_fcoe_mode = g_strdup(NM_SETTING_DCB_FCOE_MODE_FABRIC); priv->app_fcoe_priority = -1; priv->app_fip_priority = -1; priv->app_iscsi_priority = -1; @@ -982,7 +974,10 @@ nm_setting_dcb_class_init(NMSettingDcbClass *klass) * NMSettingDcb:app-fcoe-mode: * * The FCoE controller mode; either %NM_SETTING_DCB_FCOE_MODE_FABRIC - * (default) or %NM_SETTING_DCB_FCOE_MODE_VN2VN. + * or %NM_SETTING_DCB_FCOE_MODE_VN2VN. + * + * Since 1.34, %NULL is the default and means %NM_SETTING_DCB_FCOE_MODE_FABRIC. + * Before 1.34, %NULL was rejected as invalid and the default was %NM_SETTING_DCB_FCOE_MODE_FABRIC. **/ /* ---ifcfg-rh--- * property: app-fcoe-mode @@ -996,7 +991,7 @@ nm_setting_dcb_class_init(NMSettingDcbClass *klass) g_param_spec_string(NM_SETTING_DCB_APP_FCOE_MODE, "", "", - NM_SETTING_DCB_FCOE_MODE_FABRIC, + NULL, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); /** diff --git a/src/libnm-core-impl/tests/test-setting.c b/src/libnm-core-impl/tests/test-setting.c index e263b2143..b71b49fed 100644 --- a/src/libnm-core-impl/tests/test-setting.c +++ b/src/libnm-core-impl/tests/test-setting.c @@ -4647,24 +4647,17 @@ check_done:; g_object_get_property(G_OBJECT(setting), sip->name, &val); if (sip->param_spec->value_type == G_TYPE_STRING) { - const char *exp_default_value = NULL; - const char *default_value; - - default_value = ((const GParamSpecString *) sip->param_spec)->default_value; - - /* having a string property with a default != NULL is really ugly. They - * should be best avoided... Only one property is known to have a non-NULL - * default. Assert that this stays. */ - if (meta_type == NM_META_SETTING_TYPE_DCB - && nm_streq(sip->name, NM_SETTING_DCB_APP_FCOE_MODE)) { - exp_default_value = NM_SETTING_DCB_FCOE_MODE_FABRIC; - } - g_assert_cmpstr(default_value, ==, exp_default_value); + /* String properties should all have a default value of NULL. Otherwise, + * it's ugly. */ + g_assert_cmpstr(((const GParamSpecString *) sip->param_spec)->default_value, + ==, + NULL); + g_assert(!NM_G_PARAM_SPEC_GET_DEFAULT_STRING(sip->param_spec)); if (nm_streq(sip->name, NM_SETTING_NAME)) g_assert_cmpstr(g_value_get_string(&val), ==, msi->setting_name); else - g_assert_cmpstr(g_value_get_string(&val), ==, default_value); + g_assert_cmpstr(g_value_get_string(&val), ==, NULL); } if (NM_FLAGS_HAS(sip->param_spec->flags, NM_SETTING_PARAM_TO_DBUS_IGNORE_FLAGS)) diff --git a/src/libnmc-setting/settings-docs.h.in b/src/libnmc-setting/settings-docs.h.in index d9dba23ee..06346b1eb 100644 --- a/src/libnmc-setting/settings-docs.h.in +++ b/src/libnmc-setting/settings-docs.h.in @@ -178,7 +178,7 @@ #define DESCRIBE_DOC_NM_SETTING_CONNECTION_WAIT_DEVICE_TIMEOUT N_("Timeout in milliseconds to wait for device at startup. During boot, devices may take a while to be detected by the driver. This property will cause to delay NetworkManager-wait-online.service and nm-online to give the device a chance to appear. This works by waiting for the given timeout until a compatible device for the profile is available and managed. The value 0 means no wait time. The default value is -1, which currently has the same meaning as no wait time.") #define DESCRIBE_DOC_NM_SETTING_CONNECTION_ZONE N_("The trust level of a the connection. Free form case-insensitive string (for example \"Home\", \"Work\", \"Public\"). NULL or unspecified zone means the connection will be placed in the default zone as defined by the firewall. When updating this property on a currently activated connection, the change takes effect immediately.") #define DESCRIBE_DOC_NM_SETTING_DCB_APP_FCOE_FLAGS N_("Specifies the NMSettingDcbFlags for the DCB FCoE application. Flags may be any combination of NM_SETTING_DCB_FLAG_ENABLE (0x1), NM_SETTING_DCB_FLAG_ADVERTISE (0x2), and NM_SETTING_DCB_FLAG_WILLING (0x4).") -#define DESCRIBE_DOC_NM_SETTING_DCB_APP_FCOE_MODE N_("The FCoE controller mode; either \"fabric\" (default) or \"vn2vn\".") +#define DESCRIBE_DOC_NM_SETTING_DCB_APP_FCOE_MODE N_("The FCoE controller mode; either \"fabric\" or \"vn2vn\". Since 1.34, NULL is the default and means \"fabric\". Before 1.34, NULL was rejected as invalid and the default was \"fabric\".") #define DESCRIBE_DOC_NM_SETTING_DCB_APP_FCOE_PRIORITY N_("The highest User Priority (0 - 7) which FCoE frames should use, or -1 for default priority. Only used when the \"app-fcoe-flags\" property includes the NM_SETTING_DCB_FLAG_ENABLE (0x1) flag.") #define DESCRIBE_DOC_NM_SETTING_DCB_APP_FIP_FLAGS N_("Specifies the NMSettingDcbFlags for the DCB FIP application. Flags may be any combination of NM_SETTING_DCB_FLAG_ENABLE (0x1), NM_SETTING_DCB_FLAG_ADVERTISE (0x2), and NM_SETTING_DCB_FLAG_WILLING (0x4).") #define DESCRIBE_DOC_NM_SETTING_DCB_APP_FIP_PRIORITY N_("The highest User Priority (0 - 7) which FIP frames should use, or -1 for default priority. Only used when the \"app-fip-flags\" property includes the NM_SETTING_DCB_FLAG_ENABLE (0x1) flag.") diff --git a/src/nmcli/generate-docs-nm-settings-nmcli.xml.in b/src/nmcli/generate-docs-nm-settings-nmcli.xml.in index 46339e59c..02fd8800f 100644 --- a/src/nmcli/generate-docs-nm-settings-nmcli.xml.in +++ b/src/nmcli/generate-docs-nm-settings-nmcli.xml.in @@ -430,7 +430,7 @@ + description="The FCoE controller mode; either "fabric" or "vn2vn". Since 1.34, NULL is the default and means "fabric". Before 1.34, NULL was rejected as invalid and the default was "fabric"." />