ifcfg-rh: merge branch 'th/ifcfg-fix-empty-ethtool'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/905

(cherry picked from commit f9040c3ccae84b9b483f015f8e7bd2e503132f82)

8 files changed, 186 insertions, 13 deletions

diff --git a/Makefile.am b/Makefile.am
index e57d01f11c..fd1342a925 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3431,8 +3431,8 @@ EXTRA_DIST += \
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-static-routes-legacy \
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-static-routes-legacy.cexpected \
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc \
-	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc-write.cexpected \
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc-write-empty.cexpected \
+	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc-write.cexpected \
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-1 \
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-2 \
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-invalid \
@@ -3537,6 +3537,11 @@ EXTRA_DIST += \
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-write-unknown-3.expected \
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-write-unknown-4 \
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-write-unknown-4.expected \
+	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-1.cexpected \
+	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-2.cexpected \
+	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-3.cexpected \
+	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-4.cexpected \
+	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-5.cexpected \
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_write_wired_auto_negotiate_on.cexpected \
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/keys-test-wifi-dynamic-wep-leap \
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/keys-test-wifi-leap \
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 82516a6692..1c5bbbb43c 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
@@ -1376,6 +1376,13 @@ write_ethtool_setting(NMConnection *connection, shvarFile *ifcfg, GError **error
             g_string_append(str, nms_ifcfg_rh_utils_get_ethtool_name(ethtool_id));
             g_string_append(str, b ? " on" : " off");
         }
+
+        if (!str) {
+            /* 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. */
+            _ethtool_gstring_prepare(&str, &is_first, 'A', iface);
+        }
     }
 
     if (str) {
diff --git a/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-1.cexpected b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-1.cexpected
new file mode 100644
index 0000000000..5186f90c5e
--- /dev/null
+++ b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-1.cexpected
@@ -0,0 +1,14 @@
+TYPE=Ethernet
+PROXY_METHOD=none
+BROWSER_ONLY=no
+BOOTPROTO=dhcp
+DEFROUTE=yes
+IPV4_FAILURE_FATAL=no
+IPV6INIT=yes
+IPV6_AUTOCONF=yes
+IPV6_DEFROUTE=yes
+IPV6_FAILURE_FATAL=no
+IPV6_ADDR_GEN_MODE=stable-privacy
+NAME=test_roundtrip_ethtool
+UUID=${UUID}
+ONBOOT=yes
diff --git a/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-2.cexpected b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-2.cexpected
new file mode 100644
index 0000000000..5632ebec8e
--- /dev/null
+++ b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-2.cexpected
@@ -0,0 +1,15 @@
+TYPE=Ethernet
+PROXY_METHOD=none
+BROWSER_ONLY=no
+ETHTOOL_OPTS="-A net0"
+BOOTPROTO=dhcp
+DEFROUTE=yes
+IPV4_FAILURE_FATAL=no
+IPV6INIT=yes
+IPV6_AUTOCONF=yes
+IPV6_DEFROUTE=yes
+IPV6_FAILURE_FATAL=no
+IPV6_ADDR_GEN_MODE=stable-privacy
+NAME=test_roundtrip_ethtool
+UUID=${UUID}
+ONBOOT=yes
diff --git a/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-3.cexpected b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-3.cexpected
new file mode 100644
index 0000000000..96eec43bb4
--- /dev/null
+++ b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-3.cexpected
@@ -0,0 +1,15 @@
+TYPE=Ethernet
+PROXY_METHOD=none
+BROWSER_ONLY=no
+ETHTOOL_OPTS="autoneg on"
+BOOTPROTO=dhcp
+DEFROUTE=yes
+IPV4_FAILURE_FATAL=no
+IPV6INIT=yes
+IPV6_AUTOCONF=yes
+IPV6_DEFROUTE=yes
+IPV6_FAILURE_FATAL=no
+IPV6_ADDR_GEN_MODE=stable-privacy
+NAME=test_roundtrip_ethtool
+UUID=${UUID}
+ONBOOT=yes
diff --git a/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-4.cexpected b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-4.cexpected
new file mode 100644
index 0000000000..90250a3051
--- /dev/null
+++ b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-4.cexpected
@@ -0,0 +1,15 @@
+TYPE=Ethernet
+PROXY_METHOD=none
+BROWSER_ONLY=no
+ETHTOOL_OPTS="-K net0 rx on"
+BOOTPROTO=dhcp
+DEFROUTE=yes
+IPV4_FAILURE_FATAL=no
+IPV6INIT=yes
+IPV6_AUTOCONF=yes
+IPV6_DEFROUTE=yes
+IPV6_FAILURE_FATAL=no
+IPV6_ADDR_GEN_MODE=stable-privacy
+NAME=test_roundtrip_ethtool
+UUID=${UUID}
+ONBOOT=yes
diff --git a/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-5.cexpected b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-5.cexpected
new file mode 100644
index 0000000000..721eacb282
--- /dev/null
+++ b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test_roundtrip_ethtool-5.cexpected
@@ -0,0 +1,15 @@
+TYPE=Ethernet
+PROXY_METHOD=none
+BROWSER_ONLY=no
+ETHTOOL_OPTS="autoneg on ; -K net0 rx on"
+BOOTPROTO=dhcp
+DEFROUTE=yes
+IPV4_FAILURE_FATAL=no
+IPV6INIT=yes
+IPV6_AUTOCONF=yes
+IPV6_DEFROUTE=yes
+IPV6_FAILURE_FATAL=no
+IPV6_ADDR_GEN_MODE=stable-privacy
+NAME=test_roundtrip_ethtool
+UUID=${UUID}
+ONBOOT=yes
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 5516171e5f..6193b952b9 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
@@ -181,8 +181,20 @@ _assert_expected_content(NMConnection *connection, const char *filename, const c
     }
 
     if (len_expectd != len_written || memcmp(content_expectd, content_written, len_expectd) != 0) {
-        if (g_getenv("NMTST_IFCFG_RH_UPDATE_EXPECTED")
-            || nm_streq0(g_getenv("NM_TEST_REGENERATE"), "1")) {
+        static int rewrite_static = 0;
+        int        rewrite;
+
+        rewrite = g_atomic_int_get(&rewrite_static);
+        if (G_UNLIKELY(rewrite == 0)) {
+            rewrite = (g_getenv("NMTST_IFCFG_RH_UPDATE_EXPECTED")
+                       || nm_streq0(g_getenv("NM_TEST_REGENERATE"), "1"))
+                          ? -1
+                          : 1;
+            if (!g_atomic_int_compare_and_exchange(&rewrite_static, 0, rewrite))
+                g_assert_not_reached();
+        }
+
+        if (rewrite > 0) {
             if (uuid) {
                 gs_free char *search = g_strdup_printf("UUID=%s\n", uuid);
                 const char *  s;
@@ -210,15 +222,16 @@ _assert_expected_content(NMConnection *connection, const char *filename, const c
             success = g_file_set_contents(expected, content_written, len_written, &error);
             nmtst_assert_success(success, error);
         } else {
-            g_error("The content of \"%s\" (%zu) differs from \"%s\" (%zu). Set "
-                    "NMTST_IFCFG_RH_UPDATE_EXPECTED=yes to update the files "
-                    "inplace\n\n>>>%s<<<\n\n>>>%s<<<\n",
-                    filename,
-                    len_written,
-                    expected,
-                    len_expectd,
-                    content_written,
-                    content_expectd);
+            g_error(
+                "The content of \"%s\" (%zu) differs from \"%s\" (%zu). Set "
+                "NMTST_IFCFG_RH_UPDATE_EXPECTED=yes (or NM_TEST_REGENERATE=1) to update the files "
+                "inplace\n\n>>>%s<<<\n\n>>>%s<<<\n",
+                filename,
+                len_written,
+                expected,
+                len_expectd,
+                content_written,
+                content_expectd);
         }
     }
 }
@@ -360,8 +373,10 @@ _writer_new_connection_reread(NMConnection * connection,
 
     if (out_filename)
         *out_filename = filename;
-    else
+    else {
+        nmtst_file_unlink(filename);
         g_free(filename);
+    }
 }
 
 static void
@@ -3934,6 +3949,76 @@ test_read_wired_unknown_ethtool_opt(void)
 }
 
 static void
+test_roundtrip_ethtool(void)
+{
+    gs_unref_object NMConnection *connection = NULL;
+    NMSetting *                   s_ethtool;
+    NMSetting *                   s_wired;
+
+    connection = nmtst_create_minimal_connection("test_roundtrip_ethtool",
+                                                 NULL,
+                                                 NM_SETTING_WIRED_SETTING_NAME,
+                                                 NULL);
+    _writer_new_connec_exp(connection,
+                           TEST_SCRATCH_DIR,
+                           TEST_IFCFG_DIR "/ifcfg-test_roundtrip_ethtool-1.cexpected",
+                           NULL);
+    g_clear_object(&connection);
+
+    connection = nmtst_create_minimal_connection("test_roundtrip_ethtool",
+                                                 NULL,
+                                                 NM_SETTING_WIRED_SETTING_NAME,
+                                                 NULL);
+    s_ethtool  = nm_setting_ethtool_new();
+    nm_connection_add_setting(connection, s_ethtool);
+    _writer_new_connec_exp(connection,
+                           TEST_SCRATCH_DIR,
+                           TEST_IFCFG_DIR "/ifcfg-test_roundtrip_ethtool-2.cexpected",
+                           NULL);
+    g_clear_object(&connection);
+
+    connection = nmtst_create_minimal_connection("test_roundtrip_ethtool",
+                                                 NULL,
+                                                 NM_SETTING_WIRED_SETTING_NAME,
+                                                 NULL);
+    s_wired    = nm_connection_get_setting(connection, NM_TYPE_SETTING_WIRED);
+    g_object_set(s_wired, NM_SETTING_WIRED_AUTO_NEGOTIATE, TRUE, NULL);
+    _writer_new_connec_exp(connection,
+                           TEST_SCRATCH_DIR,
+                           TEST_IFCFG_DIR "/ifcfg-test_roundtrip_ethtool-3.cexpected",
+                           NULL);
+    g_clear_object(&connection);
+
+    connection = nmtst_create_minimal_connection("test_roundtrip_ethtool",
+                                                 NULL,
+                                                 NM_SETTING_WIRED_SETTING_NAME,
+                                                 NULL);
+    s_ethtool  = nm_setting_ethtool_new();
+    nm_connection_add_setting(connection, s_ethtool);
+    nm_setting_option_set_boolean(s_ethtool, NM_ETHTOOL_OPTNAME_FEATURE_RX, TRUE);
+    _writer_new_connec_exp(connection,
+                           TEST_SCRATCH_DIR,
+                           TEST_IFCFG_DIR "/ifcfg-test_roundtrip_ethtool-4.cexpected",
+                           NULL);
+    g_clear_object(&connection);
+
+    connection = nmtst_create_minimal_connection("test_roundtrip_ethtool",
+                                                 NULL,
+                                                 NM_SETTING_WIRED_SETTING_NAME,
+                                                 NULL);
+    s_wired    = nm_connection_get_setting(connection, NM_TYPE_SETTING_WIRED);
+    g_object_set(s_wired, NM_SETTING_WIRED_AUTO_NEGOTIATE, TRUE, NULL);
+    s_ethtool = nm_setting_ethtool_new();
+    nm_connection_add_setting(connection, s_ethtool);
+    nm_setting_option_set_boolean(s_ethtool, NM_ETHTOOL_OPTNAME_FEATURE_RX, TRUE);
+    _writer_new_connec_exp(connection,
+                           TEST_SCRATCH_DIR,
+                           TEST_IFCFG_DIR "/ifcfg-test_roundtrip_ethtool-5.cexpected",
+                           NULL);
+    g_clear_object(&connection);
+}
+
+static void
 test_read_wifi_hidden(void)
 {
     NMConnection *       connection;
@@ -11633,6 +11718,8 @@ main(int argc, char **argv)
     g_test_add_func(TPATH "802-1x/password_raw", test_read_write_802_1x_password_raw);
     g_test_add_func(TPATH "802-1x/tls-p12-no-client-cert", test_read_802_1x_tls_p12_no_client_cert);
 
+    g_test_add_func(TPATH "wired/roundtrip/ethtool", test_roundtrip_ethtool);
+
     g_test_add_data_func(TPATH "wired/read/aliases/good/0",
                          GINT_TO_POINTER(0),
                          test_read_wired_aliases_good);