[libvirt] [PATCH 13/21] Make use of private data structure for monitor state

Introduce a new qemuDomainObjPrivate object which is used to store the private QEMU specific data associated with each virDomainObjPtr instance. This contains a single member, an instance of the new qemuMonitorPtr object which encapsulates the QEMU monitor state. The internals of the latter are private to the qemu_monitor* files, not to be shown to qemu_driver.c * src/qemu/qemu_conf.h: Definition of qemuDomainObjPrivate. * src/qemu/qemu_driver.c: Register a functions for creating and freeing qemuDomainObjPrivate instances with the domain capabilities. Remove the qemudDispatchVMEvent() watch since I/O watches are now handled by the monitor code itself. Pass a new qemuHandleMonitorEOF() callback into qemuMonitorOpen to allow notification when the monitor quits. * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h: Introduce the 'qemuMonitor' object. Temporarily add new APIs qemuMonitorWrite, qemuMonitorRead, qemuMonitorWaitForInput to allow text based monitor impl to perform I/O. * src/qemu/qemu_monitor_text.c: Call APIs for reading/writing to monitor instead of accessing the file handle directly. --- src/qemu/qemu_conf.h | 11 +++ src/qemu/qemu_driver.c | 160 ++++++++++++++++++------------------ src/qemu/qemu_monitor.c | 186 +++++++++++++++++++++++++++++++++++++----- src/qemu/qemu_monitor.h | 28 ++++++- src/qemu/qemu_monitor_text.c | 75 +++-------------- 5 files changed, 293 insertions(+), 167 deletions(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index a6e68f8..4961074 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -36,6 +36,7 @@ #include "security/security_driver.h" #include "cgroup.h" #include "pci.h" +#include "qemu_monitor.h" #define qemudDebug(fmt, ...) do {} while(0) @@ -128,6 +129,16 @@ struct qemud_driver { pciDeviceList *activePciHostdevs; }; +/* XXX temporarily exposed. + * This will be moved back into qemu_driver.c, once the + * qemu_monitor* code is refactored a little more + */ +typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate; +typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr; +struct _qemuDomainObjPrivate { + qemuMonitorPtr mon; +}; + /* Port numbers used for KVM migration. */ #define QEMUD_MIGRATION_FIRST_PORT 49152 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b36839b..d25afb9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -98,11 +98,6 @@ static void qemuDomainEventFlush(int timer, void *opaque); static void qemuDomainEventQueue(struct qemud_driver *driver, virDomainEventPtr event); -static void qemudDispatchVMEvent(int watch, - int fd, - int events, - void *opaque); - static int qemudStartVMDaemon(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, @@ -123,6 +118,30 @@ static int qemuUpdateActivePciHostdevs(struct qemud_driver *driver, static struct qemud_driver *qemu_driver = NULL; + +static void *qemuDomainObjPrivateAlloc(void) +{ + qemuDomainObjPrivatePtr priv; + + if (VIR_ALLOC(priv) < 0) + return NULL; + + return priv; +} + +static void qemuDomainObjPrivateFree(void *data) +{ + qemuDomainObjPrivatePtr priv = data; + + /* This should never be non-NULL if we get here, but just in case... */ + if (priv->mon) { + VIR_ERROR0("Unexpected QEMU monitor still active during domain deletion"); + qemuMonitorClose(priv->mon); + } + VIR_FREE(priv); +} + + static int qemuCgroupControllerActive(struct qemud_driver *driver, int controller) { @@ -299,25 +318,44 @@ cleanup: return rc; } -static int -qemuConnectMonitor(virDomainObjPtr vm, int reconnect) -{ - int rc; - if ((rc = qemuMonitorOpen(vm, reconnect)) != 0) { - VIR_ERROR(_("Failed to connect monitor for %s: %d\n"), - vm->def->name, rc); - return -1; - } - if ((vm->monitorWatch = virEventAddHandle(vm->monitor, - VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR, - qemudDispatchVMEvent, - vm, NULL)) < 0) - return -1; +/* + * This is a callback registered with a qemuMonitorPtr instance, + * and to be invoked when the monitor console hits an end of file + * condition, or error, thus indicating VM shutdown should be + * performed + */ +static void +qemuHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + int hasError) { + struct qemud_driver *driver = qemu_driver; + virDomainEventPtr event = NULL; - return 0; + qemuDriverLockRO(driver); + virDomainObjLock(vm); + qemuDriverUnlock(driver); + + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STOPPED, + hasError ? + VIR_DOMAIN_EVENT_STOPPED_FAILED : + VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN); + + qemudShutdownVMDaemon(NULL, driver, vm); + if (!vm->persistent) + virDomainRemoveInactive(&driver->domains, vm); + else + virDomainObjUnlock(vm); + + if (event) { + qemuDriverLock(driver); + qemuDomainEventQueue(driver, event); + qemuDriverUnlock(driver); + } } + /* * Open an existing VM's monitor, re-detect VCPU threads * and re-reserve the security labels in use @@ -327,11 +365,16 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq { virDomainObjPtr obj = payload; struct qemud_driver *driver = opaque; + qemuDomainObjPrivatePtr priv; virDomainObjLock(obj); - if (qemuConnectMonitor(obj, 1) < 0) + priv = obj->privateData; + + if ((priv->mon = qemuMonitorOpen(obj, 1, qemuHandleMonitorEOF)) == NULL) { + VIR_ERROR(_("Failed to connect monitor for %s\n"), obj->def->name); goto error; + } if (qemuUpdateActivePciHostdevs(driver, obj->def) < 0) { goto error; @@ -560,6 +603,9 @@ qemudStartup(int privileged) { if ((qemu_driver->caps = qemudCapsInit(NULL)) == NULL) goto out_of_memory; + qemu_driver->caps->privateDataAllocFunc = qemuDomainObjPrivateAlloc; + qemu_driver->caps->privateDataFreeFunc = qemuDomainObjPrivateFree; + if ((qemu_driver->activePciHostdevs = pciDeviceListNew(NULL)) == NULL) goto error; @@ -932,6 +978,7 @@ qemudWaitForMonitor(virConnectPtr conn, char buf[4096]; /* Plenty of space to get startup greeting */ int logfd; int ret; + qemuDomainObjPrivatePtr priv = vm->privateData; if ((logfd = qemudLogReadFD(conn, driver->logDir, vm->def->name, pos)) < 0) @@ -953,8 +1000,10 @@ qemudWaitForMonitor(virConnectPtr conn, return -1; } - if (qemuConnectMonitor(vm, 0) < 0) + if ((priv->mon = qemuMonitorOpen(vm, 0, qemuHandleMonitorEOF)) == NULL) { + VIR_ERROR(_("Failed to connect monitor for %s\n"), vm->def->name); return -1; + } return 0; } @@ -1946,6 +1995,7 @@ static void qemudShutdownVMDaemon(virConnectPtr conn, virDomainObjPtr vm) { int ret; int retries = 0; + qemuDomainObjPrivatePtr priv = vm->privateData; if (!virDomainIsActive(vm)) return; @@ -1958,15 +2008,11 @@ static void qemudShutdownVMDaemon(virConnectPtr conn, _("Failed to send SIGTERM to %s (%d)"), vm->def->name, vm->pid); - if (vm->monitorWatch != -1) { - virEventRemoveHandle(vm->monitorWatch); - vm->monitorWatch = -1; + if (priv->mon) { + qemuMonitorClose(priv->mon); + priv->mon = NULL; } - if (vm->monitor != -1) - close(vm->monitor); - vm->monitor = -1; - if (vm->monitor_chr) { if (vm->monitor_chr->type == VIR_DOMAIN_CHR_TYPE_UNIX) unlink(vm->monitor_chr->data.nix.path); @@ -2029,59 +2075,6 @@ retry: } -static void -qemudDispatchVMEvent(int watch, int fd, int events, void *opaque) { - struct qemud_driver *driver = qemu_driver; - virDomainObjPtr vm = opaque; - virDomainEventPtr event = NULL; - int quit = 0, failed = 0; - - /* XXX Normally we have to lock the driver first, to protect - * against someone adding/removing the domain. We know, - * however, then if we're getting data in this callback - * the VM must be running. Nowhere is allowed to remove - * a domain while it is running, so it is safe to not - * lock the driver here... */ - qemuDriverLockRO(driver); - virDomainObjLock(vm); - qemuDriverUnlock(driver); - - if (vm->monitor != fd || vm->monitorWatch != watch) { - failed = 1; - } else { - if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) - quit = 1; - else { - VIR_ERROR(_("unhandled fd event %d for %s"), - events, vm->def->name); - failed = 1; - } - } - - if (failed || quit) { - event = virDomainEventNewFromObj(vm, - VIR_DOMAIN_EVENT_STOPPED, - quit ? - VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN : - VIR_DOMAIN_EVENT_STOPPED_FAILED); - qemudShutdownVMDaemon(NULL, driver, vm); - if (!vm->persistent) { - virDomainRemoveInactive(&driver->domains, - vm); - vm = NULL; - } - } - - virDomainObjUnlock(vm); - if (event) { - qemuDriverLock(driver); - qemuDomainEventQueue(driver, event); - qemuDriverUnlock(driver); - } -} - - - static virDrvOpenStatus qemudOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { @@ -2215,6 +2208,9 @@ static char *qemudGetCapabilities(virConnectPtr conn) { goto cleanup; } + caps->privateDataAllocFunc = qemuDomainObjPrivateAlloc; + caps->privateDataFreeFunc = qemuDomainObjPrivateFree; + if (qemu_driver->securityDriver && qemudSecurityCapsInit(qemu_driver->securityDriver, caps) < 0) { virCapabilitiesFree(caps); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index ab1a98c..5f7e20c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -32,13 +32,24 @@ #include "qemu_conf.h" #include "event.h" #include "virterror_internal.h" +#include "memory.h" +#include "logging.h" #define VIR_FROM_THIS VIR_FROM_QEMU +struct _qemuMonitor { + int fd; + int watch; + int hasSendFD; + + virDomainObjPtr vm; + + qemuMonitorEOFNotify eofCB; +}; + /* Return -1 for error, 1 to continue reading and 0 for success */ typedef int qemuMonitorHandleOutput(virDomainObjPtr vm, - const char *output, - int fd); + const char *output); /* * Returns -1 for error, 0 on end-of-file, 1 for success @@ -101,7 +112,7 @@ qemuMonitorReadOutput(virDomainObjPtr vm, } else { got += ret; buf[got] = '\0'; - ret = func(vm, buf, fd); + ret = func(vm, buf); if (ret == -1) return -1; if (ret == 1) @@ -117,15 +128,12 @@ qemuMonitorReadOutput(virDomainObjPtr vm, } static int -qemuMonitorCheckPrompt(virDomainObjPtr vm, - const char *output, - int fd) +qemuMonitorCheckPrompt(virDomainObjPtr vm ATTRIBUTE_UNUSED, + const char *output) { if (strstr(output, "(qemu) ") == NULL) return 1; /* keep reading */ - vm->monitor = fd; - return 0; } @@ -157,14 +165,10 @@ qemuMonitorOpenCommon(virDomainObjPtr vm, else ret = 0; } else { - vm->monitor = monfd; ret = 0; } - if (ret != 0) - return ret; - - return 0; + return ret; } static int @@ -218,7 +222,7 @@ qemuMonitorOpenUnix(virDomainObjPtr vm, if (qemuMonitorOpenCommon(vm, monfd, reconnect) < 0) goto error; - return 0; + return monfd; error: close(monfd); @@ -241,28 +245,168 @@ qemuMonitorOpenPty(virDomainObjPtr vm, if (qemuMonitorOpenCommon(vm, monfd, reconnect) < 0) goto error; - return 0; + return monfd; error: close(monfd); return -1; } -int + +static void +qemuMonitorIO(int watch, int fd, int events, void *opaque) { + qemuMonitorPtr mon = opaque; + int quit = 0, failed = 0; + + if (mon->fd != fd || mon->watch != watch) { + VIR_ERROR0(_("event from unexpected fd/watch")); + failed = 1; + } else { + if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) + quit = 1; + else { + VIR_ERROR(_("unhandled fd event %d for monitor fd %d"), + events, mon->fd); + failed = 1; + } + } + + mon->eofCB(mon, mon->vm, failed); +} + + + + + +qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm, - int reconnect) + int reconnect, + qemuMonitorEOFNotify eofCB) { + qemuMonitorPtr mon; + + if (VIR_ALLOC(mon) < 0) { + virReportOOMError(NULL); + return NULL; + } + + mon->fd = -1; + mon->vm = vm; + mon->eofCB = eofCB; + switch (vm->monitor_chr->type) { case VIR_DOMAIN_CHR_TYPE_UNIX: - return qemuMonitorOpenUnix(vm, vm->monitor_chr->data.nix.path, - reconnect); + mon->hasSendFD = 1; + mon->fd = qemuMonitorOpenUnix(vm, vm->monitor_chr->data.nix.path, + reconnect); + break; + case VIR_DOMAIN_CHR_TYPE_PTY: - return qemuMonitorOpenPty(vm, vm->monitor_chr->data.file.path, - reconnect); + mon->fd = qemuMonitorOpenPty(vm, vm->monitor_chr->data.file.path, + reconnect); + break; + default: qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("unable to handle monitor type: %s"), virDomainChrTypeToString(vm->monitor_chr->type)); + goto cleanup; + } + + if ((mon->watch = virEventAddHandle(mon->fd, + VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR, + qemuMonitorIO, + mon, NULL)) < 0) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to register monitor events")); + goto cleanup; + } + + return mon; + +cleanup: + qemuMonitorClose(mon); + return NULL; +} + + +void qemuMonitorClose(qemuMonitorPtr mon) +{ + if (!mon) + return; + + if (mon->watch) + virEventRemoveHandle(mon->watch); + + if (mon->fd != -1) + close(mon->fd); + VIR_FREE(mon); +} + + +int qemuMonitorWrite(qemuMonitorPtr mon, + const char *data, + size_t len) +{ + return safewrite(mon->fd, data, len); +} + +int qemuMonitorWriteWithFD(qemuMonitorPtr mon, + const char *data, + size_t len, + int fd) +{ + struct msghdr msg; + struct iovec iov[1]; + ssize_t ret; + char control[CMSG_SPACE(sizeof(int))]; + struct cmsghdr *cmsg; + + if (!mon->hasSendFD) { + errno = EINVAL; return -1; } + + memset(&msg, 0, sizeof(msg)); + + iov[0].iov_base = (void *)data; + iov[0].iov_len = len; + + msg.msg_iov = iov; + msg.msg_iovlen = 1; + + msg.msg_control = control; + msg.msg_controllen = sizeof(control); + + cmsg = CMSG_FIRSTHDR(&msg); + cmsg->cmsg_len = CMSG_LEN(sizeof(int)); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + memcpy(CMSG_DATA(cmsg), &fd, sizeof(int)); + + do { + ret = sendmsg(mon->fd, &msg, 0); + } while (ret < 0 && errno == EINTR); + + return ret == len ? 0 : -1; +} + +int qemuMonitorRead(qemuMonitorPtr mon, + char *data, + size_t len) +{ + return read(mon->fd, data, len); +} + +int qemuMonitorWaitForInput(qemuMonitorPtr mon) +{ + struct pollfd fd = { mon->fd, POLLIN | POLLERR | POLLHUP, 0 }; + +retry: + if (poll(&fd, 1, -1) < 0) { + if (errno == EINTR) + goto retry; + return -1; + } + return 0; } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index bdeafe0..e863e20 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -29,9 +29,33 @@ #include "domain_conf.h" -int qemuMonitorOpen(virDomainObjPtr vm, - int reconnect); +typedef struct _qemuMonitor qemuMonitor; +typedef qemuMonitor *qemuMonitorPtr; +typedef void (*qemuMonitorEOFNotify)(qemuMonitorPtr mon, + virDomainObjPtr vm, + int withError); + +qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm, + int reconnect, + qemuMonitorEOFNotify eofCB); + +void qemuMonitorClose(qemuMonitorPtr mon); + +int qemuMonitorWrite(qemuMonitorPtr mon, + const char *data, + size_t len); + +int qemuMonitorWriteWithFD(qemuMonitorPtr mon, + const char *data, + size_t len, + int fd); + +int qemuMonitorRead(qemuMonitorPtr mon, + char *data, + size_t len); + +int qemuMonitorWaitForInput(qemuMonitorPtr mon); #endif /* QEMU_MONITOR_H */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 66526dc..fa17971 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -142,6 +142,7 @@ static char *qemuMonitorEscapeShell(const char *in) */ static void qemuMonitorDiscardPendingData(virDomainObjPtr vm) { + qemuDomainObjPrivatePtr priv = vm->privateData; char buf[1024]; int ret = 0; @@ -149,54 +150,17 @@ qemuMonitorDiscardPendingData(virDomainObjPtr vm) { * get -1 or 0. Don't bother with detecting * errors, since we'll deal with that better later */ do { - ret = read(vm->monitor, buf, sizeof (buf)-1); + ret = qemuMonitorRead(priv->mon, buf, sizeof (buf)-1); } while (ret > 0); } -static int -qemuMonitorSendUnix(const virDomainObjPtr vm, - const char *cmd, - size_t cmdlen, - int scm_fd) -{ - struct msghdr msg; - struct iovec iov[1]; - ssize_t ret; - - memset(&msg, 0, sizeof(msg)); - - iov[0].iov_base = (void *)cmd; - iov[0].iov_len = cmdlen; - - msg.msg_iov = iov; - msg.msg_iovlen = 1; - - if (scm_fd != -1) { - char control[CMSG_SPACE(sizeof(int))]; - struct cmsghdr *cmsg; - - msg.msg_control = control; - msg.msg_controllen = sizeof(control); - - cmsg = CMSG_FIRSTHDR(&msg); - cmsg->cmsg_len = CMSG_LEN(sizeof(int)); - cmsg->cmsg_level = SOL_SOCKET; - cmsg->cmsg_type = SCM_RIGHTS; - memcpy(CMSG_DATA(cmsg), &scm_fd, sizeof(int)); - } - - do { - ret = sendmsg(vm->monitor, &msg, 0); - } while (ret < 0 && errno == EINTR); - - return ret == cmdlen ? 0 : -1; -} static int qemuMonitorSend(const virDomainObjPtr vm, const char *cmd, int scm_fd) { + qemuDomainObjPrivatePtr priv = vm->privateData; char *full; size_t len; int ret = -1; @@ -206,20 +170,11 @@ qemuMonitorSend(const virDomainObjPtr vm, len = strlen(full); - switch (vm->monitor_chr->type) { - case VIR_DOMAIN_CHR_TYPE_UNIX: - if (qemuMonitorSendUnix(vm, full, len, scm_fd) < 0) - goto out; - break; - default: - case VIR_DOMAIN_CHR_TYPE_PTY: - if (safewrite(vm->monitor, full, len) != len) - goto out; - break; - } + if (scm_fd == -1) + ret = qemuMonitorWrite(priv->mon, full, len); + else + ret = qemuMonitorWriteWithFD(priv->mon, full, len, scm_fd); - ret = 0; -out: VIR_FREE(full); return ret; } @@ -232,12 +187,13 @@ qemuMonitorCommandWithHandler(const virDomainObjPtr vm, void *handlerData, int scm_fd, char **reply) { + qemuDomainObjPrivatePtr priv = vm->privateData; int size = 0; char *buf = NULL; /* Should never happen, but just in case, protect * against null monitor (ocurrs when VM is inactive) */ - if (!vm->monitor_chr) + if (!priv->mon) return -1; qemuMonitorDiscardPendingData(vm); @@ -249,13 +205,10 @@ qemuMonitorCommandWithHandler(const virDomainObjPtr vm, *reply = NULL; for (;;) { - struct pollfd fd = { vm->monitor, POLLIN | POLLERR | POLLHUP, 0 }; - char *tmp; - /* Read all the data QEMU has sent thus far */ for (;;) { char data[1024]; - int got = read(vm->monitor, data, sizeof(data)); + int got = qemuMonitorRead(priv->mon, data, sizeof(data)); if (got == 0) goto error; @@ -277,6 +230,7 @@ qemuMonitorCommandWithHandler(const virDomainObjPtr vm, /* Look for QEMU prompt to indicate completion */ if (buf) { char *foundPrompt; + char *tmp; if (extraPrompt && (foundPrompt = strstr(buf, extraPrompt)) != NULL) { @@ -312,13 +266,10 @@ qemuMonitorCommandWithHandler(const virDomainObjPtr vm, break; } } - pollagain: + /* Need to wait for more data */ - if (poll(&fd, 1, -1) < 0) { - if (errno == EINTR) - goto pollagain; + if (qemuMonitorWaitForInput(priv->mon) < 0) goto error; - } } *reply = buf; DEBUG("reply='%s'", buf); -- 1.6.2.5