Re: [libvirt] [PATCH v3 6/8] virt-admin: Tweak command parsing logic so that aliases point to new commands

[Ján Tomko]

On Fri, Sep 16, 2016 at 12:50:43PM +0200, Erik Skultety wrote:

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 an internal
error.

Signed-off-by: Erik Skultety <eskul...@redhat.com>
---
tools/virsh-nodedev.c |  6 ++----
tools/virsh.pod       |  2 --
tools/vsh.c           | 15 +++++++++++++--
tools/vsh.h           |  4 +++-
4 files changed, 18 insertions(+), 9 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.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)

This man page change is unrelated to the logic change and would fit
better in a separate commit.

diff --git a/tools/vsh.c b/tools/vsh.c
index a66e2f9..88a6f37 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -329,6 +329,9 @@ vshCmddefCheckInternals(const vshCmdDef *cmd)
{
    size_t i;

+    if (cmd->flags & VSH_CMD_FLAG_ALIAS && !cmd->alias)
+        return -1;
+
    if (!cmd->opts)
        return 0;

@@ -1408,6 +1411,13 @@ 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);
+                }

There is a call to vshCmddefSearch in vshReadlineParse which also seems
to need to resolve the alias.

ACK with that fixed.

Without it, the behavior is a bit unexpected:
virsh # nodedev-det
nodedev-detach   nodedev-dettach

But the options are only offered for nodedev-detach, not the alias.

Jan

signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list