[libvirt] [PATCH v3 02/11] qemu: Introduce qemuProcessHandleDumpCompleted

[Jiri Denemark]

On Mon, Jan 29, 2018 at 11:32:00 -0500, John Ferlan wrote:
> Add data to qemuDomainJobObj in order to store the dump completion
> event information. Once the event has been received future code
> waiting on the event will be able to process the stats and error
> buffer. If there's no async job, we can just ignore.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/qemu/qemu_domain.c  |  5 +++++
>  src/qemu/qemu_domain.h  |  3 +++
>  src/qemu/qemu_process.c | 32 ++++++++++++++++++++++++++++++++
>  3 files changed, 40 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 6b4bd3cca..d8b2b3067 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -334,6 +334,11 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
>      job->spiceMigration = false;
>      job->spiceMigrated = false;
>      job->postcopyEnabled = false;
> +    job->dumpCompleted = false;

OK

> +    qemuMonitorEventDumpStatsFree(job->dumpCompletedStats);
> +    job->dumpCompletedStats = NULL;

VIR_FREE would be a bit shorter, but... (see below).

> +    VIR_FREE(job->dumpCompletedError);

OK

> +    job->dumpCompletedError = NULL;

This is redundant.

>      VIR_FREE(job->current);
>  }
>  
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index ddfc46dcd..7dab758fb 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -164,6 +164,9 @@ struct qemuDomainJobObj {
>                                           * should wait for it to finish */
>      bool spiceMigrated;                 /* spice migration completed */
>      bool postcopyEnabled;               /* post-copy migration was enabled */
> +    bool dumpCompleted;                 /* dump completed */

OK

> +    qemuMonitorDumpStatsPtr dumpCompletedStats; /* dump completion stats */

Heh, why do we need to structures for storing statistics? Just use the
qemuDomainJobInfoPtr structure we already point to from current.

> +    char *dumpCompletedError;           /* dump completion event error */

Since there can only be one job at a time and it should fail at most
once, I think we could just have a generic char *error and possibly
reuse it for other jobs in the future.

>  };
>  
>  typedef void (*qemuDomainCleanupCallback)(virQEMUDriverPtr driver,
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 3a697de03..de43f6ac0 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -1690,6 +1690,37 @@ qemuProcessHandleMigrationPass(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>  }
>  
>  
> +static int
> +qemuProcessHandleDumpCompleted(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
> +                               virDomainObjPtr vm,
> +                               qemuMonitorDumpStatsPtr stats,
> +                               const char *error,
> +                               void *opaque ATTRIBUTE_UNUSED)
> +{
> +    qemuDomainObjPrivatePtr priv;
> +
> +    virObjectLock(vm);
> +
> +    VIR_DEBUG("Dump completed for domain %p %s with stats=%p error='%s'",
> +              vm, vm->def->name, stats, NULLSTR(error));
> +
> +    priv = vm->privateData;
> +    if (priv->job.asyncJob == QEMU_ASYNC_JOB_NONE) {
> +        VIR_DEBUG("got DUMP_COMPLETED event without a dump_completed job");
> +        goto cleanup;
> +    }
> +    priv->job.dumpCompleted = true;
> +    VIR_STEAL_PTR(priv->job.dumpCompletedStats, stats);

       priv->jobs.current->... = *stats;

> +    ignore_value(VIR_STRDUP_QUIET(priv->job.dumpCompletedError, error));
> +    virDomainObjBroadcast(vm);
> +
> + cleanup:
> +    qemuMonitorEventDumpStatsFree(stats);

Ah so in fact there wouldn't be any memory leak in
qemuMonitorJSONHandleDumpCompleted since QEMU driver will always
register this handler, but it's awkward anyway.

> +    virObjectUnlock(vm);
> +    return 0;
> +}
> +
> +
>  static qemuMonitorCallbacks monitorCallbacks = {
>      .eofNotify = qemuProcessHandleMonitorEOF,
>      .errorNotify = qemuProcessHandleMonitorError,
> @@ -1718,6 +1749,7 @@ static qemuMonitorCallbacks monitorCallbacks = {
>      .domainMigrationPass = qemuProcessHandleMigrationPass,
>      .domainAcpiOstInfo = qemuProcessHandleAcpiOstInfo,
>      .domainBlockThreshold = qemuProcessHandleBlockThreshold,
> +    .domainDumpCompleted = qemuProcessHandleDumpCompleted,
>  };

Jirka