[libvirt] [PATCH v2 0/3] Introduce aliases for virt-admin's srv-* commands
the original version: https://www.redhat.com/archives/libvir-list/2016-September/msg00312.html since v1: - tweaked the virsh-self-test so that it also checks the aliased commands instead of skipping them (since there was a good reason for that before the changes this series introduces) - patches 2-3 remained untouched Erik Skultety (3): virt-admin: Tweak command parsing logic so that aliases point to new commands virt-admin: Add some command aliases to provide syntax sugar over ugly commands virt-admin: Replace the (now) aliases with new command names in the man page tools/virsh-nodedev.c | 6 ++---- tools/virsh.c | 10 ++++++---- tools/virsh.pod | 2 -- tools/virt-admin.c | 24 ++++++++++++++++++++++++ tools/virt-admin.pod | 30 +++++++++++++++--------------- tools/vsh.c | 6 ++++++ tools/vsh.h | 1 + 7 files changed, 54 insertions(+), 25 deletions(-) -- 2.5.5
Change the logic in a way, so that VSH_CMD_FLAG_ALIAS behaves similarly to how VSH_OT_ALIAS for command options, i.e. there is no need for code duplication for the alias and the aliased command structures. Along with that change, switch any existing VSH_CMD_FLAG_ALIAS occurrences to this new format. Also, since this patch introduces a new command structure element, adjust the virsh-self-test test to make sure we won't ever miss to specify the '.alias' member for an aliased command because doing that would lead to a segfault, as would be the case for any missing element of the command structure. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- tools/virsh-nodedev.c | 6 ++---- tools/virsh.c | 10 ++++++---- tools/virsh.pod | 2 -- tools/vsh.c | 6 ++++++ tools/vsh.h | 1 + 5 files changed, 15 insertions(+), 10 deletions(-) diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 321f15c..9446664 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -986,10 +986,8 @@ const vshCmdDef nodedevCmds[] = { .flags = 0 }, {.name = "nodedev-dettach", - .handler = cmdNodeDeviceDetach, - .opts = opts_node_device_detach, - .info = info_node_device_detach, - .flags = VSH_CMD_FLAG_ALIAS + .flags = VSH_CMD_FLAG_ALIAS, + .alias = "nodedev-detach" }, {.name = "nodedev-dumpxml", .handler = cmdNodeDeviceDumpXML, diff --git a/tools/virsh.c b/tools/virsh.c index cb60edc..918058c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -373,10 +373,11 @@ cmdSelfTest(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) for (grp = cmdGroups; grp->name; grp++) { for (def = grp->commands; def->name; def++) { - if (def->flags & VSH_CMD_FLAG_ALIAS) - continue; + const vshCmdDef *c = def; + if (c->flags & VSH_CMD_FLAG_ALIAS) + c = vshCmddefSearch(c->alias); - if (!vshCmddefHelp(ctl, def->name)) + if (!vshCmddefHelp(ctl, c->name)) return false; } } @@ -904,7 +905,8 @@ static const vshCmdDef virshCmds[] = { .handler = cmdSelfTest, .opts = NULL, .info = info_selftest, - .flags = VSH_CMD_FLAG_NOCONNECT | VSH_CMD_FLAG_ALIAS + .flags = VSH_CMD_FLAG_NOCONNECT | VSH_CMD_FLAG_ALIAS, + .alias = "self-test" }, {.name = NULL} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index 3da7879..49abda9 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2936,8 +2936,6 @@ make that device unusable by the rest of the physical host until a reboot. Detach I<nodedev> from the host, so that it can safely be used by guests via <hostdev> passthrough. This is reversed with B<nodedev-reattach>, and is done automatically for managed devices. -For compatibility purposes, this command can also be spelled -B<nodedev-dettach>. Different backend drivers expect the device to be bound to different dummy devices. For example, QEMU's "kvm" backend driver (the default) diff --git a/tools/vsh.c b/tools/vsh.c index be6a073..ad3e1c7 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1410,6 +1410,12 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) vshError(ctl, _("unknown command: '%s'"), tkdata); goto syntaxError; /* ... or ignore this command only? */ } + /* aliases need to be resolved to the actual commands */ + if (cmd->flags & VSH_CMD_FLAG_ALIAS) { + VIR_FREE(tkdata); + tkdata = vshStrdup(ctl, cmd->alias); + cmd = vshCmddefSearch(tkdata); + } if (vshCmddefOptParse(cmd, &opts_need_arg, &opts_required) < 0) { vshError(ctl, diff --git a/tools/vsh.h b/tools/vsh.h index e53910f..e383e54 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -179,6 +179,7 @@ struct _vshCmdDef { const vshCmdOptDef *opts; /* definition of command options */ const vshCmdInfo *info; /* details about command */ unsigned int flags; /* bitwise OR of VSH_CMD_FLAG */ + const char *alias; /* name of the aliased command */ }; /* -- 2.5.5
Make use of the new recently introduced alias handling for virt-admin srv-* commands. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- tools/virt-admin.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 12ec057..5d6beda 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -1265,18 +1265,30 @@ static const vshCmdDef vshAdmCmds[] = { static const vshCmdDef monitoringCmds[] = { {.name = "srv-list", + .flags = VSH_CMD_FLAG_ALIAS, + .alias = "server-list" + }, + {.name = "server-list", .handler = cmdSrvList, .opts = NULL, .info = info_srv_list, .flags = 0 }, {.name = "srv-threadpool-info", + .flags = VSH_CMD_FLAG_ALIAS, + .alias = "server-threadpool-info" + }, + {.name = "server-threadpool-info", .handler = cmdSrvThreadpoolInfo, .opts = opts_srv_threadpool_info, .info = info_srv_threadpool_info, .flags = 0 }, {.name = "srv-clients-list", + .flags = VSH_CMD_FLAG_ALIAS, + .alias = "client-list" + }, + {.name = "client-list", .handler = cmdSrvClientsList, .opts = opts_srv_clients_list, .info = info_srv_clients_list, @@ -1289,6 +1301,10 @@ static const vshCmdDef monitoringCmds[] = { .flags = 0 }, {.name = "srv-clients-info", + .flags = VSH_CMD_FLAG_ALIAS, + .alias = "server-clients-info" + }, + {.name = "server-clients-info", .handler = cmdSrvClientsInfo, .opts = opts_srv_clients_info, .info = info_srv_clients_info, @@ -1299,6 +1315,10 @@ static const vshCmdDef monitoringCmds[] = { static const vshCmdDef managementCmds[] = { {.name = "srv-threadpool-set", + .flags = VSH_CMD_FLAG_ALIAS, + .alias = "server-threadpool-set" + }, + {.name = "server-threadpool-set", .handler = cmdSrvThreadpoolSet, .opts = opts_srv_threadpool_set, .info = info_srv_threadpool_set, @@ -1311,6 +1331,10 @@ static const vshCmdDef managementCmds[] = { .flags = 0 }, {.name = "srv-clients-set", + .flags = VSH_CMD_FLAG_ALIAS, + .alias = "server-clients-set" + }, + {.name = "server-clients-set", .handler = cmdSrvClientsSet, .opts = opts_srv_clients_set, .info = info_srv_clients_set, -- 2.5.5
Since the old command names are being shadowed by the new ones in the code as well as in the help messages, update the man page accordingly. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- tools/virt-admin.pod | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod index 020eb51..443730e 100644 --- a/tools/virt-admin.pod +++ b/tools/virt-admin.pod @@ -150,7 +150,7 @@ change its internal configuration. =over 4 -=item B<srv-list> +=item B<server-list> Lists all manageable servers contained within the daemon the client is currently connected to. @@ -164,7 +164,7 @@ The I<server> is specified by its name. =over 4 -=item B<srv-threadpool-info> I<server> +=item B<server-threadpool-info> I<server> Retrieve server's threadpool attributes. These attributes include: @@ -207,11 +207,11 @@ control and libvirt guarantees that such a task cannot hang, thus will always finish. An example of such a task this would be destroying a domain: $ virsh destroy <domain>. -=item B<srv-threadpool-set> I<server> [I<--min-workers> B<count>] +=item B<server-threadpool-set> I<server> [I<--min-workers> B<count>] [I<--max-workers> B<count>] [I<--priority-workers> B<count>] Change threadpool attributes on a server. Only a fraction of all attributes as -described in I<srv-threadpool-info> is supported for the setter. +described in I<server-threadpool-info> is supported for the setter. =over 4 @@ -232,14 +232,7 @@ The current number of active priority workers in a threadpool. =back -=item B<srv-clients-list> I<server> - -Print a table showing the list of clients connected to <server>, also providing -information about transport type used on client's connection (supported -transports include B<unix>, B<tcp>, and B<tls>), as well as providing -information about client's connection time (system local time is used). - -=item B<srv-clients-info> I<server> +=item B<server-clients-info> I<server> Get information about the current setting of limits regarding connections of new clients. This information comprises of the limits to the maximum number of @@ -249,13 +242,13 @@ runtime values, more specifically, the current number of clients connected to I<server> and the current number of clients waiting for authentication. B<Example> - # virt-admin srv-clients-info libvirtd + # virt-admin server-clients-info libvirtd nclients_max : 120 nclients : 3 nclients_unauth_max : 20 nclients_unauth : 0 -=item B<srv-clients-set> I<server> [I<--max-clients> B<count>] +=item B<server-clients-set> I<server> [I<--max-clients> B<count>] [I<--max-unauth-clients> B<count>] Set new client-related limits on I<server>. @@ -284,10 +277,17 @@ I<--max-clients>. The following commands provide management and monitoring of clients connected to one of daemon's available servers. Clients are specified by their numeric ID which is obtained by listing all clients connected to a specified server -(see command B<srv-clients-list>). +(see command B<client-list>). =over 4 +=item B<client-list> I<server> + +Print a table showing the list of clients connected to <server>, also providing +information about transport type used on client's connection (supported +transports include B<unix>, B<tcp>, and B<tls>), as well as providing +information about client's connection time (system local time is used). + =item B<client-info> I<server> I<client> Retrieve identity information about I<client> from I<server>. The attributes -- 2.5.5
On 13/09/16 17:11, Erik Skultety wrote:
the original version: https://www.redhat.com/archives/libvir-list/2016-September/msg00312.html
since v1: - tweaked the virsh-self-test so that it also checks the aliased commands instead of skipping them (since there was a good reason for that before the changes this series introduces) - patches 2-3 remained untouched
Erik Skultety (3): virt-admin: Tweak command parsing logic so that aliases point to new commands virt-admin: Add some command aliases to provide syntax sugar over ugly commands virt-admin: Replace the (now) aliases with new command names in the man page
tools/virsh-nodedev.c | 6 ++---- tools/virsh.c | 10 ++++++---- tools/virsh.pod | 2 -- tools/virt-admin.c | 24 ++++++++++++++++++++++++ tools/virt-admin.pod | 30 +++++++++++++++--------------- tools/vsh.c | 6 ++++++ tools/vsh.h | 1 + 7 files changed, 54 insertions(+), 25 deletions(-)
SNACK. Having an error like "internal error: bad command options" would be nice indeed - thanks Michal [1] - which could be achieved by a tiny refactor, also fixing a semantically incorrect check that can cause a segfault on missing '.info' as well, pretty much the same way as the missing '.alias' does. [1] https://www.redhat.com/archives/libvir-list/2016-September/msg00386.html Erik
participants (1)
-
Erik Skultety