[libvirt] [PATCH 07/10] qemu: agent: simplify io loop

Nikolay Shirokovskiy nshirokovskiy at virtuozzo.com
Tue Oct 4 06:56:11 UTC 2016


Now we don't need to differentiate error and eof cases in the loop function.
So let's simplify it radically using goto instead of flags.
---
 src/qemu/qemu_agent.c        | 185 ++++++++++++++++++-------------------------
 src/qemu/qemu_agent.h        |   2 -
 src/qemu/qemu_process.c      |  22 +----
 tests/qemumonitortestutils.c |   1 -
 4 files changed, 83 insertions(+), 127 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index ec8d47e..43d78c9 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -595,8 +595,8 @@ static void
 qemuAgentIO(int watch, int fd, int events, void *opaque)
 {
     qemuAgentPtr mon = opaque;
-    bool error = false;
-    bool eof = false;
+    void (*errorNotify)(qemuAgentPtr, virDomainObjPtr);
+    virDomainObjPtr vm;
 
     virObjectRef(mon);
     /* lock access to the monitor and protect fd */
@@ -606,122 +606,95 @@ qemuAgentIO(int watch, int fd, int events, void *opaque)
 #endif
 
     if (mon->fd != fd || mon->watch != watch) {
-        if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR))
-            eof = true;
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("event from unexpected fd %d!=%d / watch %d!=%d"),
                        mon->fd, fd, mon->watch, watch);
-        error = true;
-    } else if (mon->lastError.code != VIR_ERR_OK) {
-        if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR))
-            eof = true;
-        error = true;
-    } else {
-        if (events & VIR_EVENT_HANDLE_WRITABLE) {
-            if (mon->connectPending) {
-                if (qemuAgentIOConnect(mon) < 0)
-                    error = true;
-            } else {
-                if (qemuAgentIOWrite(mon) < 0)
-                    error = true;
-            }
-            events &= ~VIR_EVENT_HANDLE_WRITABLE;
-        }
-
-        if (!error &&
-            events & VIR_EVENT_HANDLE_READABLE) {
-            int got = qemuAgentIORead(mon);
-            events &= ~VIR_EVENT_HANDLE_READABLE;
-            if (got < 0) {
-                error = true;
-            } else if (got == 0) {
-                eof = true;
-            } else {
-                /* Ignore hangup/error events if we read some data, to
-                 * give time for that data to be consumed */
-                events = 0;
-
-                if (qemuAgentIOProcess(mon) < 0)
-                    error = true;
-            }
-        }
-
-        if (!error &&
-            events & VIR_EVENT_HANDLE_HANGUP) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("End of file from agent monitor"));
-            eof = true;
-            events &= ~VIR_EVENT_HANDLE_HANGUP;
-        }
-
-        if (!error && !eof &&
-            events & VIR_EVENT_HANDLE_ERROR) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("Invalid file descriptor while waiting for monitor"));
-            eof = true;
-            events &= ~VIR_EVENT_HANDLE_ERROR;
-        }
-        if (!error && events) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Unhandled event %d for monitor fd %d"),
-                           events, mon->fd);
-            error = true;
-        }
+        goto error;
     }
 
-    if (error || eof) {
-        if (mon->lastError.code != VIR_ERR_OK) {
-            /* Already have an error, so clear any new error */
-            virResetLastError();
+    if (mon->lastError.code != VIR_ERR_OK)
+        goto error;
+
+    if (events & VIR_EVENT_HANDLE_WRITABLE) {
+        if (mon->connectPending) {
+            if (qemuAgentIOConnect(mon) < 0)
+                goto error;
         } else {
-            virErrorPtr err = virGetLastError();
-            if (!err)
-                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                               _("Error while processing monitor IO"));
-            virCopyLastError(&mon->lastError);
-            virResetLastError();
+            if (qemuAgentIOWrite(mon) < 0)
+                goto error;
         }
+        events &= ~VIR_EVENT_HANDLE_WRITABLE;
+    }
 
-        VIR_DEBUG("Error on monitor %s", NULLSTR(mon->lastError.message));
-        /* If IO process resulted in an error & we have a message,
-         * then wakeup that waiter */
-        if (mon->msg && !mon->msg->finished) {
-            mon->msg->finished = 1;
-            virCondSignal(&mon->notify);
-        }
+    if (events & VIR_EVENT_HANDLE_READABLE) {
+        int got = qemuAgentIORead(mon);
+
+        if (got <= 0)
+            goto error;
+
+        if (qemuAgentIOProcess(mon) < 0)
+            goto error;
+
+        /* Ignore hangup/error events if we read some data, to
+         * give time for that data to be consumed */
+        events = 0;
+    }
+
+    if (events & VIR_EVENT_HANDLE_HANGUP) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("End of file from agent monitor"));
+        goto error;
+    }
+
+    if (events & VIR_EVENT_HANDLE_ERROR) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Invalid file descriptor while waiting for monitor"));
+        goto error;
+    }
+
+    if (events) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unhandled event %d for monitor fd %d"),
+                       events, mon->fd);
+        goto error;
     }
 
     qemuAgentUpdateWatch(mon);
+    virObjectUnlock(mon);
+    virObjectUnref(mon);
+    return;
 
-    /* We have to unlock to avoid deadlock against command thread,
-     * but is this safe ?  I think it is, because the callback
-     * will try to acquire the virDomainObjPtr mutex next */
-    if (eof) {
-        void (*eofNotify)(qemuAgentPtr, virDomainObjPtr)
-            = mon->cb->eofNotify;
-        virDomainObjPtr vm = mon->vm;
-
-        /* Make sure anyone waiting wakes up now */
-        virCondSignal(&mon->notify);
-        virObjectUnlock(mon);
-        virObjectUnref(mon);
-        VIR_DEBUG("Triggering EOF callback");
-        (eofNotify)(mon, vm);
-    } else if (error) {
-        void (*errorNotify)(qemuAgentPtr, virDomainObjPtr)
-            = mon->cb->errorNotify;
-        virDomainObjPtr vm = mon->vm;
-
-        /* Make sure anyone waiting wakes up now */
-        virCondSignal(&mon->notify);
-        virObjectUnlock(mon);
-        virObjectUnref(mon);
-        VIR_DEBUG("Triggering error callback");
-        (errorNotify)(mon, vm);
+ error:
+    if (mon->lastError.code != VIR_ERR_OK) {
+        /* Already have an error, so clear any new error */
+        virResetLastError();
     } else {
-        virObjectUnlock(mon);
-        virObjectUnref(mon);
+        virErrorPtr err = virGetLastError();
+        if (!err)
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("Error while processing monitor IO"));
+        virCopyLastError(&mon->lastError);
+        virResetLastError();
     }
+
+    VIR_DEBUG("Error on monitor %s", NULLSTR(mon->lastError.message));
+    /* If IO process resulted in an error & we have a message,
+     * then wakeup that waiter */
+    if (mon->msg && !mon->msg->finished) {
+        mon->msg->finished = 1;
+        virCondSignal(&mon->notify);
+    }
+
+    qemuAgentUpdateWatch(mon);
+
+    errorNotify = mon->cb->errorNotify;
+    vm = mon->vm;
+
+    /* Make sure anyone waiting wakes up now */
+    virCondSignal(&mon->notify);
+    virObjectUnlock(mon);
+    virObjectUnref(mon);
+    (errorNotify)(mon, vm);
 }
 
 
@@ -732,9 +705,9 @@ qemuAgentOpen(virDomainObjPtr vm,
 {
     qemuAgentPtr mon;
 
-    if (!cb || !cb->eofNotify) {
+    if (!cb || !cb->errorNotify) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("EOF notify callback must be supplied"));
+                       _("error notify callback must be supplied"));
         return NULL;
     }
 
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 6dd9c70..76e8772 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -36,8 +36,6 @@ typedef qemuAgentCallbacks *qemuAgentCallbacksPtr;
 struct _qemuAgentCallbacks {
     void (*destroy)(qemuAgentPtr mon,
                     virDomainObjPtr vm);
-    void (*eofNotify)(qemuAgentPtr mon,
-                      virDomainObjPtr vm);
     void (*errorNotify)(qemuAgentPtr mon,
                         virDomainObjPtr vm);
 };
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3da1bd5..fe7682e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -128,12 +128,12 @@ extern virQEMUDriverPtr qemu_driver;
  * performed
  */
 static void
-qemuProcessHandleAgentEOF(qemuAgentPtr agent,
-                          virDomainObjPtr vm)
+qemuProcessHandleAgentError(qemuAgentPtr agent,
+                            virDomainObjPtr vm)
 {
     qemuDomainObjPrivatePtr priv;
 
-    VIR_DEBUG("Received EOF from agent on %p '%s'", vm, vm->def->name);
+    VIR_DEBUG("Received error from agent on %p '%s'", vm, vm->def->name);
 
     virObjectLock(vm);
 
@@ -145,7 +145,7 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent,
     }
 
     if (priv->beingDestroyed) {
-        VIR_DEBUG("Domain is being destroyed, agent EOF is expected");
+        VIR_DEBUG("Domain is being destroyed, agent error is expected");
         goto unlock;
     }
 
@@ -161,19 +161,6 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent,
 }
 
 
-/*
- * This is invoked when there is some kind of error
- * parsing data to/from the agent. The VM can continue
- * to run, but no further agent commands will be
- * allowed
- */
-static void
-qemuProcessHandleAgentError(qemuAgentPtr agent,
-                            virDomainObjPtr vm)
-{
-    qemuProcessHandleAgentEOF(agent, vm);
-}
-
 static void qemuProcessHandleAgentDestroy(qemuAgentPtr agent,
                                           virDomainObjPtr vm)
 {
@@ -185,7 +172,6 @@ static void qemuProcessHandleAgentDestroy(qemuAgentPtr agent,
 
 static qemuAgentCallbacks agentCallbacks = {
     .destroy = qemuProcessHandleAgentDestroy,
-    .eofNotify = qemuProcessHandleAgentEOF,
     .errorNotify = qemuProcessHandleAgentError,
 };
 
diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index c86a27a..d9b2726 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -752,7 +752,6 @@ qemuMonitorTestAgentNotify(qemuAgentPtr agent ATTRIBUTE_UNUSED,
 
 
 static qemuAgentCallbacks qemuMonitorTestAgentCallbacks = {
-    .eofNotify = qemuMonitorTestAgentNotify,
     .errorNotify = qemuMonitorTestAgentNotify,
 };
 
-- 
1.8.3.1




More information about the libvir-list mailing list