[libvirt] [PATCH 3/9] virsh: Implement VIR_DOMAIN_JOB_STATS_COMPLETED_KEEP for 'domjobinfo'

Peter Krempa posted 9 patches 12 weeks ago

[libvirt] [PATCH 3/9] virsh: Implement VIR_DOMAIN_JOB_STATS_COMPLETED_KEEP for 'domjobinfo'

Posted by Peter Krempa 12 weeks ago
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 tools/virsh-domain.c | 9 +++++++++
 tools/virsh.pod      | 7 ++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

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",
+     .type = VSH_OT_BOOL,
+     .help = N_("don't destroy statistics of a recently completed job when reading")
+    },
     {.name = NULL}
 };

@@ -6117,12 +6121,17 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd)
     int op;
     int rc;

+    VSH_REQUIRE_OPTION("keep-completed", "completed");
+
     if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
         return false;

     if (vshCommandOptBool(cmd, "completed"))
         flags |= VIR_DOMAIN_JOB_STATS_COMPLETED;

+    if (vshCommandOptBool(cmd, "keep-completed"))
+        flags |= VIR_DOMAIN_JOB_STATS_COMPLETED_KEEP;
+
     memset(&info, 0, sizeof(info));

     rc = virDomainGetJobStats(dom, &info.type, &params, &nparams, flags);
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 2dce4493cb..6c14520780 100644
--- a/tools/virsh.pod
+++ 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>]

 Returns information about jobs running on a domain. I<--completed> tells
 virsh to return information about a recently finished job. Statistics of
-a completed job are automatically destroyed once read or when libvirtd
-is restarted. Note that time information returned for completed
+a completed job are automatically destroyed once read (unless
+I<--keep-completed> is used) or when libvirtd is restarted.
+Note that time information returned for completed
 migrations may be completely irrelevant unless both source and
 destination hosts have synchronized time (i.e., NTP daemon is running
 on both of them).
-- 
2.23.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/9] virsh: Implement VIR_DOMAIN_JOB_STATS_COMPLETED_KEEP for 'domjobinfo'

Posted by Eric Blake 12 weeks ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list