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 
---
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 from the host, so that it can safely be used by
guests via  passthrough.  This is reversed with
B, and is done automatically for managed devices.
-For compatibility purposes, this command can also be spelled
-B.

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

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

[Erik Skultety]

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 
---
 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 from the host, so that it can safely be used by
 guests via  passthrough.  This is reversed with
 B, and is done automatically for managed devices.
-For compatibility purposes, this command can also be spelled
-B.
 
 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 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);
+}
 if (vshCmddefCheckInternals(cmd) < 0) {
 vshError(ctl, _("internal error: wrong command structure: "
 "'%s'"), tkdata);
@@ -3359,10 +3369,11 @@ cmdSelfTest(vshControl *ctl, const vshCmd *cmd 
ATTRIBUTE_UNUSED)
 
 for (grp = cmdGroups; grp->name; grp++) {
 for (def = grp->commands; def->name; def++) {
+const vshCmdDef *c = def;
 if (def->flags & VSH_CMD_FLAG_ALIAS)
-continue;
+c = vshCmddefSearch(c->alias);
 
-if (!vshCmddefHelp(ctl, def->name))
+if (!vshCmddefHelp(ctl, c->name))
 return false;
 }
 }
diff --git a/tools/vsh.h b/tools/vsh.h
index 7f08191..8f5d1a6 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 */
 };
 
 /*
@@ -445,7 +446,8 @@ bool cmdSelfTest(vshControl *ctl, const vshCmd *cmd);
 .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"\
 }
 
 
-- 
2.5.5

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