[libvirt] [PATCH 1/2] virsh: list required options first

Eric Blake eblake at redhat.com
Tue Apr 12 21:35:06 UTC 2011


The current state of virsh parsing is that:

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:

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.  Therefore, 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:

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:

instead of the longer

* 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.
---
 tools/virsh.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 2e35021..cd1d246 100644
--- 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}
 };

-- 
1.7.1




More information about the libvir-list mailing list