From 6b75a1a5b0d10b42e3fd344b2067a176ee294f46 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 12 Apr 2011 14:58:02 -0600 Subject: [PATCH] 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. --- tools/virsh.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 2e35021441..cd1d2465f2 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} };