[libvirt] [PATCH 0/9] domain job stats handling improvements (incremental backup prequels)
Improve few aspecs of the domain job stats handling. I felt these make life simpler when using the domain stats specifically when trying to figure out how stuff works. Peter Krempa (9): virsh: domain: Extract the code converting domain job stats to virDomainJobInfo api: Allow keeping completed domain job stats when reading them virsh: Implement VIR_DOMAIN_JOB_STATS_COMPLETED_KEEP for 'domjobinfo' qemu: Implement VIR_DOMAIN_JOB_STATS_COMPLETED_KEEP virsh: domjobinfo: Print also job operation for failed jobs virsh: domjobinfo: Allow printing stats also for failed and other jobs virsh: domjobinfo: Add switch to print raw fields API: Introduce VIR_DOMAIN_JOB_SUCCESS field for virDomainGetJobStats qemu: Always reset @info in qemuDomainGetJobInfo include/libvirt/libvirt-domain.h | 9 +++ src/libvirt-domain.c | 6 +- src/qemu/qemu_driver.c | 9 ++- tools/virsh-domain.c | 124 +++++++++++++++++++++---------- tools/virsh.pod | 16 +++- 5 files changed, 115 insertions(+), 49 deletions(-) -- 2.23.0
To simplify the stats printer code we convert the new statistics from the typed parameter list into the old stats structure. Extract this code since it takes a lot of space. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-domain.c | 72 +++++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 6be9780836..99194c2f81 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6065,6 +6065,42 @@ virshDomainJobOperationToString(int op) return str ? _(str) : _("unknown"); } + +static int +virshDomainJobStatsToDomainJobInfo(virTypedParameterPtr params, + int nparams, + virDomainJobInfo *info) +{ + if (virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_TIME_ELAPSED, + &info->timeElapsed) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_TIME_REMAINING, + &info->timeRemaining) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_DATA_TOTAL, + &info->dataTotal) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_DATA_PROCESSED, + &info->dataProcessed) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_DATA_REMAINING, + &info->dataRemaining) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_MEMORY_TOTAL, + &info->memTotal) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_MEMORY_PROCESSED, + &info->memProcessed) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_MEMORY_REMAINING, + &info->memRemaining) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_DISK_TOTAL, + &info->fileTotal) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_DISK_PROCESSED, + &info->fileProcessed) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_DISK_REMAINING, + &info->fileRemaining) < 0) { + vshSaveLibvirtError(); + return -1; + } + + return 0; +} + + static bool cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) { @@ -6091,40 +6127,8 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) rc = virDomainGetJobStats(dom, &info.type, ¶ms, &nparams, flags); if (rc == 0) { - if (virTypedParamsGetULLong(params, nparams, - VIR_DOMAIN_JOB_TIME_ELAPSED, - &info.timeElapsed) < 0 || - virTypedParamsGetULLong(params, nparams, - VIR_DOMAIN_JOB_TIME_REMAINING, - &info.timeRemaining) < 0 || - virTypedParamsGetULLong(params, nparams, - VIR_DOMAIN_JOB_DATA_TOTAL, - &info.dataTotal) < 0 || - virTypedParamsGetULLong(params, nparams, - VIR_DOMAIN_JOB_DATA_PROCESSED, - &info.dataProcessed) < 0 || - virTypedParamsGetULLong(params, nparams, - VIR_DOMAIN_JOB_DATA_REMAINING, - &info.dataRemaining) < 0 || - virTypedParamsGetULLong(params, nparams, - VIR_DOMAIN_JOB_MEMORY_TOTAL, - &info.memTotal) < 0 || - virTypedParamsGetULLong(params, nparams, - VIR_DOMAIN_JOB_MEMORY_PROCESSED, - &info.memProcessed) < 0 || - virTypedParamsGetULLong(params, nparams, - VIR_DOMAIN_JOB_MEMORY_REMAINING, - &info.memRemaining) < 0 || - virTypedParamsGetULLong(params, nparams, - VIR_DOMAIN_JOB_DISK_TOTAL, - &info.fileTotal) < 0 || - virTypedParamsGetULLong(params, nparams, - VIR_DOMAIN_JOB_DISK_PROCESSED, - &info.fileProcessed) < 0 || - virTypedParamsGetULLong(params, nparams, - VIR_DOMAIN_JOB_DISK_REMAINING, - &info.fileRemaining) < 0) - goto save_error; + if (virshDomainJobStatsToDomainJobInfo(params, nparams, &info) < 0) + goto cleanup; } else if (last_error->code == VIR_ERR_NO_SUPPORT) { if (flags) { vshError(ctl, "%s", _("Optional flags are not supported by the " -- 2.23.0
On 11/25/19 9:01 AM, Peter Krempa wrote:
To simplify the stats printer code we convert the new statistics from the typed parameter list into the old stats structure.
Extract this code since it takes a lot of space.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-domain.c | 72 +++++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 34 deletions(-)
A wash in lines of code for now, but aids reusability later. Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 6be9780836..99194c2f81 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6065,6 +6065,42 @@ virshDomainJobOperationToString(int op) return str ? _(str) : _("unknown"); }
+ +static int +virshDomainJobStatsToDomainJobInfo(virTypedParameterPtr params, + int nparams, + virDomainJobInfo *info) +{ + if (virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_TIME_ELAPSED, + &info->timeElapsed) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_TIME_REMAINING, + &info->timeRemaining) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_DATA_TOTAL, + &info->dataTotal) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_DATA_PROCESSED, + &info->dataProcessed) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_DATA_REMAINING, + &info->dataRemaining) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_MEMORY_TOTAL, + &info->memTotal) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_MEMORY_PROCESSED, + &info->memProcessed) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_MEMORY_REMAINING, + &info->memRemaining) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_DISK_TOTAL, + &info->fileTotal) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_DISK_PROCESSED, + &info->fileProcessed) < 0 || + virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_DISK_REMAINING, + &info->fileRemaining) < 0) { + vshSaveLibvirtError(); + return -1; + }
Any additional parameters not mapping to the old struct are silently ignored. I guess that's okay, especially since here it's just code motion. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
virDomainGetJobStats destroys the completed statistics on the first read. Give the user possibility to keep them around if they wish so. Add a flag VIR_DOMAIN_JOB_STATS_COMPLETED_KEEP which will read the stats without destroying them. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- include/libvirt/libvirt-domain.h | 2 ++ src/libvirt-domain.c | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a2f007568c..84b3cfdff7 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3246,6 +3246,8 @@ struct _virDomainJobInfo { typedef enum { VIR_DOMAIN_JOB_STATS_COMPLETED = 1 << 0, /* return stats of a recently * completed job */ + VIR_DOMAIN_JOB_STATS_COMPLETED_KEEP = 1 << 1, /* don't remove completed + stats when reading them */ } virDomainGetJobStatsFlags; int virDomainGetJobInfo(virDomainPtr dom, diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 51fb79cddd..77169ec4ca 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -8796,7 +8796,8 @@ virDomainGetJobInfo(virDomainPtr domain, virDomainJobInfoPtr info) * flag may be used to query statistics of a completed incoming pre-copy * migration (statistics for post-copy migration are only available on the * source host). Statistics of a completed job are automatically destroyed - * once read or when libvirtd is restarted. Note that time information + * once read (unless the VIR_DOMAIN_JOB_STATS_COMPLETED_KEEP is used as well) + * 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). The statistics of a completed job can also be @@ -8823,6 +8824,9 @@ virDomainGetJobStats(virDomainPtr domain, virCheckNonNullArgGoto(type, error); virCheckNonNullArgGoto(params, error); virCheckNonNullArgGoto(nparams, error); + VIR_REQUIRE_FLAG_GOTO(VIR_DOMAIN_JOB_STATS_COMPLETED_KEEP, + VIR_DOMAIN_JOB_STATS_COMPLETED, + error); conn = domain->conn; -- 2.23.0
On 11/25/19 9:01 AM, Peter Krempa wrote:
virDomainGetJobStats destroys the completed statistics on the first read. Give the user possibility to keep them around if they wish so.
Add a flag VIR_DOMAIN_JOB_STATS_COMPLETED_KEEP which will read the stats without destroying them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- include/libvirt/libvirt-domain.h | 2 ++ src/libvirt-domain.c | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a2f007568c..84b3cfdff7 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3246,6 +3246,8 @@ struct _virDomainJobInfo { typedef enum { VIR_DOMAIN_JOB_STATS_COMPLETED = 1 << 0, /* return stats of a recently * completed job */ + VIR_DOMAIN_JOB_STATS_COMPLETED_KEEP = 1 << 1, /* don't remove completed + stats when reading them */
Bike-shedding - STATS_KEEP_COMPLETED sounds like a better naming to me. A renaming has ripple effects, but that's my only complaint, so whichever name you go with: Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
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, ¶ms, &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
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
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 669c12d6ca..5d6a82bc13 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13958,7 +13958,8 @@ qemuDomainGetJobStats(virDomainPtr dom, bool completed = !!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED); int ret = -1; - virCheckFlags(VIR_DOMAIN_JOB_STATS_COMPLETED, -1); + virCheckFlags(VIR_DOMAIN_JOB_STATS_COMPLETED | + VIR_DOMAIN_JOB_STATS_COMPLETED_KEEP, -1); if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; @@ -13980,7 +13981,7 @@ qemuDomainGetJobStats(virDomainPtr dom, ret = qemuDomainJobInfoToParams(&jobInfo, type, params, nparams); - if (completed && ret == 0) + if (completed && ret == 0 && !(flags & VIR_DOMAIN_JOB_STATS_COMPLETED_KEEP)) VIR_FREE(priv->job.completed); cleanup: -- 2.23.0
On 11/25/19 9:01 AM, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com> (Any renaming ripple from 2/9 should be obvious) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Printing that a job failed is rather unhelpful. Print at least the operation which failed. Achieve this by moving the check whether to print stats later but replace it with a check which will skip printing of the operation if there's no job. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-domain.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 6e3814f1fd..577dec30c5 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6153,10 +6153,8 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-17s %-12s\n", _("Job type:"), virshDomainJobToString(info.type)); - if (info.type != VIR_DOMAIN_JOB_BOUNDED && - info.type != VIR_DOMAIN_JOB_UNBOUNDED && - (!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED) || - info.type != VIR_DOMAIN_JOB_COMPLETED)) { + + if (info.type == VIR_DOMAIN_JOB_NONE) { ret = true; goto cleanup; } @@ -6169,6 +6167,14 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-17s %-12s\n", _("Operation:"), virshDomainJobOperationToString(op)); + if (info.type != VIR_DOMAIN_JOB_BOUNDED && + info.type != VIR_DOMAIN_JOB_UNBOUNDED && + (!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED) || + info.type != VIR_DOMAIN_JOB_COMPLETED)) { + ret = true; + goto cleanup; + } + vshPrint(ctl, "%-17s %-12llu ms\n", _("Time elapsed:"), info.timeElapsed); if ((rc = virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_TIME_ELAPSED_NET, -- 2.23.0
On 11/25/19 9:01 AM, Peter Krempa wrote:
Printing that a job failed is rather unhelpful. Print at least the operation which failed.
Achieve this by moving the check whether to print stats later but replace it with a check which will skip printing of the operation if there's no job.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-domain.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Introduce the --anystats flag which does not skip the printing of the stats if the job was unsuccessful. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-domain.c | 7 ++++++- tools/virsh.pod | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 577dec30c5..81229fa5ca 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6029,6 +6029,10 @@ static const vshCmdOptDef opts_domjobinfo[] = { .type = VSH_OT_BOOL, .help = N_("don't destroy statistics of a recently completed job when reading") }, + {.name = "anystats", + .type = VSH_OT_BOOL, + .help = N_("print statistics for any kind of job (even failed ones)") + }, {.name = NULL} }; @@ -6167,7 +6171,8 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-17s %-12s\n", _("Operation:"), virshDomainJobOperationToString(op)); - if (info.type != VIR_DOMAIN_JOB_BOUNDED && + if (!vshCommandOptBool(cmd, "anystats") && + info.type != VIR_DOMAIN_JOB_BOUNDED && info.type != VIR_DOMAIN_JOB_UNBOUNDED && (!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED) || info.type != VIR_DOMAIN_JOB_COMPLETED)) { diff --git a/tools/virsh.pod b/tools/virsh.pod index 6c14520780..e5428976a0 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1381,11 +1381,16 @@ Returns basic information about the domain. Abort the currently running domain job. =item B<domjobinfo> I<domain> [I<--completed>] [I<--keep-completed>] +[I<--anystats>] 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 (unless I<--keep-completed> is used) or when libvirtd is restarted. + +Normally only statistics for running and successful completed jobs are printed. +I<--anystats> can be used to display statistics also for failed jobs. + 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 -- 2.23.0
On 11/25/19 9:01 AM, Peter Krempa wrote:
Introduce the --anystats flag which does not skip the printing of the stats if the job was unsuccessful.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> ---
+Normally only statistics for running and successful completed jobs are printed. +I<--anystats> can be used to display statistics also for failed jobs.
to also display statistics for with that tweak, Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Introduce --rawstats which prints all statistics fields from the new API similarly to how the virsh event handler prints them. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-domain.c | 24 +++++++++++++++++++++--- tools/virsh.pod | 6 +++++- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 81229fa5ca..325d748b49 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6033,6 +6033,10 @@ static const vshCmdOptDef opts_domjobinfo[] = { .type = VSH_OT_BOOL, .help = N_("print statistics for any kind of job (even failed ones)") }, + {.name = "rawstats", + .type = VSH_OT_BOOL, + .help = N_("print the raw data returned by libvirt") + }, {.name = NULL} }; @@ -6124,6 +6128,8 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) int ivalue; int op; int rc; + size_t i; + bool rawstats = vshCommandOptBool(cmd, "rawstats"); VSH_REQUIRE_OPTION("keep-completed", "completed"); @@ -6143,9 +6149,9 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) if (virshDomainJobStatsToDomainJobInfo(params, nparams, &info) < 0) goto cleanup; } else if (last_error->code == VIR_ERR_NO_SUPPORT) { - if (flags) { - vshError(ctl, "%s", _("Optional flags are not supported by the " - "daemon")); + if (flags != 0 || rawstats) { + vshError(ctl, "%s", + _("Optional flags or --rawstats are not supported by the daemon")); goto cleanup; } vshDebug(ctl, VSH_ERR_DEBUG, "detailed statistics not supported\n"); @@ -6155,6 +6161,18 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) if (rc < 0) goto cleanup; + if (rawstats) { + vshPrint(ctl, "Job type: %d\n\n", info.type); + + for (i = 0; i < nparams; i++) { + g_autofree char *par = virTypedParameterToString(¶ms[i]); + vshPrint(ctl, "%s: %s\n", params[i].field, NULLSTR(par)); + } + + ret = true; + goto cleanup; + } + vshPrint(ctl, "%-17s %-12s\n", _("Job type:"), virshDomainJobToString(info.type)); diff --git a/tools/virsh.pod b/tools/virsh.pod index e5428976a0..951cb738a0 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1381,7 +1381,7 @@ Returns basic information about the domain. Abort the currently running domain job. =item B<domjobinfo> I<domain> [I<--completed>] [I<--keep-completed>] -[I<--anystats>] +[I<--anystats>] [I<--rawstats>] Returns information about jobs running on a domain. I<--completed> tells virsh to return information about a recently finished job. Statistics of @@ -1391,6 +1391,10 @@ I<--keep-completed> is used) or when libvirtd is restarted. Normally only statistics for running and successful completed jobs are printed. I<--anystats> can be used to display statistics also for failed jobs. +In case I<--rawstats> is used, all fields are printed as received from the +server without any attempts to interpred the data. The "Job type:" field is +special, since it's reported by the API and not part of stats. + 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 -- 2.23.0
On 11/25/19 9:01 AM, Peter Krempa wrote:
Introduce --rawstats which prints all statistics fields from the new API similarly to how the virsh event handler prints them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-domain.c | 24 +++++++++++++++++++++--- tools/virsh.pod | 6 +++++- 2 files changed, 26 insertions(+), 4 deletions(-)
+In case I<--rawstats> is used, all fields are printed as received from the +server without any attempts to interpred the data. The "Job type:" field is
interpret
+special, since it's reported by the API and not part of stats. +
with the typo fix, Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
The statistics fields are used in two places: 1) virDomainGetJobStats where the job type which ultimately holds whether the job was successful or not is returned via a different argument. 2) The virConnectDomainEventJobCompleted event where we report just the statistics via typed parameters. Since it might be useful to report the event also for jobs which completed unsuccessfully and we don't have the means to transport the state via a different variable with the event let's add a new field which will hold the success state. Since this is meant primarily for completed jobs a plain boolean is sufficient to convey whether the job was successful or not. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- include/libvirt/libvirt-domain.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 84b3cfdff7..0e490254fd 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3578,6 +3578,13 @@ typedef enum { */ # define VIR_DOMAIN_JOB_AUTO_CONVERGE_THROTTLE "auto_converge_throttle" +/** + * VIR_DOMAIN_JOB_SUCCESS: + * + * virDomainGetJobStats field: Present only in statistics for a completed job. + * Successful completion of the job as VIR_TYPED_PARAM_BOOLEAN. + */ +# define VIR_DOMAIN_JOB_SUCCESS "success" /** * virConnectDomainEventGenericCallback: -- 2.23.0
On 11/25/19 9:01 AM, Peter Krempa wrote:
The statistics fields are used in two places:
1) virDomainGetJobStats where the job type which ultimately holds whether the job was successful or not is returned via a different argument.
2) The virConnectDomainEventJobCompleted event where we report just the statistics via typed parameters.
Since it might be useful to report the event also for jobs which completed unsuccessfully and we don't have the means to transport the state via a different variable with the event let's add a new field which will hold the success state.
Since this is meant primarily for completed jobs a plain boolean is sufficient to convey whether the job was successful or not.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- include/libvirt/libvirt-domain.h | 7 +++++++ 1 file changed, 7 insertions(+)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
qemuDomainGetJobInfo didn't always reset the return data in @info. Thankfully this wouldn't be a problem as the RPC layer does it but we should do it anyways. Since we reset the struct we don't have to set the type to VIR_DOMAIN_JOB_NONE is as the value is 0. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5d6a82bc13..112d7a6861 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13920,6 +13920,8 @@ qemuDomainGetJobInfo(virDomainPtr dom, virDomainObjPtr vm; int ret = -1; + memset(info, 0, sizeof(*info)); + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; @@ -13930,8 +13932,6 @@ qemuDomainGetJobInfo(virDomainPtr dom, goto cleanup; if (jobInfo.status == QEMU_DOMAIN_JOB_STATUS_NONE) { - memset(info, 0, sizeof(*info)); - info->type = VIR_DOMAIN_JOB_NONE; ret = 0; goto cleanup; } -- 2.23.0
On 11/25/19 9:01 AM, Peter Krempa wrote:
qemuDomainGetJobInfo didn't always reset the return data in @info. Thankfully this wouldn't be a problem as the RPC layer does it but we should do it anyways.
Since we reset the struct we don't have to set the type to VIR_DOMAIN_JOB_NONE is as the value is 0.
s/is as/as/
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
participants (2)
-
Eric Blake -
Peter Krempa