[libvirt] [PATCH 0/2] qemu_agent: Switch to event driven impl
Qemu fixed some qemu-ga commands reporting bogus success: http://www.mail-archive.com/qemu-devel@nongnu.org/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(-) -- 1.7.8.5
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(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index bc4ceff..36610c2 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -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); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 544eed5..2b47bf6 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -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) -- 1.7.8.5
On 06/15/2012 10:10 AM, Michal Privoznik wrote:
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. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 36610c2..95aec67 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -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); diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 98c23b0..0816d90 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -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, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2b47bf6..1df3637 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -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); -- 1.7.8.5
On 06/15/2012 10:10 AM, Michal Privoznik wrote:
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(-)
@@ -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. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 15.06.2012 19:16, Eric Blake wrote:
On 06/15/2012 10:10 AM, Michal Privoznik wrote:
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(-)
@@ -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.
Fixed and pushed. Thanks. Michal
participants (2)
-
Eric Blake -
Michal Privoznik