qemu_agent: Switch to event driven impl

PATCH In review Michal Privoznik 2 participants 6 messages Jun 15, 2012 – Jun 16, 2012

lore

Share & tools

lore

Michal Privoznik

author

2012-06-15 16:10 UTC #0 lore raw

cover qemu_agent: Switch to event driven impl

Qemu fixed some qemu-ga commands reporting bogus success:

http://www.mail-archive.com/[email protected]/msg110261.html

Therefore we need an implementation which won't wait for qemu-ga to reply and hence block indefinitely. Fortunately, we can take events on qemu monitor as indication of success.

Michal Privoznik (2):
qemu_agent: Add some more debug prints
qemu_agent: Wait for events instead of agent response

 src/qemu/qemu_agent.c   |   52 +++++++++++++++++++++++++++++++++++++++++-----
 src/qemu/qemu_agent.h   |    9 ++++++++
 src/qemu/qemu_process.c |    9 ++++++++
 3 files changed, 64 insertions(+), 6 deletions(-)

Signature · 1 line

1.7.8.5

Michal Privoznik

author

2012-06-15 16:10 UTC #1 lore raw

PATCH 1/2 qemu_agent: Add some more debug prints

for agent ref count and qemuProcessHandleAgentDestroy

---
 src/qemu/qemu_agent.c   |    3 ++-
 src/qemu/qemu_process.c |    2 ++
 2 files changed, 4 insertions(+), 1 deletions(-)

src/qemu/qemu_agent.c +2 -1

index bc4ceff..36610c2 100644
@@ -155,13 +155,14 @@ static void qemuAgentFree(qemuAgentPtr mon)
 int qemuAgentRef(qemuAgentPtr mon)
 {
     mon->refs++;
+    VIR_DEBUG("%d", mon->refs);
     return mon->refs;
 }
 
 int qemuAgentUnref(qemuAgentPtr mon)
 {
     mon->refs--;
-
+    VIR_DEBUG("%d", mon->refs);
     if (mon->refs == 0) {
         qemuAgentUnlock(mon);
         qemuAgentFree(mon);

src/qemu/qemu_process.c +2 -0

index 544eed5..2b47bf6 100644
@@ -167,6 +167,8 @@ static void qemuProcessHandleAgentDestroy(qemuAgentPtr agent,
 {
     qemuDomainObjPrivatePtr priv;
 
+    VIR_DEBUG("Received destroy agent=%p vm=%p", agent, vm);
+
     virDomainObjLock(vm);
     priv = vm->privateData;
     if (priv->agent == agent)

Signature · 1 line

1.7.8.5

Eric Blake

2012-06-15 17:09 UTC #2 lore raw

Re: [Qemu-devel] [libvirt] [PATCH 1/2] qemu_agent: Add some more debug prints

Quoted 5 lines · Michal Privoznik

for agent ref count and qemuProcessHandleAgentDestroy

---
 src/qemu/qemu_agent.c   |    3 ++-
 src/qemu/qemu_process.c |    2 ++
 2 files changed, 4 insertions(+), 1 deletions(-)

ACK.

Signature · 2 lines

Eric Blake   [email protected]    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Michal Privoznik

author

2012-06-15 16:10 UTC #3 lore raw

PATCH 2/2 qemu_agent: Wait for events instead of agent response

In reply to Michal Privoznik

With latest changes to qemu-ga success on some commands is not reported anymore, e.g. guest-shutdown or guest-suspend-*. However, errors are still being reported. Therefore, we need to find different source of indication if operation was successful. Events.

---
 src/qemu/qemu_agent.c   |   49 ++++++++++++++++++++++++++++++++++++++++++----
 src/qemu/qemu_agent.h   |    9 ++++++++
 src/qemu/qemu_process.c |    7 ++++++
 3 files changed, 60 insertions(+), 5 deletions(-)

src/qemu/qemu_agent.c +44 -5

index 36610c2..95aec67 100644
@@ -107,6 +107,11 @@ struct _qemuAgent {
     /* If anything went wrong, this will be fed back
      * the next monitor msg */
     virError lastError;
+
+    /* Some guest agent commands don't return anything
+     * but fire up an event on qemu monitor instead.
+     * Take that as indication of successful completion */
+    qemuAgentEvent await_event;
 };
 
 #if DEBUG_RAW_IO

@@ -826,6 +831,13 @@ void qemuAgentClose(qemuAgentPtr mon)
         VIR_FORCE_CLOSE(mon->fd);
     }
 
+    /* If there is somebody waiting for a message
+     * wake him up. No message will arrive anyway. */
+    if (mon->msg && !mon->msg->finished) {
+        mon->msg->finished = 1;
+        virCondSignal(&mon->notify);
+    }
+
     if (qemuAgentUnref(mon) > 0)
         qemuAgentUnlock(mon);
 }

@@ -982,6 +994,7 @@ qemuAgentCommand(qemuAgentPtr mon,
     int ret = -1;
     qemuAgentMessage msg;
     char *cmdstr = NULL;
+    int await_event = mon->await_event;
 
     *reply = NULL;

@@ -1010,10 +1023,16 @@ qemuAgentCommand(qemuAgentPtr mon,
               ret, msg.rxObject);
 
     if (ret == 0) {
+        /* If we haven't obtained any reply but we wait for an
+         * event, then don't report this as error */
         if (!msg.rxObject) {
-            qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                            _("Missing monitor reply object"));
-            ret = -1;
+            if (await_event) {
+                VIR_DEBUG("Woken up by event %d", await_event);
+            } else {
+                qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                _("Missing monitor reply object"));
+                ret = -1;
+            }
         } else {
             *reply = msg.rxObject;
         }

@@ -1238,6 +1257,24 @@ error:
     return NULL;
 }
 
+void qemuAgentNotifyEvent(qemuAgentPtr mon,
+                          qemuAgentEvent event)
+{
+    VIR_DEBUG("mon=%p event=%d", mon, event);
+    if (mon->await_event == event) {
+        VIR_DEBUG("Wakening up a tragedian");
+        mon->await_event = QEMU_AGENT_EVENT_NONE;
+        /* somebody waiting for this event, wake him up. */
+        if (mon->msg && !mon->msg->finished) {
+            mon->msg->finished = 1;
+            virCondSignal(&mon->notify);
+        }
+    } else {
+        /* shouldn't happen but one never knows */
+        VIR_WARN("Received unexpected event %d", event);
+    }
+}
+
 VIR_ENUM_DECL(qemuAgentShutdownMode);
 
 VIR_ENUM_IMPL(qemuAgentShutdownMode,

@@ -1257,9 +1294,10 @@ int qemuAgentShutdown(qemuAgentPtr mon,
     if (!cmd)
         return -1;
 
+    mon->await_event = QEMU_AGENT_EVENT_SHUTDOWN;
     ret = qemuAgentCommand(mon, cmd, &reply);
 
-    if (ret == 0)
+    if (reply && ret == 0)
         ret = qemuAgentCheckError(cmd, reply);
 
     virJSONValueFree(cmd);

@@ -1362,9 +1400,10 @@ qemuAgentSuspend(qemuAgentPtr mon,
     if (!cmd)
         return -1;
 
+    mon->await_event = QEMU_AGENT_EVENT_SUSPEND;
     ret = qemuAgentCommand(mon, cmd, &reply);
 
-    if (ret == 0)
+    if (reply && ret == 0)
         ret = qemuAgentCheckError(cmd, reply);
 
     virJSONValueFree(cmd);

src/qemu/qemu_agent.h +9 -0

index 98c23b0..0816d90 100644
@@ -56,6 +56,15 @@ int qemuAgentUnref(qemuAgentPtr mon) ATTRIBUTE_RETURN_CHECK;
 void qemuAgentClose(qemuAgentPtr mon);
 
 typedef enum {
+    QEMU_AGENT_EVENT_NONE = 0,
+    QEMU_AGENT_EVENT_SHUTDOWN,
+    QEMU_AGENT_EVENT_SUSPEND
+} qemuAgentEvent;
+
+void qemuAgentNotifyEvent(qemuAgentPtr mon,
+                          qemuAgentEvent event);
+
+typedef enum {
     QEMU_AGENT_SHUTDOWN_POWERDOWN,
     QEMU_AGENT_SHUTDOWN_REBOOT,
     QEMU_AGENT_SHUTDOWN_HALT,

src/qemu/qemu_process.c +7 -0

index 2b47bf6..1df3637 100644
@@ -665,6 +665,9 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
                  vm->def->name);
     }
 
+    if (priv->agent)
+        qemuAgentNotifyEvent(priv->agent, QEMU_AGENT_EVENT_SHUTDOWN);
+
     qemuProcessShutdownOrReboot(driver, vm);
 
 unlock:

@@ -1118,6 +1121,7 @@ qemuProcessHandlePMSuspend(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
     event = virDomainEventPMSuspendNewFromObj(vm);
 
     if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
+        qemuDomainObjPrivatePtr priv = vm->privateData;
         VIR_DEBUG("Transitioned guest %s to pmsuspended state due to "
                   "QMP suspend event", vm->def->name);

@@ -1128,6 +1132,9 @@ qemuProcessHandlePMSuspend(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
             VIR_WARN("Unable to save status on vm %s after suspend event",
                      vm->def->name);
         }
+
+        if (priv->agent)
+            qemuAgentNotifyEvent(priv->agent, QEMU_AGENT_EVENT_SUSPEND);
     }
 
     virDomainObjUnlock(vm);

Signature · 1 line

1.7.8.5

Eric Blake

2012-06-15 17:17 UTC #4 lore raw

Re: [Qemu-devel] [libvirt] [PATCH 2/2] qemu_agent: Wait for events instead of agent response

Quoted 12 lines · Michal Privoznik

With latest changes to qemu-ga success on some commands is not reported anymore, e.g. guest-shutdown or guest-suspend-*. However, errors are still being reported. Therefore, we need to find different source of indication if operation was successful. Events.
---
 src/qemu/qemu_agent.c   |   49 ++++++++++++++++++++++++++++++++++++++++++----
 src/qemu/qemu_agent.h   |    9 ++++++++
 src/qemu/qemu_process.c |    7 ++++++
 3 files changed, 60 insertions(+), 5 deletions(-)

src/qemu/qemu_agent.c error: · line 1265

Quoted 9 lines

src/qemu/qemu_agent.c +6 -0

@@ -1238,6 +1257,24 @@ error:
     return NULL;
 }
 
+void qemuAgentNotifyEvent(qemuAgentPtr mon,
+                          qemuAgentEvent event)
+{
+    VIR_DEBUG("mon=%p event=%d", mon, event);
+    if (mon->await_event == event) {
+        VIR_DEBUG("Wakening up a tragedian");

s/Wakening/Waking/

ACK.

Signature · 2 lines

Eric Blake   [email protected]    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Michal Privoznik

author

2012-06-16 07:51 UTC #5 lore raw

Re: [Qemu-devel] [libvirt] [PATCH 2/2] qemu_agent: Wait for events instead of agent response

Quoted 25 lines · Eric Blake

Quoted 12 lines · Michal Privoznik
With latest changes to qemu-ga success on some commands is not reported anymore, e.g. guest-shutdown or guest-suspend-*. However, errors are still being reported. Therefore, we need to find different source of indication if operation was successful. Events.
---
 src/qemu/qemu_agent.c   |   49 ++++++++++++++++++++++++++++++++++++++++++----
 src/qemu/qemu_agent.h   |    9 ++++++++
 src/qemu/qemu_process.c |    7 ++++++
 3 files changed, 60 insertions(+), 5 deletions(-)
src/qemu/qemu_agent.c error: · line 1265
Quoted 9 lines
src/qemu/qemu_agent.c +6 -0
@@ -1238,6 +1257,24 @@ error:
     return NULL;
 }
 
+void qemuAgentNotifyEvent(qemuAgentPtr mon,
+                          qemuAgentEvent event)
+{
+    VIR_DEBUG("mon=%p event=%d", mon, event);
+    if (mon->await_event == event) {
+        VIR_DEBUG("Wakening up a tragedian");
s/Wakening/Waking/

Quoted 3 lines

ACK.

Fixed and pushed. Thanks.

Michal