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(a)redhat.com&gt; --- 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, &params, &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

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(a)redhat.com&gt; --- 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

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- 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

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- 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

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(a)redhat.com&gt; --- 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

Introduce the --anystats flag which does not skip the printing of the stats if the job was unsuccessful. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- 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

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(a)redhat.com&gt; --- 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(&params[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

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(a)redhat.com&gt; --- 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

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(a)redhat.com&gt; --- 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