3:04 a.m.
This patch series is similar in implementation to [1] but the motivation
is simply to avoid suspended event duplication.
[1] https://www.redhat.com/archives/libvir-list/2018-September/msg01304.html
Nikolay Shirokovskiy (3):
qemu: pass stop reason from qemuProcessStopCPUs to stop handler
qemu: map suspended state reason to suspended event detail
qemu: don't duplicate suspended events and state changes
src/qemu/qemu_domain.c | 34 ++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 7 +++++++
src/qemu/qemu_driver.c | 26 +++-----------------------
src/qemu/qemu_migration.c | 42 ++++++------------------------------------
src/qemu/qemu_migration.h | 4 ----
src/qemu/qemu_process.c | 37 +++++++++++++++++++++++++------------
6 files changed, 75 insertions(+), 75 deletions(-)
--
1.8.3.1
3:04 a.m.
New subject: [libvirt] [PATCH 1/3] qemu: pass stop reason from qemuProcessStopCPUs to stop handler
We send duplicate suspended lifecycle events on qemu process stop in several
places. The reason is stop event handler always send suspended event and
we addidionally send same event but with more presise reason after call
to qemuProcessStopCPUs. Domain state change is also duplicated.
Let's change domain state and send event only in handler. For this
purpuse first let's pass state change reason to event handler (event
reason is deducible from it).
Inspired by similar patch for resume: 5dab984ed
"qemu: Pass running reason to RESUME event handler".
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_domain.h | 4 ++++
src/qemu/qemu_process.c | 15 ++++++++++++---
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 80bd4bd..380ea14 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -370,6 +370,10 @@ struct _qemuDomainObjPrivate {
/* qemuProcessStartCPUs stores the reason for starting vCPUs here for the
* RESUME event handler to use it */
virDomainRunningReason runningReason;
+
+ /* qemuProcessStopCPUs stores the reason for starting vCPUs here for the
+ * STOP event handler to use it */
+ virDomainPausedReason pausedReason;
};
# define QEMU_DOMAIN_PRIVATE(vm) \
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e9c7618..27021b9 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -641,14 +641,17 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
{
virQEMUDriverPtr driver = opaque;
virObjectEventPtr event = NULL;
- virDomainPausedReason reason = VIR_DOMAIN_PAUSED_UNKNOWN;
+ virDomainPausedReason reason;
virDomainEventSuspendedDetailType detail = VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ qemuDomainObjPrivatePtr priv = vm->privateData;
virObjectLock(vm);
+
+ reason = priv->pausedReason;
+ priv->pausedReason = VIR_DOMAIN_PAUSED_UNKNOWN;
+
if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
- qemuDomainObjPrivatePtr priv = vm->privateData;
-
if (priv->gotShutdown) {
VIR_DEBUG("Ignoring STOP event after SHUTDOWN");
goto unlock;
@@ -3132,6 +3135,8 @@ int qemuProcessStopCPUs(virQEMUDriverPtr driver,
VIR_FREE(priv->lockState);
+ priv->pausedReason = reason;
+
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
goto cleanup;
@@ -3154,6 +3159,9 @@ int qemuProcessStopCPUs(virQEMUDriverPtr driver,
VIR_DEBUG("Preserving lock state '%s'",
NULLSTR(priv->lockState));
cleanup:
+ if (ret < 0)
+ priv->pausedReason = VIR_DOMAIN_PAUSED_UNKNOWN;
+
return ret;
}
@@ -5996,6 +6004,7 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
priv->monStart = 0;
priv->gotShutdown = false;
priv->runningReason = VIR_DOMAIN_RUNNING_UNKNOWN;
+ priv->pausedReason = VIR_DOMAIN_PAUSED_UNKNOWN;
VIR_DEBUG("Updating guest CPU definition");
if (qemuProcessUpdateGuestCPU(vm->def, priv->qemuCaps, caps, flags) < 0)
--
1.8.3.1
1:40 p.m.
New subject: [libvirt] [PATCH 1/3] qemu: pass stop reason from qemuProcessStopCPUs to stop handler
$SUBJ:
s/pass/Pass
On 10/10/18 4:04 AM, Nikolay Shirokovskiy wrote:
We send duplicate suspended lifecycle events on qemu process stop in
several
places. The reason is stop event handler always send suspended event and
we addidionally send same event but with more presise reason after call
*additionally
*precise
to qemuProcessStopCPUs. Domain state change is also duplicated.
Let's change domain state and send event only in handler. For this
purpuse first let's pass state change reason to event handler (event
*purpose
reason is deducible from it).
Inspired by similar patch for resume: 5dab984ed
"qemu: Pass running reason to RESUME event handler".
In any case, I think the above was a bit more appropriate for the cover
since it details a few things being altered in the 3rd patch of the
series. So, maybe this could change to:
Similar to commit 5dab984ed which saves and passes the running reason to
the RESUME event handler, during qemuProcessStopCPUs let's save and pass
the pause reason in the domain private data so that the STOP event
handler can use it.
Signed-off-by: Nikolay Shirokovskiy
<nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_domain.h | 4 ++++
src/qemu/qemu_process.c | 15 ++++++++++++---
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 80bd4bd..380ea14 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -370,6 +370,10 @@ struct _qemuDomainObjPrivate {
/* qemuProcessStartCPUs stores the reason for starting vCPUs here for the
* RESUME event handler to use it */
virDomainRunningReason runningReason;
+
+ /* qemuProcessStopCPUs stores the reason for starting vCPUs here for the
s/starting/pausing/
too much copy-pasta
FWIW: Similar to the comment I made to Jirka for his series, I assume
this STOP processing would have the similar issue with the event going
to the old libvirtd and thus not needing to save/restore via XML state
processing. For context see:
https://www.redhat.com/archives/libvir-list/2018-September/msg01145.html
+ * STOP event handler to use it */
+ virDomainPausedReason pausedReason;
};
With a couple of adjustments,
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
I can make the adjustments so that you don't need to send another series
- just need your acknowledgment for that.
3:41 a.m.
New subject: [libvirt] [PATCH 1/3] qemu: pass stop reason from qemuProcessStopCPUs to stop handler
On 16.10.2018 21:40, John Ferlan wrote:
$SUBJ:
s/pass/Pass
On 10/10/18 4:04 AM, Nikolay Shirokovskiy wrote:
> We send duplicate suspended lifecycle events on qemu process stop in several
> places. The reason is stop event handler always send suspended event and
> we addidionally send same event but with more presise reason after call
*additionally
*precise
> to qemuProcessStopCPUs. Domain state change is also duplicated.
>
> Let's change domain state and send event only in handler. For this
> purpuse first let's pass state change reason to event handler (event
*purpose
> reason is deducible from it).
>
> Inspired by similar patch for resume: 5dab984ed
> "qemu: Pass running reason to RESUME event handler".
>
In any case, I think the above was a bit more appropriate for the cover
since it details a few things being altered in the 3rd patch of the
series. So, maybe this could change to:
Similar to commit 5dab984ed which saves and passes the running reason to
the RESUME event handler, during qemuProcessStopCPUs let's save and pass
the pause reason in the domain private data so that the STOP event
handler can use it.
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
> ---
> src/qemu/qemu_domain.h | 4 ++++
> src/qemu/qemu_process.c | 15 ++++++++++++---
> 2 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 80bd4bd..380ea14 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -370,6 +370,10 @@ struct _qemuDomainObjPrivate {
> /* qemuProcessStartCPUs stores the reason for starting vCPUs here for the
> * RESUME event handler to use it */
> virDomainRunningReason runningReason;
> +
> + /* qemuProcessStopCPUs stores the reason for starting vCPUs here for the
s/starting/pausing/
too much copy-pasta
FWIW: Similar to the comment I made to Jirka for his series, I assume
this STOP processing would have the similar issue with the event going
to the old libvirtd and thus not needing to save/restore via XML state
processing. For context see:
https://www.redhat.com/archives/libvir-list/2018-September/msg01145.html
> + * STOP event handler to use it */
> + virDomainPausedReason pausedReason;
> };
>
With a couple of adjustments,
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
I can make the adjustments so that you don't need to send another series
- just need your acknowledgment for that.
I'm ok with you changes
Nikolay
3:21 a.m.
New subject: [libvirt] [PATCH 1/3] qemu: pass stop reason from qemuProcessStopCPUs to stop handler
Hi, John!
Looks like this series is stucked somehow even though there is almost 100% agreement.
On 17.10.2018 11:41, Nikolay Shirokovskiy wrote:
On 16.10.2018 21:40, John Ferlan wrote:
>
> $SUBJ:
>
> s/pass/Pass
>
> On 10/10/18 4:04 AM, Nikolay Shirokovskiy wrote:
>> We send duplicate suspended lifecycle events on qemu process stop in several
>> places. The reason is stop event handler always send suspended event and
>> we addidionally send same event but with more presise reason after call
>
> *additionally
> *precise
>
>> to qemuProcessStopCPUs. Domain state change is also duplicated.
>>
>> Let's change domain state and send event only in handler. For this
>> purpuse first let's pass state change reason to event handler (event
>
> *purpose
>
>> reason is deducible from it).
>>
>> Inspired by similar patch for resume: 5dab984ed
>> "qemu: Pass running reason to RESUME event handler".
>>
>
> In any case, I think the above was a bit more appropriate for the cover
> since it details a few things being altered in the 3rd patch of the
> series. So, maybe this could change to:
>
> Similar to commit 5dab984ed which saves and passes the running reason to
> the RESUME event handler, during qemuProcessStopCPUs let's save and pass
> the pause reason in the domain private data so that the STOP event
> handler can use it.
>
>> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
>> ---
>> src/qemu/qemu_domain.h | 4 ++++
>> src/qemu/qemu_process.c | 15 ++++++++++++---
>> 2 files changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
>> index 80bd4bd..380ea14 100644
>> --- a/src/qemu/qemu_domain.h
>> +++ b/src/qemu/qemu_domain.h
>> @@ -370,6 +370,10 @@ struct _qemuDomainObjPrivate {
>> /* qemuProcessStartCPUs stores the reason for starting vCPUs here for the
>> * RESUME event handler to use it */
>> virDomainRunningReason runningReason;
>> +
>> + /* qemuProcessStopCPUs stores the reason for starting vCPUs here for the
>
> s/starting/pausing/
>
> too much copy-pasta
>
> FWIW: Similar to the comment I made to Jirka for his series, I assume
> this STOP processing would have the similar issue with the event going
> to the old libvirtd and thus not needing to save/restore via XML state
> processing. For context see:
>
> https://www.redhat.com/archives/libvir-list/2018-September/msg01145.html
>
>> + * STOP event handler to use it */
>> + virDomainPausedReason pausedReason;
>> };
>>
>
> With a couple of adjustments,
>
> Reviewed-by: John Ferlan <jferlan(a)redhat.com>
>
> John
>
> I can make the adjustments so that you don't need to send another series
> - just need your acknowledgment for that.
>
I'm ok with you changes
Nikolay
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
8:53 a.m.
New subject: [libvirt] [PATCH 1/3] qemu: pass stop reason from qemuProcessStopCPUs to stop handler
On 11/2/18 4:21 AM, Nikolay Shirokovskiy wrote:
Hi, John!
Looks like this series is stucked somehow even though there is almost 100% agreement.
Right, but there's been a few events in between that pushed this down
the stack of things I was involved with (beyond my own work) - KVM
Forum, an acquisition, code freeze, ...
Prior to KVM Forum I asked Peter Krempa (via internal IRC) to look at
patch 3, but he may have forgotten or pushed it down his todo list. Of
course he took an extra week after KVM Forum for holiday - so
Thanks for the nudge, it's on my shorter list of things to do! I'm
50/50 on the patch 3 event change. I would like to understand why it was
ignored previously. Sometimes there's some very strange and hidden
reason which becomes painfully obvious at some point in the future. Then
it's a search to figure out why the event just popped up, followed by
time spent wondering why we're not filtering the event any more, and why
we filtered it in the first place.
John
On 17.10.2018 11:41, Nikolay Shirokovskiy wrote:
>
>
> On 16.10.2018 21:40, John Ferlan wrote:
>>
>> $SUBJ:
>>
>> s/pass/Pass
>>
>> On 10/10/18 4:04 AM, Nikolay Shirokovskiy wrote:
>>> We send duplicate suspended lifecycle events on qemu process stop in several
>>> places. The reason is stop event handler always send suspended event and
>>> we addidionally send same event but with more presise reason after call
>>
>> *additionally
>> *precise
>>
>>> to qemuProcessStopCPUs. Domain state change is also duplicated.
>>>
>>> Let's change domain state and send event only in handler. For this
>>> purpuse first let's pass state change reason to event handler (event
>>
>> *purpose
>>
>>> reason is deducible from it).
>>>
>>> Inspired by similar patch for resume: 5dab984ed
>>> "qemu: Pass running reason to RESUME event handler".
>>>
>>
>> In any case, I think the above was a bit more appropriate for the cover
>> since it details a few things being altered in the 3rd patch of the
>> series. So, maybe this could change to:
>>
>> Similar to commit 5dab984ed which saves and passes the running reason to
>> the RESUME event handler, during qemuProcessStopCPUs let's save and pass
>> the pause reason in the domain private data so that the STOP event
>> handler can use it.
>>
>>> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
>>> ---
>>> src/qemu/qemu_domain.h | 4 ++++
>>> src/qemu/qemu_process.c | 15 ++++++++++++---
>>> 2 files changed, 16 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
>>> index 80bd4bd..380ea14 100644
>>> --- a/src/qemu/qemu_domain.h
>>> +++ b/src/qemu/qemu_domain.h
>>> @@ -370,6 +370,10 @@ struct _qemuDomainObjPrivate {
>>> /* qemuProcessStartCPUs stores the reason for starting vCPUs here for
the
>>> * RESUME event handler to use it */
>>> virDomainRunningReason runningReason;
>>> +
>>> + /* qemuProcessStopCPUs stores the reason for starting vCPUs here for
the
>>
>> s/starting/pausing/
>>
>> too much copy-pasta
>>
>> FWIW: Similar to the comment I made to Jirka for his series, I assume
>> this STOP processing would have the similar issue with the event going
>> to the old libvirtd and thus not needing to save/restore via XML state
>> processing. For context see:
>>
>> https://www.redhat.com/archives/libvir-list/2018-September/msg01145.html
>>
>>> + * STOP event handler to use it */
>>> + virDomainPausedReason pausedReason;
>>> };
>>>
>>
>> With a couple of adjustments,
>>
>> Reviewed-by: John Ferlan <jferlan(a)redhat.com>
>>
>> John
>>
>> I can make the adjustments so that you don't need to send another series
>> - just need your acknowledgment for that.
>>
>
> I'm ok with you changes
>
> Nikolay
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
3:04 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: map suspended state reason to suspended event detail
Map is based on existing cases in code where we send suspended
event after changing domain state to paused.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_domain.c | 34 ++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 3 +++
src/qemu/qemu_process.c | 9 ++++++---
3 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f00f1b3..557743b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -13558,3 +13558,37 @@ qemuDomainRunningReasonToResumeEvent(virDomainRunningReason
reason)
return VIR_DOMAIN_EVENT_RESUMED_UNPAUSED;
}
+
+
+virDomainEventSuspendedDetailType
+qemuDomainPausedReasonToSuspendedEvent(virDomainPausedReason reason)
+{
+ switch (reason) {
+ case VIR_DOMAIN_PAUSED_MIGRATION:
+ return VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED;
+
+ case VIR_DOMAIN_PAUSED_FROM_SNAPSHOT:
+ return VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT;
+
+ case VIR_DOMAIN_PAUSED_POSTCOPY_FAILED:
+ return VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY_FAILED;
+
+ case VIR_DOMAIN_PAUSED_POSTCOPY:
+ return VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY;
+
+ case VIR_DOMAIN_PAUSED_UNKNOWN:
+ case VIR_DOMAIN_PAUSED_USER:
+ case VIR_DOMAIN_PAUSED_SAVE:
+ case VIR_DOMAIN_PAUSED_DUMP:
+ case VIR_DOMAIN_PAUSED_IOERROR:
+ case VIR_DOMAIN_PAUSED_WATCHDOG:
+ case VIR_DOMAIN_PAUSED_SHUTTING_DOWN:
+ case VIR_DOMAIN_PAUSED_SNAPSHOT:
+ case VIR_DOMAIN_PAUSED_CRASHED:
+ case VIR_DOMAIN_PAUSED_STARTING_UP:
+ case VIR_DOMAIN_PAUSED_LAST:
+ break;
+ }
+
+ return VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 380ea14..ee88266 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1093,4 +1093,7 @@ void qemuDomainStorageIdReset(qemuDomainObjPrivatePtr priv);
virDomainEventResumedDetailType
qemuDomainRunningReasonToResumeEvent(virDomainRunningReason reason);
+virDomainEventSuspendedDetailType
+qemuDomainPausedReasonToSuspendedEvent(virDomainPausedReason reason);
+
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 27021b9..6858377 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -642,7 +642,7 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
virQEMUDriverPtr driver = opaque;
virObjectEventPtr event = NULL;
virDomainPausedReason reason;
- virDomainEventSuspendedDetailType detail = VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
+ virDomainEventSuspendedDetailType detail;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -668,8 +668,11 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
}
}
- VIR_DEBUG("Transitioned guest %s to paused state, reason %s",
- vm->def->name, virDomainPausedReasonTypeToString(reason));
+ detail = qemuDomainPausedReasonToSuspendedEvent(reason);
+ VIR_DEBUG("Transitioned guest %s to paused state, "
+ "reason %s, event detail %d",
+ vm->def->name, virDomainPausedReasonTypeToString(reason),
+ detail);
if (priv->job.current)
ignore_value(virTimeMillisNow(&priv->job.current->stopped));
--
1.8.3.1
1:42 p.m.
New subject: [libvirt] [PATCH 2/3] qemu: map suspended state reason to suspended event detail
$SUBJ:
s/map/Map
On 10/10/18 4:04 AM, Nikolay Shirokovskiy wrote:
Map is based on existing cases in code where we send suspended
event after changing domain state to paused.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_domain.c | 34 ++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 3 +++
src/qemu/qemu_process.c | 9 ++++++---
3 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f00f1b3..557743b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -13558,3 +13558,37 @@ qemuDomainRunningReasonToResumeEvent(virDomainRunningReason
reason)
return VIR_DOMAIN_EVENT_RESUMED_UNPAUSED;
}
+
+
+virDomainEventSuspendedDetailType
+qemuDomainPausedReasonToSuspendedEvent(virDomainPausedReason reason)
+{
+ switch (reason) {
+ case VIR_DOMAIN_PAUSED_MIGRATION:
+ return VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED;
+
+ case VIR_DOMAIN_PAUSED_FROM_SNAPSHOT:
+ return VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT;
+
+ case VIR_DOMAIN_PAUSED_POSTCOPY_FAILED:
+ return VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY_FAILED;
+
+ case VIR_DOMAIN_PAUSED_POSTCOPY:
+ return VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY;
+
+ case VIR_DOMAIN_PAUSED_UNKNOWN:
+ case VIR_DOMAIN_PAUSED_USER:
+ case VIR_DOMAIN_PAUSED_SAVE:
+ case VIR_DOMAIN_PAUSED_DUMP:
+ case VIR_DOMAIN_PAUSED_IOERROR:
+ case VIR_DOMAIN_PAUSED_WATCHDOG:
+ case VIR_DOMAIN_PAUSED_SHUTTING_DOWN:
+ case VIR_DOMAIN_PAUSED_SNAPSHOT:
This one may cause issues in the next patch...
+ case VIR_DOMAIN_PAUSED_CRASHED:
+ case VIR_DOMAIN_PAUSED_STARTING_UP:
+ case VIR_DOMAIN_PAUSED_LAST:
+ break;
+ }
+
+ return VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 380ea14..ee88266 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1093,4 +1093,7 @@ void qemuDomainStorageIdReset(qemuDomainObjPrivatePtr priv);
virDomainEventResumedDetailType
qemuDomainRunningReasonToResumeEvent(virDomainRunningReason reason);
+virDomainEventSuspendedDetailType
+qemuDomainPausedReasonToSuspendedEvent(virDomainPausedReason reason);
+
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 27021b9..6858377 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -642,7 +642,7 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
virQEMUDriverPtr driver = opaque;
virObjectEventPtr event = NULL;
virDomainPausedReason reason;
- virDomainEventSuspendedDetailType detail = VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
+ virDomainEventSuspendedDetailType detail;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -668,8 +668,11 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
}
}
- VIR_DEBUG("Transitioned guest %s to paused state, reason %s",
- vm->def->name, virDomainPausedReasonTypeToString(reason));
+ detail = qemuDomainPausedReasonToSuspendedEvent(reason);
This setting of @detail overrides the @detail set just above...
It seems we can remove the @detail setting above in this patch as well
as the need for the { } and { } around the setting of @reason in this
patch instead of the subsequent one. Also, may as well place the check
on the same line since it fits, e.g.:
if (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY)
reason = VIR_DOMAIN_PAUSED_POSTCOPY;
else
reason = VIR_DOMAIN_PAUSED_MIGRATION;
With that adjustment,
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
Similar to patch 1, I can make the changes for you...
+ VIR_DEBUG("Transitioned guest %s to paused state,
"
+ "reason %s, event detail %d",
+ vm->def->name, virDomainPausedReasonTypeToString(reason),
+ detail);
if (priv->job.current)
ignore_value(virTimeMillisNow(&priv->job.current->stopped));
3:45 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: map suspended state reason to suspended event detail
On 16.10.2018 21:42, John Ferlan wrote:
$SUBJ:
s/map/Map
On 10/10/18 4:04 AM, Nikolay Shirokovskiy wrote:
> Map is based on existing cases in code where we send suspended
> event after changing domain state to paused.
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
> ---
> src/qemu/qemu_domain.c | 34 ++++++++++++++++++++++++++++++++++
> src/qemu/qemu_domain.h | 3 +++
> src/qemu/qemu_process.c | 9 ++++++---
> 3 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index f00f1b3..557743b 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -13558,3 +13558,37 @@ qemuDomainRunningReasonToResumeEvent(virDomainRunningReason
reason)
>
> return VIR_DOMAIN_EVENT_RESUMED_UNPAUSED;
> }
> +
> +
> +virDomainEventSuspendedDetailType
> +qemuDomainPausedReasonToSuspendedEvent(virDomainPausedReason reason)
> +{
> + switch (reason) {
> + case VIR_DOMAIN_PAUSED_MIGRATION:
> + return VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED;
> +
> + case VIR_DOMAIN_PAUSED_FROM_SNAPSHOT:
> + return VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT;
> +
> + case VIR_DOMAIN_PAUSED_POSTCOPY_FAILED:
> + return VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY_FAILED;
> +
> + case VIR_DOMAIN_PAUSED_POSTCOPY:
> + return VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY;
> +
> + case VIR_DOMAIN_PAUSED_UNKNOWN:
> + case VIR_DOMAIN_PAUSED_USER:
> + case VIR_DOMAIN_PAUSED_SAVE:
> + case VIR_DOMAIN_PAUSED_DUMP:
> + case VIR_DOMAIN_PAUSED_IOERROR:
> + case VIR_DOMAIN_PAUSED_WATCHDOG:
> + case VIR_DOMAIN_PAUSED_SHUTTING_DOWN:
> + case VIR_DOMAIN_PAUSED_SNAPSHOT:
This one may cause issues in the next patch...
> + case VIR_DOMAIN_PAUSED_CRASHED:
> + case VIR_DOMAIN_PAUSED_STARTING_UP:
> + case VIR_DOMAIN_PAUSED_LAST:
> + break;
> + }
> +
> + return VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
> +}
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 380ea14..ee88266 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -1093,4 +1093,7 @@ void qemuDomainStorageIdReset(qemuDomainObjPrivatePtr priv);
> virDomainEventResumedDetailType
> qemuDomainRunningReasonToResumeEvent(virDomainRunningReason reason);
>
> +virDomainEventSuspendedDetailType
> +qemuDomainPausedReasonToSuspendedEvent(virDomainPausedReason reason);
> +
> #endif /* __QEMU_DOMAIN_H__ */
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 27021b9..6858377 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -642,7 +642,7 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
> virQEMUDriverPtr driver = opaque;
> virObjectEventPtr event = NULL;
> virDomainPausedReason reason;
> - virDomainEventSuspendedDetailType detail = VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
> + virDomainEventSuspendedDetailType detail;
> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> qemuDomainObjPrivatePtr priv = vm->privateData;
>
> @@ -668,8 +668,11 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
> }
> }
>
> - VIR_DEBUG("Transitioned guest %s to paused state, reason %s",
> - vm->def->name, virDomainPausedReasonTypeToString(reason));
> + detail = qemuDomainPausedReasonToSuspendedEvent(reason);
This setting of @detail overrides the @detail set just above...
But overrides to the same value.
It seems we can remove the @detail setting above in this patch as well
as the need for the { } and { } around the setting of @reason in this
patch instead of the subsequent one. Also, may as well place the check
on the same line since it fits, e.g.:
if (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY)
reason = VIR_DOMAIN_PAUSED_POSTCOPY;
else
reason = VIR_DOMAIN_PAUSED_MIGRATION;
With that adjustment,
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
Similar to patch 1, I can make the changes for you...
I'm ok here too, but we can left removing setting details for
QEMU_DOMAIN_JOB_STATUS_POSTCOPY
as well as this patch correct in this aspect.
Nikolay
> + VIR_DEBUG("Transitioned guest %s to paused state, "
> + "reason %s, event detail %d",
> + vm->def->name, virDomainPausedReasonTypeToString(reason),
> + detail);
>
> if (priv->job.current)
> ignore_value(virTimeMillisNow(&priv->job.current->stopped));
>
3:04 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: don't duplicate suspended events and state changes
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(a)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
1:46 p.m.
New subject: [libvirt] [PATCH 3/3] qemu: don't duplicate suspended events and state changes
$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(a)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;
- }
[...]
3:58 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: don't duplicate suspended events and state changes
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(a)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;
> - }
>
[...]
10:47 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: don't duplicate suspended events and state changes
On 10/17/18 4:58 AM, Nikolay Shirokovskiy wrote:
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.
>
>
Placed pkrempa on the CC line to hopefully get his feedback in order to
either move this along or require some rework due to the change possibly
generating a lifecycle event now. It's not clear from the referenced
commit why no event was desired and I'd prefer Peter's input just in
case there's some particular reason that wasn't immediately evident or
widely known...
Tks -
John
>>
>> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)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;
>> - }
>>
>
> [...]
>
2210
days inactive
2238
days old
12 comments
2 participants
participants (2)
-
John Ferlan -
Nikolay Shirokovskiy