From 0cde514252e91b77394639f298196b40821457f5 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Mon, 17 Oct 2016 16:00:10 +0200 Subject: [PATCH 1/6] cli/trivial: fix some whitespace errors --- clients/cli/nmcli.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/clients/cli/nmcli.c b/clients/cli/nmcli.c index 6e27c3d668..abc344fcbb 100644 --- a/clients/cli/nmcli.c +++ b/clients/cli/nmcli.c @@ -282,7 +282,7 @@ parse_command_line (NmCli *nmc, int argc, char **argv) } else if (matches (opt, "-mode") == 0) { nmc->mode_specified = TRUE; if (next_arg (&argc, &argv) != 0) { - g_string_printf (nmc->return_text, _("Error: missing argument for '%s' option."), opt); + g_string_printf (nmc->return_text, _("Error: missing argument for '%s' option."), opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; return nmc->return_value; } @@ -293,13 +293,13 @@ parse_command_line (NmCli *nmc, int argc, char **argv) else if (matches (argv[0], "multiline") == 0) nmc->multiline_output = TRUE; else { - g_string_printf (nmc->return_text, _("Error: '%s' is not valid argument for '%s' option."), argv[0], opt); + g_string_printf (nmc->return_text, _("Error: '%s' is not valid argument for '%s' option."), argv[0], opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; return nmc->return_value; } } else if (matches (opt, "-colors") == 0) { if (next_arg (&argc, &argv) != 0) { - g_string_printf (nmc->return_text, _("Error: missing argument for '%s' option."), opt); + g_string_printf (nmc->return_text, _("Error: missing argument for '%s' option."), opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; return nmc->return_value; } @@ -312,13 +312,13 @@ parse_command_line (NmCli *nmc, int argc, char **argv) else if (matches (argv[0], "no") == 0) nmc->use_colors = NMC_USE_COLOR_NO; else { - g_string_printf (nmc->return_text, _("Error: '%s' is not valid argument for '%s' option."), argv[0], opt); + g_string_printf (nmc->return_text, _("Error: '%s' is not valid argument for '%s' option."), argv[0], opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; return nmc->return_value; } } else if (matches (opt, "-escape") == 0) { if (next_arg (&argc, &argv) != 0) { - g_string_printf (nmc->return_text, _("Error: missing argument for '%s' option."), opt); + g_string_printf (nmc->return_text, _("Error: missing argument for '%s' option."), opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; return nmc->return_value; } @@ -329,13 +329,13 @@ parse_command_line (NmCli *nmc, int argc, char **argv) else if (matches (argv[0], "no") == 0) nmc->escape_values = FALSE; else { - g_string_printf (nmc->return_text, _("Error: '%s' is not valid argument for '%s' option."), argv[0], opt); + g_string_printf (nmc->return_text, _("Error: '%s' is not valid argument for '%s' option."), argv[0], opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; return nmc->return_value; } } else if (matches (opt, "-fields") == 0) { if (next_arg (&argc, &argv) != 0) { - g_string_printf (nmc->return_text, _("Error: fields for '%s' options are missing."), opt); + g_string_printf (nmc->return_text, _("Error: fields for '%s' options are missing."), opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; return nmc->return_value; } @@ -351,13 +351,13 @@ parse_command_line (NmCli *nmc, int argc, char **argv) } else if (matches (opt, "-wait") == 0) { unsigned long timeout; if (next_arg (&argc, &argv) != 0) { - g_string_printf (nmc->return_text, _("Error: missing argument for '%s' option."), opt); + g_string_printf (nmc->return_text, _("Error: missing argument for '%s' option."), opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; return nmc->return_value; } if (!nmc_string_to_uint (argv[0], TRUE, 0, G_MAXINT, &timeout)) { - g_string_printf (nmc->return_text, _("Error: '%s' is not a valid timeout for '%s' option."), - argv[0], opt); + g_string_printf (nmc->return_text, _("Error: '%s' is not a valid timeout for '%s' option."), + argv[0], opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; return nmc->return_value; } From 6499bb893f6965314738592a6ac2ca2184dad3d7 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Wed, 31 Aug 2016 22:50:02 +0200 Subject: [PATCH 2/6] cli: get rid of client-global connections list Caching it in the NmCli object is unnecessary, ugly and would be cumbersome in future when we'll be creating the client object only when needed. --- clients/cli/common.c | 4 +- clients/cli/connections.c | 98 ++++++++++++++++++++++++--------------- clients/cli/devices.c | 2 - clients/cli/nmcli.c | 2 - clients/cli/nmcli.h | 2 - clients/cli/settings.c | 6 ++- 6 files changed, 67 insertions(+), 47 deletions(-) diff --git a/clients/cli/common.c b/clients/cli/common.c index 8e25dbffd3..a2f9adc480 100644 --- a/clients/cli/common.c +++ b/clients/cli/common.c @@ -1217,6 +1217,7 @@ nmc_secrets_requested (NMSecretAgentSimple *agent, NMConnection *connection = NULL; char *path, *p; gboolean success = FALSE; + const GPtrArray *connections; if (nmc->print_output == NMC_PRINT_PRETTY) nmc_terminal_erase_line (); @@ -1227,7 +1228,8 @@ nmc_secrets_requested (NMSecretAgentSimple *agent, p = strrchr (path, '/'); if (p) *p = '\0'; - connection = nmc_find_connection (nmc->connections, "path", path, NULL, FALSE); + connections = nm_client_get_connections (nmc->client); + connection = nmc_find_connection (connections, "path", path, NULL, FALSE); g_free (path); } diff --git a/clients/cli/connections.c b/clients/cli/connections.c index f99449ffd2..ab6c578af1 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -1579,6 +1579,7 @@ static GPtrArray * get_invisible_active_connections (NmCli *nmc) { const GPtrArray *acons; + const GPtrArray *connections; GPtrArray *invisibles; int a, c; @@ -1586,13 +1587,14 @@ get_invisible_active_connections (NmCli *nmc) invisibles = g_ptr_array_new (); acons = nm_client_get_active_connections (nmc->client); + connections = nm_client_get_connections (nmc->client); for (a = 0; a < acons->len; a++) { gboolean found = FALSE; NMActiveConnection *acon = g_ptr_array_index (acons, a); const char *a_uuid = nm_active_connection_get_uuid (acon); - for (c = 0; c < nmc->connections->len; c++) { - NMConnection *con = g_ptr_array_index (nmc->connections, c); + for (c = 0; c < connections->len; c++) { + NMConnection *con = g_ptr_array_index (connections, c); const char *c_uuid = nm_connection_get_uuid (con); if (strcmp (a_uuid, c_uuid) == 0) { @@ -1671,6 +1673,7 @@ parse_preferred_connection_order (const char *order, GError **error) static NMConnection * get_connection (NmCli *nmc, int *argc, char ***argv, int *pos, GError **error) { + const GPtrArray *connections; NMConnection *connection = NULL; const char *selector = NULL; @@ -1694,7 +1697,8 @@ get_connection (NmCli *nmc, int *argc, char ***argv, int *pos, GError **error) } } - connection = nmc_find_connection (nmc->connections, selector, **argv, pos, + connections = nm_client_get_connections (nmc->client); + connection = nmc_find_connection (connections, selector, **argv, pos, *argc == 1 && nmc->complete); if (!connection) { g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_NOT_FOUND, @@ -1754,6 +1758,7 @@ do_connections_show (NmCli *nmc, int argc, char **argv) show_secrets = nmc->show_secrets || show_secrets; if (argc == 0) { + const GPtrArray *connections; char *fields_str; char *fields_all = NMC_FIELDS_CON_SHOW_ALL; char *fields_common = NMC_FIELDS_CON_SHOW_COMMON; @@ -1793,7 +1798,8 @@ do_connections_show (NmCli *nmc, int argc, char **argv) g_ptr_array_free (invisibles, TRUE); /* Sort the connections and fill the output data */ - sorted_cons = sort_connections (nmc->connections, nmc, order); + connections = nm_client_get_connections (nmc->client); + sorted_cons = sort_connections (connections, nmc, order); for (i = 0; i < sorted_cons->len; i++) fill_output_connection (sorted_cons->pdata[i], nmc, active_only); g_ptr_array_free (sorted_cons, TRUE); @@ -1816,6 +1822,7 @@ do_connections_show (NmCli *nmc, int argc, char **argv) nmc->required_fields = NULL; while (argc > 0) { + const GPtrArray *connections; gboolean res; NMConnection *con; NMActiveConnection *acon = NULL; @@ -1837,11 +1844,12 @@ do_connections_show (NmCli *nmc, int argc, char **argv) } /* Try to find connection by id, uuid or path first */ - con = nmc_find_connection (nmc->connections, selector, *argv, &pos, + connections = nm_client_get_connections (nmc->client); + con = nmc_find_connection (connections, selector, *argv, &pos, argc == 1 && nmc->complete); if (!con && (!selector || strcmp (selector, "apath") == 0)) { /* Try apath too */ - acon = find_active_connection (active_cons, nmc->connections, "apath", *argv, NULL, + acon = find_active_connection (active_cons, connections, "apath", *argv, NULL, argc == 1 && nmc->complete); if (acon) con = NM_CONNECTION (nm_active_connection_get_connection (acon)); @@ -2784,6 +2792,7 @@ do_connection_down (NmCli *nmc, int argc, char **argv) /* Get active connections */ active_cons = nm_client_get_active_connections (nmc->client); while (arg_num > 0) { + const GPtrArray *connections; const char *selector = NULL; if (arg_num == 1) @@ -2802,7 +2811,8 @@ do_connection_down (NmCli *nmc, int argc, char **argv) } } - active = find_active_connection (active_cons, nmc->connections, selector, *arg_ptr, &idx, + connections = nm_client_get_connections (nmc->client); + active = find_active_connection (active_cons, connections, selector, *arg_ptr, &idx, arg_num == 1 && nmc->complete); if (active) { /* Check if the connection is unique. */ @@ -3522,19 +3532,22 @@ unique_master_iface_ifname (const GPtrArray *connections, static void set_default_interface_name (NmCli *nmc, NMSettingConnection *s_con) { + const GPtrArray *connections; char *ifname = NULL; const char *con_type = nm_setting_connection_get_connection_type (s_con); if (nm_setting_connection_get_interface_name (s_con)) return; + connections = nm_client_get_connections (nmc->client); + /* Set a sensible bond/team/bridge interface name by default */ if (g_strcmp0 (con_type, NM_SETTING_BOND_SETTING_NAME) == 0) - ifname = unique_master_iface_ifname (nmc->connections, "nm-bond"); + ifname = unique_master_iface_ifname (connections, "nm-bond"); else if (g_strcmp0 (con_type, NM_SETTING_TEAM_SETTING_NAME) == 0) - ifname = unique_master_iface_ifname (nmc->connections, "nm-team"); + ifname = unique_master_iface_ifname (connections, "nm-team"); else if (g_strcmp0 (con_type, NM_SETTING_BRIDGE_SETTING_NAME) == 0) - ifname = unique_master_iface_ifname (nmc->connections, "nm-bridge"); + ifname = unique_master_iface_ifname (connections, "nm-bridge"); else return; @@ -3880,16 +3893,16 @@ gen_func_master_ifnames (const char *text, int state) NMConnection *con; NMSettingConnection *s_con; const char *con_type, *ifname; + const GPtrArray *connections; - if (!nm_cli.connections) - return NULL; + connections = nm_client_get_connections (nm_cli.client); /* Disable appending space after completion */ rl_completion_append_character = '\0'; ifnames = g_ptr_array_sized_new (20); - for (i = 0; i < nm_cli.connections->len; i++) { - con = NM_CONNECTION (nm_cli.connections->pdata[i]); + for (i = 0; i < connections->len; i++) { + con = NM_CONNECTION (connections->pdata[i]); s_con = nm_connection_get_setting_connection (con); g_assert (s_con); con_type = nm_setting_connection_get_connection_type (s_con); @@ -3992,6 +4005,7 @@ set_connection_iface (NmCli *nmc, NMConnection *con, OptionInfo *option, const c static gboolean set_connection_master (NmCli *nmc, NMConnection *con, OptionInfo *option, const char *value, GError **error) { + const GPtrArray *connections; NMSettingConnection *s_con; const char *slave_type; @@ -4005,7 +4019,8 @@ set_connection_master (NmCli *nmc, NMConnection *con, OptionInfo *option, const } slave_type = nm_setting_connection_get_slave_type (s_con); - value = normalized_master_for_slave (nmc->connections, value, slave_type, &slave_type); + connections = nm_client_get_connections (nmc->client); + value = normalized_master_for_slave (connections, value, slave_type, &slave_type); if (!set_property (con, NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE, slave_type, @@ -4944,9 +4959,12 @@ read_properties: /* If only bother when there's a type, which is not guaranteed at this point. * Otherwise the validation will fail anyway. */ if (type) { + const GPtrArray *connections; + + connections = nm_client_get_connections (nmc->client); try_name = ifname ? g_strdup_printf ("%s-%s", get_name_alias (type, slave_type, nmc_valid_connection_types), ifname) : g_strdup (get_name_alias (type, slave_type, nmc_valid_connection_types)); - default_name = nmc_unique_connection_name (nmc->connections, try_name); + default_name = nmc_unique_connection_name (connections, try_name); g_free (try_name); g_object_set (s_con, NM_SETTING_CONNECTION_ID, default_name, NULL); g_free (default_name); @@ -5004,12 +5022,14 @@ finish: static void uuid_display_hook (char **array, int len, int max_len) { + const GPtrArray *connections; NMConnection *con; int i, max = 0; char *tmp; const char *id; for (i = 1; i <= len; i++) { - con = nmc_find_connection (nmc_tab_completion.nmc->connections, "uuid", array[i], NULL, FALSE); + connections = nm_client_get_connections (nmc_tab_completion.nmc->client); + con = nmc_find_connection (connections, "uuid", array[i], NULL, FALSE); id = con ? nm_connection_get_id (con) : NULL; if (id) { tmp = g_strdup_printf ("%s (%s)", array[i], id); @@ -5313,10 +5333,11 @@ _create_vpn_array (const GPtrArray *connections, gboolean uuid) static char * gen_vpn_uuids (const char *text, int state) { - const GPtrArray *connections = nm_cli.connections; + const GPtrArray *connections; const char **uuids; char *ret; + connections = nm_client_get_connections (nm_cli.client); if (connections->len < 1) return NULL; @@ -5329,10 +5350,11 @@ gen_vpn_uuids (const char *text, int state) static char * gen_vpn_ids (const char *text, int state) { - const GPtrArray *connections = nm_cli.connections; + const GPtrArray *connections; const char **ids; char *ret; + connections = nm_client_get_connections (nm_cli.client); if (connections->len < 1) return NULL; @@ -7783,6 +7805,7 @@ editor_init_existing_connection (NMConnection *connection) static NMCResultCode do_connection_edit (NmCli *nmc, int argc, char **argv) { + const GPtrArray *connections; NMConnection *connection = NULL; NMSettingConnection *s_con; const char *connection_type; @@ -7829,6 +7852,8 @@ do_connection_edit (NmCli *nmc, int argc, char **argv) /* Use ' ' and '.' as word break characters */ rl_completer_word_break_characters = ". "; + connections = nm_client_get_connections (nmc->client); + if (!con) { if (con_id && !con_uuid && !con_path) { con = con_id; @@ -7853,7 +7878,7 @@ do_connection_edit (NmCli *nmc, int argc, char **argv) /* Existing connection */ NMConnection *found_con; - found_con = nmc_find_connection (nmc->connections, selector, con, NULL, FALSE); + found_con = nmc_find_connection (connections, selector, con, NULL, FALSE); if (!found_con) { g_string_printf (nmc->return_text, _("Error: Unknown connection '%s'."), con); nmc->return_value = NMC_RESULT_ERROR_NOT_FOUND; @@ -7911,7 +7936,7 @@ do_connection_edit (NmCli *nmc, int argc, char **argv) if (con_name) default_name = g_strdup (con_name); else - default_name = nmc_unique_connection_name (nmc->connections, + default_name = nmc_unique_connection_name (connections, get_name_alias (connection_type, NULL, nmc_valid_connection_types)); g_object_set (s_con, @@ -8344,14 +8369,15 @@ do_connection_monitor (NmCli *nmc, int argc, char **argv) if (argc == 0) { /* No connections specified. Monitor all. */ + const GPtrArray *connections; int i; /* nmc_do_cmd() should not call this with argc=0. */ g_assert (!nmc->complete); - nmc->connections = nm_client_get_connections (nmc->client); - for (i = 0; i < nmc->connections->len; i++) - connection_watch (nmc, g_ptr_array_index (nmc->connections, i)); + connections = nm_client_get_connections (nmc->client); + for (i = 0; i < connections->len; i++) + connection_watch (nmc, g_ptr_array_index (connections, i)); /* We'll watch the connection additions too, never exit. */ nmc->should_wait++; @@ -8688,23 +8714,22 @@ static char * gen_func_connection_names (const char *text, int state) { int i; - const char **connections; + const GPtrArray *connections; + const char **connection_names; char *ret; - if (nm_cli.connections->len == 0) + connections = nm_client_get_connections (nm_cli.client); + if (connections->len == 0) return NULL; - connections = g_new (const char *, nm_cli.connections->len + 1); - for (i = 0; i < nm_cli.connections->len; i++) { - NMConnection *con = NM_CONNECTION (nm_cli.connections->pdata[i]); - const char *id = nm_connection_get_id (con); - connections[i] = id; - } - connections[i] = NULL; + connection_names = g_new (const char *, connections->len + 1); + for (i = 0; i < connections->len; i++) + connection_names[i] = nm_connection_get_id (NM_CONNECTION (connections->pdata[i])); + connection_names[i] = NULL; - ret = nmc_rl_gen_func_basic (text, state, connections); + ret = nmc_rl_gen_func_basic (text, state, connection_names); - g_free (connections); + g_free (connection_names); return ret; } @@ -8809,9 +8834,6 @@ do_connections (NmCli *nmc, int argc, char **argv) return nmc->return_value; } - /* Get the connection list */ - nmc->connections = nm_client_get_connections (nmc->client); - return nmc_do_cmd (nmc, connection_cmds, *argv, argc, argv); } diff --git a/clients/cli/devices.c b/clients/cli/devices.c index d77686686f..c63cf74dda 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -1856,8 +1856,6 @@ do_device_connect (NmCli *nmc, int argc, char **argv) nmc->nowait_flag = (nmc->timeout == 0); nmc->should_wait++; - nmc->connections = nm_client_get_connections (nmc->client); - /* Create secret agent */ nmc->secret_agent = nm_secret_agent_simple_new ("nmcli-connect"); if (nmc->secret_agent) { diff --git a/clients/cli/nmcli.c b/clients/cli/nmcli.c index abc344fcbb..56c80d94d1 100644 --- a/clients/cli/nmcli.c +++ b/clients/cli/nmcli.c @@ -541,8 +541,6 @@ nmc_init (NmCli *nmc) nmc->timeout = -1; - nmc->connections = NULL; - nmc->secret_agent = NULL; nmc->pwds_hash = NULL; nmc->pk_listener = NULL; diff --git a/clients/cli/nmcli.h b/clients/cli/nmcli.h index dc9549db3c..8fb5060772 100644 --- a/clients/cli/nmcli.h +++ b/clients/cli/nmcli.h @@ -139,8 +139,6 @@ typedef struct _NmCli { int timeout; /* Operation timeout */ - const GPtrArray *connections; /* List of connections */ - NMSecretAgentOld *secret_agent; /* Secret agent */ GHashTable *pwds_hash; /* Hash table with passwords in passwd-file */ NMPolkitListener *pk_listener ; /* polkit agent listener */ diff --git a/clients/cli/settings.c b/clients/cli/settings.c index 80838f780c..2bd7550ba8 100644 --- a/clients/cli/settings.c +++ b/clients/cli/settings.c @@ -3355,19 +3355,21 @@ DEFINE_ALLOWED_VAL_FUNC (nmc_property_con_allowed_slave_type, con_valid_slave_ty static gboolean nmc_property_connection_set_secondaries (NMSetting *setting, const char *prop, const char *val, GError **error) { + const GPtrArray *connections; NMConnection *con; char **strv = NULL, **iter; guint i = 0; g_return_val_if_fail (error == NULL || *error == NULL, FALSE); + connections = nm_client_get_connections (nm_cli.client); strv = nmc_strsplit_set (val, " \t,", 0); for (iter = strv; iter && *iter; iter++) { if (**iter == '\0') continue; if (nm_utils_is_uuid (*iter)) { - con = nmc_find_connection (nm_cli.connections, "uuid", *iter, NULL, FALSE); + con = nmc_find_connection (connections, "uuid", *iter, NULL, FALSE); if (!con) g_print (_("Warning: %s is not an UUID of any existing connection profile\n"), *iter); else { @@ -3379,7 +3381,7 @@ nmc_property_connection_set_secondaries (NMSetting *setting, const char *prop, c } } } else { - con = nmc_find_connection (nm_cli.connections, "id", *iter, NULL, FALSE); + con = nmc_find_connection (connections, "id", *iter, NULL, FALSE); if (!con) { g_set_error (error, 1, 0, _("'%s' is not a name of any exiting profile"), *iter); g_strfreev (strv); From 3ee03afeccb74b79023e8668a1ef684974a1061b Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Wed, 31 Aug 2016 21:04:33 +0200 Subject: [PATCH 3/6] cli: make it possible to call sub-commands with client obtained asynchronously --- clients/cli/agent.c | 16 ++--- clients/cli/common.c | 130 +++++++++++++++++++++++++++++++++----- clients/cli/common.h | 4 +- clients/cli/connections.c | 32 +++++----- clients/cli/devices.c | 50 ++++++++------- clients/cli/general.c | 26 ++++---- clients/cli/nmcli.c | 72 +++++++++------------ 7 files changed, 215 insertions(+), 115 deletions(-) diff --git a/clients/cli/agent.c b/clients/cli/agent.c index 9211d9a0fb..1343d8bb97 100644 --- a/clients/cli/agent.c +++ b/clients/cli/agent.c @@ -209,13 +209,12 @@ do_agent_all (NmCli *nmc, int argc, char **argv) } static const NMCCommand agent_cmds[] = { - {"secret", do_agent_secret, usage_agent_secret }, - {"polkit", do_agent_polkit, usage_agent_polkit }, - {"all", do_agent_all, usage_agent_all }, - {NULL, do_agent_all, usage } + { "secret", do_agent_secret, usage_agent_secret, FALSE }, + { "polkit", do_agent_polkit, usage_agent_polkit, FALSE }, + { "all", do_agent_all, usage_agent_all, FALSE }, + { NULL, do_agent_all, usage, FALSE }, }; - NMCResultCode do_agent (NmCli *nmc, int argc, char **argv) { @@ -225,9 +224,10 @@ do_agent (NmCli *nmc, int argc, char **argv) /* Check whether NetworkManager is running */ if (!nm_client_get_nm_running (nmc->client)) { g_string_printf (nmc->return_text, _("Error: NetworkManager is not running.")); - nmc->return_value = NMC_RESULT_ERROR_NM_NOT_RUNNING; - return nmc->return_value; + return NMC_RESULT_ERROR_NM_NOT_RUNNING; } - return nmc_do_cmd (nmc, agent_cmds, *argv, argc, argv); + nmc_do_cmd (nmc, agent_cmds, *argv, argc, argv); + + return nmc->return_value; } diff --git a/clients/cli/common.c b/clients/cli/common.c index a2f9adc480..2ea6efaf97 100644 --- a/clients/cli/common.c +++ b/clients/cli/common.c @@ -1565,13 +1565,92 @@ nmc_parse_lldp_capabilities (guint value) return g_string_free (str, FALSE); } +extern GMainLoop *loop; + +static void +command_done (GObject *object, GAsyncResult *res, gpointer user_data) +{ + GSimpleAsyncResult *simple = (GSimpleAsyncResult *)res; + NmCli *nmc = user_data; + GError *error = NULL; + + if (g_simple_async_result_propagate_error (simple, &error)) { + nmc->return_value = error->code; + g_string_assign (nmc->return_text, error->message); + g_error_free (error); + } + + if (!nmc->should_wait) + g_main_loop_quit (loop); +} + +typedef struct { + NmCli *nmc; + const NMCCommand *cmd; + int argc; + char **argv; + GSimpleAsyncResult *simple; +} CmdCall; + +static void +call_cmd (NmCli *nmc, GSimpleAsyncResult *simple, const NMCCommand *cmd, int argc, char **argv); + +static void +got_client (GObject *source_object, GAsyncResult *res, gpointer user_data) +{ + GError *error = NULL; + CmdCall *call = user_data; + NmCli *nmc = call->nmc; + + nmc->should_wait--; + nmc->client = nm_client_new_finish (res, &error); + + if (!nmc->client) { + g_simple_async_result_set_error (call->simple, NMCLI_ERROR, NMC_RESULT_ERROR_UNKNOWN, + _("Error: Could not create NMClient object: %s."), error->message); + g_error_free (error); + g_simple_async_result_complete (call->simple); + } else { + call_cmd (nmc, call->simple, call->cmd, call->argc, call->argv); + } + + g_slice_free (CmdCall, call); +} + +static void +call_cmd (NmCli *nmc, GSimpleAsyncResult *simple, const NMCCommand *cmd, int argc, char **argv) +{ + CmdCall *call; + + if (nmc->client || !cmd->needs_client) { + + /* Check whether NetworkManager is running */ + if (cmd->needs_nm_running && !nm_client_get_nm_running (nmc->client)) { + g_simple_async_result_set_error (simple, NMCLI_ERROR, NMC_RESULT_ERROR_NM_NOT_RUNNING, + _("Error: NetworkManager is not running.")); + } else + nmc->return_value = cmd->func (nmc, argc, argv); + g_simple_async_result_complete_in_idle (simple); + g_object_unref (simple); + } else { + nmc->should_wait++; + call = g_slice_new0 (CmdCall); + call->nmc = nmc; + call->cmd = cmd; + call->argc = argc; + call->argv = argv; + call->simple = simple; + nm_client_new_async (NULL, got_client, call); + } +} + /** * nmc_do_cmd: * @nmc: Client instance * @cmds: Command table * @cmd: Command * @argc: Argument count - * @argv: Arguments vector + * @argv: Arguments vector. Must be a global variable. * * Picks the right callback to handle command from the command table. * If --help argument follows and the usage callback is specified for the command @@ -1580,22 +1659,35 @@ nmc_parse_lldp_capabilities (guint value) * The command table is terminated with a %NULL command. The terminating * entry's handlers are called if the command is empty. * - * Returns: a nmcli return code + * The argument vector needs to be a pointer to the global arguments vector that is + * never freed, since the command handler will be called asynchronously and there's + * no callback to free the memory in (for simplicity). */ -NMCResultCode +void nmc_do_cmd (NmCli *nmc, const NMCCommand cmds[], const char *cmd, int argc, char **argv) { const NMCCommand *c; + GSimpleAsyncResult *simple; - if (argc == 0 && nmc->complete) - return nmc->return_value; + simple = g_simple_async_result_new (NULL, + command_done, + nmc, + nmc_do_cmd); + + if (argc == 0 && nmc->complete) { + g_simple_async_result_complete_in_idle (simple); + g_object_unref (simple); + return; + } if (argc == 1 && nmc->complete) { for (c = cmds; c->cmd; ++c) { if (!*cmd || matches (cmd, c->cmd) == 0) g_print ("%s\n", c->cmd); } - return nmc->return_value; + g_simple_async_result_complete_in_idle (simple); + g_object_unref (simple); + return; } for (c = cmds; c->cmd; ++c) { @@ -1605,28 +1697,34 @@ nmc_do_cmd (NmCli *nmc, const NMCCommand cmds[], const char *cmd, int argc, char if (c->cmd) { /* A valid command was specified. */ - if (c->usage && nmc_arg_is_help (*(argv+1))) + if (c->usage && nmc_arg_is_help (*(argv+1))) { c->usage (); - else - nmc->return_value = c->func (nmc, argc-1, argv+1); + g_simple_async_result_complete_in_idle (simple); + g_object_unref (simple); + } else + call_cmd (nmc, simple, c, argc-1, argv+1); } else if (cmd) { /* Not a known command. */ if (nmc_arg_is_help (cmd) && c->usage) { c->usage (); + g_simple_async_result_complete_in_idle (simple); + g_object_unref (simple); } else { - g_string_printf (nmc->return_text, _("Error: argument '%s' not understood. Try passing --help instead."), cmd); - nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; + g_simple_async_result_set_error (simple, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, + _("Error: argument '%s' not understood. Try passing --help instead."), cmd); + g_simple_async_result_complete_in_idle (simple); + g_object_unref (simple); } } else if (c->func) { /* No command, run the default handler. */ - nmc->return_value = c->func (nmc, argc, argv); + call_cmd (nmc, simple, c, argc, argv); } else { /* No command and no default handler. */ - g_string_printf (nmc->return_text, _("Error: missing argument. Try passing --help.")); - nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; + g_simple_async_result_set_error (simple, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, + _("Error: missing argument. Try passing --help.")); + g_simple_async_result_complete_in_idle (simple); + g_object_unref (simple); } - - return nmc->return_value; } /** diff --git a/clients/cli/common.h b/clients/cli/common.h index 1887e910b1..4209136350 100644 --- a/clients/cli/common.h +++ b/clients/cli/common.h @@ -80,9 +80,11 @@ typedef struct { const char *cmd; NMCResultCode (*func) (NmCli *nmc, int argc, char **argv); void (*usage) (void); + gboolean needs_client; + gboolean needs_nm_running; } NMCCommand; -NMCResultCode nmc_do_cmd (NmCli *nmc, const NMCCommand cmds[], const char *argv0, int argc, char **argv); +void nmc_do_cmd (NmCli *nmc, const NMCCommand cmds[], const char *cmd, int argc, char **argv); void nmc_complete_strings (const char *prefix, ...) G_GNUC_NULL_TERMINATED; diff --git a/clients/cli/connections.c b/clients/cli/connections.c index ab6c578af1..2ad965ab66 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -8796,20 +8796,20 @@ nmcli_con_tab_completion (const char *text, int start, int end) } static const NMCCommand connection_cmds[] = { - {"show", do_connections_show, usage_connection_show }, - {"up", do_connection_up, usage_connection_up }, - {"down", do_connection_down, usage_connection_down }, - {"add", do_connection_add, usage_connection_add }, - {"edit", do_connection_edit, usage_connection_edit }, - {"delete", do_connection_delete, usage_connection_delete }, - {"reload", do_connection_reload, usage_connection_reload }, - {"load", do_connection_load, usage_connection_load }, - {"modify", do_connection_modify, usage_connection_modify }, - {"clone", do_connection_clone, usage_connection_clone }, - {"import", do_connection_import, usage_connection_import }, - {"export", do_connection_export, usage_connection_export }, - {"monitor", do_connection_monitor, usage_connection_monitor }, - {NULL, do_connections_show, usage }, + { "show", do_connections_show, usage_connection_show, FALSE }, + { "up", do_connection_up, usage_connection_up, FALSE }, + { "down", do_connection_down, usage_connection_down, FALSE }, + { "add", do_connection_add, usage_connection_add, FALSE }, + { "edit", do_connection_edit, usage_connection_edit, FALSE }, + { "delete", do_connection_delete, usage_connection_delete, FALSE }, + { "reload", do_connection_reload, usage_connection_reload, FALSE }, + { "load", do_connection_load, usage_connection_load, FALSE }, + { "modify", do_connection_modify, usage_connection_modify, FALSE }, + { "clone", do_connection_clone, usage_connection_clone, FALSE }, + { "import", do_connection_import, usage_connection_import, FALSE }, + { "export", do_connection_export, usage_connection_export, FALSE }, + { "monitor", do_connection_monitor, usage_connection_monitor, FALSE }, + { NULL, do_connections_show, usage, FALSE }, }; /* Entry point function for connections-related commands: 'nmcli connection' */ @@ -8834,7 +8834,9 @@ do_connections (NmCli *nmc, int argc, char **argv) return nmc->return_value; } - return nmc_do_cmd (nmc, connection_cmds, *argv, argc, argv); + nmc_do_cmd (nmc, connection_cmds, *argv, argc, argv); + + return nmc->return_value; } void diff --git a/clients/cli/devices.c b/clients/cli/devices.c index c63cf74dda..cf5d825fb8 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -3607,11 +3607,11 @@ finish: } static NMCCommand device_wifi_cmds[] = { - {"list", do_device_wifi_list, NULL }, - {"connect", do_device_wifi_connect_network, NULL }, - {"hotspot", do_device_wifi_hotspot, NULL }, - {"rescan", do_device_wifi_rescan, NULL }, - {NULL, do_device_wifi_list, NULL } + { "list", do_device_wifi_list, NULL, FALSE }, + { "connect", do_device_wifi_connect_network, NULL, FALSE }, + { "hotspot", do_device_wifi_hotspot, NULL, FALSE }, + { "rescan", do_device_wifi_rescan, NULL, FALSE }, + { NULL, do_device_wifi_list, NULL, FALSE }, }; static NMCResultCode @@ -3625,7 +3625,9 @@ do_device_wifi (NmCli *nmc, int argc, char **argv) return NMC_RESULT_ERROR_USER_INPUT; } - return nmc_do_cmd (nmc, device_wifi_cmds, *argv, argc, argv); + nmc_do_cmd (nmc, device_wifi_cmds, *argv, argc, argv); + + return nmc->return_value; } static int @@ -3780,8 +3782,8 @@ do_device_lldp_list (NmCli *nmc, int argc, char **argv) } static NMCCommand device_lldp_cmds[] = { - {"list", do_device_lldp_list, NULL }, - {NULL, do_device_lldp_list, NULL } + { "list", do_device_lldp_list, NULL, FALSE }, + { NULL, do_device_lldp_list, NULL, FALSE }, }; static NMCResultCode @@ -3798,7 +3800,9 @@ do_device_lldp (NmCli *nmc, int argc, char **argv) if (!nmc->mode_specified) nmc->multiline_output = TRUE; /* multiline mode is default for 'device lldp' */ - return nmc_do_cmd (nmc, device_lldp_cmds, *argv, argc, argv); + nmc_do_cmd (nmc, device_lldp_cmds, *argv, argc, argv); + + return nmc->return_value; } static gboolean @@ -3844,18 +3848,18 @@ nmcli_device_tab_completion (const char *text, int start, int end) } static const NMCCommand device_cmds[] = { - {"status", do_devices_status, usage_device_status }, - {"show", do_device_show, usage_device_show }, - {"connect", do_device_connect, usage_device_connect }, - {"reapply", do_device_reapply, usage_device_reapply }, - {"disconnect", do_devices_disconnect, usage_device_disconnect }, - {"delete", do_devices_delete, usage_device_delete }, - {"set", do_device_set, usage_device_set }, - {"monitor", do_devices_monitor, usage_device_monitor }, - {"wifi", do_device_wifi, usage_device_wifi }, - {"lldp", do_device_lldp, usage_device_lldp }, - {"modify", do_device_modify, usage_device_modify }, - {NULL, do_devices_status, usage }, + { "status", do_devices_status, usage_device_status, FALSE }, + { "show", do_device_show, usage_device_show, FALSE }, + { "connect", do_device_connect, usage_device_connect, FALSE }, + { "reapply", do_device_reapply, usage_device_reapply, FALSE }, + { "disconnect", do_devices_disconnect, usage_device_disconnect, FALSE }, + { "delete", do_devices_delete, usage_device_delete, FALSE }, + { "set", do_device_set, usage_device_set, FALSE }, + { "monitor", do_devices_monitor, usage_device_monitor, FALSE }, + { "wifi", do_device_wifi, usage_device_wifi, FALSE }, + { "lldp", do_device_lldp, usage_device_lldp, FALSE }, + { "modify", do_device_modify, usage_device_modify, FALSE }, + { NULL, do_devices_status, usage, FALSE }, }; NMCResultCode @@ -3875,7 +3879,9 @@ do_devices (NmCli *nmc, int argc, char **argv) return NMC_RESULT_ERROR_NM_NOT_RUNNING; } - return nmc_do_cmd (nmc, device_cmds, *argv, argc, argv); + nmc_do_cmd (nmc, device_cmds, *argv, argc, argv); + + return nmc->return_value; } void diff --git a/clients/cli/general.c b/clients/cli/general.c index e97091251f..d844f1fb19 100644 --- a/clients/cli/general.c +++ b/clients/cli/general.c @@ -678,11 +678,11 @@ do_general_hostname (NmCli *nmc, int argc, char **argv) } static const NMCCommand general_cmds[] = { - {"status", do_general_status, usage_general_status }, - {"hostname", do_general_hostname, usage_general_hostname }, - {"permissions", do_general_permissions, usage_general_permissions }, - {"logging", do_general_logging, usage_general_logging }, - {NULL, do_general_status, usage_general } + { "status", do_general_status, usage_general_status, FALSE }, + { "hostname", do_general_hostname, usage_general_hostname, FALSE }, + { "permissions", do_general_permissions, usage_general_permissions, FALSE }, + { "logging", do_general_logging, usage_general_logging, FALSE }, + { NULL, do_general_status, usage_general, FALSE }, }; /* @@ -694,7 +694,9 @@ do_general (NmCli *nmc, int argc, char **argv) /* Register polkit agent */ nmc_start_polkit_agent_start_try (nmc); - return nmc_do_cmd (nmc, general_cmds, *argv, argc, argv); + nmc_do_cmd (nmc, general_cmds, *argv, argc, argv); + + return nmc->return_value; } static gboolean @@ -916,10 +918,10 @@ do_radio_wwan (NmCli *nmc, int argc, char **argv) } static const NMCCommand radio_cmds[] = { - {"all", do_radio_all, usage_radio_all }, - {"wifi", do_radio_wifi, usage_radio_wifi }, - {"wwan", do_radio_wwan, usage_radio_wwan }, - {NULL, do_radio_all, usage_radio } + { "all", do_radio_all, usage_radio_all, FALSE }, + { "wifi", do_radio_wifi, usage_radio_wifi, FALSE }, + { "wwan", do_radio_wwan, usage_radio_wwan, FALSE }, + { NULL, do_radio_all, usage_radio, FALSE }, }; /* @@ -931,7 +933,9 @@ do_radio (NmCli *nmc, int argc, char **argv) /* Register polkit agent */ nmc_start_polkit_agent_start_try (nmc); - return nmc_do_cmd (nmc, radio_cmds, *argv, argc, argv); + nmc_do_cmd (nmc, radio_cmds, *argv, argc, argv); + + return nmc->return_value; } static void diff --git a/clients/cli/nmcli.c b/clients/cli/nmcli.c index 56c80d94d1..a102b2a249 100644 --- a/clients/cli/nmcli.c +++ b/clients/cli/nmcli.c @@ -205,18 +205,18 @@ usage (void) } static const NMCCommand nmcli_cmds[] = { - { "general", do_general, NULL }, - { "monitor", do_monitor, NULL }, - { "networking", do_networking, NULL }, - { "radio", do_radio, NULL }, - { "connection", do_connections, NULL }, - { "device", do_devices, NULL }, - { "agent", do_agent, NULL }, - { NULL, do_overview, usage } + { "general", do_general, NULL, FALSE }, + { "monitor", do_monitor, NULL, FALSE }, + { "networking", do_networking, NULL, FALSE }, + { "radio", do_radio, NULL, FALSE }, + { "connection", do_connections, NULL, FALSE }, + { "device", do_devices, NULL, FALSE }, + { "agent", do_agent, NULL, FALSE }, + { NULL, do_overview, usage, FALSE }, }; -static NMCResultCode -parse_command_line (NmCli *nmc, int argc, char **argv) +static gboolean +process_command_line (NmCli *nmc, int argc, char **argv) { char *base; @@ -257,12 +257,12 @@ parse_command_line (NmCli *nmc, int argc, char **argv) if (nmc->print_output == NMC_PRINT_TERSE) { g_string_printf (nmc->return_text, _("Error: Option '--terse' is specified the second time.")); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; - return nmc->return_value; + return FALSE; } else if (nmc->print_output == NMC_PRINT_PRETTY) { g_string_printf (nmc->return_text, _("Error: Option '--terse' is mutually exclusive with '--pretty'.")); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; - return nmc->return_value; + return FALSE; } else nmc->print_output = NMC_PRINT_TERSE; @@ -270,12 +270,12 @@ parse_command_line (NmCli *nmc, int argc, char **argv) if (nmc->print_output == NMC_PRINT_PRETTY) { g_string_printf (nmc->return_text, _("Error: Option '--pretty' is specified the second time.")); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; - return nmc->return_value; + return FALSE; } else if (nmc->print_output == NMC_PRINT_TERSE) { g_string_printf (nmc->return_text, _("Error: Option '--pretty' is mutually exclusive with '--terse'.")); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; - return nmc->return_value; + return FALSE; } else nmc->print_output = NMC_PRINT_PRETTY; @@ -284,7 +284,7 @@ parse_command_line (NmCli *nmc, int argc, char **argv) if (next_arg (&argc, &argv) != 0) { g_string_printf (nmc->return_text, _("Error: missing argument for '%s' option."), opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; - return nmc->return_value; + return FALSE; } if (argc == 1 && nmc->complete) nmc_complete_strings (argv[0], "tabular", "multiline", NULL); @@ -295,13 +295,13 @@ parse_command_line (NmCli *nmc, int argc, char **argv) else { g_string_printf (nmc->return_text, _("Error: '%s' is not valid argument for '%s' option."), argv[0], opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; - return nmc->return_value; + return FALSE; } } else if (matches (opt, "-colors") == 0) { if (next_arg (&argc, &argv) != 0) { g_string_printf (nmc->return_text, _("Error: missing argument for '%s' option."), opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; - return nmc->return_value; + return FALSE; } if (argc == 1 && nmc->complete) nmc_complete_strings (argv[0], "yes", "no", "auto", NULL); @@ -314,13 +314,13 @@ parse_command_line (NmCli *nmc, int argc, char **argv) else { g_string_printf (nmc->return_text, _("Error: '%s' is not valid argument for '%s' option."), argv[0], opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; - return nmc->return_value; + return FALSE; } } else if (matches (opt, "-escape") == 0) { if (next_arg (&argc, &argv) != 0) { g_string_printf (nmc->return_text, _("Error: missing argument for '%s' option."), opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; - return nmc->return_value; + return FALSE; } if (argc == 1 && nmc->complete) nmc_complete_strings (argv[0], "yes", "no", NULL); @@ -331,13 +331,13 @@ parse_command_line (NmCli *nmc, int argc, char **argv) else { g_string_printf (nmc->return_text, _("Error: '%s' is not valid argument for '%s' option."), argv[0], opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; - return nmc->return_value; + return FALSE; } } else if (matches (opt, "-fields") == 0) { if (next_arg (&argc, &argv) != 0) { g_string_printf (nmc->return_text, _("Error: fields for '%s' options are missing."), opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; - return nmc->return_value; + return FALSE; } if (argc == 1 && nmc->complete) complete_fields (argv[0]); @@ -353,13 +353,13 @@ parse_command_line (NmCli *nmc, int argc, char **argv) if (next_arg (&argc, &argv) != 0) { g_string_printf (nmc->return_text, _("Error: missing argument for '%s' option."), opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; - return nmc->return_value; + return FALSE; } if (!nmc_string_to_uint (argv[0], TRUE, 0, G_MAXINT, &timeout)) { g_string_printf (nmc->return_text, _("Error: '%s' is not a valid timeout for '%s' option."), argv[0], opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; - return nmc->return_value; + return FALSE; } nmc->timeout = (int) timeout; } else if (matches (opt, "-version") == 0) { @@ -373,14 +373,16 @@ parse_command_line (NmCli *nmc, int argc, char **argv) } else { g_string_printf (nmc->return_text, _("Error: Option '%s' is unknown, try 'nmcli -help'."), opt); nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; - return nmc->return_value; + return FALSE; } argc--; argv++; } /* Now run the requested command */ - return nmc_do_cmd (nmc, nmcli_cmds, *argv, argc, argv); + nmc_do_cmd (nmc, nmcli_cmds, *argv, argc, argv); + + return TRUE; } static gboolean nmcli_sigint = FALSE; @@ -587,19 +589,6 @@ nmc_cleanup (NmCli *nmc) nmc_polkit_agent_fini (nmc); } -static gboolean -start (gpointer data) -{ - ArgsInfo *info = (ArgsInfo *) data; - info->nmc->return_value = parse_command_line (info->nmc, info->argc, info->argv); - - if (!info->nmc->should_wait) - g_main_loop_quit (loop); - - return FALSE; -} - - int main (int argc, char *argv[]) { @@ -626,10 +615,9 @@ main (int argc, char *argv[]) nmc_value_transforms_register (); nmc_init (&nm_cli); - g_idle_add (start, &args_info); - - loop = g_main_loop_new (NULL, FALSE); /* create main loop */ - g_main_loop_run (loop); /* run main loop */ + loop = g_main_loop_new (NULL, FALSE); + if (process_command_line (&nm_cli, argc, argv)) + g_main_loop_run (loop); if (nm_cli.complete) { /* Remove error statuses from command completion runs. */ From 01a20015e0bfd6d7c9aa872cfa31c4642ccb6825 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Wed, 31 Aug 2016 20:52:48 +0200 Subject: [PATCH 4/6] cli: use nmc_do_cmd to get the client and check if the daemon is running The makes use of asynchronous client initialization, making things a bit faster and reduces code duplication too. --- clients/cli/agent.c | 17 +++--------- clients/cli/common.c | 1 - clients/cli/connections.c | 40 ++++++++++------------------- clients/cli/devices.c | 47 ++++++++++++++-------------------- clients/cli/general.c | 54 +++++++-------------------------------- clients/cli/nmcli.c | 36 ++++++-------------------- clients/cli/nmcli.h | 1 - 7 files changed, 54 insertions(+), 142 deletions(-) diff --git a/clients/cli/agent.c b/clients/cli/agent.c index 1343d8bb97..4cbf7d2d37 100644 --- a/clients/cli/agent.c +++ b/clients/cli/agent.c @@ -209,24 +209,15 @@ do_agent_all (NmCli *nmc, int argc, char **argv) } static const NMCCommand agent_cmds[] = { - { "secret", do_agent_secret, usage_agent_secret, FALSE }, - { "polkit", do_agent_polkit, usage_agent_polkit, FALSE }, - { "all", do_agent_all, usage_agent_all, FALSE }, - { NULL, do_agent_all, usage, FALSE }, + { "secret", do_agent_secret, usage_agent_secret, TRUE, TRUE }, + { "polkit", do_agent_polkit, usage_agent_polkit, TRUE, TRUE }, + { "all", do_agent_all, usage_agent_all, TRUE, TRUE }, + { NULL, do_agent_all, usage, TRUE, TRUE }, }; NMCResultCode do_agent (NmCli *nmc, int argc, char **argv) { - /* Get NMClient object */ - nmc->get_client (nmc); - - /* Check whether NetworkManager is running */ - if (!nm_client_get_nm_running (nmc->client)) { - g_string_printf (nmc->return_text, _("Error: NetworkManager is not running.")); - return NMC_RESULT_ERROR_NM_NOT_RUNNING; - } - nmc_do_cmd (nmc, agent_cmds, *argv, argc, argv); return nmc->return_value; diff --git a/clients/cli/common.c b/clients/cli/common.c index 2ea6efaf97..47e858e778 100644 --- a/clients/cli/common.c +++ b/clients/cli/common.c @@ -1487,7 +1487,6 @@ nmc_rl_gen_func_ifnames (const char *text, int state) const char **ifnames; char *ret; - nm_cli.get_client (&nm_cli); devices = nm_client_get_devices (nm_cli.client); if (devices->len == 0) return NULL; diff --git a/clients/cli/connections.c b/clients/cli/connections.c index 2ad965ab66..187d416421 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -8796,20 +8796,20 @@ nmcli_con_tab_completion (const char *text, int start, int end) } static const NMCCommand connection_cmds[] = { - { "show", do_connections_show, usage_connection_show, FALSE }, - { "up", do_connection_up, usage_connection_up, FALSE }, - { "down", do_connection_down, usage_connection_down, FALSE }, - { "add", do_connection_add, usage_connection_add, FALSE }, - { "edit", do_connection_edit, usage_connection_edit, FALSE }, - { "delete", do_connection_delete, usage_connection_delete, FALSE }, - { "reload", do_connection_reload, usage_connection_reload, FALSE }, - { "load", do_connection_load, usage_connection_load, FALSE }, - { "modify", do_connection_modify, usage_connection_modify, FALSE }, - { "clone", do_connection_clone, usage_connection_clone, FALSE }, - { "import", do_connection_import, usage_connection_import, FALSE }, - { "export", do_connection_export, usage_connection_export, FALSE }, - { "monitor", do_connection_monitor, usage_connection_monitor, FALSE }, - { NULL, do_connections_show, usage, FALSE }, + { "show", do_connections_show, usage_connection_show, TRUE, TRUE }, + { "up", do_connection_up, usage_connection_up, TRUE, TRUE }, + { "down", do_connection_down, usage_connection_down, TRUE, TRUE }, + { "add", do_connection_add, usage_connection_add, TRUE, TRUE }, + { "edit", do_connection_edit, usage_connection_edit, TRUE, TRUE }, + { "delete", do_connection_delete, usage_connection_delete, TRUE, TRUE }, + { "reload", do_connection_reload, usage_connection_reload, TRUE, TRUE }, + { "load", do_connection_load, usage_connection_load, TRUE, TRUE }, + { "modify", do_connection_modify, usage_connection_modify, TRUE, TRUE }, + { "clone", do_connection_clone, usage_connection_clone, TRUE, TRUE }, + { "import", do_connection_import, usage_connection_import, TRUE, TRUE }, + { "export", do_connection_export, usage_connection_export, TRUE, TRUE }, + { "monitor", do_connection_monitor, usage_connection_monitor, TRUE, TRUE }, + { NULL, do_connections_show, usage, TRUE, TRUE }, }; /* Entry point function for connections-related commands: 'nmcli connection' */ @@ -8822,18 +8822,6 @@ do_connections (NmCli *nmc, int argc, char **argv) /* Set completion function for 'nmcli con' */ rl_attempted_completion_function = (rl_completion_func_t *) nmcli_con_tab_completion; - /* Get NMClient object early */ - nmc->get_client (nmc); - - /* Check whether NetworkManager is running */ - if (!nm_client_get_nm_running (nmc->client)) { - if (!nmc->complete) { - g_string_printf (nmc->return_text, _("Error: NetworkManager is not running.")); - nmc->return_value = NMC_RESULT_ERROR_NM_NOT_RUNNING; - } - return nmc->return_value; - } - nmc_do_cmd (nmc, connection_cmds, *argv, argc, argv); return nmc->return_value; diff --git a/clients/cli/devices.c b/clients/cli/devices.c index cf5d825fb8..eaded0ca85 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -3607,11 +3607,11 @@ finish: } static NMCCommand device_wifi_cmds[] = { - { "list", do_device_wifi_list, NULL, FALSE }, - { "connect", do_device_wifi_connect_network, NULL, FALSE }, - { "hotspot", do_device_wifi_hotspot, NULL, FALSE }, - { "rescan", do_device_wifi_rescan, NULL, FALSE }, - { NULL, do_device_wifi_list, NULL, FALSE }, + { "list", do_device_wifi_list, NULL, TRUE, TRUE }, + { "connect", do_device_wifi_connect_network, NULL, TRUE, TRUE }, + { "hotspot", do_device_wifi_hotspot, NULL, TRUE, TRUE }, + { "rescan", do_device_wifi_rescan, NULL, TRUE, TRUE }, + { NULL, do_device_wifi_list, NULL, TRUE, TRUE }, }; static NMCResultCode @@ -3782,8 +3782,8 @@ do_device_lldp_list (NmCli *nmc, int argc, char **argv) } static NMCCommand device_lldp_cmds[] = { - { "list", do_device_lldp_list, NULL, FALSE }, - { NULL, do_device_lldp_list, NULL, FALSE }, + { "list", do_device_lldp_list, NULL, TRUE, TRUE }, + { NULL, do_device_lldp_list, NULL, TRUE, TRUE }, }; static NMCResultCode @@ -3848,18 +3848,18 @@ nmcli_device_tab_completion (const char *text, int start, int end) } static const NMCCommand device_cmds[] = { - { "status", do_devices_status, usage_device_status, FALSE }, - { "show", do_device_show, usage_device_show, FALSE }, - { "connect", do_device_connect, usage_device_connect, FALSE }, - { "reapply", do_device_reapply, usage_device_reapply, FALSE }, - { "disconnect", do_devices_disconnect, usage_device_disconnect, FALSE }, - { "delete", do_devices_delete, usage_device_delete, FALSE }, - { "set", do_device_set, usage_device_set, FALSE }, - { "monitor", do_devices_monitor, usage_device_monitor, FALSE }, - { "wifi", do_device_wifi, usage_device_wifi, FALSE }, - { "lldp", do_device_lldp, usage_device_lldp, FALSE }, - { "modify", do_device_modify, usage_device_modify, FALSE }, - { NULL, do_devices_status, usage, FALSE }, + { "status", do_devices_status, usage_device_status, TRUE, TRUE }, + { "show", do_device_show, usage_device_show, TRUE, TRUE }, + { "connect", do_device_connect, usage_device_connect, TRUE, TRUE }, + { "reapply", do_device_reapply, usage_device_reapply, TRUE, TRUE }, + { "disconnect", do_devices_disconnect, usage_device_disconnect, TRUE, TRUE }, + { "delete", do_devices_delete, usage_device_delete, TRUE, TRUE }, + { "set", do_device_set, usage_device_set, TRUE, TRUE }, + { "monitor", do_devices_monitor, usage_device_monitor, TRUE, TRUE }, + { "wifi", do_device_wifi, usage_device_wifi, FALSE, FALSE }, + { "lldp", do_device_lldp, usage_device_lldp, FALSE, FALSE }, + { "modify", do_device_modify, usage_device_modify, TRUE, TRUE }, + { NULL, do_devices_status, usage, TRUE, TRUE }, }; NMCResultCode @@ -3870,15 +3870,6 @@ do_devices (NmCli *nmc, int argc, char **argv) rl_attempted_completion_function = (rl_completion_func_t *) nmcli_device_tab_completion; - /* Get NMClient object early */ - nmc->get_client (nmc); - - /* Check whether NetworkManager is running */ - if (!nm_client_get_nm_running (nmc->client)) { - g_string_printf (nmc->return_text, _("Error: NetworkManager is not running.")); - return NMC_RESULT_ERROR_NM_NOT_RUNNING; - } - nmc_do_cmd (nmc, device_cmds, *argv, argc, argv); return nmc->return_value; diff --git a/clients/cli/general.c b/clients/cli/general.c index d844f1fb19..cd1548baf0 100644 --- a/clients/cli/general.c +++ b/clients/cli/general.c @@ -340,14 +340,6 @@ show_nm_status (NmCli *nmc, const char *pretty_header_name, const char *print_fl return FALSE; } - nmc->get_client (nmc); /* create NMClient */ - - if (!nm_client_get_nm_running (nmc->client)) { - g_string_printf (nmc->return_text, _("Error: NetworkManager is not running.")); - nmc->return_value = NMC_RESULT_ERROR_NM_NOT_RUNNING; - return FALSE; - } - state = nm_client_get_state (nmc->client); startup = nm_client_get_startup (nmc->client); connectivity = nm_client_get_connectivity (nmc->client); @@ -490,14 +482,6 @@ show_nm_permissions (NmCli *nmc) return FALSE; } - nmc->get_client (nmc); /* create NMClient */ - - if (!nm_client_get_nm_running (nmc->client)) { - g_string_printf (nmc->return_text, _("Error: NetworkManager is not running.")); - nmc->return_value = NMC_RESULT_ERROR_NM_NOT_RUNNING; - return FALSE; - } - nmc->print_fields.header_name = _("NetworkManager permissions"); arr = nmc_dup_fields_array (tmpl, tmpl_len, NMC_OF_FLAG_MAIN_HEADER_ADD | NMC_OF_FLAG_FIELD_NAMES); g_ptr_array_add (nmc->output_data, arr); @@ -562,7 +546,6 @@ show_general_logging (NmCli *nmc) return FALSE; } - nmc->get_client (nmc); /* create NMClient */ nm_client_get_logging (nmc->client, &level, &domains, &error); if (error) { g_string_printf (nmc->return_text, _("Error: %s."), error->message); @@ -618,7 +601,6 @@ do_general_logging (NmCli *nmc, int argc, char **argv) return error->code; } - nmc->get_client (nmc); /* create NMClient */ nm_client_set_logging (nmc->client, level, domains, &error); if (error) { g_string_printf (nmc->return_text, _("Error: failed to set logging: %s"), @@ -656,7 +638,6 @@ do_general_hostname (NmCli *nmc, int argc, char **argv) /* no arguments -> get hostname */ char *hostname = NULL; - nmc->get_client (nmc); /* create NMClient */ g_object_get (nmc->client, NM_CLIENT_HOSTNAME, &hostname, NULL); if (hostname) g_print ("%s\n", hostname); @@ -669,7 +650,6 @@ do_general_hostname (NmCli *nmc, int argc, char **argv) g_print ("Warning: ignoring extra garbage after '%s' hostname\n", hostname); nmc->should_wait++; - nmc->get_client (nmc); /* create NMClient */ nm_client_save_hostname_async (nmc->client, hostname, NULL, save_hostname_cb, nmc); } @@ -678,11 +658,11 @@ do_general_hostname (NmCli *nmc, int argc, char **argv) } static const NMCCommand general_cmds[] = { - { "status", do_general_status, usage_general_status, FALSE }, - { "hostname", do_general_hostname, usage_general_hostname, FALSE }, - { "permissions", do_general_permissions, usage_general_permissions, FALSE }, - { "logging", do_general_logging, usage_general_logging, FALSE }, - { NULL, do_general_status, usage_general, FALSE }, + { "status", do_general_status, usage_general_status, TRUE, TRUE }, + { "hostname", do_general_hostname, usage_general_hostname, TRUE, TRUE }, + { "permissions", do_general_permissions, usage_general_permissions, TRUE, TRUE }, + { "logging", do_general_logging, usage_general_logging, TRUE, TRUE }, + { NULL, do_general_status, usage_general, TRUE, TRUE }, }; /* @@ -788,7 +768,6 @@ do_networking (NmCli *nmc, int argc, char **argv) } else if (matches (*argv, "check") == 0) { GError *error = NULL; - nmc->get_client (nmc); /* create NMClient */ nm_client_check_connectivity (nmc->client, NULL, &error); if (error) { g_string_printf (nmc->return_text, _("Error: %s."), error->message); @@ -812,7 +791,6 @@ do_networking (NmCli *nmc, int argc, char **argv) goto finish; } - nmc->get_client (nmc); /* create NMClient */ nm_client_networking_set_enabled (nmc->client, enable_flag, NULL); } else { if (nmc->complete) @@ -854,7 +832,6 @@ do_radio_all (NmCli *nmc, int argc, char **argv) if (!nmc_switch_parse_on_off (nmc, *(argv-1), *argv, &enable_flag)) return nmc->return_value; - nmc->get_client (nmc); /* create NMClient */ nm_client_wireless_set_enabled (nmc->client, enable_flag); nm_client_wimax_set_enabled (nmc->client, enable_flag); nm_client_wwan_set_enabled (nmc->client, enable_flag); @@ -883,7 +860,6 @@ do_radio_wifi (NmCli *nmc, int argc, char **argv) if (!nmc_switch_parse_on_off (nmc, *(argv-1), *argv, &enable_flag)) return nmc->return_value; - nmc->get_client (nmc); /* create NMClient */ nm_client_wireless_set_enabled (nmc->client, enable_flag); } @@ -910,7 +886,6 @@ do_radio_wwan (NmCli *nmc, int argc, char **argv) if (!nmc_switch_parse_on_off (nmc, *(argv-1), *argv, &enable_flag)) return nmc->return_value; - nmc->get_client (nmc); /* create NMClient */ nm_client_wwan_set_enabled (nmc->client, enable_flag); } @@ -918,10 +893,10 @@ do_radio_wwan (NmCli *nmc, int argc, char **argv) } static const NMCCommand radio_cmds[] = { - { "all", do_radio_all, usage_radio_all, FALSE }, - { "wifi", do_radio_wifi, usage_radio_wifi, FALSE }, - { "wwan", do_radio_wwan, usage_radio_wwan, FALSE }, - { NULL, do_radio_all, usage_radio, FALSE }, + { "all", do_radio_all, usage_radio_all, TRUE, TRUE }, + { "wifi", do_radio_wifi, usage_radio_wifi, TRUE, TRUE }, + { "wwan", do_radio_wwan, usage_radio_wwan, TRUE, TRUE }, + { NULL, do_radio_all, usage_radio, TRUE, TRUE }, }; /* @@ -1157,15 +1132,6 @@ do_overview (NmCli *nmc, int argc, char **argv) /* Register polkit agent */ nmc_start_polkit_agent_start_try (nmc); - /* Get NMClient object early */ - nmc->get_client (nmc); - - /* Check whether NetworkManager is running */ - if (!nm_client_get_nm_running (nmc->client)) { - g_string_printf (nmc->return_text, _("Error: NetworkManager is not running.")); - return NMC_RESULT_ERROR_NM_NOT_RUNNING; - } - /* The VPN connections don't have devices (yet?). */ p = nm_client_get_active_connections (nmc->client); for (i = 0; i < p->len; i++) { @@ -1242,8 +1208,6 @@ do_monitor (NmCli *nmc, int argc, char **argv) return nmc->return_value; } - nmc->get_client (nmc); /* create NMClient */ - if (!nm_client_get_nm_running (nmc->client)) { char *str; diff --git a/clients/cli/nmcli.c b/clients/cli/nmcli.c index a102b2a249..5c47e44083 100644 --- a/clients/cli/nmcli.c +++ b/clients/cli/nmcli.c @@ -205,14 +205,14 @@ usage (void) } static const NMCCommand nmcli_cmds[] = { - { "general", do_general, NULL, FALSE }, - { "monitor", do_monitor, NULL, FALSE }, - { "networking", do_networking, NULL, FALSE }, - { "radio", do_radio, NULL, FALSE }, - { "connection", do_connections, NULL, FALSE }, - { "device", do_devices, NULL, FALSE }, - { "agent", do_agent, NULL, FALSE }, - { NULL, do_overview, usage, FALSE }, + { "general", do_general, NULL, FALSE, FALSE }, + { "monitor", do_monitor, NULL, TRUE, FALSE }, + { "networking", do_networking, NULL, FALSE, FALSE }, + { "radio", do_radio, NULL, FALSE, FALSE }, + { "connection", do_connections, NULL, FALSE, FALSE }, + { "device", do_devices, NULL, FALSE, FALSE }, + { "agent", do_agent, NULL, FALSE, FALSE }, + { NULL, do_overview, usage, TRUE, TRUE }, }; static gboolean @@ -514,29 +514,11 @@ nmc_value_transforms_register (void) nmc_convert_bytes_to_string); } -static NMClient * -nmc_get_client (NmCli *nmc) -{ - GError *error = NULL; - - if (!nmc->client) { - nmc->client = nm_client_new (NULL, &error); - if (!nmc->client) { - g_printerr ("%s\n", error->message); - g_clear_error (&error); - exit (NMC_RESULT_ERROR_UNKNOWN); - } - } - - return nmc->client; -} - /* Initialize NmCli structure - set default values */ static void nmc_init (NmCli *nmc) { nmc->client = NULL; - nmc->get_client = &nmc_get_client; nmc->return_value = NMC_RESULT_SUCCESS; nmc->return_text = g_string_new (_("Success")); @@ -592,8 +574,6 @@ nmc_cleanup (NmCli *nmc) int main (int argc, char *argv[]) { - ArgsInfo args_info = { &nm_cli, argc, argv }; - /* Set locale to use environment variables */ setlocale (LC_ALL, ""); diff --git a/clients/cli/nmcli.h b/clients/cli/nmcli.h index 8fb5060772..30600d21d1 100644 --- a/clients/cli/nmcli.h +++ b/clients/cli/nmcli.h @@ -132,7 +132,6 @@ typedef enum { /* NmCli - main structure */ typedef struct _NmCli { NMClient *client; /* Pointer to NMClient of libnm */ - NMClient *(*get_client) (struct _NmCli *nmc); /* Pointer to function for creating NMClient */ NMCResultCode return_value; /* Return code of nmcli */ GString *return_text; /* Reason text */ From 32dfa563d1eaf4eea1245d26ac02b035a0a38605 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Sat, 8 Oct 2016 14:44:45 +0200 Subject: [PATCH 5/6] libnm/nm-manager: don't block the object creation on permissions The GetPermissions call is very expensive (~400ms here, an extra NM->polkit call for every known permission while polkit being really slow to answer) yet seldom needed. There's no methods to access the permissions -- they're only communicated via signals. Unfortunately, we don't know when a signal is hooked, so we still need to kick of the call. Nevertheless, we don't need to wait for it to finish. --- libnm/nm-manager.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/libnm/nm-manager.c b/libnm/nm-manager.c index 5a2918b7a9..acff85cbfe 100644 --- a/libnm/nm-manager.c +++ b/libnm/nm-manager.c @@ -1199,18 +1199,18 @@ init_async_complete (NMManagerInitData *init_data) static void init_async_got_permissions (GObject *object, GAsyncResult *result, gpointer user_data) { - NMManagerInitData *init_data = user_data; + NMManager *manager = user_data; GVariant *permissions; if (nmdbus_manager_call_get_permissions_finish (NMDBUS_MANAGER (object), &permissions, result, NULL)) { - update_permissions (init_data->manager, permissions); + update_permissions (manager, permissions); g_variant_unref (permissions); } else - update_permissions (init_data->manager, NULL); + update_permissions (manager, NULL); - init_async_complete (init_data); + g_object_unref (manager); } static void @@ -1228,7 +1228,10 @@ init_async_parent_inited (GObject *source, GAsyncResult *result, gpointer user_d nmdbus_manager_call_get_permissions (priv->manager_proxy, init_data->cancellable, - init_async_got_permissions, init_data); + init_async_got_permissions, + g_object_ref (init_data->manager)); + + init_async_complete (init_data); } static void From f2099f5b796a5069a070eba2bb6580dae8b0b626 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Wed, 19 Oct 2016 10:08:46 +0200 Subject: [PATCH 6/6] cli/general: defer printing the permissions until we know them The async client might be constructed before we know the permissions. --- clients/cli/general.c | 66 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/clients/cli/general.c b/clients/cli/general.c index cd1548baf0..0f1d556986 100644 --- a/clients/cli/general.c +++ b/clients/cli/general.c @@ -454,8 +454,20 @@ permission_result_to_string (NMClientPermissionResult perm_result) } static gboolean -show_nm_permissions (NmCli *nmc) +timeout_cb (gpointer user_data) { + NmCli *nmc = (NmCli *) user_data; + + g_string_printf (nmc->return_text, _("Error: Timeout %d sec expired."), nmc->timeout); + nmc->return_value = NMC_RESULT_ERROR_TIMEOUT_EXPIRED; + quit (); + return FALSE; +} + +static int +print_permissions (void *user_data) +{ + NmCli *nmc = user_data; NMClientPermission perm; GError *error = NULL; const char *fields_str; @@ -486,6 +498,7 @@ show_nm_permissions (NmCli *nmc) arr = nmc_dup_fields_array (tmpl, tmpl_len, NMC_OF_FLAG_MAIN_HEADER_ADD | NMC_OF_FLAG_FIELD_NAMES); g_ptr_array_add (nmc->output_data, arr); + for (perm = NM_CLIENT_PERMISSION_NONE + 1; perm <= NM_CLIENT_PERMISSION_LAST; perm++) { NMClientPermissionResult perm_result = nm_client_get_permission_result (nmc->client, perm); @@ -496,6 +509,57 @@ show_nm_permissions (NmCli *nmc) } print_data (nmc); /* Print all data */ + quit (); + return G_SOURCE_REMOVE; +} + +static gboolean +got_permissions (NmCli *nmc) +{ + NMClientPermission perm; + + /* The server returns all the permissions at once, so if at least one is there + * we already received the reply. */ + for (perm = NM_CLIENT_PERMISSION_NONE + 1; perm <= NM_CLIENT_PERMISSION_LAST; perm++) { + if (nm_client_get_permission_result (nmc->client, perm) != NM_CLIENT_PERMISSION_RESULT_UNKNOWN) + return TRUE; + } + + return FALSE; +} + +static void +permission_changed (NMClient *client, + NMClientPermission permission, + NMClientPermissionResult result, + NmCli *nmc) +{ + if (got_permissions (nmc)) { + /* Defer the printing, so that we have a chance to process the other + * permission-changed signals. */ + g_idle_remove_by_data (nmc); + g_idle_add (print_permissions, nmc); + } +} + +static gboolean +show_nm_permissions (NmCli *nmc) +{ + /* The permissions are available now, just print them. */ + if (got_permissions (nmc)) { + print_permissions (nmc); + return TRUE; + } + + /* The client didn't get the permissions reply yet. Subscribe to changes. */ + g_signal_connect (nmc->client, NM_CLIENT_PERMISSION_CHANGED, + G_CALLBACK (permission_changed), nmc); + + if (nmc->timeout == -1) + nmc->timeout = 10; + g_timeout_add_seconds (nmc->timeout, timeout_cb, nmc); + + nmc->should_wait++; return TRUE; }