[libvirt] [PATCH v2 0/7] Fix and enhance statistics of a completed job

Jiri Denemark (7): qemu: Store completed stats at the very end of migration qemu: Properly update completed migration stats qemu: Don't explicitly stop CPUs after migration qemu: Fix a race when computing migration downtime qemu: Do not report completed stats until the job finishes Introduce job completed event qemu: Add support for job completed event daemon/remote.c | 38 ++++++++++++++++++ include/libvirt/libvirt-domain.h | 24 ++++++++++++ src/conf/domain_event.c | 85 +++++++++++++++++++++++++++++++++++++++- src/conf/domain_event.h | 10 +++++ src/libvirt-domain.c | 4 +- src/libvirt_private.syms | 2 + src/qemu/qemu_domain.c | 24 ++++++++++++ src/qemu/qemu_domain.h | 4 ++ src/qemu/qemu_driver.c | 4 +- src/qemu/qemu_migration.c | 74 ++++++++++++++++++++++------------ src/qemu/qemu_process.c | 20 +++++++--- src/remote/remote_driver.c | 42 ++++++++++++++++++++ src/remote/remote_protocol.x | 14 ++++++- src/remote_protocol-structs | 9 +++++ tools/virsh-domain.c | 25 ++++++++++++ 15 files changed, 344 insertions(+), 35 deletions(-) -- 2.7.2

Statistics for a completed migration only make sense if the migration was successful. Let's don't store them in priv->job.completed until we are sure it was a success. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: Version 2: - no change src/qemu/qemu_migration.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 704e182..97e98bb 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5716,6 +5716,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver, unsigned long long timeReceived = 0; virObjectEventPtr event; int rc; + qemuDomainJobInfoPtr jobInfo = NULL; VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d", @@ -5859,8 +5860,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver, } if (mig->jobInfo) { - qemuDomainJobInfoPtr jobInfo = mig->jobInfo; - priv->job.completed = jobInfo; + jobInfo = mig->jobInfo; mig->jobInfo = NULL; if (jobInfo->sent && timeReceived) { @@ -5868,8 +5868,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver, jobInfo->received = timeReceived; jobInfo->timeDeltaSet = true; } - qemuDomainJobInfoUpdateTime(priv->job.completed); - qemuDomainJobInfoUpdateDowntime(priv->job.completed); + qemuDomainJobInfoUpdateTime(jobInfo); + qemuDomainJobInfoUpdateDowntime(jobInfo); } dom = virGetDomain(dconn, vm->def->name, vm->def->uuid); @@ -5908,16 +5908,20 @@ qemuMigrationFinish(virQEMUDriverPtr driver, qemuDomainEventQueue(driver, event); } - if (dom && - qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, - QEMU_MIGRATION_COOKIE_STATS) < 0) - VIR_WARN("Unable to encode migration cookie"); + if (dom) { + priv->job.completed = jobInfo; + jobInfo = NULL; + if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, + QEMU_MIGRATION_COOKIE_STATS) < 0) + VIR_WARN("Unable to encode migration cookie"); + } qemuMigrationJobFinish(driver, vm); if (!virDomainObjIsActive(vm)) qemuDomainRemoveInactive(driver, vm); cleanup: + VIR_FREE(jobInfo); virPortAllocatorRelease(driver->migrationPorts, port); if (priv->mon) qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL); -- 2.7.2

On Tue, Mar 01, 2016 at 13:55:27 +0100, Jiri Denemark wrote:
Statistics for a completed migration only make sense if the migration was successful. Let's don't store them in priv->job.completed until we are sure it was a success.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
Notes: Version 2: - no change
src/qemu/qemu_migration.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)a
ACK

On Mon, Mar 07, 2016 at 14:27:46 +0100, Peter Krempa wrote:
On Tue, Mar 01, 2016 at 13:55:27 +0100, Jiri Denemark wrote:
Statistics for a completed migration only make sense if the migration was successful. Let's don't store them in priv->job.completed until we
Let's not ...
are sure it was a success.

We should not overwrite all migration statistics on the source with the numbers sent by the destination since the source may have an updated view in some cases (such as post-copy migration). It's safer to update just the timing info we need to get from the destination and be prepared for the future. And we should only do all this after a successful migration. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: Version 2: - do not touch jobInfo->type src/qemu/qemu_migration.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 97e98bb..d587bb6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3867,6 +3867,8 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, virObjectEventPtr event = NULL; int rv = -1; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuDomainJobInfoPtr jobInfo = NULL; VIR_DEBUG("driver=%p, conn=%p, vm=%p, cookiein=%s, cookieinlen=%d, " "flags=%x, retcode=%d", @@ -3884,12 +3886,18 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, QEMU_MIGRATION_COOKIE_STATS))) goto cleanup; - /* Update total times with the values sent by the destination daemon */ - if (mig->jobInfo) { - qemuDomainObjPrivatePtr priv = vm->privateData; + if (retcode == 0) + jobInfo = priv->job.completed; + else VIR_FREE(priv->job.completed); - priv->job.completed = mig->jobInfo; - mig->jobInfo = NULL; + + /* Update times with the values sent by the destination daemon */ + if (mig->jobInfo && jobInfo) { + qemuDomainJobInfoUpdateTime(jobInfo); + jobInfo->timeDeltaSet = mig->jobInfo->timeDeltaSet; + jobInfo->timeDelta = mig->jobInfo->timeDelta; + jobInfo->stats.downtime_set = mig->jobInfo->stats.downtime_set; + jobInfo->stats.downtime = mig->jobInfo->stats.downtime; } if (flags & VIR_MIGRATE_OFFLINE) -- 2.7.2

On Tue, Mar 01, 2016 at 13:55:28 +0100, Jiri Denemark wrote:
We should not overwrite all migration statistics on the source with the numbers sent by the destination since the source may have an updated view in some cases (such as post-copy migration). It's safer to update just the timing info we need to get from the destination and be prepared for the future. And we should only do all this after a successful migration.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
ACK

With a very old QEMU which doesn't support events we need to explicitly call qemuMigrationSetOffline at the end of migration to update our internal state. On the other hand, if we talk to QEMU using QMP, we should just wait for the STOP event and let the event handler update the state and trigger a libvirt event. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: Version 2: - no change src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_migration.c | 27 ++++++++++++++++----------- src/qemu/qemu_process.c | 20 +++++++++++++++----- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index fe890a7..fb05b8b 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -203,6 +203,8 @@ struct _qemuDomainObjPrivate { bool signalIOError; /* true if the domain condition should be signalled on I/O error */ + bool signalStop; /* true if the domain condition should be signalled on + QMP STOP event */ char *machineName; char *libDir; /* base path for per-domain files */ char *channelTargetDir; /* base path for per-domain channel targets */ diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d587bb6..dec4fc9 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4489,19 +4489,24 @@ qemuMigrationRun(virQEMUDriverPtr driver, else if (rc == -1) goto cleanup; - /* When migration completed, QEMU will have paused the - * CPUs for us, but unless we're using the JSON monitor - * we won't have been notified of this, so might still - * think we're running. For v2 protocol this doesn't - * matter because we'll kill the VM soon, but for v3 - * this is important because we stay paused until the - * confirm3 step, but need to release the lock state + /* When migration completed, QEMU will have paused the CPUs for us. + * Wait for the STOP event to be processed or explicitly stop CPUs + * (for old QEMU which does not send events) to release the lock state. */ - if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { - if (qemuMigrationSetOffline(driver, vm) < 0) { - priv->job.current->type = VIR_DOMAIN_JOB_FAILED; - goto cleanup; + if (priv->monJSON) { + while (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + priv->signalStop = true; + rc = virDomainObjWait(vm); + priv->signalStop = false; + if (rc < 0) { + priv->job.current->type = VIR_DOMAIN_JOB_FAILED; + goto cleanup; + } } + } else if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING && + qemuMigrationSetOffline(driver, vm) < 0) { + priv->job.current->type = VIR_DOMAIN_JOB_FAILED; + goto cleanup; } ret = 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7d8cf9d..828c1ff 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -697,6 +697,8 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED, { virQEMUDriverPtr driver = opaque; virObjectEventPtr event = NULL; + virDomainPausedReason reason = VIR_DOMAIN_PAUSED_UNKNOWN; + virDomainEventSuspendedDetailType detail = VIR_DOMAIN_EVENT_SUSPENDED_PAUSED; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virObjectLock(vm); @@ -708,16 +710,24 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED, goto unlock; } - VIR_DEBUG("Transitioned guest %s to paused state", - vm->def->name); + if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) { + reason = VIR_DOMAIN_PAUSED_MIGRATION; + detail = VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED; + } + + VIR_DEBUG("Transitioned guest %s to paused state, reason %s", + vm->def->name, virDomainPausedReasonTypeToString(reason)); if (priv->job.current) ignore_value(virTimeMillisNow(&priv->job.current->stopped)); - virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_UNKNOWN); + if (priv->signalStop) + virDomainObjBroadcast(vm); + + virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, reason); event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_SUSPENDED, - VIR_DOMAIN_EVENT_SUSPENDED_PAUSED); + VIR_DOMAIN_EVENT_SUSPENDED, + detail); VIR_FREE(priv->lockState); if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) -- 2.7.2

On Tue, Mar 01, 2016 at 13:55:29 +0100, Jiri Denemark wrote:
With a very old QEMU which doesn't support events we need to explicitly call qemuMigrationSetOffline at the end of migration to update our internal state. On the other hand, if we talk to QEMU using QMP, we should just wait for the STOP event and let the event handler update the state and trigger a libvirt event.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
ACK

Computing a total downtime during a migration requires us to store a time stamp when guest CPUs get stopped. The value (and all other statistics) is then transferred to the destination to compute the downtime. Because the stopped time stamp is stored by a STOP event handler while the statistics which will be sent over to the destination are copied synchronously within qemuMigrationWaitForCompletion. Depending on the timing of STOP and MIGRATION events, we may end up copying (and transferring) statistics without the stopped time stamp set. Let's make sure we always use the correct time stamp. https://bugzilla.redhat.com/show_bug.cgi?id=1282744 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: Version 2: - no change src/qemu/qemu_migration.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index dec4fc9..da0fc18 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4508,6 +4508,8 @@ qemuMigrationRun(virQEMUDriverPtr driver, priv->job.current->type = VIR_DOMAIN_JOB_FAILED; goto cleanup; } + if (priv->job.completed) + priv->job.completed->stopped = priv->job.current->stopped; ret = 0; -- 2.7.2

On Tue, Mar 01, 2016 at 13:55:30 +0100, Jiri Denemark wrote:
Computing a total downtime during a migration requires us to store a time stamp when guest CPUs get stopped. The value (and all other statistics) is then transferred to the destination to compute the downtime. Because the stopped time stamp is stored by a STOP event handler while the statistics which will be sent over to the destination are copied synchronously within qemuMigrationWaitForCompletion.
Depending on the timing of STOP and MIGRATION events, we may end up copying (and transferring) statistics without the stopped time stamp set. Let's make sure we always use the correct time stamp.
https://bugzilla.redhat.com/show_bug.cgi?id=1282744
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
ACK

We would happily report and free statistics of a completed migration even before it actually completed (on the source host while migration is in the Finish phase). Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: Version 2: - no change src/qemu/qemu_driver.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8a529af..8015fe3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12910,7 +12910,9 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, goto cleanup; } - if (completed) + if (completed && priv->job.current) + info = NULL; + else if (completed) info = priv->job.completed; else info = priv->job.current; -- 2.7.2

On Tue, Mar 01, 2016 at 13:55:31 +0100, Jiri Denemark wrote:
We would happily report and free statistics of a completed migration even before it actually completed (on the source host while migration is in the Finish phase).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
ACK

The VIR_DOMAIN_EVENT_ID_JOB_COMPLETED event will be triggered once a job (such as migration) finishes and it will contain statistics for the job as one would get by calling virDomainGetJobStats. Thanks to this event it is now possible to get statistics of a completed migration of a transient domain on the source host. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: Version 2: - remove useless 'type' parameter from the event daemon/remote.c | 38 ++++++++++++++++++ include/libvirt/libvirt-domain.h | 24 ++++++++++++ src/conf/domain_event.c | 85 +++++++++++++++++++++++++++++++++++++++- src/conf/domain_event.h | 10 +++++ src/libvirt-domain.c | 4 +- src/libvirt_private.syms | 2 + src/remote/remote_driver.c | 42 ++++++++++++++++++++ src/remote/remote_protocol.x | 14 ++++++- src/remote_protocol-structs | 9 +++++ tools/virsh-domain.c | 25 ++++++++++++ 10 files changed, 250 insertions(+), 3 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index ca692a9..04608bf 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1099,6 +1099,43 @@ remoteRelayDomainEventMigrationIteration(virConnectPtr conn, } +static int +remoteRelayDomainEventJobCompleted(virConnectPtr conn, + virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + void *opaque) +{ + daemonClientEventCallbackPtr callback = opaque; + remote_domain_event_callback_job_completed_msg data; + + if (callback->callbackID < 0 || + !remoteRelayDomainEventCheckACL(callback->client, conn, dom)) + return -1; + + VIR_DEBUG("Relaying domain migration completed event %s %d, " + "callback %d, params %p %d", + dom->name, dom->id, callback->callbackID, params, nparams); + + /* build return data */ + memset(&data, 0, sizeof(data)); + data.callbackID = callback->callbackID; + make_nonnull_domain(&data.dom, dom); + + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &data.params.params_val, + &data.params.params_len, + VIR_TYPED_PARAM_STRING_OKAY) < 0) + return -1; + + remoteDispatchObjectEventSend(callback->client, remoteProgram, + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_JOB_COMPLETED, + (xdrproc_t)xdr_remote_domain_event_callback_job_completed_msg, + &data); + return 0; +} + + static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventReboot), @@ -1121,6 +1158,7 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventAgentLifecycle), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceAdded), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventMigrationIteration), + VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventJobCompleted), }; verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 65f1618..cf3dbc5 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3305,6 +3305,29 @@ typedef void (*virConnectDomainEventMigrationIterationCallback)(virConnectPtr co void *opaque); /** + * virConnectDomainEventJobCompletedCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @params: job statistics stored as an array of virTypedParameter + * @nparams: size of the params array + * @opaque: application specific data + * + * This callback occurs when a job (such as migration) running on the domain + * is completed. The params array will contain statistics of the just completed + * job as virDomainGetJobStats would return. The callback must not free @params + * (the array will be freed once the callback finishes). + * + * The callback signature to use when registering for an event of type + * VIR_DOMAIN_EVENT_ID_JOB_COMPLETED with + * virConnectDomainEventRegisterAny(). + */ +typedef void (*virConnectDomainEventJobCompletedCallback)(virConnectPtr conn, + virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + void *opaque); + +/** * VIR_DOMAIN_TUNABLE_CPU_VCPUPIN: * * Macro represents formatted pinning for one vcpu specified by id which is @@ -3588,6 +3611,7 @@ typedef enum { VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE = 18,/* virConnectDomainEventAgentLifecycleCallback */ VIR_DOMAIN_EVENT_ID_DEVICE_ADDED = 19, /* virConnectDomainEventDeviceAddedCallback */ VIR_DOMAIN_EVENT_ID_MIGRATION_ITERATION = 20, /* virConnectDomainEventMigrationIterationCallback */ + VIR_DOMAIN_EVENT_ID_JOB_COMPLETED = 21, /* virConnectDomainEventJobCompletedCallback */ # ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_ID_LAST diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 5cb3ccd..a9107e5 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -57,7 +57,7 @@ static virClassPtr virDomainEventTunableClass; static virClassPtr virDomainEventAgentLifecycleClass; static virClassPtr virDomainEventDeviceAddedClass; static virClassPtr virDomainEventMigrationIterationClass; - +static virClassPtr virDomainEventJobCompletedClass; static void virDomainEventDispose(void *obj); static void virDomainEventLifecycleDispose(void *obj); @@ -76,6 +76,7 @@ static void virDomainEventTunableDispose(void *obj); static void virDomainEventAgentLifecycleDispose(void *obj); static void virDomainEventDeviceAddedDispose(void *obj); static void virDomainEventMigrationIterationDispose(void *obj); +static void virDomainEventJobCompletedDispose(void *obj); static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, @@ -246,6 +247,14 @@ struct _virDomainEventMigrationIteration { typedef struct _virDomainEventMigrationIteration virDomainEventMigrationIteration; typedef virDomainEventMigrationIteration *virDomainEventMigrationIterationPtr; +struct _virDomainEventJobCompleted { + virDomainEvent parent; + + virTypedParameterPtr params; + int nparams; +}; +typedef struct _virDomainEventJobCompleted virDomainEventJobCompleted; +typedef virDomainEventJobCompleted *virDomainEventJobCompletedPtr; static int virDomainEventsOnceInit(void) @@ -352,6 +361,12 @@ virDomainEventsOnceInit(void) sizeof(virDomainEventMigrationIteration), virDomainEventMigrationIterationDispose))) return -1; + if (!(virDomainEventJobCompletedClass = + virClassNew(virDomainEventClass, + "virDomainEventJobCompleted", + sizeof(virDomainEventJobCompleted), + virDomainEventJobCompletedDispose))) + return -1; return 0; } @@ -519,6 +534,15 @@ virDomainEventMigrationIterationDispose(void *obj) VIR_DEBUG("obj=%p", event); }; +static void +virDomainEventJobCompletedDispose(void *obj) +{ + virDomainEventJobCompletedPtr event = obj; + VIR_DEBUG("obj=%p", event); + + virTypedParamsFree(event->params, event->nparams); +} + static void * virDomainEventNew(virClassPtr klass, @@ -1394,6 +1418,53 @@ virDomainEventMigrationIterationNewFromDom(virDomainPtr dom, iteration); } +/* This function consumes @params, the caller must not free it. + */ +static virObjectEventPtr +virDomainEventJobCompletedNew(int id, + const char *name, + const unsigned char *uuid, + virTypedParameterPtr params, + int nparams) +{ + virDomainEventJobCompletedPtr ev; + + if (virDomainEventsInitialize() < 0) + goto error; + + if (!(ev = virDomainEventNew(virDomainEventJobCompletedClass, + VIR_DOMAIN_EVENT_ID_JOB_COMPLETED, + id, name, uuid))) + goto error; + + ev->params = params; + ev->nparams = nparams; + + return (virObjectEventPtr) ev; + + error: + virTypedParamsFree(params, nparams); + return NULL; +} + +virObjectEventPtr +virDomainEventJobCompletedNewFromObj(virDomainObjPtr obj, + virTypedParameterPtr params, + int nparams) +{ + return virDomainEventJobCompletedNew(obj->def->id, obj->def->name, + obj->def->uuid, params, nparams); +} + +virObjectEventPtr +virDomainEventJobCompletedNewFromDom(virDomainPtr dom, + virTypedParameterPtr params, + int nparams) +{ + return virDomainEventJobCompletedNew(dom->id, dom->name, dom->uuid, + params, nparams); +} + /* This function consumes the params so caller don't have to care about * freeing it even if error occurs. The reason is to not have to do deep @@ -1685,6 +1756,18 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, goto cleanup; } + case VIR_DOMAIN_EVENT_ID_JOB_COMPLETED: + { + virDomainEventJobCompletedPtr ev; + + ev = (virDomainEventJobCompletedPtr) event; + ((virConnectDomainEventJobCompletedCallback) cb)(conn, dom, + ev->params, + ev->nparams, + cbopaque); + goto cleanup; + } + case VIR_DOMAIN_EVENT_ID_LAST: break; } diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index b7cddb5..3eb13c8 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -217,6 +217,16 @@ virObjectEventPtr virDomainEventMigrationIterationNewFromDom(virDomainPtr dom, int iteration); +virObjectEventPtr +virDomainEventJobCompletedNewFromObj(virDomainObjPtr obj, + virTypedParameterPtr params, + int nparams); + +virObjectEventPtr +virDomainEventJobCompletedNewFromDom(virDomainPtr dom, + virTypedParameterPtr params, + int nparams); + int virDomainEventStateRegister(virConnectPtr conn, virObjectEventStatePtr state, diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 9491845..ca32dc1 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -8873,7 +8873,9 @@ virDomainGetJobInfo(virDomainPtr domain, virDomainJobInfoPtr info) * 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). + * both of them). The statistics of a completed job can also be obtained by + * listening to a VIR_DOMAIN_EVENT_ID_JOB_COMPLETED event (on the source host + * in case of a migration job). * * Returns 0 in case of success and -1 in case of failure. */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4b40612..858ec06 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -508,6 +508,8 @@ virDomainEventIOErrorNewFromDom; virDomainEventIOErrorNewFromObj; virDomainEventIOErrorReasonNewFromDom; virDomainEventIOErrorReasonNewFromObj; +virDomainEventJobCompletedNewFromDom; +virDomainEventJobCompletedNewFromObj; virDomainEventLifecycleNew; virDomainEventLifecycleNewFromDef; virDomainEventLifecycleNewFromDom; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7cf61cf..e84c827 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -342,6 +342,11 @@ remoteDomainBuildEventCallbackMigrationIteration(virNetClientProgramPtr prog, void *evdata, void *opaque); static void +remoteDomainBuildEventCallbackJobCompleted(virNetClientProgramPtr prog, + virNetClientPtr client, + void *evdata, void *opaque); + +static void remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, virNetClientPtr client ATTRIBUTE_UNUSED, void *evdata, void *opaque); @@ -508,6 +513,10 @@ static virNetClientProgramEvent remoteEvents[] = { remoteDomainBuildEventCallbackMigrationIteration, sizeof(remote_domain_event_callback_migration_iteration_msg), (xdrproc_t)xdr_remote_domain_event_callback_migration_iteration_msg }, + { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_JOB_COMPLETED, + remoteDomainBuildEventCallbackJobCompleted, + sizeof(remote_domain_event_callback_job_completed_msg), + (xdrproc_t)xdr_remote_domain_event_callback_job_completed_msg }, }; @@ -5365,6 +5374,39 @@ remoteDomainBuildEventCallbackMigrationIteration(virNetClientProgramPtr prog ATT static void +remoteDomainBuildEventCallbackJobCompleted(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, + virNetClientPtr client ATTRIBUTE_UNUSED, + void *evdata, + void *opaque) +{ + virConnectPtr conn = opaque; + remote_domain_event_callback_job_completed_msg *msg = evdata; + struct private_data *priv = conn->privateData; + virDomainPtr dom; + virObjectEventPtr event = NULL; + virTypedParameterPtr params = NULL; + int nparams = 0; + + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) msg->params.params_val, + msg->params.params_len, + REMOTE_DOMAIN_JOB_STATS_MAX, + ¶ms, &nparams) < 0) + return; + + if (!(dom = get_nonnull_domain(conn, msg->dom))) { + virTypedParamsFree(params, nparams); + return; + } + + event = virDomainEventJobCompletedNewFromDom(dom, params, nparams); + + virObjectUnref(dom); + + remoteEventQueue(priv, event, msg->callbackID); +} + + +static void remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, virNetClientPtr client ATTRIBUTE_UNUSED, void *evdata, void *opaque) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index bfdbce7..3b51a4e 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3228,6 +3228,12 @@ struct remote_domain_event_callback_migration_iteration_msg { int iteration; }; +struct remote_domain_event_callback_job_completed_msg { + int callbackID; + remote_nonnull_domain dom; + remote_typed_param params<REMOTE_DOMAIN_JOB_STATS_MAX>; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -5706,5 +5712,11 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_DOMAIN_EVENT_CALLBACK_MIGRATION_ITERATION = 359 + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_MIGRATION_ITERATION = 359, + + /** + * @generate: both + * @acl: none + */ + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_JOB_COMPLETED = 360 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index dff54e8..1fd7c27 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2697,6 +2697,14 @@ struct remote_domain_event_callback_migration_iteration_msg { remote_nonnull_domain dom; int iteration; }; +struct remote_domain_event_callback_job_completed_msg { + int callbackID; + remote_nonnull_domain dom; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -3057,4 +3065,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_USER_PASSWORD = 357, REMOTE_PROC_DOMAIN_RENAME = 358, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_MIGRATION_ITERATION = 359, + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_JOB_COMPLETED = 360, }; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 979f115..f66faca 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11975,6 +11975,29 @@ virshEventMigrationIterationPrint(virConnectPtr conn ATTRIBUTE_UNUSED, virshEventPrint(opaque, &buf); } +static void +virshEventJobCompletedPrint(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + void *opaque) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + size_t i; + char *value; + + virBufferAsprintf(&buf, _("event 'job-completed' for domain %s:\n"), + virDomainGetName(dom)); + for (i = 0; i < nparams; i++) { + value = virTypedParameterToString(¶ms[i]); + if (value) { + virBufferAsprintf(&buf, "\t%s: %s\n", params[i].field, value); + VIR_FREE(value); + } + } + virshEventPrint(opaque, &buf); +} + static vshEventCallback vshEventCallbacks[] = { { "lifecycle", VIR_DOMAIN_EVENT_CALLBACK(virshEventLifecyclePrint), }, @@ -12016,6 +12039,8 @@ static vshEventCallback vshEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(virshEventDeviceAddedPrint), }, { "migration-iteration", VIR_DOMAIN_EVENT_CALLBACK(virshEventMigrationIterationPrint), }, + { "job-completed", + VIR_DOMAIN_EVENT_CALLBACK(virshEventJobCompletedPrint), }, }; verify(VIR_DOMAIN_EVENT_ID_LAST == ARRAY_CARDINALITY(vshEventCallbacks)); -- 2.7.2

On Tue, Mar 01, 2016 at 13:55:32 +0100, Jiri Denemark wrote:
The VIR_DOMAIN_EVENT_ID_JOB_COMPLETED event will be triggered once a job (such as migration) finishes and it will contain statistics for the job as one would get by calling virDomainGetJobStats. Thanks to this event it is now possible to get statistics of a completed migration of a transient domain on the source host.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
ACK

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: Version 2: - remove useless 'type' parameter from the event src/qemu/qemu_domain.c | 24 ++++++++++++++++++++++++ src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_migration.c | 7 +++++-- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9044792..5165c97 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -149,6 +149,30 @@ void qemuDomainEventQueue(virQEMUDriverPtr driver, } +void +qemuDomainEventEmitJobCompleted(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virObjectEventPtr event; + virTypedParameterPtr params = NULL; + int nparams = 0; + int type; + + if (!priv->job.completed) + return; + + if (qemuDomainJobInfoToParams(priv->job.completed, &type, + ¶ms, &nparams) < 0) { + VIR_WARN("Could not get stats for completed job; domain %s", + vm->def->name); + } + + event = virDomainEventJobCompletedNewFromObj(vm, params, nparams); + qemuDomainEventQueue(driver, event); +} + + static int qemuDomainObjInitJob(qemuDomainObjPrivatePtr priv) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index fb05b8b..84ce6ba 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -263,6 +263,8 @@ void qemuDomainEventFlush(int timer, void *opaque); void qemuDomainEventQueue(virQEMUDriverPtr driver, virObjectEventPtr event); +void qemuDomainEventEmitJobCompleted(virQEMUDriverPtr driver, + virDomainObjPtr vm); int qemuDomainObjBeginJob(virQEMUDriverPtr driver, virDomainObjPtr obj, diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index da0fc18..744bbe6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3864,7 +3864,7 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, int retcode) { qemuMigrationCookiePtr mig; - virObjectEventPtr event = NULL; + virObjectEventPtr event; int rv = -1; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; @@ -3919,6 +3919,8 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_MIGRATED); + qemuDomainEventQueue(driver, event); + qemuDomainEventEmitJobCompleted(driver, vm); } else { virErrorPtr orig_err = virSaveLastError(); @@ -3933,6 +3935,7 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_RESUMED, VIR_DOMAIN_EVENT_RESUMED_MIGRATED); + qemuDomainEventQueue(driver, event); } if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) @@ -3944,7 +3947,6 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, rv = 0; cleanup: - qemuDomainEventQueue(driver, event); virObjectUnref(cfg); return rv; } @@ -6069,6 +6071,7 @@ qemuMigrationToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, if (cmd && virCommandWait(cmd, NULL) < 0) goto cleanup; + qemuDomainEventEmitJobCompleted(driver, vm); ret = 0; cleanup: -- 2.7.2

On Tue, Mar 01, 2016 at 13:55:26 +0100, Jiri Denemark wrote:
Jiri Denemark (7): qemu: Store completed stats at the very end of migration qemu: Properly update completed migration stats qemu: Don't explicitly stop CPUs after migration qemu: Fix a race when computing migration downtime qemu: Do not report completed stats until the job finishes Introduce job completed event qemu: Add support for job completed event
This series doesn't apply cleanly on master anymore (because of new APIs pushed in the meantime). The rebased version can be found in completed-stats branch of https://gitlab.com/jirkade/libvirt repository. Jirka
participants (2)
-
Jiri Denemark
-
Peter Krempa