[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] PATCH: Process QEMU monitor at startup



Hi Dan,
	Looks fine to commit, but how about re-factoring out the
read/poll/check loop? Suggested, but untested, patch attached.

Cheers,
Mark.
Index: libvirt/qemud/qemud.c
===================================================================
--- libvirt.orig/qemud/qemud.c	2007-03-05 08:38:13.000000000 +0000
+++ libvirt.orig/qemud/qemud.c	2007-03-05 08:38:13.000000000 +0000
@@ -615,74 +615,121 @@ qemudExec(struct qemud_server *server, c
     return -1;
 }
 
+/* Return -1 for error, 1 to continue reading and 0 for success */
+typedef int qemudHandlerMonitorOutput(struct qemud_server *server,
+                                      struct qemud_vm *vm,
+                                      const char *output,
+                                      int fd);
+
+static int
+qemudReadMonitorOutput(struct qemud_server *server,
+                       struct qemud_vm *vm,
+                       int fd,
+                       char *buffer,
+                       int buflen,
+                       qemudHandlerMonitorOutput func)
+{
 #define MONITOR_TIMEOUT 3000
 
-static int qemudOpenMonitor(struct qemud_server *server, struct qemud_vm *vm, const char *monitor) {
-    int monfd;
-    char buffer[1024];
     int got = 0;
 
-    if (!(monfd = open(monitor, O_RDWR))) {
-        qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
-                         "Unable to open monitor path %s", monitor);
-        return -1;
-    }
-    if (qemudSetCloseExec(monfd) < 0) {
-        qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
-                         "Unable to set monitor close-on-exec flag");
-        goto error;
-    }
-    if (qemudSetNonBlock(monfd) < 0) {
-        qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
-                         "Unable to put monitor into non-blocking mode");
-        goto error;
-    }
-
-    /* Consume & discard the initial greeting */
-    while (got < (sizeof(buffer)-1)) {
+   /* Consume & discard the initial greeting */
+    while (got < (buflen-1)) {
         int ret;
 
-        ret = read(monfd, buffer+got, sizeof(buffer)-got-1);
-        if (ret == 0)
-            goto error;
+        ret = read(fd, buffer+got, buflen-got-1);
+        if (ret == 0) {
+            qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
+                             "End-of-file while reading monitor startup output");
+            return -1;
+        }
         if (ret < 0) {
-            struct pollfd fd = { .fd = monfd, .events = POLLIN };
+            struct pollfd pfd = { .fd = fd, .events = POLLIN };
             if (errno != EAGAIN &&
-                errno != EINTR)
-                goto error;
+                errno != EINTR) {
+                qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
+                                 "Failure while reading monitor startup output: %s",
+                                 strerror(errno));
+                return -1;
+            }
 
-            ret = poll(&fd, 1, MONITOR_TIMEOUT);
+            ret = poll(&pfd, 1, MONITOR_TIMEOUT);
             if (ret == 0) {
                 qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
-                                 "Timed out while waiting for monitor prompt");
-                goto error;
+                                 "Timed out while reading monitor startup output");
+                return -1;
             } else if (ret == -1) {
                 if (errno != EINTR) {
                     qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
-                                     "Failure while waiting for monitor prompt");
-                    goto error;
+                                     "Failure while reading monitor startup output: %s",
+                                     strerror(errno));
+                    return -1;
                 }
-            } else if (fd.revents != POLLIN) {
+            } else if (pfd.revents & POLLHUP) {
+                qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
+                                 "End-of-file while reading monitor startup output");
+                return -1;
+            } else if (pfd.revents != POLLIN) {
                 qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
-                                 "Failure while waiting for monitor prompt");
-                goto error;
+                                 "Failure while reading monitor startup output");
+                return -1;
             }
         } else {
             got += ret;
             buffer[got] = '\0';
-            if (strstr(buffer, "(qemu) ") != NULL) {
-                vm->monitor = monfd;
-                return 0;
-            }
+            if ((ret = func(server, vm, buffer, fd)) != 1)
+                return ret;
         }
     }
 
     qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
-                     "No monitor prompt found");
+                     "Out of space while reading monitor startup output");
+    return -1;
 
+#undef MONITOR_TIMEOUT
+}
+
+static int
+qemudCheckMonitorPrompt(struct qemud_server *server ATTRIBUTE_UNUSED,
+                        struct qemud_vm *vm,
+                        const char *output,
+                        int fd)
+{
+    if (strstr(output, "(qemu) ") == NULL)
+        return 1; /* keep reading */
+
+    vm->monitor = fd;
+
+    return 0;
+}
+
+static int qemudOpenMonitor(struct qemud_server *server, struct qemud_vm *vm, const char *monitor) {
+    int monfd;
+    char buffer[1024];
+    int ret = -1;
+
+    if (!(monfd = open(monitor, O_RDWR))) {
+        qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
+                         "Unable to open monitor path %s", monitor);
+        return -1;
+    }
+    if (qemudSetCloseExec(monfd) < 0) {
+        qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
+                         "Unable to set monitor close-on-exec flag");
+        goto error;
+    }
+    if (qemudSetNonBlock(monfd) < 0) {
+        qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
+                         "Unable to put monitor into non-blocking mode");
+        goto error;
+    }
+
+    ret = qemudReadMonitorOutput(server, vm, monfd,
+                                 buffer, sizeof(buffer),
+                                 qemudCheckMonitorPrompt);
  error:
     close(monfd);
-    return -1;
+    return ret;
 }
 
 static int qemudExtractMonitorPath(const char *haystack, char *path, int pathmax) {
@@ -715,60 +762,26 @@ static int qemudExtractMonitorPath(const
     return -1;
 }
 
-static int qemudWaitForMonitor(struct qemud_server *server, struct qemud_vm *vm) {
-    char buffer[1024]; /* Plenty of space to get startup greeting */
-    int got = 0;
+static int
+qemudOpenMonitorPath(struct qemud_server *server,
+                     struct qemud_vm *vm,
+                     const char *output,
+                     int fd ATTRIBUTE_UNUSED)
+{
+    char monitor[PATH_MAX];
 
-    while (got < (sizeof(buffer)-1)) {
-        int ret;
+    if (qemudExtractMonitorPath(output, monitor, sizeof(monitor)) < 0)
+        return 1; /* keep reading */
 
-        ret = read(vm->stderr, buffer+got, sizeof(buffer)-got-1);
-        if (ret == 0) {
-            break; /* End of file */
-        }
-        if (ret < 0) {
-            struct pollfd fd = { .fd = vm->stderr, .events = POLLIN };
-            if (errno != EAGAIN &&
-                errno != EINTR) {
-                qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
-                                 "Failure while looking for monitor PTY path");
-                return -1;
-            }
-
-            ret = poll(&fd, 1, MONITOR_TIMEOUT);
-            if (ret == 0) {
-                qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
-                                 "Timed out while waiting for monitor PTY path");
-                return -1;
-            } else if (ret < 0) {
-                if (errno != EINTR) {
-                    qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
-                                     "Failure while looking for monitor PTY path");
-                    return -1;
-                }
-            } else if (fd.revents & POLLHUP) {
-                break; /* End of file */
-            } else if (fd.revents != POLLIN) {
-                qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
-                                 "Failure while looking for monitor PTY path");
-                return -1;
-            }
-        } else {
-            char monitor[PATH_MAX];
-            got += ret;
-            buffer[got] = '\0';
+    return qemudOpenMonitor(server, vm, monitor);
+}
 
-            if (qemudExtractMonitorPath(buffer, monitor, sizeof(monitor)) == 0) {
-                if (qemudOpenMonitor(server, vm, monitor) < 0)
-                    return -1;
-                return 0;
-            }
-        }
-    }
+static int qemudWaitForMonitor(struct qemud_server *server, struct qemud_vm *vm) {
+    char buffer[1024]; /* Plenty of space to get startup greeting */
 
-    qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
-                     "Domain shutdown before sending monitor PTY path");
-    return -1;
+    return qemudReadMonitorOutput(server, vm, vm->stderr,
+                                  buffer, sizeof(buffer),
+                                  qemudOpenMonitorPath);
 }
 
 static int qemudNextFreeVNCPort(struct qemud_server *server ATTRIBUTE_UNUSED) {

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]