1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] qemu: Don't ignore resume events

Jiri Denemark posted 1 patch 5 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/b6e3d6860809c808fb053d1f7dbb7f7dc3b19219.1541599871.git.jdenemar@redhat.com
Test syntax-check passed
src/qemu/qemu_process.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
[libvirt] [PATCH] qemu: Don't ignore resume events
Posted by Jiri Denemark 5 years, 5 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't ignore resume events
Posted by Peter Krempa 5 years, 5 months ago 
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't ignore resume events
Posted by Jiri Denemark 5 years, 5 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't ignore resume events
Posted by Peter Krempa 5 years, 5 months ago 
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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.