virsh: list required options first

The current state of virsh parsing is that:

$ virsh vol-info /path/to/image
$ virsh vol-info --pool default /path/to/image
$ virsh vol-info --pool default --vol /path/to/image

all lookup the volume by path (technically, the last two also attempt
a name lookup within a pool, whereas the first skips that step, but
the end result is the same); meanwhile:

$ virsh vol-info default /path/to/image

complains about unexpected data.  Why?  Because the --pool option is
optional, so default was parsed as the --vol argument, and
/path/to/image.img doesn't match up with any remaining options that
require an argument.  For proof, note that:

$ virsh vol-info default --vol /path/to/image

complains about looking up 'default' - the parser mis-associated both
arguments with --vol.  Given the above, the only way to specify pool
is with an explicit "--pool" argument (you can't specify it
positionally).  However, named arguments can appear in any order, so:

$ virsh vol-info /path/to/image --pool default
$ virsh vol-info --vol /path/to/image --pool default

have also always worked.  Therefore, this patch has no functional
change on vol-info option parsing, but only on 'virsh help vol-info'
synopsis layout.  However, it also allows the next patch to 1) enforce
that required options are always first (without this patch, the next
patch would fail the testsuite), and 2) allow the user to omit the
"--pool" argument.  That is, the next patch makes it possible to do:

$ virsh vol-info /path/to/image default

which to date was not possible.

* tools/virsh.c (opts_vol_create_from, opts_vol_clone)
(opts_vol_upload, opts_vol_download, opts_vol_delete)
(opts_vol_wipe, opts_vol_info, opts_vol_dumpxml, opts_vol_key)
(opts_vol_path): List optional pool parameter after required
arguments.

diff --git a/tools/virsh.c b/tools/virsh.c
index 2e350214412db39e89857c638d0273aaea6e7746..cd1d2465f262cc5e354e592c2b351f99b99bcfdb 100644 (file)
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -6963,8 +6963,8 @@ static const vshCmdInfo info_vol_create_from[] = {
 static const vshCmdOptDef opts_vol_create_from[] = {
     {"pool", VSH_OT_DATA, VSH_OFLAG_REQ, N_("pool name")},
     {"file", VSH_OT_DATA, VSH_OFLAG_REQ, N_("file containing an XML vol description")},
-    {"inputpool", VSH_OT_STRING, 0, N_("pool name or uuid of the input volume's pool")},
     {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("input vol name or key")},
+    {"inputpool", VSH_OT_STRING, 0, N_("pool name or uuid of the input volume's pool")},
     {NULL, 0, 0, NULL}
 };
 
@@ -7059,9 +7059,9 @@ static const vshCmdInfo info_vol_clone[] = {
 };
 
 static const vshCmdOptDef opts_vol_clone[] = {
-    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("orig vol name or key")},
     {"newname", VSH_OT_DATA, VSH_OFLAG_REQ, N_("clone name")},
+    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {NULL, 0, 0, NULL}
 };
 
@@ -7136,9 +7136,9 @@ static const vshCmdInfo info_vol_upload[] = {
 };
 
 static const vshCmdOptDef opts_vol_upload[] = {
-    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")},
     {"file", VSH_OT_DATA, VSH_OFLAG_REQ, N_("file")},
+    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {"offset", VSH_OT_INT, 0, N_("volume offset to upload to") },
     {"length", VSH_OT_INT, 0, N_("amount of data to upload") },
     {NULL, 0, 0, NULL}
@@ -7236,9 +7236,9 @@ static const vshCmdInfo info_vol_download[] = {
 };
 
 static const vshCmdOptDef opts_vol_download[] = {
-    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")},
     {"file", VSH_OT_DATA, VSH_OFLAG_REQ, N_("file")},
+    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {"offset", VSH_OT_INT, 0, N_("volume offset to download from") },
     {"length", VSH_OT_INT, 0, N_("amount of data to download") },
     {NULL, 0, 0, NULL}
@@ -7342,8 +7342,8 @@ static const vshCmdInfo info_vol_delete[] = {
 };
 
 static const vshCmdOptDef opts_vol_delete[] = {
-    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")},
+    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {NULL, 0, 0, NULL}
 };
 
@@ -7383,8 +7383,8 @@ static const vshCmdInfo info_vol_wipe[] = {
 };
 
 static const vshCmdOptDef opts_vol_wipe[] = {
-    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")},
+    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {NULL, 0, 0, NULL}
 };
 
@@ -7424,8 +7424,8 @@ static const vshCmdInfo info_vol_info[] = {
 };
 
 static const vshCmdOptDef opts_vol_info[] = {
-    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")},
+    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {NULL, 0, 0, NULL}
 };
 
@@ -7475,8 +7475,8 @@ static const vshCmdInfo info_vol_dumpxml[] = {
 };
 
 static const vshCmdOptDef opts_vol_dumpxml[] = {
-    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")},
+    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {NULL, 0, 0, NULL}
 };
 
@@ -7888,8 +7888,8 @@ static const vshCmdInfo info_vol_key[] = {
 };
 
 static const vshCmdOptDef opts_vol_key[] = {
-    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("volume name or path")},
+    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {NULL, 0, 0, NULL}
 };
 
@@ -7921,8 +7921,8 @@ static const vshCmdInfo info_vol_path[] = {
 };
 
 static const vshCmdOptDef opts_vol_path[] = {
-    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("volume name or key")},
+    {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
     {NULL, 0, 0, NULL}
 };