[libvirt] [PATCH] vsh: print help more often

So, consider following scenario: virsh # migrate --help In this case migrate help is printed out. So far so good. However, you start writing this long migrate command (I bet you know the arguments there can get quite long), but then, at some point you need to print out the help. Something like this: virsh # migrate --copy-storage-all --migrate-disks --help In this specific case you just want to see the format that --migrate-disks accepts. So you append --help and expect help to be printed out. Well, it will not, because "--help" is taken as data to --migrate-disks. Therefore we will get this error instead: virsh # migrate --copy-storage-all --migrate-disks --help error: command 'migrate' requires <domain> option error: command 'migrate' requires <desturi> option Teach our parsing code, that --help may occur even in argument data, and therefore anywhere on the command line. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/vsh.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/vsh.c b/tools/vsh.c index e57c324..d67f5de 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1415,7 +1415,8 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) /* if we encountered --help, replace parsed command with * 'help <cmdname>' */ for (tmpopt = first; tmpopt; tmpopt = tmpopt->next) { - if (STRNEQ(tmpopt->def->name, "help")) + if (STRNEQ(tmpopt->def->name, "help") && + STRNEQ_NULLABLE(tmpopt->data, "--help")) continue; const vshCmdDef *help = vshCmddefSearch("help"); -- 2.4.10

On Wed, Oct 21, 2015 at 10:48:13 +0200, Michal Privoznik wrote:
So, consider following scenario:
virsh # migrate --help
In this case migrate help is printed out. So far so good. However, you start writing this long migrate command (I bet you know the arguments there can get quite long), but then, at some point you need to print out the help. Something like this:
virsh # migrate --copy-storage-all --migrate-disks --help
In this specific case you just want to see the format that --migrate-disks accepts. So you append --help and expect help to be printed out. Well, it will not, because "--help" is taken as data to --migrate-disks. Therefore we will get this error instead:
virsh # migrate --copy-storage-all --migrate-disks --help error: command 'migrate' requires <domain> option error: command 'migrate' requires <desturi> option
Teach our parsing code, that --help may occur even in argument data, and therefore anywhere on the command line.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/vsh.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
The following then won't be possible: $ virsh start --domain --help Domain --help started $ virsh list Id Name State ---------------------------------------------------- 2 --help running $ virsh destroy --domain --help Domain --help destroyed If you are curious whether such config is actually valid ... # virt-xml-validate /etc/libvirt/qemu/--help.xml /etc/libvirt/qemu/--help.xml validates Since the schema looks like: <define name="domainName"> <data type="string"> <!-- Use literal newline instead of \n for bug in libxml2 2.7.6 --> <param name="pattern">[^ ]+</param> </data> </define> Peter

On 21.10.2015 13:19, Peter Krempa wrote:
On Wed, Oct 21, 2015 at 10:48:13 +0200, Michal Privoznik wrote:
So, consider following scenario:
virsh # migrate --help
In this case migrate help is printed out. So far so good. However, you start writing this long migrate command (I bet you know the arguments there can get quite long), but then, at some point you need to print out the help. Something like this:
virsh # migrate --copy-storage-all --migrate-disks --help
In this specific case you just want to see the format that --migrate-disks accepts. So you append --help and expect help to be printed out. Well, it will not, because "--help" is taken as data to --migrate-disks. Therefore we will get this error instead:
virsh # migrate --copy-storage-all --migrate-disks --help error: command 'migrate' requires <domain> option error: command 'migrate' requires <desturi> option
Teach our parsing code, that --help may occur even in argument data, and therefore anywhere on the command line.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/vsh.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
The following then won't be possible:
$ virsh start --domain --help Domain --help started
$ virsh list Id Name State ---------------------------------------------------- 2 --help running
$ virsh destroy --domain --help Domain --help destroyed
If you are curious whether such config is actually valid ...
# virt-xml-validate /etc/libvirt/qemu/--help.xml /etc/libvirt/qemu/--help.xml validates
Since the schema looks like: <define name="domainName"> <data type="string"> <!-- Use literal newline instead of \n for bug in libxml2 2.7.6 --> <param name="pattern">[^ ]+</param> </data> </define>
Peter
Well, I'd say <not_personal>if you are that stupid to call your domain like that, you should suffer</not_personal>, but I don't care that much if this patch goes in or not. Michal

On 10/21/2015 07:23 AM, Michal Privoznik wrote:
On 21.10.2015 13:19, Peter Krempa wrote:
On Wed, Oct 21, 2015 at 10:48:13 +0200, Michal Privoznik wrote:
So, consider following scenario:
virsh # migrate --help
In this case migrate help is printed out. So far so good. However, you start writing this long migrate command (I bet you know the arguments there can get quite long), but then, at some point you need to print out the help. Something like this:
virsh # migrate --copy-storage-all --migrate-disks --help
In this specific case you just want to see the format that --migrate-disks accepts. So you append --help and expect help to be printed out. Well, it will not, because "--help" is taken as data to --migrate-disks. Therefore we will get this error instead:
virsh # migrate --copy-storage-all --migrate-disks --help error: command 'migrate' requires <domain> option error: command 'migrate' requires <desturi> option
Teach our parsing code, that --help may occur even in argument data, and therefore anywhere on the command line.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/vsh.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
The following then won't be possible:
$ virsh start --domain --help Domain --help started
$ virsh list Id Name State ---------------------------------------------------- 2 --help running
$ virsh destroy --domain --help Domain --help destroyed
If you are curious whether such config is actually valid ...
# virt-xml-validate /etc/libvirt/qemu/--help.xml /etc/libvirt/qemu/--help.xml validates
Since the schema looks like: <define name="domainName"> <data type="string"> <!-- Use literal newline instead of \n for bug in libxml2 2.7.6 --> <param name="pattern">[^ ]+</param> </data> </define>
Peter
Well, I'd say <not_personal>if you are that stupid to call your domain like that, you should suffer</not_personal>, but I don't care that much if this patch goes in or not.
Well for someone that wants to name their domain '--help', we could require they escape it first - eg '\--help'. Alternatively, can "--?" be used instead? Or is it felt someone would name their domain that too? John

On 10/21/2015 02:48 AM, Michal Privoznik wrote:
So, consider following scenario:
virsh # migrate --help
In this case migrate help is printed out. So far so good. However, you start writing this long migrate command (I bet you know the arguments there can get quite long), but then, at some point you need to print out the help. Something like this:
virsh # migrate --copy-storage-all --migrate-disks --help
In this specific case you just want to see the format that --migrate-disks accepts. So you append --help and expect help to be printed out. Well, it will not, because "--help" is taken as data to --migrate-disks.
But this is the behavior of ALL applications that use getopt_long(): $ ls --block-size --help ls: invalid --block-size argument '--help' NACK; if you use --help in the position where it will be eaten as the argument to an incomplete long option, that's too bad for you, but we shouldn't special case it differently than getopt_long(). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 10/21/2015 06:26 AM, Eric Blake wrote:
In this case migrate help is printed out. So far so good. However, you start writing this long migrate command (I bet you know the arguments there can get quite long), but then, at some point you need to print out the help. Something like this:
virsh # migrate --copy-storage-all --migrate-disks --help
In this specific case you just want to see the format that --migrate-disks accepts. So you append --help and expect help to be printed out. Well, it will not, because "--help" is taken as data to --migrate-disks.
But this is the behavior of ALL applications that use getopt_long():
$ ls --block-size --help ls: invalid --block-size argument '--help'
NACK; if you use --help in the position where it will be eaten as the argument to an incomplete long option, that's too bad for you, but we shouldn't special case it differently than getopt_long().
By the way, you CAN get in the habit of typing '--help --help' any time you want help but don't know if the first --help will just end up as filler to an incomplete option. It doesn't globally work, but happens to work for virsh. For 'virsh migrate --copy-storage-all --migrate-disks --help --help', we get help output (since we do all option parsing before any interpretation of the options, then make sure to interpret --help before any other option once --help is seen in an option position). Whereas with my ls example, 'ls --block-size --help --help' doesn't work; arguably something that could be patched in coreutils to be more user-friendly but not our problem here. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
Eric Blake
-
John Ferlan
-
Michal Privoznik
-
Peter Krempa