[PATCH] virsh: fix regression in argv parsing

[Eric Blake <eblake@xxxxxxxxxx>]

Commit 85d2810 broke commands with mandatory argv (send-key,
qemu-monitor-command).  This fixes things, and enhances the
test to cover it.

* tools/virsh.c (vshCmddefGetOption, vshCmddefGetData)
(vshCommandParse): Fix option parsing for required argv option.
(vshCmddefOptParse): Check that argv option is last.
* tests/virsh-optparse: Enhance test.
---
 tests/virsh-optparse |    9 +++++++++
 tools/virsh.c        |   24 +++++++++++++++---------
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/tests/virsh-optparse b/tests/virsh-optparse
index 18252d2..d0d4329 100755
--- a/tests/virsh-optparse
+++ b/tests/virsh-optparse
@@ -118,6 +118,7 @@ for args in \
     'test name desc vda vdb' \
     'test --diskspec vda name --diskspec vdb desc' \
     '--description desc --name name --domain test vda vdb' \
+    '--description desc --diskspec vda --name name --domain test vdb' \
 ; do
   virsh -q -c $test_url snapshot-create-as --print-xml $args \
     >out 2>>err || fail=1
@@ -126,4 +127,12 @@ done

 test -s err && fail=1

+# Test a required argv
+cat <<\EOF > exp-err || framework_failure
+error: this function is not supported by the connection driver: virDomainQemuMonitorCommand
+EOF
+virsh -q -c $test_url qemu-monitor-command test a >out 2>err && fail=1
+test -s out && fail=1
+compare err exp-err || fail=1
+
 (exit $fail); exit $fail
diff --git a/tools/virsh.c b/tools/virsh.c
index 371346a..0b187bc 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -13834,6 +13834,7 @@ vshCmddefGetInfo(const vshCmdDef * cmd, const char *name)
     return NULL;
 }

+/* Validate that the options associated with cmd can be parsed.  */
 static int
 vshCmddefOptParse(const vshCmdDef *cmd, uint32_t *opts_need_arg,
                   uint32_t *opts_required)
@@ -13871,13 +13872,16 @@ vshCmddefOptParse(const vshCmdDef *cmd, uint32_t *opts_need_arg,
         } else {
             optional = true;
         }
+
+        if (opt->type == VSH_OT_ARGV && cmd->opts[i + 1].name)
+            return -1; /* argv option must be listed last */
     }
     return 0;
 }

 static const vshCmdOptDef *
 vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char *name,
-                   uint32_t *opts_seen)
+                   uint32_t *opts_seen, int *opt_index)
 {
     int i;

@@ -13885,12 +13889,12 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char *name,
         const vshCmdOptDef *opt = &cmd->opts[i];

         if (STREQ(opt->name, name)) {
-            if (*opts_seen & (1 << i)) {
+            if ((*opts_seen & (1 << i)) && opt->type != VSH_OT_ARGV) {
                 vshError(ctl, _("option --%s already seen"), name);
                 return NULL;
             }
-            if (opt->type != VSH_OT_ARGV)
-                *opts_seen |= 1 << i;
+            *opts_seen |= 1 << i;
+            *opt_index = i;
             return opt;
         }
     }
@@ -13913,10 +13917,9 @@ vshCmddefGetData(const vshCmdDef *cmd, uint32_t *opts_need_arg,
     /* Grab least-significant set bit */
     i = ffs(*opts_need_arg) - 1;
     opt = &cmd->opts[i];
-    if (opt->type != VSH_OT_ARGV) {
+    if (opt->type != VSH_OT_ARGV)
         *opts_need_arg &= ~(1 << i);
-        *opts_seen |= 1 << i;
-    }
+    *opts_seen |= 1 << i;
     return opt;
 }

@@ -14878,12 +14881,14 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser)
             } else if (tkdata[0] == '-' && tkdata[1] == '-' &&
                        c_isalnum(tkdata[2])) {
                 char *optstr = strchr(tkdata + 2, '=');
+                int opt_index;
+
                 if (optstr) {
                     *optstr = '\0'; /* convert the '=' to '\0' */
                     optstr = vshStrdup(ctl, optstr + 1);
                 }
                 if (!(opt = vshCmddefGetOption(ctl, cmd, tkdata + 2,
-                                               &opts_seen))) {
+                                               &opts_seen, &opt_index))) {
                     VIR_FREE(optstr);
                     goto syntaxError;
                 }
@@ -14905,7 +14910,8 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser)
                                  VSH_OT_INT ? _("number") : _("string"));
                         goto syntaxError;
                     }
-                    opts_need_arg &= ~opts_seen;
+                    if (opt->type != VSH_OT_ARGV)
+                        opts_need_arg &= ~(1 << opt_index);
                 } else {
                     tkdata = NULL;
                     if (optstr) {
-- 
1.7.4.4

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