From 5490604084589b5387601b5821f86b76d8497001 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Mon, 17 Jul 2023 10:54:42 +0200 Subject: [PATCH 1/2] nmcli: move offline flag from NmCli to NmcConfig struct This flag is a setting that changes the behaviour of nmcli, it's not only the current state of the program, so it makes more sense to put it in NmcConfig than in NmCli. Furthermore, it's needed to fix a bug in next commit, too. --- src/nmcli/common.c | 2 +- src/nmcli/connections.c | 12 ++++++------ src/nmcli/nmcli.c | 2 +- src/nmcli/nmcli.h | 6 +++--- src/nmcli/settings.c | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/nmcli/common.c b/src/nmcli/common.c index fcf1ed81d0..01fe9a375f 100644 --- a/src/nmcli/common.c +++ b/src/nmcli/common.c @@ -1371,7 +1371,7 @@ call_cmd(NmCli *nmc, GTask *task, const NMCCommand *cmd, int argc, const char *c { CmdCall *call; - if (nmc->offline) { + if (nmc->nmc_config.offline) { if (!cmd->supports_offline) { g_task_return_new_error(task, NMCLI_ERROR, diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index 38ce23cf0a..5a8ed2a8d9 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -237,7 +237,7 @@ error: static const GPtrArray * nmc_get_connections(const NmCli *nmc) { - if (nmc->offline) { + if (nmc->nmc_config.offline) { g_return_val_if_fail(!nmc->client, nmc->offline_connections); return nmc->offline_connections; } else { @@ -251,7 +251,7 @@ nmc_get_active_connections(const NmCli *nmc) { static const GPtrArray offline_active_connections = {.len = 0}; - if (nmc->offline) { + if (nmc->nmc_config.offline) { g_return_val_if_fail(!nmc->client, &offline_active_connections); return &offline_active_connections; } else { @@ -2242,7 +2242,7 @@ get_connection(NmCli *nmc, if (nmc->offline_connections && nmc->offline_connections->len) return nmc->offline_connections->pdata[0]; - g_return_val_if_fail(!nmc->offline, NULL); + g_return_val_if_fail(!nmc->nmc_config.offline, NULL); if (*argc == 0) { g_set_error_literal(error, @@ -5844,7 +5844,7 @@ again: static void nmc_add_connection(NmCli *nmc, NMConnection *connection, gboolean temporary) { - if (nmc->offline) { + if (nmc->nmc_config.offline) { nmc_print_connection_and_quit(nmc, connection); } else { add_connection(nmc->client, @@ -9146,7 +9146,7 @@ modify_connection_cb(GObject *connection, GAsyncResult *result, gpointer user_da static void nmc_update_connection(NmCli *nmc, NMConnection *connection, gboolean temporary) { - if (nmc->offline) { + if (nmc->nmc_config.offline) { nmc_print_connection_and_quit(nmc, connection); } else { nm_remote_connection_commit_changes_async(NM_REMOTE_CONNECTION(connection), @@ -9177,7 +9177,7 @@ do_connection_modify(const NMCCommand *cmd, NmCli *nmc, int argc, const char *co } /* Don't insist on having argument if we're running in offline mode. */ - if (!nmc->offline || argc > 0) { + if (!nmc->nmc_config.offline || argc > 0) { if (!nmc_process_connection_properties(nmc, connection, &argc, &argv, TRUE, &error)) { g_string_assign(nmc->return_text, error->message); nmc->return_value = error->code; diff --git a/src/nmcli/nmcli.c b/src/nmcli/nmcli.c index ff496874b4..792edf6e0e 100644 --- a/src/nmcli/nmcli.c +++ b/src/nmcli/nmcli.c @@ -785,7 +785,7 @@ process_command_line(NmCli *nmc, int argc, char **argv_orig) if (matches_arg(nmc, &argc, &argv, "-overview", NULL)) { nmc->nmc_config_mutable.overview = TRUE; } else if (matches_arg(nmc, &argc, &argv, "-offline", NULL)) { - nmc->offline = TRUE; + nmc->nmc_config_mutable.offline = TRUE; } else if (matches_arg(nmc, &argc, &argv, "-terse", NULL)) { if (nmc->nmc_config.print_output == NMC_PRINT_TERSE) { g_string_printf(nmc->return_text, diff --git a/src/nmcli/nmcli.h b/src/nmcli/nmcli.h index f9b4cc7de0..e704c76f5b 100644 --- a/src/nmcli/nmcli.h +++ b/src/nmcli/nmcli.h @@ -132,6 +132,9 @@ typedef struct _NmcConfig { /* Overview mode (hide default values) */ bool overview : 1; + /* Communicate the connection data over stdin/stdout instead of talking to the daemon. */ + bool offline : 1; + NmcColorPalette palette; } NmcConfig; @@ -178,9 +181,6 @@ typedef struct _NmCli { /* Whether tabular/multiline mode was specified via '--mode' option */ bool mode_specified : 1; - /* Communicate the connection data over stdin/stdout instead of talking to the daemon. */ - bool offline : 1; - /* Ask for missing parameters: option '--ask' */ bool ask : 1; diff --git a/src/nmcli/settings.c b/src/nmcli/settings.c index e637bc53b1..4c864d847f 100644 --- a/src/nmcli/settings.c +++ b/src/nmcli/settings.c @@ -498,7 +498,7 @@ _env_get_env_flags(const NMMetaEnvironment *environment, gpointer environment_us nm_assert(nmc); - return (nmc->offline ? NM_META_ENV_FLAGS_OFFLINE : NM_META_ENV_FLAGS_NONE); + return (nmc->nmc_config.offline ? NM_META_ENV_FLAGS_OFFLINE : NM_META_ENV_FLAGS_NONE); } /*****************************************************************************/ From d414265ab17c54f7b5bc27a55536ca99c77ee80a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Mon, 17 Jul 2023 10:56:35 +0200 Subject: [PATCH 2/2] nmcli: fix endless loop with --offline --ask If --offline and --ask were used at the same time, and endless loop showing the readline's prompt but without waiting for user's input happened. This was because when using --offline, all arguments are parsed and resolved before running the g_main_loop. In nmc_readline_helper it was checked that the main loop is running, so if g_main_loop_quit is called we can stop waiting for user's input. Fix this bug by continue polling for user input if the main loop is running or if we are in offline mode. Cancelling the user input is still possible both in normal and offline mode with Ctrl+C or Ctrl+D. Added a test case to verify that this still works after future changes. --- src/nmcli/common.c | 5 +-- src/tests/client/test-client.py | 56 +++++++++++++++++++++++++++++++-- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/nmcli/common.c b/src/nmcli/common.c index 01fe9a375f..2e8ded7e44 100644 --- a/src/nmcli/common.c +++ b/src/nmcli/common.c @@ -880,7 +880,8 @@ read_again: rl_got_line = FALSE; rl_callback_handler_install(prompt, readline_cb); - while (!rl_got_line && g_main_loop_is_running(loop) && !nmc_seen_sigint()) + while (!rl_got_line && (g_main_loop_is_running(loop) || nmc_config->offline) + && !nmc_seen_sigint()) g_main_context_iteration(NULL, TRUE); /* If Ctrl-C was detected, complete the line */ @@ -909,7 +910,7 @@ read_again: } } else if (!rl_string) { /* Ctrl-D, exit */ - if (g_main_loop_is_running(loop)) + if (g_main_loop_is_running(loop) || nmc_config->offline) nmc_exit(); } diff --git a/src/tests/client/test-client.py b/src/tests/client/test-client.py index 391c14d1c3..b618fb9703 100755 --- a/src/tests/client/test-client.py +++ b/src/tests/client/test-client.py @@ -703,11 +703,14 @@ class Util: return typ(pexp, valgrind_log) @staticmethod - def cmd_call_pexpect_nmcli(args): + def cmd_call_pexpect_nmcli(args, extra_env={}): + extra_env = extra_env.copy() + extra_env.update({"NO_COLOR": "1"}) + return Util.cmd_call_pexpect( ENV_NM_TEST_CLIENT_NMCLI_PATH, args, - {"NO_COLOR": "1"}, + extra_env, ) @@ -2158,6 +2161,55 @@ class TestNmcli(unittest.TestCase): nmc.pexp.expect(pexpect.EOF) Util.valgrind_check_log(nmc.valgrind_log, "test_ask_mode") + @Util.skip_without_pexpect + @nm_test + def test_ask_offline(self): + # Make sure we're not using D-Bus + no_dbus_env = { + "DBUS_SYSTEM_BUS_ADDRESS": "very:invalid", + "DBUS_SESSION_BUS_ADDRESS": "very:invalid", + } + + nmc = Util.cmd_call_pexpect_nmcli( + ["--offline", "--ask", "c", "add"], extra_env=no_dbus_env + ) + nmc.pexp.expect("Connection type:") + nmc.pexp.sendline("ethernet") + nmc.pexp.expect("Interface name:") + nmc.pexp.sendline("eth0") + nmc.pexp.expect("There are 3 optional settings for Wired Ethernet.") + nmc.pexp.expect("Do you want to provide them\? \(yes/no\) \[yes]") + nmc.pexp.sendline("no") + nmc.pexp.expect("There are 2 optional settings for IPv4 protocol.") + nmc.pexp.expect("Do you want to provide them\? \(yes/no\) \[yes]") + nmc.pexp.sendline("no") + nmc.pexp.expect("There are 2 optional settings for IPv6 protocol.") + nmc.pexp.expect("Do you want to provide them\? \(yes/no\) \[yes]") + nmc.pexp.sendline("no") + nmc.pexp.expect("There are 4 optional settings for Proxy.") + nmc.pexp.expect("Do you want to provide them\? \(yes/no\) \[yes]") + nmc.pexp.sendline("no") + nmc.pexp.expect( + "\[connection\]\r\n" + + "id=ethernet\r\n" + + "uuid=.*\r\n" + + "type=ethernet\r\n" + + "interface-name=eth0\r\n" + + "\r\n" + + "\[ethernet\]\r\n" + + "\r\n" + + "\[ipv4\]\r\n" + + "method=auto\r\n" + + "\r\n" + + "\[ipv6\]\r\n" + + "addr-gen-mode=default\r\n" + + "method=auto\r\n" + + "\r\n" + + "\[proxy\]\r\n" + ) + nmc.pexp.expect(pexpect.EOF) + Util.valgrind_check_log(nmc.valgrind_log, "test_ask_offline") + @Util.skip_without_pexpect @nm_test def test_monitor(self):