[Libvir] PATCH: Process QEMU monitor at startup
Daniel P. Berrange
berrange at redhat.com
Fri Mar 2 16:47:10 UTC 2007
On Fri, Mar 02, 2007 at 03:34:33PM +0000, Daniel P. Berrange wrote:
> On Fri, Mar 02, 2007 at 03:06:56PM +0000, Mark McLoughlin wrote:
> > > +static int qemudWaitForMonitor(struct qemud_vm *vm) {
> >
> > We don't set an error in here, so how about returning the appropriate
> > errno from the function?
>
> I'll add in some error reporting.
We now get errors back like:
$ ./virsh --connect qemu:///session start Fedora
libvir: QEMU error : internal error Domain shutdown before sending monitor PTY path
error: Failed to start domain Fedora
> > > + if (sscanf(tmp, "char device redirected to %19s", monitor) == 1) {
> >
> > Path length of 100, maximum field with of 19? That's all rather
> > voodooish ... hope about strstr() to find it, buffer length of PATH_MAX
> > and then just strncpy()? Also, it'd be nice to split that out into e.g.
> > qemudMonitorPathFromStr(buffer, monitorPath, PATH_MAX)
>
> I'll have a go at splitting it out.
This is now done.
> > index() is a bit odd, why not strstr() ?
>
> Well we're only looking for a single character match - index() is for
> single characters, strstr() is for strings.
Changed to use strchr() - although there's several other bits in existing
code which already using 'index()' which also need updating.
> > > + if (connect(fd, (struct sockaddr*)&addr, sizeof(addr)) == 0)
> >
> > Um, why not bind() (with SO_REUSEADDR)?
>
> No particular reason - either will work just fine.
Actually one case where bind() is better is if the server is not accepting new
connections - connect() will block indefinitely unless the server actually
promptly accepts them. So I've changed to bind() + REUSEADDR.
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
-------------- next part --------------
Index: conf.c
===================================================================
RCS file: /data/cvs/libvirt/qemud/conf.c,v
retrieving revision 1.38
diff -u -p -r1.38 conf.c
--- conf.c 26 Feb 2007 15:34:24 -0000 1.38
+++ conf.c 2 Mar 2007 16:42:49 -0000
@@ -1308,6 +1308,7 @@ qemudAssignVMDef(struct qemud_server *se
vm->monitor = -1;
vm->pid = -1;
vm->id = -1;
+ vm->state = QEMUD_STATE_STOPPED;
vm->def = def;
vm->next = server->vms;
Index: driver.c
===================================================================
RCS file: /data/cvs/libvirt/qemud/driver.c,v
retrieving revision 1.17
diff -u -p -r1.17 driver.c
--- driver.c 26 Feb 2007 14:21:21 -0000 1.17
+++ driver.c 2 Mar 2007 16:42:49 -0000
@@ -60,6 +60,7 @@ int qemudMonitorCommand(struct qemud_ser
char **reply) {
int size = 0;
char *buf = NULL;
+
if (write(vm->monitor, cmd, strlen(cmd)) < 0) {
return -1;
}
@@ -74,13 +75,20 @@ int qemudMonitorCommand(struct qemud_ser
for (;;) {
char data[1024];
int got = read(vm->monitor, data, sizeof(data));
+
+ if (got == 0) {
+ if (buf)
+ free(buf);
+ return -1;
+ }
if (got < 0) {
if (errno == EINTR)
continue;
if (errno == EAGAIN)
break;
- free(buf);
+ if (buf)
+ free(buf);
return -1;
}
if (!(buf = realloc(buf, size+got+1)))
@@ -92,7 +100,7 @@ int qemudMonitorCommand(struct qemud_ser
if (buf)
qemudDebug("Mon [%s]", buf);
/* Look for QEMU prompt to indicate completion */
- if (buf && ((tmp = strstr(buf, "\n(qemu)")) != NULL)) {
+ if (buf && ((tmp = strstr(buf, "\n(qemu) ")) != NULL)) {
tmp[0] = '\0';
break;
}
@@ -314,11 +322,14 @@ int qemudDomainSuspend(struct qemud_serv
qemudReportError(server, VIR_ERR_OPERATION_FAILED, "domain is not running");
return -1;
}
+ if (vm->state == QEMUD_STATE_PAUSED)
+ return 0;
if (qemudMonitorCommand(server, vm, "stop\n", &info) < 0) {
qemudReportError(server, VIR_ERR_OPERATION_FAILED, "suspend operation failed");
return -1;
}
+ vm->state = QEMUD_STATE_PAUSED;
qemudDebug("Reply %s", info);
free(info);
return 0;
@@ -336,13 +347,16 @@ int qemudDomainResume(struct qemud_serve
qemudReportError(server, VIR_ERR_OPERATION_FAILED, "domain is not running");
return -1;
}
+ if (vm->state == QEMUD_STATE_RUNNING)
+ return 0;
if (qemudMonitorCommand(server, vm, "cont\n", &info) < 0) {
qemudReportError(server, VIR_ERR_OPERATION_FAILED, "resume operation failed");
return -1;
}
+ vm->state = QEMUD_STATE_RUNNING;
qemudDebug("Reply %s", info);
free(info);
- return -1;
+ return 0;
}
@@ -371,12 +385,7 @@ int qemudDomainGetInfo(struct qemud_serv
return -1;
}
- if (!qemudIsActiveVM(vm)) {
- *runstate = QEMUD_STATE_STOPPED;
- } else {
- /* XXX in future need to add PAUSED */
- *runstate = QEMUD_STATE_RUNNING;
- }
+ *runstate = vm->state;
if (!qemudIsActiveVM(vm)) {
*cputime = 0;
@@ -432,7 +441,7 @@ int qemudDomainDumpXML(struct qemud_serv
strncpy(xml, vmxml, xmllen);
xml[xmllen-1] = '\0';
- free(xml);
+ free(vmxml);
return 0;
}
Index: internal.h
===================================================================
RCS file: /data/cvs/libvirt/qemud/internal.h,v
retrieving revision 1.17
diff -u -p -r1.17 internal.h
--- internal.h 23 Feb 2007 17:15:18 -0000 1.17
+++ internal.h 2 Mar 2007 16:42:49 -0000
@@ -216,6 +216,7 @@ struct qemud_vm {
int monitor;
int pid;
int id;
+ int state;
int *tapfds;
int ntapfds;
Index: qemud.c
===================================================================
RCS file: /data/cvs/libvirt/qemud/qemud.c,v
retrieving revision 1.29
diff -u -p -r1.29 qemud.c
--- qemud.c 23 Feb 2007 17:15:18 -0000 1.29
+++ qemud.c 2 Mar 2007 16:42:50 -0000
@@ -43,6 +43,7 @@
#include <string.h>
#include <errno.h>
#include <getopt.h>
+#include <ctype.h>
#include <libvirt/virterror.h>
@@ -614,6 +615,197 @@ qemudExec(struct qemud_server *server, c
return -1;
}
+#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)) {
+ int ret;
+
+ ret = read(monfd, buffer+got, sizeof(buffer)-got-1);
+ if (ret == 0)
+ goto error;
+ if (ret < 0) {
+ struct pollfd fd = { .fd = monfd, .events = POLLIN };
+ if (errno != EAGAIN &&
+ errno != EINTR)
+ goto error;
+
+ ret = poll(&fd, 1, MONITOR_TIMEOUT);
+ if (ret == 0) {
+ qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
+ "Timed out while waiting for monitor prompt");
+ goto error;
+ } else if (ret == -1) {
+ if (errno != EINTR) {
+ qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
+ "Failure while waiting for monitor prompt");
+ goto error;
+ }
+ } else if (fd.revents != POLLIN) {
+ qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
+ "Failure while waiting for monitor prompt");
+ goto error;
+ }
+ } else {
+ got += ret;
+ buffer[got] = '\0';
+ if (strstr(buffer, "(qemu) ") != NULL) {
+ vm->monitor = monfd;
+ return 0;
+ }
+ }
+ }
+
+ qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
+ "No monitor prompt found");
+
+ error:
+ close(monfd);
+ return -1;
+}
+
+static int qemudExtractMonitorPath(const char *haystack, char *path, int pathmax) {
+ static const char needle[] = "char device redirected to";
+ char *tmp;
+
+ if (!(tmp = strstr(haystack, needle)))
+ return -1;
+
+ strncpy(path, tmp+sizeof(needle), pathmax-1);
+ path[pathmax-1] = '\0';
+
+ while (*path) {
+ /*
+ * The monitor path ends at first whitespace char
+ * so lets search for it & NULL terminate it there
+ */
+ if (isspace(*path)) {
+ *path = '\0';
+ return 0;
+ }
+ path++;
+ }
+
+ /*
+ * We found a path, but didn't find any whitespace,
+ * so it must be still incomplete - we should at
+ * least see a \n
+ */
+ 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;
+
+ while (got < (sizeof(buffer)-1)) {
+ int ret;
+
+ 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';
+
+ if (qemudExtractMonitorPath(buffer, monitor, sizeof(monitor)) == 0) {
+ if (qemudOpenMonitor(server, vm, monitor) < 0)
+ return -1;
+ return 0;
+ }
+ }
+ }
+
+ qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
+ "Domain shutdown before sending monitor PTY path");
+ return -1;
+}
+
+static int qemudNextFreeVNCPort(struct qemud_server *server ATTRIBUTE_UNUSED) {
+ int i;
+
+ for (i = 5900 ; i < 6000 ; i++) {
+ int fd;
+ int reuse = 1;
+ struct sockaddr_in addr;
+ addr.sin_family = AF_INET;
+ addr.sin_port = htons(i);
+ addr.sin_addr.s_addr = htonl(INADDR_ANY);
+ fd = socket(PF_INET, SOCK_STREAM, 0);
+ if (fd < 0)
+ return -1;
+
+ if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void*)&reuse, sizeof(reuse)) < 0) {
+ close(fd);
+ break;
+ }
+
+ if (bind(fd, (struct sockaddr*)&addr, sizeof(addr)) == 0) {
+ /* Not in use, lets grab it */
+ close(fd);
+ return i;
+ }
+ close(fd);
+
+ if (errno == EADDRINUSE) {
+ /* In use, try next */
+ continue;
+ }
+ /* Some other bad failure, get out.. */
+ break;
+ }
+ return -1;
+}
int qemudStartVMDaemon(struct qemud_server *server,
struct qemud_vm *vm) {
@@ -626,9 +818,15 @@ int qemudStartVMDaemon(struct qemud_serv
return -1;
}
- if (vm->def->vncPort < 0)
- vm->def->vncActivePort = 5900 + server->nextvmid;
- else
+ if (vm->def->vncPort < 0) {
+ int port = qemudNextFreeVNCPort(server);
+ if (port < 0) {
+ qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
+ "Unable to find an unused VNC port");
+ return -1;
+ }
+ vm->def->vncActivePort = port;
+ } else
vm->def->vncActivePort = vm->def->vncPort;
if (qemudBuildCommandLine(server, vm, &argv) < 0)
@@ -636,12 +834,18 @@ int qemudStartVMDaemon(struct qemud_serv
if (qemudExec(server, argv, &vm->pid, &vm->stdout, &vm->stderr) == 0) {
vm->id = server->nextvmid++;
+ vm->state = QEMUD_STATE_RUNNING;
server->ninactivevms--;
server->nactivevms++;
server->nvmfds += 2;
ret = 0;
+
+ if (qemudWaitForMonitor(server, vm) < 0) {
+ qemudShutdownVMDaemon(server, vm);
+ ret = -1;
+ }
}
if (vm->tapfds) {
@@ -804,50 +1008,6 @@ static int qemudVMData(struct qemud_serv
}
buf[ret] = '\0';
- /*
- * XXX this is bad - we should wait for tty and open the
- * monitor when actually starting the guest, so we can
- * reliably trap startup failures
- */
- if (vm->monitor == -1) {
- char monitor[20];
- /* Fairly lame assuming we receive the data all in one chunk.
- This isn't guarenteed, but in practice it seems good enough.
- This will probably bite me in the future.... */
- if (sscanf(buf, "char device redirected to %19s", monitor) == 1) {
- int monfd;
-
- if (!(monfd = open(monitor, O_RDWR))) {
- perror("cannot open monitor");
- return -1;
- }
- if (qemudSetCloseExec(monfd) < 0) {
- close(monfd);
- return -1;
- }
- if (qemudSetNonBlock(monfd) < 0) {
- close(monfd);
- return -1;
- }
-
- /* Consume & discard the initial greeting */
- /* XXX this is broken, we need to block until
- we see the initial prompt to ensure startup
- has completed */
- for(;;) {
- char line[1024];
- if (read(monfd, line, sizeof(line)) < 0) {
- if (errno == EAGAIN) {
- break;
- }
- close(monfd);
- return -1;
- }
- qemudDebug("[%s]", line);
- }
- vm->monitor = monfd;
- }
- }
qemudDebug("[%s]", buf);
}
}
@@ -896,6 +1056,7 @@ int qemudShutdownVMDaemon(struct qemud_s
vm->pid = -1;
vm->id = -1;
+ vm->state = QEMUD_STATE_STOPPED;
if (vm->newDef) {
qemudFreeVMDef(vm->def);
@@ -1305,30 +1466,6 @@ static int qemudDispatchPoll(struct qemu
if (server->shutdown)
return 0;
- while (sock) {
- struct qemud_socket *next = sock->next;
- /* FIXME: the daemon shouldn't exit on error here */
- if (fds[fd].revents)
- if (qemudDispatchServer(server, sock) < 0)
- return -1;
- fd++;
- sock = next;
- }
-
- while (client) {
- struct qemud_client *next = client->next;
- if (fds[fd].revents) {
- qemudDebug("Poll data normal");
- if (fds[fd].revents == POLLOUT)
- qemudDispatchClientWrite(server, client);
- else if (fds[fd].revents == POLLIN)
- qemudDispatchClientRead(server, client);
- else
- qemudDispatchClientFailure(server, client);
- }
- fd++;
- client = next;
- }
vm = server->vms;
while (vm) {
struct qemud_vm *next = vm->next;
@@ -1371,6 +1508,29 @@ static int qemudDispatchPoll(struct qemu
if (failed)
ret = -1; /* FIXME: the daemon shouldn't exit on failure here */
}
+ while (client) {
+ struct qemud_client *next = client->next;
+ if (fds[fd].revents) {
+ qemudDebug("Poll data normal");
+ if (fds[fd].revents == POLLOUT)
+ qemudDispatchClientWrite(server, client);
+ else if (fds[fd].revents == POLLIN)
+ qemudDispatchClientRead(server, client);
+ else
+ qemudDispatchClientFailure(server, client);
+ }
+ fd++;
+ client = next;
+ }
+ while (sock) {
+ struct qemud_socket *next = sock->next;
+ /* FIXME: the daemon shouldn't exit on error here */
+ if (fds[fd].revents)
+ if (qemudDispatchServer(server, sock) < 0)
+ return -1;
+ fd++;
+ sock = next;
+ }
/* Cleanup any VMs which shutdown & dont have an associated
config file */
@@ -1408,22 +1568,6 @@ static void qemudPreparePoll(struct qemu
fds[fd].events = POLLIN;
fd++;
- for (sock = server->sockets ; sock ; sock = sock->next) {
- fds[fd].fd = sock->fd;
- fds[fd].events = POLLIN;
- fd++;
- }
-
- for (client = server->clients ; client ; client = client->next) {
- fds[fd].fd = client->fd;
- /* Refuse to read more from client if tx is pending to
- rate limit */
- if (client->tx)
- fds[fd].events = POLLOUT | POLLERR | POLLHUP;
- else
- fds[fd].events = POLLIN | POLLERR | POLLHUP;
- fd++;
- }
for (vm = server->vms ; vm ; vm = vm->next) {
if (!qemudIsActiveVM(vm))
continue;
@@ -1438,6 +1582,21 @@ static void qemudPreparePoll(struct qemu
fd++;
}
}
+ for (client = server->clients ; client ; client = client->next) {
+ fds[fd].fd = client->fd;
+ /* Refuse to read more from client if tx is pending to
+ rate limit */
+ if (client->tx)
+ fds[fd].events = POLLOUT | POLLERR | POLLHUP;
+ else
+ fds[fd].events = POLLIN | POLLERR | POLLHUP;
+ fd++;
+ }
+ for (sock = server->sockets ; sock ; sock = sock->next) {
+ fds[fd].fd = sock->fd;
+ fds[fd].events = POLLIN;
+ fd++;
+ }
}
More information about the libvir-list
mailing list