[libvirt] [REBASE PATCH v2 6/9] qemu: Introduce qemuDomainGetJobInfoDumpStats

[Jiri Denemark]

On Fri, Jan 19, 2018 at 14:53:13 -0500, John Ferlan wrote:
> Add an API to allow fetching the Dump statistics for the job
> via the qemuDomainGetJobInfo API.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/qemu/qemu_driver.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 00a010b45..adf66228b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -13186,6 +13186,53 @@ qemuDomainGetJobInfoMigrationStats(virQEMUDriverPtr driver,
>  
>  
>  static int
> +qemuDomainGetJobInfoDumpStats(virQEMUDriverPtr driver,
> +                              virDomainObjPtr vm,
> +                              qemuDomainJobInfoPtr jobInfo)
> +{
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    qemuMonitorDumpStats stats;
> +    int rv;
> +
> +    if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
> +        return -1;
> +
> +    rv = qemuMonitorQueryDump(priv->mon, &stats);
> +
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0)
> +        return -1;
> +
> +    /* Save the stats in the migration stats so that qemuDomainJobInfoToInfo
> +     * will be copy properly */
> +    jobInfo->stats.ram_total = stats.total;
> +    jobInfo->stats.ram_remaining = stats.total - stats.completed;
> +    jobInfo->stats.ram_transferred = stats.completed;

I think we should store the raw DumpStats in jobInfo similarly to what
we do with migration stats and let qemuDomainJobInfoTo* translate them.
If we store the stats type in jobInfo (see below), we can even change
jobInfo->stats into a union to make it clear what stats structures are
used for each type.

> +    switch (stats.status) {
> +    case QEMU_MONITOR_DUMP_STATUS_NONE:
> +    case QEMU_MONITOR_DUMP_STATUS_FAILED:
> +    case QEMU_MONITOR_DUMP_STATUS_LAST:
> +        virReportError(VIR_ERR_OPERATION_FAILED,
> +                       _("dump query failed, status=%d"), stats.status);
> +        return -1;
> +        break;
> +
> +    case QEMU_MONITOR_DUMP_STATUS_ACTIVE:
> +        jobInfo->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
> +        VIR_DEBUG("dump active, bytes written='%llu' remaining='%llu'",
> +                  stats.completed, stats.total - stats.completed);
> +        break;
> +
> +    case QEMU_MONITOR_DUMP_STATUS_COMPLETED:
> +        jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED;
> +        VIR_DEBUG("dump completed, bytes written='%llu'", stats.completed);
> +        break;
> +    }
> +
> +    return 0;
> +}
> +
> +
> +static int
>  qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
>                                virDomainObjPtr vm,
>                                bool completed,
> @@ -13226,8 +13273,13 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
>      }
>      *jobInfo = *priv->job.current;
>  
> -    if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0)
> -        goto cleanup;
> +    if (priv->job.asyncJob == QEMU_ASYNC_JOB_DUMP && priv->job.dumpCompletion) {

Oh, so this is the only place where job.dumpCompletion is actually doing
something useful. It's distinguishing whether we need to get dump or
migration stats. We can just use job.dump_memory_only for this as
priv->job.current is NULL if job.dump_memory_only is true but the
DUMP_COMPLETED event is not supported. Or alternatively we could have an
explicit enum for distinguishing the two types of statistics, store it
in qemuDomainJobInfo, and use it here to select the right function for
fetching the statistics.

> +        if (qemuDomainGetJobInfoDumpStats(driver, vm, jobInfo) < 0)
> +            goto cleanup;
> +    } else {
> +        if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0)
> +            goto cleanup;
> +    }

Jirka