Change the QEMU monitor file handle watch to poll for both read & write events, as well as EOF. All I/O to/from the QEMU monitor FD is now done in the event callback thread. When the QEMU driver needs to send a command, it puts the data to be sent into a qemuMonitorMessagePtr object instance, queues it for dispatch, and then goes to sleep on a condition variable. The event thread sends all the data, and then waits for the reply to arrive, putting the response / error data back into the qemuMonitorMessagePtr and notifying the condition variable. * src/qemu/qemu_driver.c: XXX this change shouldn't be here * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h: Remove raw I/O functions, and a generic qemuMonitorSend() for invoking a command * src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Remove all low level I/O, and use the new qemuMonitorSend() API. Provide a qemuMonitorTextIOProcess() method for detecting command/reply/prompt boundaries in the monitor data stream --- src/conf/domain_conf.c | 3 +- src/qemu/qemu_driver.c | 60 ++++- src/qemu/qemu_monitor.c | 550 ++++++++++++++++++++++++++---------------- src/qemu/qemu_monitor.h | 46 +++-- src/qemu/qemu_monitor_text.c | 402 +++++++++++++++++-------------- src/qemu/qemu_monitor_text.h | 5 + 6 files changed, 647 insertions(+), 419 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 82bec2a..6f8704d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4870,7 +4870,8 @@ static virDomainObjPtr virDomainLoadStatus(virConnectPtr conn, return obj; error: - virDomainObjUnref(obj); + if (obj) + virDomainObjUnref(obj); VIR_FREE(statusFile); return NULL; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 47bd58a..8f269d2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -337,9 +337,8 @@ qemuHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, struct qemud_driver *driver = qemu_driver; virDomainEventPtr event = NULL; - qemuDriverLockRO(driver); + VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name); virDomainObjLock(vm); - qemuDriverUnlock(driver); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, @@ -403,11 +402,24 @@ findVolumeQcowPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, char *passphrase; unsigned char *data; size_t size; + int ret = -1; + + /* XXX + * We ought to be taking the lock here, but that would + * require that it be released when monitor commands are + * run. Currently we deadlock if we try to take it again + * + * Until this is resolved, don't take the lock and rely + * on fact that the thread invoking this callback is + * running lock-step with the thread holding the lock + * + * virDomainObjLock(vm); + */ if (!conn) { qemudReportError(NULL, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s", _("cannot find secrets without a connection")); - return -1; + goto cleanup; } if (conn->secretDriver == NULL || @@ -415,7 +427,7 @@ findVolumeQcowPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, conn->secretDriver->getValue == NULL) { qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s", _("secret storage not supported")); - return -1; + goto cleanup; } enc = findDomainDiskEncryption(conn, vm, path); @@ -428,18 +440,18 @@ findVolumeQcowPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE) { qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_DOMAIN, _("invalid <encryption> for volume %s"), path); - return -1; + goto cleanup; } secret = conn->secretDriver->lookupByUUID(conn, enc->secrets[0]->uuid); if (secret == NULL) - return -1; + goto cleanup; data = conn->secretDriver->getValue(secret, &size, VIR_SECRET_GET_VALUE_INTERNAL_CALL); virUnrefSecret(secret); if (data == NULL) - return -1; + goto cleanup; if (memchr(data, '\0', size) != NULL) { memset(data, 0, size); @@ -447,14 +459,14 @@ findVolumeQcowPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_SECRET, _("format='qcow' passphrase for %s must not contain a " "'\\0'"), path); - return -1; + goto cleanup; } if (VIR_ALLOC_N(passphrase, size + 1) < 0) { memset(data, 0, size); VIR_FREE(data); virReportOOMError(conn); - return -1; + goto cleanup; } memcpy(passphrase, data, size); passphrase[size] = '\0'; @@ -465,7 +477,16 @@ findVolumeQcowPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, *secretRet = passphrase; *secretLen = size; - return 0; + ret = 0; + +cleanup: + /* + * XXX + * See earlier comment about lock + * + * virDomainObjUnlock(vm); + */ + return ret; } /* @@ -483,7 +504,10 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq priv = obj->privateData; - if ((priv->mon = qemuMonitorOpen(obj, 1, qemuHandleMonitorEOF)) == NULL) { + VIR_DEBUG("Reconnect monitor to %p '%s'", obj, obj->def->name); + + /* XXX check PID liveliness & EXE path */ + if ((priv->mon = qemuMonitorOpen(obj, qemuHandleMonitorEOF)) == NULL) { VIR_ERROR(_("Failed to connect monitor for %s\n"), obj->def->name); goto error; } @@ -512,7 +536,10 @@ error: * to remove danger of it ending up running twice if * user tries to start it again later */ qemudShutdownVMDaemon(NULL, driver, obj); - virDomainObjUnlock(obj); + if (!obj->persistent) + virDomainRemoveInactive(&driver->domains, obj); + else + virDomainObjUnlock(obj); } /** @@ -1115,7 +1142,8 @@ qemudWaitForMonitor(virConnectPtr conn, return -1; } - if ((priv->mon = qemuMonitorOpen(vm, 0, qemuHandleMonitorEOF)) == NULL) { + VIR_DEBUG("Connect monitor to %p '%s'", vm, vm->def->name); + if ((priv->mon = qemuMonitorOpen(vm, qemuHandleMonitorEOF)) == NULL) { VIR_ERROR(_("Failed to connect monitor for %s\n"), vm->def->name); return -1; } @@ -2121,7 +2149,7 @@ static void qemudShutdownVMDaemon(virConnectPtr conn, if (!virDomainIsActive(vm)) return; - VIR_DEBUG(_("Shutting down VM '%s'\n"), vm->def->name); + VIR_DEBUG("Shutting down VM '%s'", vm->def->name); if (virKillProcess(vm->pid, 0) == 0 && virKillProcess(vm->pid, SIGTERM) < 0) @@ -6088,6 +6116,10 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, qemust = qemuStreamMigOpen(st, unixfile); if (qemust == NULL) { qemudShutdownVMDaemon(NULL, driver, vm); + if (!vm->persistent) { + virDomainRemoveInactive(&driver->domains, vm); + vm = NULL; + } virReportSystemError(dconn, errno, _("cannot open unix socket '%s' for tunnelled migration"), unixfile); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 33dbe1b..fb89f9d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -40,6 +40,9 @@ struct _qemuMonitor { virMutex lock; + virCond notify; + + virDomainObjPtr dom; int fd; int watch; @@ -49,6 +52,25 @@ struct _qemuMonitor { qemuMonitorEOFNotify eofCB; qemuMonitorDiskSecretLookup secretCB; + + /* If there's a command being processed this will be + * non-NULL */ + qemuMonitorMessagePtr msg; + + /* Buffer incoming data ready for Text/QMP monitor + * code to process & find message boundaries */ + size_t bufferOffset; + size_t bufferLength; + char *buffer; + + /* If anything went wrong, this will be fed back + * the next monitor msg */ + int lastErrno; + + /* If the monitor callback is currently active */ + unsigned eofcb: 1; + /* If the monitor callback should free the closed monitor */ + unsigned closed: 1; }; static void qemuMonitorLock(qemuMonitorPtr mon) @@ -61,134 +83,25 @@ static void qemuMonitorUnlock(qemuMonitorPtr mon) virMutexUnlock(&mon->lock); } -/* Return -1 for error, 1 to continue reading and 0 for success */ -typedef int qemuMonitorHandleOutput(virDomainObjPtr vm, - const char *output); - -/* - * Returns -1 for error, 0 on end-of-file, 1 for success - */ -static int -qemuMonitorReadOutput(virDomainObjPtr vm, - int fd, - char *buf, - size_t buflen, - qemuMonitorHandleOutput func, - const char *what, - int timeout) -{ - size_t got = 0; - buf[0] = '\0'; - timeout *= 1000; /* poll wants milli seconds */ - - /* Consume & discard the initial greeting */ - while (got < (buflen-1)) { - ssize_t ret; - - ret = read(fd, buf+got, buflen-got-1); - - if (ret < 0) { - struct pollfd pfd = { .fd = fd, .events = POLLIN }; - if (errno == EINTR) - continue; - - if (errno != EAGAIN) { - virReportSystemError(NULL, errno, - _("Failure while reading %s startup output"), - what); - return -1; - } - - ret = poll(&pfd, 1, timeout); - if (ret == 0) { - qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Timed out while reading %s startup output"), what); - return -1; - } else if (ret == -1) { - if (errno != EINTR) { - virReportSystemError(NULL, errno, - _("Failure while reading %s startup output"), - what); - return -1; - } - } else { - /* Make sure we continue loop & read any further data - available before dealing with EOF */ - if (pfd.revents & (POLLIN | POLLHUP)) - continue; - - qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Failure while reading %s startup output"), what); - return -1; - } - } else if (ret == 0) { - return 0; - } else { - got += ret; - buf[got] = '\0'; - ret = func(vm, buf); - if (ret == -1) - return -1; - if (ret == 1) - continue; - return 1; - } - } - - qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Out of space while reading %s startup output"), what); - return -1; - -} - -static int -qemuMonitorCheckPrompt(virDomainObjPtr vm ATTRIBUTE_UNUSED, - const char *output) -{ - if (strstr(output, "(qemu) ") == NULL) - return 1; /* keep reading */ - - return 0; -} -static int -qemuMonitorOpenCommon(virDomainObjPtr vm, - int monfd, - int reconnect) +static void qemuMonitorFree(qemuMonitorPtr mon, int lockDomain) { - char buf[1024]; - int ret; - - if (virSetCloseExec(monfd) < 0) { - qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("Unable to set monitor close-on-exec flag")); - return -1; - } - if (virSetNonBlock(monfd) < 0) { - qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("Unable to put monitor into non-blocking mode")); - return -1; + VIR_DEBUG("mon=%p, lockDomain=%d", mon, lockDomain); + if (mon->vm) { + if (lockDomain) + virDomainObjLock(mon->vm); + if (!virDomainObjUnref(mon->vm) && lockDomain) + virDomainObjUnlock(mon->vm); } - - if (!reconnect) { - if (qemuMonitorReadOutput(vm, monfd, - buf, sizeof(buf), - qemuMonitorCheckPrompt, - "monitor", 10) <= 0) - ret = -1; - else - ret = 0; - } else { - ret = 0; - } - - return ret; + if (virCondDestroy(&mon->notify) < 0) + {} + virMutexDestroy(&mon->lock); + VIR_FREE(mon); } + static int -qemuMonitorOpenUnix(virDomainObjPtr vm, - const char *monitor, - int reconnect) +qemuMonitorOpenUnix(const char *monitor) { struct sockaddr_un addr; int monfd; @@ -233,9 +146,6 @@ qemuMonitorOpenUnix(virDomainObjPtr vm, goto error; } - if (qemuMonitorOpenCommon(vm, monfd, reconnect) < 0) - goto error; - return monfd; error: @@ -244,9 +154,7 @@ error: } static int -qemuMonitorOpenPty(virDomainObjPtr vm, - const char *monitor, - int reconnect) +qemuMonitorOpenPty(const char *monitor) { int monfd; @@ -256,14 +164,181 @@ qemuMonitorOpenPty(virDomainObjPtr vm, return -1; } - if (qemuMonitorOpenCommon(vm, monfd, reconnect) < 0) - goto error; - return monfd; +} -error: - close(monfd); - return -1; + +static int +qemuMonitorIOProcess(qemuMonitorPtr mon) +{ + int len; + qemuMonitorMessagePtr msg = NULL; + + /* See if there's a message & whether its ready for its reply + * ie whether its completed writing all its data */ + if (mon->msg && mon->msg->txOffset == mon->msg->txLength) + msg = mon->msg; + + VIR_DEBUG("Process %d", mon->bufferOffset); + len = qemuMonitorTextIOProcess(mon, + mon->buffer, mon->bufferOffset, + msg); + + if (len < 0) { + mon->lastErrno = errno; + return -1; + } + + if (len < mon->bufferOffset) { + memmove(mon->buffer, mon->buffer + len, mon->bufferOffset - len); + mon->bufferOffset -= len; + } else { + VIR_FREE(mon->buffer); + mon->bufferOffset = mon->bufferLength = 0; + } + VIR_DEBUG("Process done %d used %d", mon->bufferOffset, len); + if (msg && msg->finished) + virCondBroadcast(&mon->notify); + return len; +} + + +static int +qemuMonitorIOWriteWithFD(qemuMonitorPtr mon, + const char *data, + size_t len, + int fd) +{ + struct msghdr msg; + struct iovec iov[1]; + int 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; +} + +/* Called when the monitor is able to write data */ +static int +qemuMonitorIOWrite(qemuMonitorPtr mon) +{ + int done; + + /* If no active message, or fully transmitted, the no-op */ + if (!mon->msg || mon->msg->txOffset == mon->msg->txLength) + return 0; + + if (mon->msg->txFD == -1) + done = write(mon->fd, + mon->msg->txBuffer + mon->msg->txOffset, + mon->msg->txLength - mon->msg->txOffset); + else + done = qemuMonitorIOWriteWithFD(mon, + mon->msg->txBuffer + mon->msg->txOffset, + mon->msg->txLength - mon->msg->txOffset, + mon->msg->txFD); + + if (done < 0) { + if (errno == EAGAIN) + return 0; + + mon->lastErrno = errno; + return -1; + } + mon->msg->txOffset += done; + return done; +} + +/* + * Called when the monitor has incoming data to read + * + * Returns -1 on error, or number of bytes read + */ +static int +qemuMonitorIORead(qemuMonitorPtr mon) +{ + size_t avail = mon->bufferLength - mon->bufferOffset; + int ret = 0; + + if (avail < 1024) { + if (VIR_REALLOC_N(mon->buffer, + mon->bufferLength + 1024) < 0) { + errno = ENOMEM; + return -1; + } + mon->bufferLength += 1024; + avail += 1024; + } + + /* Read as much as we can get into our buffer, + until we block on EAGAIN, or hit EOF */ + while (avail > 1) { + int got; + got = read(mon->fd, + mon->buffer + mon->bufferOffset, + avail - 1); + if (got < 0) { + if (errno == EAGAIN) + break; + mon->lastErrno = errno; + ret = -1; + break; + } + if (got == 0) + break; + + ret += got; + avail -= got; + mon->bufferOffset += got; + mon->buffer[mon->bufferOffset] = '\0'; + } + + VIR_DEBUG("Now read %d bytes of data", mon->bufferOffset); + + return ret; +} + + +static void qemuMonitorUpdateWatch(qemuMonitorPtr mon) +{ + int events = + VIR_EVENT_HANDLE_HANGUP | + VIR_EVENT_HANDLE_ERROR; + + if (!mon->lastErrno) { + events |= VIR_EVENT_HANDLE_READABLE; + + if (mon->msg && mon->msg->txOffset < mon->msg->txLength) + events |= VIR_EVENT_HANDLE_WRITABLE; + } + + virEventUpdateHandle(mon->watch, events); } @@ -272,29 +347,89 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { qemuMonitorPtr mon = opaque; int quit = 0, failed = 0; + qemuMonitorLock(mon); + VIR_DEBUG("Monitor %p I/O on watch %d fd %d events %d", mon, watch, fd, events); + if (mon->fd != fd || mon->watch != watch) { - VIR_ERROR0(_("event from unexpected fd/watch")); + VIR_ERROR("event from unexpected fd %d!=%d / watch %d!=%d", mon->fd, fd, mon->watch, watch); failed = 1; } else { - if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) + if (!mon->lastErrno && + events & VIR_EVENT_HANDLE_WRITABLE) { + int done = qemuMonitorIOWrite(mon); + if (done < 0) + failed = 1; + events &= ~VIR_EVENT_HANDLE_WRITABLE; + } + if (!mon->lastErrno && + events & VIR_EVENT_HANDLE_READABLE) { + int got = qemuMonitorIORead(mon); + if (got < 0) + failed = 1; + /* Ignore hangup/error events if we read some data, to + * give time for that data to be consumed */ + if (got > 0) { + events = 0; + + if (qemuMonitorIOProcess(mon) < 0) + failed = 1; + } else + events &= ~VIR_EVENT_HANDLE_READABLE; + } + + /* If IO process resulted in an error & we have a message, + * then wakeup that waiter */ + if (mon->lastErrno && mon->msg && !mon->msg->finished) { + mon->msg->lastErrno = mon->lastErrno; + mon->msg->finished = 1; + virCondSignal(&mon->notify); + } + + qemuMonitorUpdateWatch(mon); + + if (events & VIR_EVENT_HANDLE_HANGUP) { + /* If IO process resulted in EOF & we have a message, + * then wakeup that waiter */ + if (mon->msg && !mon->msg->finished) { + mon->msg->finished = 1; + mon->msg->lastErrno = EIO; + virCondSignal(&mon->notify); + } quit = 1; - else { + } else if (events) { VIR_ERROR(_("unhandled fd event %d for monitor fd %d"), events, mon->fd); failed = 1; } } - mon->eofCB(mon, mon->vm, failed); + /* 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 (failed || quit) { + /* Make sure anyone waiting wakes up now */ + virCondSignal(&mon->notify); + mon->eofcb = 1; + qemuMonitorUnlock(mon); + VIR_DEBUG("Triggering EOF callback error? %d", failed); + mon->eofCB(mon, mon->vm, failed); + + qemuMonitorLock(mon); + if (mon->closed) { + qemuMonitorUnlock(mon); + VIR_DEBUG("Delayed free of monitor %p", mon); + qemuMonitorFree(mon, 1); + } else { + qemuMonitorUnlock(mon); + } + } else { + qemuMonitorUnlock(mon); + } } - - - qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm, - int reconnect, qemuMonitorEOFNotify eofCB) { qemuMonitorPtr mon; @@ -310,20 +445,26 @@ qemuMonitorOpen(virDomainObjPtr vm, VIR_FREE(mon); return NULL; } + if (virCondInit(&mon->notify) < 0) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot initialize monitor condition")); + virMutexDestroy(&mon->lock); + VIR_FREE(mon); + return NULL; + } mon->fd = -1; mon->vm = vm; mon->eofCB = eofCB; + qemuMonitorLock(mon); switch (vm->monitor_chr->type) { case VIR_DOMAIN_CHR_TYPE_UNIX: mon->hasSendFD = 1; - mon->fd = qemuMonitorOpenUnix(vm, vm->monitor_chr->data.nix.path, - reconnect); + mon->fd = qemuMonitorOpenUnix(vm->monitor_chr->data.nix.path); break; case VIR_DOMAIN_CHR_TYPE_PTY: - mon->fd = qemuMonitorOpenPty(vm, vm->monitor_chr->data.file.path, - reconnect); + mon->fd = qemuMonitorOpenPty(vm->monitor_chr->data.file.path); break; default: @@ -333,8 +474,22 @@ qemuMonitorOpen(virDomainObjPtr vm, goto cleanup; } + if (virSetCloseExec(mon->fd) < 0) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to set monitor close-on-exec flag")); + goto cleanup; + } + if (virSetNonBlock(mon->fd) < 0) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to put monitor into non-blocking mode")); + goto cleanup; + } + + if ((mon->watch = virEventAddHandle(mon->fd, - VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR, + VIR_EVENT_HANDLE_HANGUP | + VIR_EVENT_HANDLE_ERROR | + VIR_EVENT_HANDLE_READABLE, qemuMonitorIO, mon, NULL)) < 0) { qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", @@ -342,10 +497,15 @@ qemuMonitorOpen(virDomainObjPtr vm, goto cleanup; } + virDomainObjRef(vm); + + VIR_DEBUG("New mon %p fd =%d watch=%d", mon, mon->fd, mon->watch); + qemuMonitorUnlock(mon); return mon; cleanup: + qemuMonitorUnlock(mon); qemuMonitorClose(mon); return NULL; } @@ -356,13 +516,24 @@ void qemuMonitorClose(qemuMonitorPtr mon) if (!mon) return; - if (mon->watch) - virEventRemoveHandle(mon->watch); + qemuMonitorLock(mon); + if (!mon->closed) { + if (mon->watch) + virEventRemoveHandle(mon->watch); + if (mon->fd != -1) + close(mon->fd); + /* NB: don't reset fd / watch fields, since active + * callback may still want them */ + mon->closed = 1; + } - if (mon->fd != -1) - close(mon->fd); - virMutexDestroy(&mon->lock); - VIR_FREE(mon); + if (mon->eofcb) { + VIR_DEBUG("Mark monitor to be deleted %p", mon); + qemuMonitorUnlock(mon); + } else { + VIR_DEBUG("Delete monitor now %p", mon); + qemuMonitorFree(mon, 0); + } } void qemuMonitorRegisterDiskSecretLookup(qemuMonitorPtr mon, @@ -372,71 +543,33 @@ void qemuMonitorRegisterDiskSecretLookup(qemuMonitorPtr mon, } -int qemuMonitorWrite(qemuMonitorPtr mon, - const char *data, - size_t len) +int qemuMonitorSend(qemuMonitorPtr mon, + qemuMonitorMessagePtr msg) { - 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; + int ret = -1; - if (!mon->hasSendFD) { - errno = EINVAL; + if (mon->eofcb) { + msg->lastErrno = EIO; + qemuMonitorUnlock(mon); 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)); + mon->msg = msg; + qemuMonitorUpdateWatch(mon); - do { - ret = sendmsg(mon->fd, &msg, 0); - } while (ret < 0 && errno == EINTR); - - return ret == len ? 0 : -1; -} + while (!mon->msg->finished) { + if (virCondWait(&mon->notify, &mon->lock) < 0) + goto cleanup; + } -int qemuMonitorRead(qemuMonitorPtr mon, - char *data, - size_t len) -{ - return read(mon->fd, data, len); -} + if (mon->lastErrno == 0) + ret = 0; -int qemuMonitorWaitForInput(qemuMonitorPtr mon) -{ - struct pollfd fd = { mon->fd, POLLIN | POLLERR | POLLHUP, 0 }; +cleanup: + mon->msg = NULL; + qemuMonitorUpdateWatch(mon); -retry: - if (poll(&fd, 1, -1) < 0) { - if (errno == EINTR) - goto retry; - return -1; - } - return 0; + return ret; } @@ -454,7 +587,6 @@ int qemuMonitorGetDiskSecret(qemuMonitorPtr mon, - int qemuMonitorStartCPUs(qemuMonitorPtr mon, virConnectPtr conn) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 5f06155..46fd40f 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -32,6 +32,33 @@ typedef struct _qemuMonitor qemuMonitor; typedef qemuMonitor *qemuMonitorPtr; +typedef struct _qemuMonitorMessage qemuMonitorMessage; +typedef qemuMonitorMessage *qemuMonitorMessagePtr; + +typedef int (*qemuMonitorPasswordHandler)(qemuMonitorPtr mon, + qemuMonitorMessagePtr msg, + const char *data, + size_t len, + void *opaque); + +struct _qemuMonitorMessage { + int txFD; + + char *txBuffer; + int txOffset; + int txLength; + + char *rxBuffer; + int rxLength; + + int finished; + + int lastErrno; + + qemuMonitorPasswordHandler passwordHandler; + void *passwordOpaque; +}; + typedef void (*qemuMonitorEOFNotify)(qemuMonitorPtr mon, virDomainObjPtr vm, int withError); @@ -49,7 +76,6 @@ typedef int (*qemuMonitorDiskSecretLookup)(qemuMonitorPtr mon, size_t *secretLen); qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm, - int reconnect, qemuMonitorEOFNotify eofCB); void qemuMonitorClose(qemuMonitorPtr mon); @@ -57,21 +83,11 @@ void qemuMonitorClose(qemuMonitorPtr mon); void qemuMonitorRegisterDiskSecretLookup(qemuMonitorPtr mon, qemuMonitorDiskSecretLookup secretCB); -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); +/* This API is for use by the internal Text/JSON monitor impl code only */ +int qemuMonitorSend(qemuMonitorPtr mon, + qemuMonitorMessagePtr msg); +/* XXX same comment about virConnectPtr as above */ int qemuMonitorGetDiskSecret(qemuMonitorPtr mon, virConnectPtr conn, const char *path, diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7ad7f09..db7ff57 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -133,181 +133,163 @@ static char *qemuMonitorEscapeShell(const char *in) return qemuMonitorEscape(in, 1); } -/* Throw away any data available on the monitor - * This is done before executing a command, in order - * to allow re-synchronization if something went badly - * wrong in the past. it also deals with problem of - * QEMU *sometimes* re-printing its initial greeting - * when we reconnect to the monitor after restarts. +/* When connecting to a monitor, QEMU will print a greeting like + * + * QEMU 0.11.0 monitor - type 'help' for more information + * + * Don't expect the version number bit to be stable :-) */ -static void -qemuMonitorDiscardPendingData(qemuMonitorPtr mon) { - char buf[1024]; - int ret = 0; - - /* Monitor is non-blocking, so just loop till we - * get -1 or 0. Don't bother with detecting - * errors, since we'll deal with that better later */ - do { - ret = qemuMonitorRead(mon, buf, sizeof (buf)-1); - } while (ret > 0); -} +#define GREETING_PREFIX "QEMU " +#define GREETING_POSTFIX "type 'help' for more information\r\n(qemu) " +#define BASIC_PROMPT "(qemu) " +#define PASSWORD_PROMPT "Password:" +#define DISK_ENCRYPTION_PREFIX "(" +#define DISK_ENCRYPTION_POSTFIX ") is encrypted." +#define LINE_ENDING "\r\n" + +int qemuMonitorTextIOProcess(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + const char *data, + size_t len, + qemuMonitorMessagePtr msg) +{ + int used = 0; + /* Check for & discard greeting */ + if (STRPREFIX(data, GREETING_PREFIX)) { + const char *offset = strstr(data, GREETING_POSTFIX); -static int -qemuMonitorSend(qemuMonitorPtr mon, - const char *cmd, - int scm_fd) -{ - char *full; - size_t len; - int ret = -1; + /* We see the greeting prefix, but not postfix, so pretend we've + not consumed anything. We'll restart when more data arrives. */ + if (!offset) { + VIR_DEBUG0("Partial greeting seen, getting out & waiting for more"); + return 0; + } - if (virAsprintf(&full, "%s\r", cmd) < 0) - return -1; + used = offset - data + strlen(GREETING_POSTFIX); - len = strlen(full); + VIR_DEBUG0("Discarded monitor greeting"); + } - if (scm_fd == -1) - ret = qemuMonitorWrite(mon, full, len); - else - ret = qemuMonitorWriteWithFD(mon, full, len, scm_fd); + /* Don't print raw data in debug because its full of control chars */ + /*VIR_DEBUG("Process data %d byts of data [%s]", len - used, data + used);*/ + VIR_DEBUG("Process data %d byts of data", len - used); - VIR_FREE(full); - return ret; + /* Look for a non-zero reply followed by prompt */ + if (msg && !msg->finished) { + const char *end; + + /* We might get a prompt for a password */ + end = strstr(data + used, PASSWORD_PROMPT); + if (end) { + VIR_DEBUG("Woooo passwowrd [%s]", data + used); + if (msg->passwordHandler) { + size_t consumed; + /* Try and handle the prompt */ + if (msg->passwordHandler(mon, msg, + data + used, + len - used, + msg->passwordOpaque) < 0) + return -1; + + /* Skip over prompt now */ + consumed = (end + strlen(PASSWORD_PROMPT)) + - (data + used); + used += consumed; + } else { + errno = EACCES; + return -1; + } + } + + /* We use the arrival of BASIC_PROMPT to detect when we've got a + * complete reply available from a command */ + end = strstr(data + used, BASIC_PROMPT); + if (end) { + /* QEMU echos the command back to us, full of control + * character junk that we don't want. Fortunately this + * is all terminated by LINE_ENDING, so we can easily + * skip over the control character junk */ + const char *start = strstr(data + used, LINE_ENDING); + if (!start) + start = data + used; + else + start += strlen(LINE_ENDING); + int want = end - start; + + /* Annoyingly some commands may not have any reply data + * at all upon success, but since we've detected the + * BASIC_PROMPT we can reasonably reliably cope */ + if (want) { + if (VIR_REALLOC_N(msg->rxBuffer, + msg->rxLength + want + 1) < 0) + return -1; + memcpy(msg->rxBuffer + msg->rxLength, start, want); + msg->rxLength += want; + msg->rxBuffer[msg->rxLength] = '\0'; + VIR_DEBUG("Finished %d byte reply [%s]", want, msg->rxBuffer); + } else { + VIR_DEBUG0("Finished 0 byte reply"); + } + msg->finished = 1; + used += end - (data + used); + used += strlen(BASIC_PROMPT); + } + } + + VIR_DEBUG("Total used %d", used); + return used; } static int qemuMonitorCommandWithHandler(qemuMonitorPtr mon, const char *cmd, - const char *extraPrompt, - qemuMonitorExtraPromptHandler extraHandler, - void *handlerData, + qemuMonitorPasswordHandler passwordHandler, + void *passwordOpaque, int scm_fd, char **reply) { - int size = 0; - char *buf = NULL; + int ret; + qemuMonitorMessage msg; - qemuMonitorDiscardPendingData(mon); + *reply = NULL; - VIR_DEBUG("cmd='%s' extraPrompt='%s'", cmd, NULLSTR(extraPrompt)); - if (qemuMonitorSend(mon, cmd, scm_fd) < 0) + memset(&msg, 0, sizeof msg); + + if (virAsprintf(&msg.txBuffer, "%s\r", cmd) < 0) { + virReportOOMError(NULL); return -1; + } + msg.txLength = strlen(msg.txBuffer); + msg.txFD = scm_fd; + msg.passwordHandler = passwordHandler; + msg.passwordOpaque = passwordOpaque; - *reply = NULL; + VIR_DEBUG("Send command '%s' for write with FD %d", cmd, scm_fd); - for (;;) { - /* Read all the data QEMU has sent thus far */ - for (;;) { - char data[1024]; - int got = qemuMonitorRead(mon, data, sizeof(data)); - - if (got == 0) - goto error; - if (got < 0) { - if (errno == EINTR) - continue; - if (errno == EAGAIN) - break; - goto error; - } - if (VIR_REALLOC_N(buf, size+got+1) < 0) - goto error; + ret = qemuMonitorSend(mon, &msg); - memmove(buf+size, data, got); - buf[size+got] = '\0'; - size += got; - } - - /* Look for QEMU prompt to indicate completion */ - if (buf) { - char *foundPrompt; - char *tmp; + VIR_DEBUG("Receive command reply ret=%d errno=%d %d bytes '%s'", + ret, msg.lastErrno, msg.rxLength, msg.rxBuffer); - if (extraPrompt && - (foundPrompt = strstr(buf, extraPrompt)) != NULL) { - char *promptEnd; + /* Just in case buffer had some passwords in */ + memset(msg.txBuffer, 0, msg.txLength); + VIR_FREE(msg.txBuffer); - DEBUG("prompt='%s' handler=%p", extraPrompt, extraHandler); - if (extraHandler(mon, buf, foundPrompt, handlerData) < 0) - return -1; - /* Discard output so far, necessary to detect whether - extraPrompt appears again. We don't need the output between - original command and this prompt anyway. */ - promptEnd = foundPrompt + strlen(extraPrompt); - memmove(buf, promptEnd, strlen(promptEnd)+1); - size -= promptEnd - buf; - } else if ((tmp = strstr(buf, QEMU_CMD_PROMPT)) != NULL) { - char *commptr = NULL, *nlptr = NULL; - /* Preserve the newline */ - tmp[1] = '\0'; - - /* The monitor doesn't dump clean output after we have written to - * it. Every character we write dumps a bunch of useless stuff, - * so the result looks like "cXcoXcomXcommXcommaXcommanXcommand" - * Try to throw away everything before the first full command - * occurence, and inbetween the command and the newline starting - * the response - */ - if ((commptr = strstr(buf, cmd))) { - memmove(buf, commptr, strlen(commptr)+1); - if ((nlptr = strchr(buf, '\n'))) - memmove(buf+strlen(cmd), nlptr, strlen(nlptr)+1); - } - - break; - } + /* To make life safer for callers, already ensure there's at least an empty string */ + if (msg.rxBuffer) { + *reply = msg.rxBuffer; + } else { + *reply = strdup(""); + if (!*reply) { + virReportOOMError(NULL); + return -1; } - - /* Need to wait for more data */ - if (qemuMonitorWaitForInput(mon) < 0) - goto error; } - *reply = buf; - DEBUG("reply='%s'", buf); - return 0; - error: - VIR_FREE(buf); - return -1; -} + if (ret < 0) + virReportSystemError(NULL, msg.lastErrno, + _("cannot send monitor command '%s'"), cmd); -struct extraHandlerData -{ - const char *reply; - bool first; -}; - -static int -qemuMonitorCommandSimpleExtraHandler(qemuMonitorPtr mon, - const char *buf ATTRIBUTE_UNUSED, - const char *prompt ATTRIBUTE_UNUSED, - void *data_) -{ - struct extraHandlerData *data = data_; - - if (!data->first) - return 0; - if (qemuMonitorSend(mon, data->reply, -1) < 0) - return -1; - data->first = false; - return 0; -} - -static int -qemuMonitorCommandExtra(qemuMonitorPtr mon, - const char *cmd, - const char *extra, - const char *extraPrompt, - int scm_fd, - char **reply) { - struct extraHandlerData data; - - data.reply = extra; - data.first = true; - return qemuMonitorCommandWithHandler(mon, cmd, extraPrompt, - qemuMonitorCommandSimpleExtraHandler, - &data, scm_fd, reply); + return ret; } static int @@ -315,7 +297,7 @@ qemuMonitorCommandWithFd(qemuMonitorPtr mon, const char *cmd, int scm_fd, char **reply) { - return qemuMonitorCommandExtra(mon, cmd, NULL, NULL, scm_fd, reply); + return qemuMonitorCommandWithHandler(mon, cmd, NULL, NULL, scm_fd, reply); } static int @@ -327,44 +309,74 @@ qemuMonitorCommand(qemuMonitorPtr mon, static int -qemuMonitorSendVolumePassphrase(qemuMonitorPtr mon, - const char *buf, - const char *prompt, - void *data) +qemuMonitorSendDiskPassphrase(qemuMonitorPtr mon, + qemuMonitorMessagePtr msg, + const char *data, + size_t len ATTRIBUTE_UNUSED, + void *opaque) { - virConnectPtr conn = data; - char *passphrase = NULL, *path; - const char *prompt_path; - size_t path_len, passphrase_len = 0; + virConnectPtr conn = opaque; + char *path; + char *passphrase = NULL; + size_t passphrase_len = 0; int res; + const char *pathStart; + const char *pathEnd; - /* The complete prompt looks like this: - ide0-hd0 (/path/to/volume) is encrypted. - Password: - "prompt" starts with ") is encrypted". Extract /path/to/volume. */ - for (prompt_path = prompt; prompt_path > buf && prompt_path[-1] != '('; - prompt_path--) - ; - if (prompt_path == buf) + /* + * For disk passwords: + * + * ide0-hd0 (/path/to/volume) is encrypted. + * Password: + * + */ + pathStart = strstr(data, DISK_ENCRYPTION_PREFIX); + pathEnd = strstr(data, DISK_ENCRYPTION_POSTFIX); + if (!pathStart || !pathEnd || pathStart >= pathEnd) { + errno = -EINVAL; return -1; - path_len = prompt - prompt_path; - if (VIR_ALLOC_N(path, path_len + 1) < 0) + } + + /* Extra the path */ + pathStart += strlen(DISK_ENCRYPTION_PREFIX); + path = strndup(pathStart, pathEnd - pathStart); + if (!path) { + errno = ENOMEM; return -1; - memcpy(path, prompt_path, path_len); - path[path_len] = '\0'; + } - res = qemuMonitorGetDiskSecret(mon, conn, path, - &passphrase, &passphrase_len); + /* Fetch the disk password if possible */ + res = qemuMonitorGetDiskSecret(mon, + conn, + path, + &passphrase, + &passphrase_len); VIR_FREE(path); if (res < 0) return -1; - res = qemuMonitorSend(mon, passphrase, -1); + /* Enlarge transmit buffer to allow for the extra data + * to be sent back */ + if (VIR_REALLOC_N(msg->txBuffer, + msg->txLength + passphrase_len + 1 + 1) < 0) { + memset(passphrase, 0, passphrase_len); + VIR_FREE(passphrase); + errno = ENOMEM; + return -1; + } + + /* Queue the password for sending */ + memcpy(msg->txBuffer + msg->txLength, + passphrase, passphrase_len); + msg->txLength += passphrase_len; + msg->txBuffer[msg->txLength] = '\r'; + msg->txLength++; + msg->txBuffer[msg->txLength] = '\0'; memset(passphrase, 0, passphrase_len); VIR_FREE(passphrase); - return res; + return 0; } int @@ -372,8 +384,9 @@ qemuMonitorTextStartCPUs(qemuMonitorPtr mon, virConnectPtr conn) { char *reply; - if (qemuMonitorCommandWithHandler(mon, "cont", ") is encrypted.", - qemuMonitorSendVolumePassphrase, conn, + if (qemuMonitorCommandWithHandler(mon, "cont", + qemuMonitorSendDiskPassphrase, + conn, -1, &reply) < 0) return -1; @@ -637,15 +650,44 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, } +static int +qemuMonitorSendVNCPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + qemuMonitorMessagePtr msg, + const char *data ATTRIBUTE_UNUSED, + size_t len ATTRIBUTE_UNUSED, + void *opaque) +{ + char *passphrase = opaque; + size_t passphrase_len = strlen(passphrase); + + /* Enlarge transmit buffer to allow for the extra data + * to be sent back */ + if (VIR_REALLOC_N(msg->txBuffer, + msg->txLength + passphrase_len + 1 + 1) < 0) { + errno = ENOMEM; + return -1; + } + + /* Queue the password for sending */ + memcpy(msg->txBuffer + msg->txLength, + passphrase, passphrase_len); + msg->txLength += passphrase_len; + msg->txBuffer[msg->txLength] = '\r'; + msg->txLength++; + msg->txBuffer[msg->txLength] = '\0'; + + return 0; +} + int qemuMonitorTextSetVNCPassword(qemuMonitorPtr mon, const char *password) { char *info = NULL; - if (qemuMonitorCommandExtra(mon, "change vnc password", - password, - QEMU_PASSWD_PROMPT, - -1, &info) < 0) { + if (qemuMonitorCommandWithHandler(mon, "change vnc password", + qemuMonitorSendVNCPassphrase, + (char *)password, + -1, &info) < 0) { qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("setting VNC password failed")); return -1; diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index 35eaf05..6bca07a 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -29,6 +29,11 @@ #include "qemu_monitor.h" +int qemuMonitorTextIOProcess(qemuMonitorPtr mon, + const char *data, + size_t len, + qemuMonitorMessagePtr msg); + int qemuMonitorTextStartCPUs(qemuMonitorPtr mon, virConnectPtr conn); int qemuMonitorTextStopCPUs(qemuMonitorPtr mon); -- 1.6.2.5 -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list