On 11/25/19 9:01 AM, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-domain.c | 9 +++++++++ tools/virsh.pod | 7 ++++--- 2 files changed, 13 insertions(+), 3 deletions(-)
Renaming from 2/9 would ripple here, obviously.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 99194c2f81..6e3814f1fd 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6025,6 +6025,10 @@ static const vshCmdOptDef opts_domjobinfo[] = { .type = VSH_OT_BOOL, .help = N_("return statistics of a recently completed job") }, + {.name = "keep-completed",
In fact, you named the virsh command line option opposite from the flag name, and I like the CLI ordering better.
+ .type = VSH_OT_BOOL, + .help = N_("don't destroy statistics of a recently completed job when reading") + },
Should this flag imply --completed for convenience, or do you want to force the user to write --completed --keep-completed? The latter makes it possible to test that we catch incorrect use of the flag in isolation, but doesn't aid the command line user. /me reads Your implementation is the latter (the user has to type extra, rather than virsh letting one flag imply the other).
+++ b/tools/virsh.pod @@ -1380,12 +1380,13 @@ Returns basic information about the domain.
Abort the currently running domain job.
-=item B<domjobinfo> I<domain> [I<--completed>] +=item B<domjobinfo> I<domain> [I<--completed>] [I<--keep-completed>]
Semantically, you could write this: =item B<domjobinfo> I<domain> [I<--completed> [I<--keep-completed>]] to show that the --keep-completed only makes sense with --completed. (Of course, that changes if you make one flag imply the other, in which case, the form you wrote is already best) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org