From 40564c5d6a29730cbb7bb4da469fe8d9dfe26620 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 8 Nov 2022 21:50:42 +0100 Subject: [PATCH] ifcfg-rh: fix persisting all-default NMSettingEthtool with autoneg/wol flags Fixes: 26ed9e67140a ('ifcfg-rh: fix persisting all-default NMSettingEthtool settings') (cherry picked from commit 4303d33727aac305f183dd640a93ef44a34465ae) --- src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 7 ++++++- .../ifcfg-test_roundtrip_ethtool-8.cexpected | 2 +- src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 7 ------- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c index 48b2b3f2e1..e8948c3dd0 100644 --- a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c +++ b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c @@ -1312,6 +1312,7 @@ write_ethtool_setting(NMConnection *connection, shvarFile *ifcfg, GError **error gboolean is_first; guint32 u32; gboolean b; + gboolean any_option = FALSE; s_con = nm_connection_get_setting_connection(connection); if (s_con) { @@ -1341,6 +1342,7 @@ write_ethtool_setting(NMConnection *connection, shvarFile *ifcfg, GError **error g_string_append_c(str, ' '); g_string_append(str, nms_ifcfg_rh_utils_get_ethtool_name(ethtool_id)); g_string_append(str, b ? " on" : " off"); + any_option = TRUE; } is_first = TRUE; @@ -1356,6 +1358,7 @@ write_ethtool_setting(NMConnection *connection, shvarFile *ifcfg, GError **error g_string_append_c(str, ' '); g_string_append(str, nms_ifcfg_rh_utils_get_ethtool_name(ethtool_id)); g_string_append_printf(str, " %" G_GUINT32_FORMAT, u32); + any_option = TRUE; } is_first = TRUE; @@ -1371,6 +1374,7 @@ write_ethtool_setting(NMConnection *connection, shvarFile *ifcfg, GError **error g_string_append_c(str, ' '); g_string_append(str, nms_ifcfg_rh_utils_get_ethtool_name(ethtool_id)); g_string_append_printf(str, " %" G_GUINT32_FORMAT, u32); + any_option = TRUE; } is_first = TRUE; @@ -1386,9 +1390,10 @@ write_ethtool_setting(NMConnection *connection, shvarFile *ifcfg, GError **error g_string_append_c(str, ' '); g_string_append(str, nms_ifcfg_rh_utils_get_ethtool_name(ethtool_id)); g_string_append(str, b ? " on" : " off"); + any_option = TRUE; } - if (!str) { + if (!any_option) { /* Write an empty dummy "-A" option without arguments. This is to * ensure that the reader will create an (all default) NMSettingEthtool. * Also, it seems that `ethtool -A "$IFACE"` is silently accepted. */ diff --git a/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-8.cexpected b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-8.cexpected index 784ad22427..fd76083936 100644 --- a/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-8.cexpected +++ b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-8.cexpected @@ -1,7 +1,7 @@ TYPE=Ethernet PROXY_METHOD=none BROWSER_ONLY=no -ETHTOOL_OPTS="autoneg on ; -A net0 pause-autoneg off" +ETHTOOL_OPTS="autoneg on ; -A net0" BOOTPROTO=dhcp DEFROUTE=yes IPV4_FAILURE_FATAL=no diff --git a/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index 49ca9afcc0..886a605fb2 100644 --- a/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -3527,8 +3527,6 @@ test_roundtrip_ethtool(void) g_object_set(s_wired, NM_SETTING_WIRED_AUTO_NEGOTIATE, TRUE, NULL); s_ethtool = _nm_connection_new_setting(connection, NM_TYPE_SETTING_ETHTOOL); - /* FIXME: ensure proper round-trip with no ethtool settings set. */ - nm_setting_option_set_boolean(s_ethtool, NM_ETHTOOL_OPTNAME_PAUSE_AUTONEG, FALSE); _writer_new_connec_exp(connection, TEST_SCRATCH_DIR, TEST_IFCFG_DIR "/ifcfg-test_roundtrip_ethtool-8.cexpected", @@ -3633,11 +3631,6 @@ test_roundtrip_ethtool(void) nmtst_get_rand_bool() ? g_variant_new_boolean(FALSE) : NULL); } - if (!nm_setting_option_get_all_names(s_ethtool, NULL)) { - // FIXME. - continue; - } - check_roundtrip: _writer_new_connection_reread(con2, TEST_SCRATCH_DIR,