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@xxxxxxxxxx>
> ---
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
