[libvirt] [PATCH 3/9] virsh: Implement VIR_DOMAIN_JOB_STATS_COMPLETED_KEEP for 'domjobinfo'
Eric Blake
eblake at redhat.com
Tue Nov 26 16:28:57 UTC 2019
On 11/25/19 9:01 AM, Peter Krempa wrote:
> Signed-off-by: Peter Krempa <pkrempa at 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
More information about the libvir-list
mailing list