[libvirt] [PATCH] qemu: Don't ignore resume events
Since commit v4.7.0-302-ge6d77a75c4 processing RESUME event is mandatory for updating domain state. But the event handler explicitly ignored this event in some cases. Thus the state would be wrong after a fake reboot or when a domain was rebooted after it crashed. BTW, the code to ignore RESUME event after SHUTDOWN didn't make sense even before making RESUME event mandatory. Most likely it was there as a result of careless copy&paste from qemuProcessHandleStop. https://bugzilla.redhat.com/show_bug.cgi?id=1612943 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c698c3b29c..59ca7cd333 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -719,12 +719,7 @@ qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED, priv->runningReason = VIR_DOMAIN_RUNNING_UNKNOWN; } - if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { - if (priv->gotShutdown) { - VIR_DEBUG("Ignoring RESUME event after SHUTDOWN"); - goto unlock; - } - + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) { eventDetail = qemuDomainRunningReasonToResumeEvent(reason); VIR_DEBUG("Transitioned guest %s out of paused into resumed state, " "reason '%s', event detail %d", @@ -742,7 +737,6 @@ qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } } - unlock: virObjectUnlock(vm); virObjectEventStateQueue(driver->domainEventState, event); virObjectUnref(cfg); -- 2.19.1
On Wed, Nov 07, 2018 at 15:11:11 +0100, Jiri Denemark wrote:
Since commit v4.7.0-302-ge6d77a75c4 processing RESUME event is mandatory for updating domain state. But the event handler explicitly ignored this event in some cases. Thus the state would be wrong after a fake reboot or when a domain was rebooted after it crashed.
BTW, the code to ignore RESUME event after SHUTDOWN didn't make sense even before making RESUME event mandatory. Most likely it was there as a result of careless copy&paste from qemuProcessHandleStop.
https://bugzilla.redhat.com/show_bug.cgi?id=1612943
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c698c3b29c..59ca7cd333 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -719,12 +719,7 @@ qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED, priv->runningReason = VIR_DOMAIN_RUNNING_UNKNOWN; }
- if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { - if (priv->gotShutdown) { - VIR_DEBUG("Ignoring RESUME event after SHUTDOWN"); - goto unlock; - } - + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) { eventDetail = qemuDomainRunningReasonToResumeEvent(reason); VIR_DEBUG("Transitioned guest %s out of paused into resumed state, "
This message might become misleading since the VM may not be paused, but e.g. crashed or some other state. Maybe just drop the mention of the PAUSED state?
"reason '%s', event detail %d",
ACK
On Wed, Nov 07, 2018 at 15:48:44 +0100, Peter Krempa wrote:
On Wed, Nov 07, 2018 at 15:11:11 +0100, Jiri Denemark wrote:
Since commit v4.7.0-302-ge6d77a75c4 processing RESUME event is mandatory for updating domain state. But the event handler explicitly ignored this event in some cases. Thus the state would be wrong after a fake reboot or when a domain was rebooted after it crashed.
BTW, the code to ignore RESUME event after SHUTDOWN didn't make sense even before making RESUME event mandatory. Most likely it was there as a result of careless copy&paste from qemuProcessHandleStop.
https://bugzilla.redhat.com/show_bug.cgi?id=1612943
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c698c3b29c..59ca7cd333 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -719,12 +719,7 @@ qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED, priv->runningReason = VIR_DOMAIN_RUNNING_UNKNOWN; }
- if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { - if (priv->gotShutdown) { - VIR_DEBUG("Ignoring RESUME event after SHUTDOWN"); - goto unlock; - } - + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) { eventDetail = qemuDomainRunningReasonToResumeEvent(reason); VIR_DEBUG("Transitioned guest %s out of paused into resumed state, "
This message might become misleading since the VM may not be paused, but e.g. crashed or some other state. Maybe just drop the mention of the PAUSED state?
Hmm, you're right. Should I squash the second trivial patch to this one then since I'll be touching the message here anyway? Jirka
On Wed, Nov 07, 2018 at 15:56:02 +0100, Jiri Denemark wrote:
On Wed, Nov 07, 2018 at 15:48:44 +0100, Peter Krempa wrote:
On Wed, Nov 07, 2018 at 15:11:11 +0100, Jiri Denemark wrote:
Since commit v4.7.0-302-ge6d77a75c4 processing RESUME event is mandatory for updating domain state. But the event handler explicitly ignored this event in some cases. Thus the state would be wrong after a fake reboot or when a domain was rebooted after it crashed.
BTW, the code to ignore RESUME event after SHUTDOWN didn't make sense even before making RESUME event mandatory. Most likely it was there as a result of careless copy&paste from qemuProcessHandleStop.
https://bugzilla.redhat.com/show_bug.cgi?id=1612943
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c698c3b29c..59ca7cd333 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -719,12 +719,7 @@ qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED, priv->runningReason = VIR_DOMAIN_RUNNING_UNKNOWN; }
- if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { - if (priv->gotShutdown) { - VIR_DEBUG("Ignoring RESUME event after SHUTDOWN"); - goto unlock; - } - + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) { eventDetail = qemuDomainRunningReasonToResumeEvent(reason); VIR_DEBUG("Transitioned guest %s out of paused into resumed state, "
This message might become misleading since the VM may not be paused, but e.g. crashed or some other state. Maybe just drop the mention of the PAUSED state?
Hmm, you're right. Should I squash the second trivial patch to this one then since I'll be touching the message here anyway?
Sure. Just mention it in the commit message please.
participants (2)
-
Jiri Denemark -
Peter Krempa