Commit 4b329727 authored by Thomas Haller's avatar Thomas Haller

cli: rework connection handling for multiple results

Functions like nmc_find_connection() and nmc_find_active_connection()
can easily find multiple matching results. For example, the
"connection.id" in NetworkManager is not enforced to be unique,
so if the user adds multiple connections with the same name,
they should be all selected.

The previous API had a @pos argument, that allowed to iterate over
the results. Change that, to return all matches in a GPtrArray.

Also, extend connection-show and other places, to anticipate that
a connection might be active multiple times in any moment.
parent 4029d4df
......@@ -412,8 +412,11 @@ print_dhcp_config (NMDhcpConfig *dhcp,
* @connections: array of NMConnections to search in
* @filter_type: "id", "uuid", "path" or %NULL
* @filter_val: connection to find (connection name, UUID or path)
* @start: where to start in @list. The location is updated so that the function
* can be called multiple times (for connections with the same name).
* @out_result: if not NULL, attach all matching connection to this
* list. If necessary, a new array will be allocated. If the array
* already contains a connection, it will not be added a second time.
* All object are referenced by the array. If the function allocates
* a new array, it will set the free function to g_object_unref.
* @complete: print possible completions
*
* Find a connection in @list according to @filter_val. @filter_type determines
......@@ -428,17 +431,18 @@ NMConnection *
nmc_find_connection (const GPtrArray *connections,
const char *filter_type,
const char *filter_val,
int *start,
GPtrArray **out_result,
gboolean complete)
{
NMConnection *connection;
NMConnection *found = NULL;
guint i;
NMConnection *best_candidate = NULL;
GPtrArray *result = out_result ? *out_result : NULL;
guint i, j;
nm_assert (connections);
nm_assert (filter_val);
for (i = start ? *start : 0; i < connections->len; i++) {
for (i = 0; i < connections->len; i++) {
const char *v, *v_num;
connection = NM_CONNECTION (connections->pdata[i]);
......@@ -476,35 +480,39 @@ nmc_find_connection (const GPtrArray *connections,
continue;
found:
if (!start)
if (!out_result)
return connection;
if (found) {
*start = i;
return found;
if (!best_candidate)
best_candidate = connection;
if (!result)
result = g_ptr_array_new_with_free_func (g_object_unref);
for (j = 0; j < result->len; j++) {
if (connection == result->pdata[j])
break;
}
found = connection;
if (j == result->len)
g_ptr_array_add (result, g_object_ref (connection));
}
if (start)
*start = 0;
return found;
NM_SET_OUT (out_result, result);
return best_candidate;
}
NMActiveConnection *
nmc_find_active_connection (const GPtrArray *active_cons,
const char *filter_type,
const char *filter_val,
int *idx,
GPtrArray **out_result,
gboolean complete)
{
int i;
int start = (idx && *idx > 0) ? *idx : 0;
NMRemoteConnection *con;
NMActiveConnection *found = NULL;
guint i, j;
NMActiveConnection *best_candidate = NULL;
GPtrArray *result = out_result ? *out_result : NULL;
nm_assert (filter_val);
for (i = start; i < active_cons->len; i++) {
for (i = 0; i < active_cons->len; i++) {
NMRemoteConnection *con;
NMActiveConnection *candidate = g_ptr_array_index (active_cons, i);
const char *v, *v_num;
......@@ -552,19 +560,24 @@ nmc_find_active_connection (const GPtrArray *active_cons,
}
continue;
found:
if (!idx)
if (!out_result)
return candidate;
if (found) {
*idx = i;
return found;
if (!best_candidate)
best_candidate = candidate;
if (!result)
result = g_ptr_array_new_with_free_func (g_object_unref);
for (j = 0; j < result->len; j++) {
if (candidate == result->pdata[j])
break;
}
found = candidate;
if (j == result->len)
g_ptr_array_add (result, g_object_ref (candidate));
}
if (idx)
*idx = 0;
return found;
NM_SET_OUT (out_result, result);
return best_candidate;
}
static gboolean
......
......@@ -32,13 +32,13 @@ gboolean print_dhcp_config (NMDhcpConfig *dhcp, const NmcConfig *nmc_config, con
NMConnection *nmc_find_connection (const GPtrArray *connections,
const char *filter_type,
const char *filter_val,
int *start,
GPtrArray **out_result,
gboolean complete);
NMActiveConnection *nmc_find_active_connection (const GPtrArray *active_cons,
const char *filter_type,
const char *filter_val,
int *idx,
GPtrArray **out_result,
gboolean complete);
void nmc_secrets_requested (NMSecretAgentSimple *agent,
......
......@@ -570,26 +570,37 @@ get_ac_device_string (NMActiveConnection *active)
}
static NMActiveConnection *
get_ac_for_connection (const GPtrArray *active_cons, NMConnection *connection)
get_ac_for_connection (const GPtrArray *active_cons, NMConnection *connection, GPtrArray **out_result)
{
const char *con_path, *ac_con_path;
int i;
NMActiveConnection *ac = NULL;
guint i;
NMActiveConnection *best_candidate = NULL;
GPtrArray *result = out_result ? *out_result : NULL;
/* Is the connection active? */
con_path = nm_connection_get_path (connection);
for (i = 0; i < active_cons->len; i++) {
NMActiveConnection *candidate = g_ptr_array_index (active_cons, i);
NMRemoteConnection *con;
con = nm_active_connection_get_connection (candidate);
ac_con_path = con ? nm_connection_get_path (NM_CONNECTION (con)) : NULL;
if (!g_strcmp0 (ac_con_path, con_path)) {
ac = candidate;
break;
if (NM_CONNECTION (con) != connection) {
/* also compare the D-Bus paths. Why? I don't know. */
ac_con_path = con ? nm_connection_get_path (NM_CONNECTION (con)) : NULL;
if (!nm_streq0 (ac_con_path, con_path))
continue;
}
if (!out_result)
return candidate;
if (!best_candidate)
best_candidate = candidate;
if (!result)
result = g_ptr_array_new_with_free_func (g_object_unref);
g_ptr_array_add (result, g_object_ref (candidate));
}
return ac;
NM_SET_OUT (out_result, result);
return best_candidate;
}
typedef struct {
......@@ -775,7 +786,7 @@ fill_output_connection (NMConnection *connection, NMClient *client, NMCPrintOutp
s_con = nm_connection_get_setting_connection (connection);
g_assert (s_con);
ac = get_ac_for_connection (nm_client_get_active_connections (client), connection);
ac = get_ac_for_connection (nm_client_get_active_connections (client), connection, NULL);
if (active_only && !ac)
return;
......@@ -1365,8 +1376,8 @@ compare_connections (gconstpointer a, gconstpointer b, gpointer user_data)
switch (item) {
case NMC_SORT_ACTIVE:
case NMC_SORT_ACTIVE_INV:
aca = get_ac_for_connection (nm_client_get_active_connections (info->nmc->client), ca);
acb = get_ac_for_connection (nm_client_get_active_connections (info->nmc->client), cb);
aca = get_ac_for_connection (nm_client_get_active_connections (info->nmc->client), ca, NULL);
acb = get_ac_for_connection (nm_client_get_active_connections (info->nmc->client), cb, NULL);
cmp = (aca && !acb) ? -1 : (!aca && acb) ? 1 : 0;
if (item == NMC_SORT_ACTIVE_INV)
cmp = -(cmp);
......@@ -1544,12 +1555,21 @@ parse_preferred_connection_order (const char *order, GError **error)
}
static NMConnection *
get_connection (NmCli *nmc, int *argc, char ***argv, int *pos, GError **error)
get_connection (NmCli *nmc,
int *argc,
char ***argv,
const char **out_selector,
const char **out_value,
GPtrArray **out_result,
GError **error)
{
const GPtrArray *connections;
NMConnection *connection = NULL;
const char *selector = NULL;
NM_SET_OUT (out_selector, NULL);
NM_SET_OUT (out_value, NULL);
if (*argc == 0) {
g_set_error_literal (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT,
_("No connection specified"));
......@@ -1573,20 +1593,18 @@ get_connection (NmCli *nmc, int *argc, char ***argv, int *pos, GError **error)
}
}
NM_SET_OUT (out_selector, selector);
NM_SET_OUT (out_value, **argv);
connections = nm_client_get_connections (nmc->client);
connection = nmc_find_connection (connections, selector, **argv, pos,
connection = nmc_find_connection (connections, selector, **argv, out_result,
*argc == 1 && nmc->complete);
if (!connection) {
g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_NOT_FOUND,
_("unknown connection '%s'"), **argv);
}
/* If the caller wants multiple results (pos is set) and there are any,
* don't switch to next argument.
*/
if (!pos || !*pos)
next_arg (nmc, argc, argv, NULL);
next_arg (nmc, argc, argv, NULL);
return connection;
}
......@@ -1599,7 +1617,8 @@ do_connections_show (NmCli *nmc, int argc, char **argv)
GPtrArray *invisibles, *sorted_cons;
gboolean active_only = FALSE;
gs_unref_array GArray *order = NULL;
int i, option;
guint i, j;
int option;
/* check connection show options [--active] [--order <order spec>] */
while ((option = next_arg (nmc, &argc, &argv, "--active", "--order", NULL)) > 0) {
......@@ -1674,7 +1693,6 @@ do_connections_show (NmCli *nmc, int argc, char **argv)
gboolean new_line = FALSE;
gboolean without_fields = (nmc->required_fields == NULL);
const GPtrArray *active_cons = nm_client_get_active_connections (nmc->client);
int pos = 0;
/* multiline mode is default for 'connection show <ID>' */
if (!nmc->mode_specified)
......@@ -1704,8 +1722,11 @@ do_connections_show (NmCli *nmc, int argc, char **argv)
const GPtrArray *connections;
gboolean res;
NMConnection *con;
NMActiveConnection *acon = NULL;
gs_unref_object NMActiveConnection *explicit_acon = NULL;
const char *selector = NULL;
gs_unref_ptrarray GPtrArray *found_cons = NULL;
gboolean explicit_acon_handled = FALSE;
guint i_found_cons;
if (argc == 1 && nmc->complete)
nmc_complete_strings (*argv, "id", "uuid", "path", "apath", NULL);
......@@ -1723,18 +1744,26 @@ do_connections_show (NmCli *nmc, int argc, char **argv)
/* Try to find connection by id, uuid or path first */
connections = nm_client_get_connections (nmc->client);
con = nmc_find_connection (connections, selector, *argv, &pos,
con = nmc_find_connection (connections, selector, *argv, &found_cons,
argc == 1 && nmc->complete);
if ( !con
&& NM_IN_STRSET (selector, NULL, "apath")) {
/* Try apath too */
acon = nmc_find_active_connection (active_cons, "apath", *argv, NULL,
argc == 1 && nmc->complete);
if (acon)
con = NM_CONNECTION (nm_active_connection_get_connection (acon));
explicit_acon = nmc_find_active_connection (active_cons, "apath", *argv, NULL,
argc == 1 && nmc->complete);
if (explicit_acon) {
if ( !selector
&& !nm_streq0 (*argv, nm_object_get_path (NM_OBJECT (explicit_acon)))) {
/* we matched the apath based on the last component alone (note the full D-Bus path).
* That is how nmc_find_active_connection() works, if you pass in a selector.
* Reject it. */
explicit_acon = NULL;
}
nm_g_object_ref (explicit_acon);
}
}
if (!con && !acon) {
if (!con && !explicit_acon) {
g_string_printf (nmc->return_text, _("Error: %s - no such connection profile."), *argv);
nmc->return_value = NMC_RESULT_ERROR_NOT_FOUND;
goto finish;
......@@ -1746,54 +1775,81 @@ do_connections_show (NmCli *nmc, int argc, char **argv)
* may see only the active connection.
*/
/* Filter only active connections */
if (!acon)
acon = get_ac_for_connection (active_cons, con);
if (active_only && !acon) {
next_arg (nmc, &argc, &argv, NULL);
continue;
}
if (nmc->complete) {
next_arg (nmc, &argc, &argv, NULL);
continue;
}
/* Show an empty line between connections */
if (new_line)
g_print ("\n");
/* Show profile configuration */
if (without_fields || profile_flds) {
if (con) {
nmc->required_fields = profile_flds;
if (nmc->nmc_config.show_secrets)
update_secrets_in_connection (NM_REMOTE_CONNECTION (con), con);
res = nmc_connection_profile_details (con, nmc);
nmc->required_fields = NULL;
if (!res)
goto finish;
explicit_acon_handled = FALSE;
i_found_cons = 0;
for (;;) {
gs_unref_ptrarray GPtrArray *found_acons = NULL;
if (explicit_acon) {
if (explicit_acon_handled)
break;
explicit_acon_handled = TRUE;
/* the user referenced an "apath". In this case, we can only have at most one connection
* and one apath. */
con = NM_CONNECTION (nm_active_connection_get_connection (explicit_acon));
} else {
if (i_found_cons >= found_cons->len)
break;
con = found_cons->pdata[i_found_cons++];
get_ac_for_connection (active_cons, con, &found_acons);
}
}
/* If the profile is active, print also active details */
if (without_fields || active_flds) {
if (acon) {
nmc->required_fields = active_flds;
res = nmc_active_connection_details (acon, nmc);
nmc->required_fields = NULL;
if (!res)
goto finish;
if (active_only && !explicit_acon && !found_acons) {
/* this connection is not interesting, we only print active ones. */
continue;
}
nm_assert (explicit_acon || con);
if (new_line)
g_print ("\n");
new_line = TRUE;
if (without_fields || profile_flds) {
if (con) {
nmc->required_fields = profile_flds;
if (nmc->nmc_config.show_secrets)
update_secrets_in_connection (NM_REMOTE_CONNECTION (con), con);
res = nmc_connection_profile_details (con, nmc);
nmc->required_fields = NULL;
if (!res)
goto finish;
}
}
if (without_fields || active_flds) {
guint l = explicit_acon ? 1 : (found_acons ? found_acons->len : 0);
for (j = 0; j < l; j++) {
NMActiveConnection *acon;
if (j > 0) {
/* if there are multiple active connections, separate them with newline.
* that is a bit odd, because we already separate connections with newlines,
* and commonly don't separate the connection from the first active connection. */
g_print ("\n");
}
if (explicit_acon)
acon = explicit_acon;
else
acon = found_acons->pdata[j];
nmc->required_fields = active_flds;
res = nmc_active_connection_details (acon, nmc);
nmc->required_fields = NULL;
if (!res)
goto finish;
}
}
}
new_line = TRUE;
/* Take next argument.
* But for pos != NULL we have more connections of the same name,
* so process the same argument again.
*/
if (!pos)
next_arg (nmc, &argc, &argv, NULL);
next_arg (nmc, &argc, &argv, NULL);
}
}
......@@ -2364,7 +2420,7 @@ do_connection_up (NmCli *nmc, int argc, char **argv)
}
if (argc > 0 && strcmp (*argv, "ifname") != 0) {
connection = get_connection (nmc, argc_ptr, argv_ptr, NULL, &error);
connection = get_connection (nmc, argc_ptr, argv_ptr, NULL, NULL, NULL, &error);
if (!connection) {
g_string_printf (nmc->return_text, _("Error: %s."), error->message);
return error->code;
......@@ -2443,20 +2499,108 @@ do_connection_up (NmCli *nmc, int argc, char **argv)
return nmc->return_value;
}
/*****************************************************************************/
typedef struct {
NmCli *nmc;
GSList *queue;
/* a list of object that is relevant for the callback. The object
* type differs, and depends on the type of callback. */
GPtrArray *obj_list;
guint timeout_id;
GCancellable *cancellable;
} ConnectionCbInfo;
static void connection_cb_info_finish (ConnectionCbInfo *info,
gpointer connection);
static void connection_removed_cb (NMClient *client, NMConnection *connection, ConnectionCbInfo *info);
static void down_active_connection_state_cb (NMActiveConnection *active,
GParamSpec *pspec,
ConnectionCbInfo *info);
static void
connection_cb_info_obj_list_destroy (ConnectionCbInfo *info, gpointer obj)
{
nm_assert (info);
nm_assert (info->obj_list);
nm_assert (G_IS_OBJECT (obj));
g_signal_handlers_disconnect_by_func (obj, down_active_connection_state_cb, info);
g_object_unref (obj);
}
static gssize
connection_cb_info_obj_list_idx (ConnectionCbInfo *info, gpointer obj)
{
guint i;
nm_assert (info);
nm_assert (info->obj_list);
nm_assert (G_IS_OBJECT (obj));
for (i = 0; i < info->obj_list->len; i++) {
if (info->obj_list->pdata[i] == obj)
return i;
}
return -1;
}
static gpointer
connection_cb_info_obj_list_has (ConnectionCbInfo *info, gpointer obj)
{
gssize idx;
idx = connection_cb_info_obj_list_idx (info, obj);
if (idx >= 0)
return info->obj_list->pdata[idx];
return NULL;
}
static gpointer
connection_cb_info_obj_list_steal (ConnectionCbInfo *info, gpointer obj)
{
gssize idx;
idx = connection_cb_info_obj_list_idx (info, obj);
if (idx >= 0) {
g_ptr_array_remove_index (info->obj_list, idx);
return obj;
}
return NULL;
}
static void
connection_cb_info_finish (ConnectionCbInfo *info, gpointer obj)
{
if (obj) {
obj = connection_cb_info_obj_list_steal (info, obj);
if (obj)
connection_cb_info_obj_list_destroy (info, obj);
} else {
while (info->obj_list->len > 0) {
obj = info->obj_list->pdata[info->obj_list->len - 1];
g_ptr_array_remove_index (info->obj_list, info->obj_list->len);
connection_cb_info_obj_list_destroy (info, obj);
}
}
if (info->obj_list->len > 0)
return;
nm_clear_g_source (&info->timeout_id);
nm_clear_g_cancellable (&info->cancellable);
g_signal_handlers_disconnect_by_func (info->nmc->client, connection_removed_cb, info);
g_slice_free (ConnectionCbInfo, info);
quit ();
}
/*****************************************************************************/
static void
connection_removed_cb (NMClient *client, NMConnection *connection, ConnectionCbInfo *info)
{
if (!g_slist_find (info->queue, connection))
if (!connection_cb_info_obj_list_has (info, connection))
return;
g_print (_("Connection '%s' (%s) successfully deleted.\n"),
nm_connection_get_id (connection),
......@@ -2493,49 +2637,17 @@ connection_op_timeout_cb (gpointer user_data)
return G_SOURCE_REMOVE;
}
static void
destroy_queue_element (gpointer data)
{
g_signal_handlers_disconnect_matched (data, G_SIGNAL_MATCH_FUNC, 0, 0, 0,
down_active_connection_state_cb, NULL);
g_object_unref (data);
}
static void
connection_cb_info_finish (ConnectionCbInfo *info, gpointer connection)
{
if (connection) {
info->queue = g_slist_remove (info->queue, connection);
g_object_unref (G_OBJECT (connection));
} else {
g_slist_free_full (info->queue, destroy_queue_element);
info->queue = NULL;
}
if (info->queue)
return;
if (info->timeout_id)
g_source_remove (info->timeout_id);
nm_clear_g_cancellable (&info->cancellable);
g_signal_handlers_disconnect_by_func (info->nmc->client, connection_removed_cb, info);
g_slice_free (ConnectionCbInfo, info);
quit ();
}
static NMCResultCode
do_connection_down (NmCli *nmc, int argc, char **argv)
{
NMActiveConnection *active;
ConnectionCbInfo *info = NULL;
const GPtrArray *active_cons;
GSList *queue = NULL, *iter, *next;
gs_strfreev char **arg_arr = NULL;
char **arg_ptr;
int arg_num;
int idx = 0;
guint i;
gs_unref_ptrarray GPtrArray *found_active_cons = NULL;
if (nmc->timeout == -1)
nmc->timeout = 10;
......@@ -2578,73 +2690,64 @@ do_connection_down (NmCli *nmc, int argc, char **argv)
}
}
active = nmc_find_active_connection (active_cons, selector, *arg_ptr, &idx,
active = nmc_find_active_connection (active_cons,
selector,
*arg_ptr,
&found_active_cons,
arg_num == 1 && nmc->complete);
if (active) {
/* Check if the connection is unique. */
/* Calling down for the same connection repeatedly would result in
* NM responding for the last D-Bus call only and we would stall. */
if (!g_slist_find (queue, active))
queue = g_slist_prepend (queue, g_object_ref (active));
} else {
if (!active) {
if (!nmc->complete)
g_printerr (_("Error: '%s' is not an active connection.\n"), *arg_ptr);
g_string_printf (nmc->return_text, _("Error: not all active connections found."));
nmc->return_value = NMC_RESULT_ERROR_NOT_FOUND;
}
if (idx == 0)
next_arg (nmc->ask ? NULL : nmc, &arg_num, &arg_ptr, NULL);
next_arg (nmc->ask ? NULL : nmc, &arg_num, &arg_ptr, NULL);
}
if (!queue) {
if (!found_active_cons) {