Now when STOP event handler has correct both suspended event reason
and paused state reason let's wipe out duplicated event sending and
state changed in/after qemuProcessStopCPUs.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
---
src/qemu/qemu_driver.c | 26 +++-----------------------
src/qemu/qemu_migration.c | 42 ++++++------------------------------------
src/qemu/qemu_migration.h | 4 ----
src/qemu/qemu_process.c | 13 +++++++------
4 files changed, 16 insertions(+), 69 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a52e249..7e08768 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1792,10 +1792,8 @@ static int qemuDomainSuspend(virDomainPtr dom)
virQEMUDriverPtr driver = dom->conn->privateData;
virDomainObjPtr vm;
int ret = -1;
- virObjectEventPtr event = NULL;
qemuDomainObjPrivatePtr priv;
virDomainPausedReason reason;
- int eventDetail;
int state;
virQEMUDriverConfigPtr cfg = NULL;
@@ -1814,16 +1812,12 @@ static int qemuDomainSuspend(virDomainPtr dom)
if (virDomainObjCheckActive(vm) < 0)
goto endjob;
- if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) {
+ if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT)
reason = VIR_DOMAIN_PAUSED_MIGRATION;
- eventDetail = VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED;
- } else if (priv->job.asyncJob == QEMU_ASYNC_JOB_SNAPSHOT) {
+ else if (priv->job.asyncJob == QEMU_ASYNC_JOB_SNAPSHOT)
reason = VIR_DOMAIN_PAUSED_SNAPSHOT;
- eventDetail = -1; /* don't create lifecycle events when doing snapshot */
- } else {
+ else
reason = VIR_DOMAIN_PAUSED_USER;
- eventDetail = VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
- }
state = virDomainObjGetState(vm, NULL);
if (state == VIR_DOMAIN_PMSUSPENDED) {
@@ -1833,12 +1827,6 @@ static int qemuDomainSuspend(virDomainPtr dom)
} else if (state != VIR_DOMAIN_PAUSED) {
if (qemuProcessStopCPUs(driver, vm, reason, QEMU_ASYNC_JOB_NONE) < 0)
goto endjob;
-
- if (eventDetail >= 0) {
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_SUSPENDED,
- eventDetail);
- }
}
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0)
goto endjob;
@@ -1850,7 +1838,6 @@ static int qemuDomainSuspend(virDomainPtr dom)
cleanup:
virDomainObjEndAPI(&vm);
- virObjectEventStateQueue(driver->domainEventState, event);
virObjectUnref(cfg);
return ret;
}
@@ -16175,13 +16162,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
VIR_DOMAIN_PAUSED_FROM_SNAPSHOT,
QEMU_ASYNC_JOB_START) < 0)
goto endjob;
- /* Create an event now in case the restore fails, so
- * that user will be alerted that they are now paused.
- * If restore later succeeds, we might replace this. */
- detail = VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT;
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_SUSPENDED,
- detail);
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("guest unexpectedly quit"));
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6794033..5cfad1c 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1228,29 +1228,6 @@ qemuMigrationSrcIsSafe(virDomainDefPtr def,
return true;
}
-/** qemuMigrationSrcSetOffline
- * Pause domain for non-live migration.
- */
-int
-qemuMigrationSrcSetOffline(virQEMUDriverPtr driver,
- virDomainObjPtr vm)
-{
- int ret;
- VIR_DEBUG("driver=%p vm=%p", driver, vm);
- ret = qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_MIGRATION,
- QEMU_ASYNC_JOB_MIGRATION_OUT);
- if (ret == 0) {
- virObjectEventPtr event;
-
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_SUSPENDED,
- VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED);
- virObjectEventStateQueue(driver->domainEventState, event);
- }
-
- return ret;
-}
-
void
qemuMigrationAnyPostcopyFailed(virQEMUDriverPtr driver,
@@ -1273,19 +1250,10 @@ qemuMigrationAnyPostcopyFailed(virQEMUDriverPtr driver,
"leaving the domain paused", vm->def->name);
if (state == VIR_DOMAIN_RUNNING) {
- virObjectEventPtr event;
-
if (qemuProcessStopCPUs(driver, vm,
VIR_DOMAIN_PAUSED_POSTCOPY_FAILED,
- QEMU_ASYNC_JOB_MIGRATION_IN) < 0) {
+ QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
VIR_WARN("Unable to pause guest CPUs for %s", vm->def->name);
- return;
- }
-
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_SUSPENDED,
- VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY_FAILED);
- virObjectEventStateQueue(driver->domainEventState, event);
} else {
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
VIR_DOMAIN_PAUSED_POSTCOPY_FAILED);
@@ -3493,10 +3461,11 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver,
}
}
- /* Before EnterMonitor, since qemuMigrationSetOffline already does that */
+ /* Before EnterMonitor, since already qemuProcessStopCPUs does that */
if (!(flags & VIR_MIGRATE_LIVE) &&
virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
- if (qemuMigrationSrcSetOffline(driver, vm) < 0)
+ if (qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_MIGRATION,
+ QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
goto error;
}
@@ -3606,7 +3575,8 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver,
goto error;
}
} else if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING &&
- qemuMigrationSrcSetOffline(driver, vm) < 0) {
+ qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_MIGRATION,
+ QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) {
goto error;
}
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index e12b697..a71cfe6 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -98,10 +98,6 @@ typedef enum {
} qemuMigrationJobPhase;
VIR_ENUM_DECL(qemuMigrationJobPhase)
-int
-qemuMigrationSrcSetOffline(virQEMUDriverPtr driver,
- virDomainObjPtr vm);
-
char *
qemuMigrationSrcBegin(virConnectPtr conn,
virDomainObjPtr vm,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6858377..c5203a1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -659,13 +659,10 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) {
if (priv->job.current->status ==
- QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
+ QEMU_DOMAIN_JOB_STATUS_POSTCOPY)
reason = VIR_DOMAIN_PAUSED_POSTCOPY;
- detail = VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY;
- } else {
+ else
reason = VIR_DOMAIN_PAUSED_MIGRATION;
- detail = VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED;
- }
}
detail = qemuDomainPausedReasonToSuspendedEvent(reason);
@@ -3156,7 +3153,11 @@ int qemuProcessStopCPUs(virQEMUDriverPtr driver,
if (priv->job.current)
ignore_value(virTimeMillisNow(&priv->job.current->stopped));
- virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, reason);
+ /* The STOP event handler will change the domain state with the reason
+ * saved in priv->pausedReason and it will also emit corresponding domain
+ * lifecycle event.
+ */
+
if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0)
VIR_WARN("Unable to release lease on %s", vm->def->name);
VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState));
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
$SUBJ:
s/don't/Don't
On 10/10/18 4:04 AM, Nikolay Shirokovskiy wrote:
> Now when STOP event handler has correct both suspended event reason
> and paused state reason let's wipe out duplicated event sending and
> state changed in/after qemuProcessStopCPUs.
Since the STOP event handler can use the pausedReason as sent to
qemuProcessStopCPUs, we no longer need to send duplicate suspended
lifecycle events because we know what caused the stop along with extra
details. This processing allows us to also remove the duplicated state
change from qemuProcessStopCPUs.
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
> ---
> src/qemu/qemu_driver.c | 26 +++-----------------------
> src/qemu/qemu_migration.c | 42 ++++++------------------------------------
> src/qemu/qemu_migration.h | 4 ----
> src/qemu/qemu_process.c | 13 +++++++------
> 4 files changed, 16 insertions(+), 69 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a52e249..7e08768 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1792,10 +1792,8 @@ static int qemuDomainSuspend(virDomainPtr dom)
> virQEMUDriverPtr driver = dom->conn->privateData;
> virDomainObjPtr vm;
> int ret = -1;
> - virObjectEventPtr event = NULL;
> qemuDomainObjPrivatePtr priv;
> virDomainPausedReason reason;
> - int eventDetail;
> int state;
> virQEMUDriverConfigPtr cfg = NULL;
>
> @@ -1814,16 +1812,12 @@ static int qemuDomainSuspend(virDomainPtr dom)
> if (virDomainObjCheckActive(vm) < 0)
> goto endjob;
>
> - if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) {
> + if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT)
> reason = VIR_DOMAIN_PAUSED_MIGRATION;
> - eventDetail = VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED;
> - } else if (priv->job.asyncJob == QEMU_ASYNC_JOB_SNAPSHOT) {
> + else if (priv->job.asyncJob == QEMU_ASYNC_JOB_SNAPSHOT)
> reason = VIR_DOMAIN_PAUSED_SNAPSHOT;
> - eventDetail = -1; /* don't create lifecycle events when doing snapshot */
So with these changes how do we handle this special case? See commit
f569b87f5 when this was introduced.
Do we need to adjust qemuProcessHandleStop to not generate the event
when "reason == VIR_DOMAIN_PAUSED_SNAPSHOT && priv->job.asyncJob ==
QEMU_ASYNC_JOB_SNAPSHOT"?
The rest is OK - just need your thoughts on this particular case for the
R-By.
John
> - } else {
> + else
> reason = VIR_DOMAIN_PAUSED_USER;
> - eventDetail = VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
> - }
>
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 16.10.2018 21:46, John Ferlan wrote:
>
> $SUBJ:
>
> s/don't/Don't
>
> On 10/10/18 4:04 AM, Nikolay Shirokovskiy wrote:
>> Now when STOP event handler has correct both suspended event reason
>> and paused state reason let's wipe out duplicated event sending and
>> state changed in/after qemuProcessStopCPUs.
>
> Since the STOP event handler can use the pausedReason as sent to
> qemuProcessStopCPUs, we no longer need to send duplicate suspended
> lifecycle events because we know what caused the stop along with extra
> details. This processing allows us to also remove the duplicated state
> change from qemuProcessStopCPUs.
>
>
>>
>> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
>> ---
>> src/qemu/qemu_driver.c | 26 +++-----------------------
>> src/qemu/qemu_migration.c | 42 ++++++------------------------------------
>> src/qemu/qemu_migration.h | 4 ----
>> src/qemu/qemu_process.c | 13 +++++++------
>> 4 files changed, 16 insertions(+), 69 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index a52e249..7e08768 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -1792,10 +1792,8 @@ static int qemuDomainSuspend(virDomainPtr dom)
>> virQEMUDriverPtr driver = dom->conn->privateData;
>> virDomainObjPtr vm;
>> int ret = -1;
>> - virObjectEventPtr event = NULL;
>> qemuDomainObjPrivatePtr priv;
>> virDomainPausedReason reason;
>> - int eventDetail;
>> int state;
>> virQEMUDriverConfigPtr cfg = NULL;
>>
>> @@ -1814,16 +1812,12 @@ static int qemuDomainSuspend(virDomainPtr dom)
>> if (virDomainObjCheckActive(vm) < 0)
>> goto endjob;
>>
>> - if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) {
>> + if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT)
>> reason = VIR_DOMAIN_PAUSED_MIGRATION;
>> - eventDetail = VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED;
>> - } else if (priv->job.asyncJob == QEMU_ASYNC_JOB_SNAPSHOT) {
>> + else if (priv->job.asyncJob == QEMU_ASYNC_JOB_SNAPSHOT)
>> reason = VIR_DOMAIN_PAUSED_SNAPSHOT;
>> - eventDetail = -1; /* don't create lifecycle events when doing snapshot */
>
> So with these changes how do we handle this special case? See commit
> f569b87f5 when this was introduced.
>
> Do we need to adjust qemuProcessHandleStop to not generate the event
> when "reason == VIR_DOMAIN_PAUSED_SNAPSHOT && priv->job.asyncJob ==
> QEMU_ASYNC_JOB_SNAPSHOT"?
Well we had lifecycle event anyway because of stop handler so I decided
to keep sending it. However I'm not sure why it was not sent in qemuDomainSuspend
originally. For example for migration we do send event. Looks like this
event does not hurt anyone if it survives up to now.
I'm ok with commit message to if the case.
Nikolay
>
> The rest is OK - just need your thoughts on this particular case for the
> R-By.
>
> John
>
>> - } else {
>> + else
>> reason = VIR_DOMAIN_PAUSED_USER;
>> - eventDetail = VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
>> - }
>>
>
> [...]
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.