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) {