[libvirt] [PATCH 0/4] Add support for QEMU guest agent control

==== These patches are taken from here: https://www.redhat.com/archives/libvir-list/2011-October/msg00135.html I've just rebased and polished them. ==== The QEMU guest agent "/usr/bin/qemu-ga" has some handy functions for controlling the guest, not least, shutdown/reboot and filesystem freeze/thaw. In Fedora 15/16 the semantics of the ACPI power button have been changed to suspend-to-RAM which breaks our current shutdown implementation. By adding support for the agent we gain a more predictable way to shutdown / reboot guests. NB: the code currently has the same "flaw" as the monitor, in so much as we wait forever for a guest agent reply. We need to add a timeout ability to the agent code Daniel P. Berrange (4): QEMU guest agent support Add new virDomainShutdownFlags API Wire up QEMU agent to reboot/shutdown APIs Allow choice of shutdown method via virsh include/libvirt/libvirt.h.in | 15 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/driver.h | 5 + src/esx/esx_driver.c | 11 +- src/libvirt.c | 65 +++- src/libvirt_public.syms | 4 + src/libxl/libxl_driver.c | 12 +- src/openvz/openvz_driver.c | 1 + src/qemu/qemu_agent.c | 1151 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 69 +++ src/qemu/qemu_domain.c | 97 ++++ src/qemu/qemu_domain.h | 22 + src/qemu/qemu_driver.c | 107 ++++- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_process.c | 188 +++++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 8 +- src/remote_protocol-structs | 5 + src/test/test_driver.c | 11 +- src/uml/uml_driver.c | 11 +- src/vbox/vbox_tmpl.c | 11 +- src/vmware/vmware_driver.c | 1 + src/xen/xen_driver.c | 14 +- src/xenapi/xenapi_driver.c | 12 +- tools/virsh.c | 47 ++- tools/virsh.pod | 24 +- 27 files changed, 1854 insertions(+), 42 deletions(-) create mode 100644 src/qemu/qemu_agent.c create mode 100644 src/qemu/qemu_agent.h -- 1.7.3.4

There is now a standard QEMU guest agent that can be installed and given a virtio serial channel <channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/> <target type='virtio' name='org.qemu.guest_agent.0'/> </channel> The protocol that runs over the guest agent is JSON based and very similar to the JSON monitor. We can't use exactly the same code because there are some odd differences in the way messages and errors are structured. The qemu_agent.c file is based on a combination and simplification of qemu_monitor.c and qemu_monitor_json.c * src/qemu/qemu_agent.c, src/qemu/qemu_agent.h: Support for talking to the agent for shutdown * src/qemu/qemu_domain.c, src/qemu/qemu_domain.h: Add thread helpers for talking to the agent * src/qemu/qemu_process.c: Connect to agent whenever starting a guest * src/qemu/qemu_monitor_json.c: Make variable static --- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/qemu/qemu_agent.c | 1151 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 69 +++ src/qemu/qemu_domain.c | 97 ++++ src/qemu/qemu_domain.h | 22 + src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_process.c | 188 +++++++ 8 files changed, 1530 insertions(+), 1 deletions(-) create mode 100644 src/qemu/qemu_agent.c create mode 100644 src/qemu/qemu_agent.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 3e8359a..418d48a 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -57,6 +57,7 @@ src/nwfilter/nwfilter_learnipaddr.c src/openvz/openvz_conf.c src/openvz/openvz_driver.c src/phyp/phyp_driver.c +src/qemu/qemu_agent.c src/qemu/qemu_bridge_filter.c src/qemu/qemu_capabilities.c src/qemu/qemu_cgroup.c diff --git a/src/Makefile.am b/src/Makefile.am index 0a1221a..a18bcde 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -363,6 +363,7 @@ VBOX_DRIVER_EXTRA_DIST = \ vbox/vbox_XPCOMCGlue.c vbox/vbox_XPCOMCGlue.h QEMU_DRIVER_SOURCES = \ + qemu/qemu_agent.c qemu/qemu_agent.h \ qemu/qemu_capabilities.c qemu/qemu_capabilities.h\ qemu/qemu_command.c qemu/qemu_command.h \ qemu/qemu_domain.c qemu/qemu_domain.h \ diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c new file mode 100644 index 0000000..9f31367 --- /dev/null +++ b/src/qemu/qemu_agent.c @@ -0,0 +1,1151 @@ +/* + * qemu_agent.h: interaction with QEMU guest agent + * + * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> + +#include <sys/types.h> +#include <sys/socket.h> +#include <sys/un.h> +#include <poll.h> +#include <unistd.h> +#include <fcntl.h> +#include <string.h> +#include <sys/time.h> + +#include "qemu_agent.h" +#include "qemu_command.h" +#include "memory.h" +#include "logging.h" +#include "virterror_internal.h" +#include "json.h" +#include "virfile.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +#define LINE_ENDING "\n" + +#define DEBUG_IO 0 +#define DEBUG_RAW_IO 0 + +/* When you are the first to uncomment this, + * don't forget to uncomment the corresponding + * part in qemuAgentIOProcessEvent as well. + * +static struct { + const char *type; + void (*handler)(qemuAgentPtr mon, virJSONValuePtr data); +} eventHandlers[] = { +}; +*/ + +typedef struct _qemuAgentMessage qemuAgentMessage; +typedef qemuAgentMessage *qemuAgentMessagePtr; + +struct _qemuAgentMessage { + char *txBuffer; + int txOffset; + int txLength; + + /* Used by the text monitor reply / error */ + char *rxBuffer; + int rxLength; + /* Used by the JSON monitor to hold reply / error */ + void *rxObject; + + /* True if rxBuffer / rxObject are ready, or a + * fatal error occurred on the monitor channel + */ + bool finished; +}; + + +struct _qemuAgent { + virMutex lock; /* also used to protect fd */ + virCond notify; + + int refs; + + int fd; + int watch; + + bool connectPending; + + virDomainObjPtr vm; + + qemuAgentCallbacksPtr cb; + + /* If there's a command being processed this will be + * non-NULL */ + qemuAgentMessagePtr 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 */ + virError lastError; +}; + +#if DEBUG_RAW_IO +# include <c-ctype.h> +static char * qemuAgentEscapeNonPrintable(const char *text) +{ + int i; + virBuffer buf = VIR_BUFFER_INITIALIZER; + for (i = 0 ; text[i] != '\0' ; i++) { + if (c_isprint(text[i]) || + text[i] == '\n' || + (text[i] == '\r' && text[i+1] == '\n')) + virBufferAsprintf(&buf,"%c", text[i]); + else + virBufferAsprintf(&buf, "0x%02x", text[i]); + } + return virBufferContentAndReset(&buf); +} +#endif + +void qemuAgentLock(qemuAgentPtr mon) +{ + virMutexLock(&mon->lock); +} + + +void qemuAgentUnlock(qemuAgentPtr mon) +{ + virMutexUnlock(&mon->lock); +} + + +static void qemuAgentFree(qemuAgentPtr mon) +{ + VIR_DEBUG("mon=%p", mon); + if (mon->cb && mon->cb->destroy) + (mon->cb->destroy)(mon, mon->vm); + ignore_value(virCondDestroy(&mon->notify)); + virMutexDestroy(&mon->lock); + VIR_FREE(mon->buffer); + VIR_FREE(mon); +} + +int qemuAgentRef(qemuAgentPtr mon) +{ + mon->refs++; + return mon->refs; +} + +int qemuAgentUnref(qemuAgentPtr mon) +{ + mon->refs--; + + if (mon->refs == 0) { + qemuAgentUnlock(mon); + qemuAgentFree(mon); + return 0; + } + + return mon->refs; +} + +static void +qemuAgentUnwatch(void *monitor) +{ + qemuAgentPtr mon = monitor; + + qemuAgentLock(mon); + if (qemuAgentUnref(mon) > 0) + qemuAgentUnlock(mon); +} + +static int +qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool *inProgress) +{ + struct sockaddr_un addr; + int monfd; + int timeout = 3; /* In seconds */ + int ret, i = 0; + + *inProgress = false; + + if ((monfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { + virReportSystemError(errno, + "%s", _("failed to create socket")); + return -1; + } + + if (virSetNonBlock(monfd) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to put monitor into non-blocking mode")); + goto error; + } + + if (virSetCloseExec(monfd) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to set monitor close-on-exec flag")); + goto error; + } + + memset(&addr, 0, sizeof(addr)); + addr.sun_family = AF_UNIX; + if (virStrcpyStatic(addr.sun_path, monitor) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Agent path %s too big for destination"), monitor); + goto error; + } + + do { + ret = connect(monfd, (struct sockaddr *) &addr, sizeof(addr)); + + if (ret == 0) + break; + + if ((errno == ENOENT || errno == ECONNREFUSED) && + virKillProcess(cpid, 0) == 0) { + /* ENOENT : Socket may not have shown up yet + * ECONNREFUSED : Leftover socket hasn't been removed yet */ + continue; + } + + if ((errno == EINPROGRESS) || + (errno == EAGAIN)) { + VIR_DEBUG("Connection attempt continuing in background"); + *inProgress = true; + ret = 0; + break; + } + + virReportSystemError(errno, "%s", + _("failed to connect to monitor socket")); + goto error; + + } while ((++i <= timeout*5) && (usleep(.2 * 1000000) <= 0)); + + if (ret != 0) { + virReportSystemError(errno, "%s", + _("monitor socket did not show up.")); + goto error; + } + + return monfd; + +error: + VIR_FORCE_CLOSE(monfd); + return -1; +} + +static int +qemuAgentOpenPty(const char *monitor) +{ + int monfd; + + if ((monfd = open(monitor, O_RDWR)) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to open monitor path %s"), monitor); + return -1; + } + + if (virSetNonBlock(monfd) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to put monitor into non-blocking mode")); + goto error; + } + + if (virSetCloseExec(monfd) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to set monitor close-on-exec flag")); + goto error; + } + + return monfd; + +error: + VIR_FORCE_CLOSE(monfd); + return -1; +} + + +static int +qemuAgentIOProcessEvent(qemuAgentPtr mon, + virJSONValuePtr obj) +{ + const char *type; + VIR_DEBUG("mon=%p obj=%p", mon, obj); + + type = virJSONValueObjectGetString(obj, "event"); + if (!type) { + VIR_WARN("missing event type in message"); + errno = EINVAL; + return -1; + } + +/* + for (i = 0 ; i < ARRAY_CARDINALITY(eventHandlers) ; i++) { + if (STREQ(eventHandlers[i].type, type)) { + virJSONValuePtr data = virJSONValueObjectGet(obj, "data"); + VIR_DEBUG("handle %s handler=%p data=%p", type, + eventHandlers[i].handler, data); + (eventHandlers[i].handler)(mon, data); + break; + } + } +*/ + return 0; +} + +static int +qemuAgentIOProcessLine(qemuAgentPtr mon, + const char *line, + qemuAgentMessagePtr msg) +{ + virJSONValuePtr obj = NULL; + int ret = -1; + + VIR_DEBUG("Line [%s]", line); + + if (!(obj = virJSONValueFromString(line))) + goto cleanup; + + if (obj->type != VIR_JSON_TYPE_OBJECT) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Parsed JSON reply '%s' isn't an object"), line); + goto cleanup; + } + + if (virJSONValueObjectHasKey(obj, "QMP") == 1) { + ret = 0; + } else if (virJSONValueObjectHasKey(obj, "event") == 1) { + ret = qemuAgentIOProcessEvent(mon, obj); + } else if (virJSONValueObjectHasKey(obj, "error") == 1 || + virJSONValueObjectHasKey(obj, "return") == 1) { + if (msg) { + msg->rxObject = obj; + msg->finished = 1; + obj = NULL; + ret = 0; + } else { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected JSON reply '%s'"), line); + } + } else { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown JSON reply '%s'"), line); + } + +cleanup: + virJSONValueFree(obj); + return ret; +} + +static int qemuAgentIOProcessData(qemuAgentPtr mon, + const char *data, + size_t len, + qemuAgentMessagePtr msg) +{ + int used = 0; +#if DEBUG_IO +# if DEBUG_RAW_IO + char *str1 = qemuAgentEscapeNonPrintable(data); + VIR_ERROR("[%s]", str1); + VIR_FREE(str1); +# else + VIR_DEBUG("Data %zu bytes [%s]", len, data); +# endif +#endif + + while (used < len) { + char *nl = strstr(data + used, LINE_ENDING); + + if (nl) { + int got = nl - (data + used); + char *line = strndup(data + used, got); + if (!line) { + virReportOOMError(); + return -1; + } + used += got + strlen(LINE_ENDING); + line[got] = '\0'; /* kill \n */ + if (qemuAgentIOProcessLine(mon, line, msg) < 0) { + VIR_FREE(line); + return -1; + } + + VIR_FREE(line); + } else { + break; + } + } + + VIR_DEBUG("Total used %d bytes out of %zd available in buffer", used, len); + return used; +} + +/* This method processes data that has been received + * from the monitor. Looking for async events and + * replies/errors. + */ +static int +qemuAgentIOProcess(qemuAgentPtr mon) +{ + int len; + qemuAgentMessagePtr msg = NULL; + + /* See if there's a message ready for reply; that is, + * one that has completed writing all its data. + */ + if (mon->msg && mon->msg->txOffset == mon->msg->txLength) + msg = mon->msg; + +#if DEBUG_IO +# if DEBUG_RAW_IO + char *str1 = qemuAgentEscapeNonPrintable(msg ? msg->txBuffer : ""); + char *str2 = qemuAgentEscapeNonPrintable(mon->buffer); + VIR_ERROR(_("Process %d %p %p [[[%s]]][[[%s]]]"), + (int)mon->bufferOffset, mon->msg, msg, str1, str2); + VIR_FREE(str1); + VIR_FREE(str2); +# else + VIR_DEBUG("Process %d", (int)mon->bufferOffset); +# endif +#endif + + len = qemuAgentIOProcessData(mon, + mon->buffer, mon->bufferOffset, + msg); + + if (len < 0) + 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; + } +#if DEBUG_IO + VIR_DEBUG("Process done %d used %d", (int)mon->bufferOffset, len); +#endif + if (msg && msg->finished) + virCondBroadcast(&mon->notify); + return len; +} + + +static int +qemuAgentIOConnect(qemuAgentPtr mon) +{ + int optval; + socklen_t optlen; + + VIR_DEBUG("Checking on background connection status"); + + mon->connectPending = false; + + optlen = sizeof(optval); + + if (getsockopt(mon->fd, SOL_SOCKET, SO_ERROR, + &optval, &optlen) < 0) { + virReportSystemError(errno, "%s", + _("Cannot check socket connection status")); + return -1; + } + + if (optval != 0) { + virReportSystemError(optval, "%s", + _("Cannot connect to agent socket")); + return -1; + } + + VIR_DEBUG("Agent is now connected"); + return 0; +} + +/* + * Called when the monitor is able to write data + * Call this function while holding the monitor lock. + */ +static int +qemuAgentIOWrite(qemuAgentPtr mon) +{ + int done; + + /* If no active message, or fully transmitted, then no-op */ + if (!mon->msg || mon->msg->txOffset == mon->msg->txLength) + return 0; + + done = safewrite(mon->fd, + mon->msg->txBuffer + mon->msg->txOffset, + mon->msg->txLength - mon->msg->txOffset); + + if (done < 0) { + if (errno == EAGAIN) + return 0; + + virReportSystemError(errno, "%s", + _("Unable to write to monitor")); + return -1; + } + mon->msg->txOffset += done; + return done; +} + +/* + * Called when the monitor has incoming data to read + * Call this function while holding the monitor lock. + * + * Returns -1 on error, or number of bytes read + */ +static int +qemuAgentIORead(qemuAgentPtr mon) +{ + size_t avail = mon->bufferLength - mon->bufferOffset; + int ret = 0; + + if (avail < 1024) { + if (VIR_REALLOC_N(mon->buffer, + mon->bufferLength + 1024) < 0) { + virReportOOMError(); + 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; + virReportSystemError(errno, "%s", + _("Unable to read from monitor")); + ret = -1; + break; + } + if (got == 0) + break; + + ret += got; + avail -= got; + mon->bufferOffset += got; + mon->buffer[mon->bufferOffset] = '\0'; + } + +#if DEBUG_IO + VIR_DEBUG("Now read %d bytes of data", (int)mon->bufferOffset); +#endif + + return ret; +} + + +static void qemuAgentUpdateWatch(qemuAgentPtr mon) +{ + int events = + VIR_EVENT_HANDLE_HANGUP | + VIR_EVENT_HANDLE_ERROR; + + if (mon->lastError.code == VIR_ERR_OK) { + events |= VIR_EVENT_HANDLE_READABLE; + + if (mon->msg && mon->msg->txOffset < mon->msg->txLength) + events |= VIR_EVENT_HANDLE_WRITABLE; + } + + virEventUpdateHandle(mon->watch, events); +} + + +static void +qemuAgentIO(int watch, int fd, int events, void *opaque) { + qemuAgentPtr mon = opaque; + bool error = false; + bool eof = false; + + /* lock access to the monitor and protect fd */ + qemuAgentLock(mon); + qemuAgentRef(mon); +#if DEBUG_IO + VIR_DEBUG("Agent %p I/O on watch %d fd %d events %d", mon, watch, fd, events); +#endif + + if (mon->fd != fd || mon->watch != watch) { + if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) + eof = true; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("event from unexpected fd %d!=%d / watch %d!=%d"), + mon->fd, fd, mon->watch, watch); + error = true; + } else if (mon->lastError.code != VIR_ERR_OK) { + if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) + eof = true; + error = true; + } else { + if (events & VIR_EVENT_HANDLE_WRITABLE) { + if (mon->connectPending) { + if (qemuAgentIOConnect(mon) < 0) + error = true; + } else { + if (qemuAgentIOWrite(mon) < 0) + error = true; + } + events &= ~VIR_EVENT_HANDLE_WRITABLE; + } + + if (!error && + events & VIR_EVENT_HANDLE_READABLE) { + int got = qemuAgentIORead(mon); + events &= ~VIR_EVENT_HANDLE_READABLE; + if (got < 0) { + error = true; + } else if (got == 0) { + eof = true; + } else { + /* Ignore hangup/error events if we read some data, to + * give time for that data to be consumed */ + events = 0; + + if (qemuAgentIOProcess(mon) < 0) + error = true; + } + } + + if (!error && + events & VIR_EVENT_HANDLE_HANGUP) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("End of file from monitor")); + eof = 1; + events &= ~VIR_EVENT_HANDLE_HANGUP; + } + + if (!error && !eof && + events & VIR_EVENT_HANDLE_ERROR) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid file descriptor while waiting for monitor")); + eof = 1; + events &= ~VIR_EVENT_HANDLE_ERROR; + } + if (!error && events) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unhandled event %d for monitor fd %d"), + events, mon->fd); + error = 1; + } + } + + if (error || eof) { + if (mon->lastError.code != VIR_ERR_OK) { + /* Already have an error, so clear any new error */ + virResetLastError(); + } else { + virErrorPtr err = virGetLastError(); + if (!err) + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Error while processing monitor IO")); + virCopyLastError(&mon->lastError); + virResetLastError(); + } + + VIR_DEBUG("Error on monitor %s", NULLSTR(mon->lastError.message)); + /* If IO process resulted in an error & we have a message, + * then wakeup that waiter */ + if (mon->msg && !mon->msg->finished) { + mon->msg->finished = 1; + virCondSignal(&mon->notify); + } + } + + qemuAgentUpdateWatch(mon); + + /* 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 (eof) { + void (*eofNotify)(qemuAgentPtr, virDomainObjPtr) + = mon->cb->eofNotify; + virDomainObjPtr vm = mon->vm; + + /* Make sure anyone waiting wakes up now */ + virCondSignal(&mon->notify); + if (qemuAgentUnref(mon) > 0) + qemuAgentUnlock(mon); + VIR_DEBUG("Triggering EOF callback"); + (eofNotify)(mon, vm); + } else if (error) { + void (*errorNotify)(qemuAgentPtr, virDomainObjPtr) + = mon->cb->errorNotify; + virDomainObjPtr vm = mon->vm; + + /* Make sure anyone waiting wakes up now */ + virCondSignal(&mon->notify); + if (qemuAgentUnref(mon) > 0) + qemuAgentUnlock(mon); + VIR_DEBUG("Triggering error callback"); + (errorNotify)(mon, vm); + } else { + if (qemuAgentUnref(mon) > 0) + qemuAgentUnlock(mon); + } +} + + +qemuAgentPtr +qemuAgentOpen(virDomainObjPtr vm, + virDomainChrSourceDefPtr config, + qemuAgentCallbacksPtr cb) +{ + qemuAgentPtr mon; + + if (!cb || !cb->eofNotify) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("EOF notify callback must be supplied")); + return NULL; + } + + if (VIR_ALLOC(mon) < 0) { + virReportOOMError(); + return NULL; + } + + if (virMutexInit(&mon->lock) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot initialize monitor mutex")); + VIR_FREE(mon); + return NULL; + } + if (virCondInit(&mon->notify) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot initialize monitor condition")); + virMutexDestroy(&mon->lock); + VIR_FREE(mon); + return NULL; + } + mon->fd = -1; + mon->refs = 1; + mon->vm = vm; + mon->cb = cb; + qemuAgentLock(mon); + + switch (config->type) { + case VIR_DOMAIN_CHR_TYPE_UNIX: + mon->fd = qemuAgentOpenUnix(config->data.nix.path, vm->pid, + &mon->connectPending); + break; + + case VIR_DOMAIN_CHR_TYPE_PTY: + mon->fd = qemuAgentOpenPty(config->data.file.path); + break; + + default: + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to handle monitor type: %s"), + virDomainChrTypeToString(config->type)); + goto cleanup; + } + + if (mon->fd == -1) goto cleanup; + + if ((mon->watch = virEventAddHandle(mon->fd, + VIR_EVENT_HANDLE_HANGUP | + VIR_EVENT_HANDLE_ERROR | + VIR_EVENT_HANDLE_READABLE | + (mon->connectPending ? + VIR_EVENT_HANDLE_WRITABLE : + 0), + qemuAgentIO, + mon, qemuAgentUnwatch)) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to register monitor events")); + goto cleanup; + } + qemuAgentRef(mon); + + VIR_DEBUG("New mon %p fd =%d watch=%d", mon, mon->fd, mon->watch); + qemuAgentUnlock(mon); + + return mon; + +cleanup: + /* We don't want the 'destroy' callback invoked during + * cleanup from construction failure, because that can + * give a double-unref on virDomainObjPtr in the caller, + * so kill the callbacks now. + */ + mon->cb = NULL; + qemuAgentUnlock(mon); + qemuAgentClose(mon); + return NULL; +} + + +void qemuAgentClose(qemuAgentPtr mon) +{ + if (!mon) + return; + + VIR_DEBUG("mon=%p", mon); + + qemuAgentLock(mon); + + if (mon->fd >= 0) { + if (mon->watch) + virEventRemoveHandle(mon->watch); + VIR_FORCE_CLOSE(mon->fd); + } + + if (qemuAgentUnref(mon) > 0) + qemuAgentUnlock(mon); +} + + +static int qemuAgentSend(qemuAgentPtr mon, + qemuAgentMessagePtr msg) +{ + int ret = -1; + + /* Check whether qemu quit unexpectedly */ + if (mon->lastError.code != VIR_ERR_OK) { + VIR_DEBUG("Attempt to send command while error is set %s", + NULLSTR(mon->lastError.message)); + virSetError(&mon->lastError); + return -1; + } + + mon->msg = msg; + qemuAgentUpdateWatch(mon); + + while (!mon->msg->finished) { + if (virCondWait(&mon->notify, &mon->lock) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to wait on monitor condition")); + goto cleanup; + } + } + + if (mon->lastError.code != VIR_ERR_OK) { + VIR_DEBUG("Send command resulted in error %s", + NULLSTR(mon->lastError.message)); + virSetError(&mon->lastError); + goto cleanup; + } + + ret = 0; + +cleanup: + mon->msg = NULL; + qemuAgentUpdateWatch(mon); + + return ret; +} + + +static int +qemuAgentCommand(qemuAgentPtr mon, + virJSONValuePtr cmd, + virJSONValuePtr *reply) +{ + int ret = -1; + qemuAgentMessage msg; + char *cmdstr = NULL; + + *reply = NULL; + + memset(&msg, 0, sizeof msg); + + if (!(cmdstr = virJSONValueToString(cmd))) { + virReportOOMError(); + goto cleanup; + } + if (virAsprintf(&msg.txBuffer, "%s" LINE_ENDING, cmdstr) < 0) { + virReportOOMError(); + goto cleanup; + } + msg.txLength = strlen(msg.txBuffer); + + VIR_DEBUG("Send command '%s' for write", cmdstr); + + ret = qemuAgentSend(mon, &msg); + + VIR_DEBUG("Receive command reply ret=%d rxObject=%p", + ret, msg.rxObject); + + + if (ret == 0) { + if (!msg.rxObject) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing monitor reply object")); + ret = -1; + } else { + *reply = msg.rxObject; + } + } + +cleanup: + VIR_FREE(cmdstr); + VIR_FREE(msg.txBuffer); + + return ret; +} + + +/* Ignoring OOM in this method, since we're already reporting + * a more important error + * + * XXX see qerror.h for different klasses & fill out useful params + */ +static const char * +qemuAgentStringifyError(virJSONValuePtr error) +{ + const char *klass = virJSONValueObjectGetString(error, "class"); + const char *detail = NULL; + + /* The QMP 'desc' field is usually sufficient for our generic + * error reporting needs. + */ + if (klass) + detail = virJSONValueObjectGetString(error, "desc"); + + + if (!detail) + detail = "unknown QEMU command error"; + + return detail; +} + +static const char * +qemuAgentCommandName(virJSONValuePtr cmd) +{ + const char *name = virJSONValueObjectGetString(cmd, "execute"); + if (name) + return name; + else + return "<unknown>"; +} + +static int +qemuAgentCheckError(virJSONValuePtr cmd, + virJSONValuePtr reply) +{ + if (virJSONValueObjectHasKey(reply, "error")) { + virJSONValuePtr error = virJSONValueObjectGet(reply, "error"); + char *cmdstr = virJSONValueToString(cmd); + char *replystr = virJSONValueToString(reply); + + /* Log the full JSON formatted command & error */ + VIR_DEBUG("unable to execute QEMU command %s: %s", + cmdstr, replystr); + + /* Only send the user the command name + friendly error */ + if (!error) + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to execute QEMU command '%s'"), + qemuAgentCommandName(cmd)); + else + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to execute QEMU command '%s': %s"), + qemuAgentCommandName(cmd), + qemuAgentStringifyError(error)); + + VIR_FREE(cmdstr); + VIR_FREE(replystr); + return -1; + } else if (!virJSONValueObjectHasKey(reply, "return")) { + char *cmdstr = virJSONValueToString(cmd); + char *replystr = virJSONValueToString(reply); + + VIR_DEBUG("Neither 'return' nor 'error' is set in the JSON reply %s: %s", + cmdstr, replystr); + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to execute QEMU command '%s'"), + qemuAgentCommandName(cmd)); + VIR_FREE(cmdstr); + VIR_FREE(replystr); + return -1; + } + return 0; +} + + +#if 0 +static int +qemuAgentHasError(virJSONValuePtr reply, + const char *klass) +{ + virJSONValuePtr error; + const char *thisklass; + + if (!virJSONValueObjectHasKey(reply, "error")) + return 0; + + error = virJSONValueObjectGet(reply, "error"); + if (!error) + return 0; + + if (!virJSONValueObjectHasKey(error, "class")) + return 0; + + thisklass = virJSONValueObjectGetString(error, "class"); + + if (!thisklass) + return 0; + + return STREQ(klass, thisklass); +} +#endif + + +static virJSONValuePtr ATTRIBUTE_SENTINEL +qemuAgentMakeCommand(const char *cmdname, + ...) +{ + virJSONValuePtr obj; + virJSONValuePtr jargs = NULL; + va_list args; + char *key; + + va_start(args, cmdname); + + if (!(obj = virJSONValueNewObject())) + goto no_memory; + + if (virJSONValueObjectAppendString(obj, "execute", cmdname) < 0) + goto no_memory; + + while ((key = va_arg(args, char *)) != NULL) { + int ret; + char type; + + if (strlen(key) < 3) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("argument key '%s' is too short, missing type prefix"), + key); + goto error; + } + + /* Keys look like s:name the first letter is a type code */ + type = key[0]; + key += 2; + + if (!jargs && + !(jargs = virJSONValueNewObject())) + goto no_memory; + + /* This doesn't support maps/arrays. This hasn't + * proved to be a problem..... yet :-) */ + switch (type) { + case 's': { + char *val = va_arg(args, char *); + ret = virJSONValueObjectAppendString(jargs, key, val); + } break; + case 'i': { + int val = va_arg(args, int); + ret = virJSONValueObjectAppendNumberInt(jargs, key, val); + } break; + case 'u': { + unsigned int val = va_arg(args, unsigned int); + ret = virJSONValueObjectAppendNumberUint(jargs, key, val); + } break; + case 'I': { + long long val = va_arg(args, long long); + ret = virJSONValueObjectAppendNumberLong(jargs, key, val); + } break; + case 'U': { + /* qemu silently truncates numbers larger than LLONG_MAX, + * so passing the full range of unsigned 64 bit integers + * is not safe here. Pass them as signed 64 bit integers + * instead. + */ + long long val = va_arg(args, long long); + ret = virJSONValueObjectAppendNumberLong(jargs, key, val); + } break; + case 'd': { + double val = va_arg(args, double); + ret = virJSONValueObjectAppendNumberDouble(jargs, key, val); + } break; + case 'b': { + int val = va_arg(args, int); + ret = virJSONValueObjectAppendBoolean(jargs, key, val); + } break; + case 'n': { + ret = virJSONValueObjectAppendNull(jargs, key); + } break; + default: + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported data type '%c' for arg '%s'"), type, key - 2); + goto error; + } + if (ret < 0) + goto no_memory; + } + + if (jargs && + virJSONValueObjectAppend(obj, "arguments", jargs) < 0) + goto no_memory; + + va_end(args); + + return obj; + +no_memory: + virReportOOMError(); +error: + virJSONValueFree(obj); + virJSONValueFree(jargs); + va_end(args); + return NULL; +} + +VIR_ENUM_DECL(qemuAgentShutdownMode); + +VIR_ENUM_IMPL(qemuAgentShutdownMode, + QEMU_AGENT_SHUTDOWN_LAST, + "powerdown", "reboot", "halt"); + +int qemuAgentShutdown(qemuAgentPtr mon, + qemuAgentShutdownMode mode) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuAgentMakeCommand("guest-shutdown", + "s:mode", qemuAgentShutdownModeTypeToString(mode), + NULL); + if (!cmd) + return -1; + + ret = qemuAgentCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuAgentCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h new file mode 100644 index 0000000..93c2ae7 --- /dev/null +++ b/src/qemu/qemu_agent.h @@ -0,0 +1,69 @@ +/* + * qemu_agent.h: interaction with QEMU guest agent + * + * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + + +#ifndef __QEMU_AGENT_H__ +# define __QEMU_AGENT_H__ + +# include "internal.h" +# include "domain_conf.h" + +typedef struct _qemuAgent qemuAgent; +typedef qemuAgent *qemuAgentPtr; + +typedef struct _qemuAgentCallbacks qemuAgentCallbacks; +typedef qemuAgentCallbacks *qemuAgentCallbacksPtr; +struct _qemuAgentCallbacks { + void (*destroy)(qemuAgentPtr mon, + virDomainObjPtr vm); + void (*eofNotify)(qemuAgentPtr mon, + virDomainObjPtr vm); + void (*errorNotify)(qemuAgentPtr mon, + virDomainObjPtr vm); +}; + + +qemuAgentPtr qemuAgentOpen(virDomainObjPtr vm, + virDomainChrSourceDefPtr config, + qemuAgentCallbacksPtr cb); + +void qemuAgentLock(qemuAgentPtr mon); +void qemuAgentUnlock(qemuAgentPtr mon); + +int qemuAgentRef(qemuAgentPtr mon); +int qemuAgentUnref(qemuAgentPtr mon) ATTRIBUTE_RETURN_CHECK; + +void qemuAgentClose(qemuAgentPtr mon); + +typedef enum { + QEMU_AGENT_SHUTDOWN_POWERDOWN, + QEMU_AGENT_SHUTDOWN_REBOOT, + QEMU_AGENT_SHUTDOWN_HALT, + + QEMU_AGENT_SHUTDOWN_LAST, +} qemuAgentShutdownMode; + +int qemuAgentShutdown(qemuAgentPtr mon, + qemuAgentShutdownMode mode); + +#endif /* __QEMU_AGENT_H__ */ diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6b03c62..fac5fe8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -219,6 +219,10 @@ static void qemuDomainObjPrivateFree(void *data) VIR_ERROR(_("Unexpected QEMU monitor still active during domain deletion")); qemuMonitorClose(priv->mon); } + if (priv->agent) { + VIR_ERROR(_("Unexpected QEMU agent still active during domain deletion")); + qemuAgentClose(priv->agent); + } VIR_FREE(priv); } @@ -1025,6 +1029,99 @@ void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, qemuDomainObjExitMonitorInternal(driver, true, obj); } + + +static int +qemuDomainObjEnterAgentInternal(struct qemud_driver *driver, + bool driver_locked, + virDomainObjPtr obj) +{ + qemuDomainObjPrivatePtr priv = obj->privateData; + + qemuAgentLock(priv->agent); + qemuAgentRef(priv->agent); + ignore_value(virTimeMillisNow(&priv->agentStart)); + virDomainObjUnlock(obj); + if (driver_locked) + qemuDriverUnlock(driver); + + return 0; +} + +static void ATTRIBUTE_NONNULL(1) +qemuDomainObjExitAgentInternal(struct qemud_driver *driver, + bool driver_locked, + virDomainObjPtr obj) +{ + qemuDomainObjPrivatePtr priv = obj->privateData; + int refs; + + refs = qemuAgentUnref(priv->agent); + + if (refs > 0) + qemuAgentUnlock(priv->agent); + + if (driver_locked) + qemuDriverLock(driver); + virDomainObjLock(obj); + + priv->agentStart = 0; + if (refs == 0) { + priv->agent = NULL; + } +} + +/* + * obj must be locked before calling, qemud_driver must be unlocked + * + * To be called immediately before any QEMU agent API call + * Must have already either called qemuDomainObjBeginJob() and checked + * that the VM is still active; + * + * To be followed with qemuDomainObjExitAgent() once complete + */ +void qemuDomainObjEnterAgent(struct qemud_driver *driver, + virDomainObjPtr obj) +{ + ignore_value(qemuDomainObjEnterAgentInternal(driver, false, obj)); +} + +/* obj must NOT be locked before calling, qemud_driver must be unlocked + * + * Should be paired with an earlier qemuDomainObjEnterAgent() call + */ +void qemuDomainObjExitAgent(struct qemud_driver *driver, + virDomainObjPtr obj) +{ + qemuDomainObjExitAgentInternal(driver, false, obj); +} + +/* + * obj must be locked before calling, qemud_driver must be locked + * + * To be called immediately before any QEMU agent API call + * Must have already either called qemuDomainObjBeginJobWithDriver() and + * checked that the VM is still active; may not be used for nested async jobs. + * + * To be followed with qemuDomainObjExitAgentWithDriver() once complete + */ +void qemuDomainObjEnterAgentWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj) +{ + ignore_value(qemuDomainObjEnterAgentInternal(driver, true, obj)); +} + +/* obj must NOT be locked before calling, qemud_driver must be unlocked, + * and will be locked after returning + * + * Should be paired with an earlier qemuDomainObjEnterAgentWithDriver() call + */ +void qemuDomainObjExitAgentWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj) +{ + qemuDomainObjExitAgentInternal(driver, true, obj); +} + void qemuDomainObjEnterRemoteWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f40fa09..d76d717 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -27,6 +27,7 @@ # include "threads.h" # include "domain_conf.h" # include "qemu_monitor.h" +# include "qemu_agent.h" # include "qemu_conf.h" # include "bitmap.h" @@ -102,6 +103,11 @@ struct _qemuDomainObjPrivate { int monJSON; bool monError; unsigned long long monStart; + + qemuAgentPtr agent; + bool agentError; + unsigned long long agentStart; + bool gotShutdown; bool beingDestroyed; char *pidfile; @@ -192,6 +198,22 @@ int qemuDomainObjEnterMonitorAsync(struct qemud_driver *driver, void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + + +void qemuDomainObjEnterAgent(struct qemud_driver *driver, + virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +void qemuDomainObjExitAgent(struct qemud_driver *driver, + virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +void qemuDomainObjEnterAgentWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +void qemuDomainObjExitAgentWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + + void qemuDomainObjEnterRemoteWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7eb2a92..9e01cae 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -59,7 +59,7 @@ static void qemuMonitorJSONHandleVNCInitialize(qemuMonitorPtr mon, virJSONValueP static void qemuMonitorJSONHandleVNCDisconnect(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleBlockJob(qemuMonitorPtr mon, virJSONValuePtr data); -struct { +static struct { const char *type; void (*handler)(qemuMonitorPtr mon, virJSONValuePtr data); } eventHandlers[] = { diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e16ca07..d22020b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -107,6 +107,164 @@ qemuProcessRemoveDomainStatus(struct qemud_driver *driver, extern struct qemud_driver *qemu_driver; /* + * This is a callback registered with a qemuAgentPtr instance, + * and to be invoked when the agent console hits an end of file + * condition, or error, thus indicating VM shutdown should be + * performed + */ +static void +qemuProcessHandleAgentEOF(qemuAgentPtr agent ATTRIBUTE_UNUSED, + virDomainObjPtr vm) +{ + struct qemud_driver *driver = qemu_driver; + qemuDomainObjPrivatePtr priv; + + VIR_DEBUG("Received EOF from agent on %p '%s'", vm, vm->def->name); + + qemuDriverLock(driver); + virDomainObjLock(vm); + + priv = vm->privateData; + + qemuAgentClose(agent); + priv->agent = NULL; + + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); +} + + +/* + * This is invoked when there is some kind of error + * parsing data to/from the agent. The VM can continue + * to run, but no further agent commands will be + * allowed + */ +static void +qemuProcessHandleAgentError(qemuAgentPtr agent ATTRIBUTE_UNUSED, + virDomainObjPtr vm) +{ + struct qemud_driver *driver = qemu_driver; + qemuDomainObjPrivatePtr priv; + + VIR_DEBUG("Received error from agent on %p '%s'", vm, vm->def->name); + + qemuDriverLock(driver); + virDomainObjLock(vm); + + priv = vm->privateData; + + priv->agentError = true; + + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); +} + +static void qemuProcessHandleAgentDestroy(qemuAgentPtr agent, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv; + + virDomainObjLock(vm); + priv = vm->privateData; + if (priv->agent == agent) + priv->agent = NULL; + if (virDomainObjUnref(vm) > 0) + virDomainObjUnlock(vm); +} + + +static qemuAgentCallbacks agentCallbacks = { + .destroy = qemuProcessHandleAgentDestroy, + .eofNotify = qemuProcessHandleAgentEOF, + .errorNotify = qemuProcessHandleAgentError, +}; + +static virDomainChrSourceDefPtr +qemuFindAgentConfig(virDomainDefPtr def) +{ + virDomainChrSourceDefPtr config = NULL; + int i; + + for (i = 0 ; i < def->nchannels ; i++) { + virDomainChrDefPtr channel = def->channels[i]; + + if (channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) + continue; + + if (STREQ(channel->target.name, "org.qemu.guest_agent.0")) { + config = &channel->source; + break; + } + } + + return config; +} + +static int +qemuConnectAgent(struct qemud_driver *driver, virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret = -1; + qemuAgentPtr agent = NULL; + virDomainChrSourceDefPtr config = qemuFindAgentConfig(vm->def); + + if (!config) + return 0; + + if (virSecurityManagerSetDaemonSocketLabel(driver->securityManager, + vm->def) < 0) { + VIR_ERROR(_("Failed to set security context for agent for %s"), + vm->def->name); + goto cleanup; + } + + /* Hold an extra reference because we can't allow 'vm' to be + * deleted while the agent is active */ + virDomainObjRef(vm); + + ignore_value(virTimeMillisNow(&priv->agentStart)); + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + + agent = qemuAgentOpen(vm, + config, + &agentCallbacks); + + qemuDriverLock(driver); + virDomainObjLock(vm); + priv->agentStart = 0; + + if (virSecurityManagerClearSocketLabel(driver->securityManager, + vm->def) < 0) { + VIR_ERROR(_("Failed to clear security context for agent for %s"), + vm->def->name); + goto cleanup; + } + + /* Safe to ignore value since ref count was incremented above */ + if (agent == NULL) + ignore_value(virDomainObjUnref(vm)); + + if (!virDomainObjIsActive(vm)) { + qemuAgentClose(agent); + goto cleanup; + } + priv->agent = agent; + + if (priv->agent == NULL) { + VIR_INFO("Failed to connect agent for %s", vm->def->name); + goto cleanup; + } + + ret = 0; + +cleanup: + return ret; +} + + +/* * 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 @@ -2691,6 +2849,14 @@ qemuProcessReconnect(void *opaque) if (qemuConnectMonitor(driver, obj) < 0) goto error; + /* Failure to connect to agent shouldn't be fatal */ + if (qemuConnectAgent(driver, obj) < 0) { + VIR_WARN("Cannot connect to QEMU guest agent for %s", + obj->def->name); + virResetLastError(); + priv->agentError = true; + } + if (qemuUpdateActivePciHostdevs(driver, obj->def) < 0) { goto error; } @@ -3258,6 +3424,14 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessWaitForMonitor(driver, vm, priv->qemuCaps, pos) < 0) goto cleanup; + /* Failure to connect to agent shouldn't be fatal */ + if (qemuConnectAgent(driver, vm) < 0) { + VIR_WARN("Cannot connect to QEMU guest agent for %s", + vm->def->name); + virResetLastError(); + priv->agentError = true; + } + VIR_DEBUG("Detecting VCPU PIDs"); if (qemuProcessDetectVcpuPIDs(driver, vm) < 0) goto cleanup; @@ -3460,6 +3634,12 @@ void qemuProcessStop(struct qemud_driver *driver, } } + if (priv->agent) { + qemuAgentClose(priv->agent); + priv->agent = NULL; + priv->agentError = false; + } + if (priv->mon) qemuMonitorClose(priv->mon); @@ -3713,6 +3893,14 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, if (qemuProcessWaitForMonitor(driver, vm, priv->qemuCaps, -1) < 0) goto cleanup; + /* Failure to connect to agent shouldn't be fatal */ + if (qemuConnectAgent(driver, vm) < 0) { + VIR_WARN("Cannot connect to QEMU guest agent for %s", + vm->def->name); + virResetLastError(); + priv->agentError = true; + } + VIR_DEBUG("Detecting VCPU PIDs"); if (qemuProcessDetectVcpuPIDs(driver, vm) < 0) goto cleanup; -- 1.7.3.4

On 01/17/2012 04:44 AM, Michal Privoznik wrote:
There is now a standard QEMU guest agent that can be installed and given a virtio serial channel
<channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/>
Do we really want to be documenting a path in the libvirt-controlled hierarchy? Or is this something that we should be automatically creating on the user's behalf if it is apparent that qemu supports it, rather than requiring the user to modify their XML to add it? At which point, it would _not_ be part of 'virsh dumpxml' output, but would only be visible in the /var/run/libvirt/qemu/dom.xml runtime state (similar to how <domstatus>/<monitor> is the chardev element automatically created for the monitor)?
<target type='virtio' name='org.qemu.guest_agent.0'/> </channel>
The protocol that runs over the guest agent is JSON based and very similar to the JSON monitor. We can't use exactly the same code because there are some odd differences in the way messages and errors are structured. The qemu_agent.c file is based on a combination and simplification of qemu_monitor.c and qemu_monitor_json.c
* src/qemu/qemu_agent.c, src/qemu/qemu_agent.h: Support for talking to the agent for shutdown * src/qemu/qemu_domain.c, src/qemu/qemu_domain.h: Add thread helpers for talking to the agent * src/qemu/qemu_process.c: Connect to agent whenever starting a guest * src/qemu/qemu_monitor_json.c: Make variable static
+struct _qemuAgentMessage { + char *txBuffer; + int txOffset; + int txLength; + + /* Used by the text monitor reply / error */ + char *rxBuffer; + int rxLength;
This comment is misleading, since we are a JSON monitor and not a text monitor.
+ /* Used by the JSON monitor to hold reply / error */ + void *rxObject; + + /* True if rxBuffer / rxObject are ready, or a + * fatal error occurred on the monitor channel + */ + bool finished; +}; +
+ +#if DEBUG_RAW_IO +# include <c-ctype.h> +static char * qemuAgentEscapeNonPrintable(const char *text)
Formatting - I'd do this in two lines: static char * qemuAgentEscapeNonPrintable(const char *text)
+{ + int i; + virBuffer buf = VIR_BUFFER_INITIALIZER; + for (i = 0 ; text[i] != '\0' ; i++) { + if (c_isprint(text[i]) || + text[i] == '\n' || + (text[i] == '\r' && text[i+1] == '\n')) + virBufferAsprintf(&buf,"%c", text[i]); + else + virBufferAsprintf(&buf, "0x%02x", text[i]);
That gives ambiguous output, and isn't very efficient. I'd do it more like: if (text[i] == '\\') virBufferAddLit(&buf, "\\\\"); else if (c_isprint(text[i]) || text[i] == '\n' || (text[i] == '\r' && text[i+1] == '\n')) virBufferAddChar(&buf, text[i]); else virBufferAsprintf(&buf, "\\x%02x", text[i]);
+static int +qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool *inProgress) +{ + struct sockaddr_un addr; + int monfd; + int timeout = 3; /* In seconds */ + int ret, i = 0; + + *inProgress = false; + + if ((monfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { + virReportSystemError(errno, + "%s", _("failed to create socket")); + return -1; + } + + if (virSetNonBlock(monfd) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to put monitor into non-blocking mode")); + goto error; + } + + if (virSetCloseExec(monfd) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to set monitor close-on-exec flag")); + goto error; + }
You know, if I ever got around to fixing gnulib to guarantee things for older platforms, we could use socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0) and shave off a few syscalls. But for now, your code is the best we can do.
+static int +qemuAgentOpenPty(const char *monitor) +{ + int monfd; + + if ((monfd = open(monitor, O_RDWR)) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to open monitor path %s"), monitor); + return -1; + } + + if (virSetNonBlock(monfd) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to put monitor into non-blocking mode")); + goto error; + }
Why not open(monitor, O_RDWR | O_NONBLOCK), and shave off a syscall here?
+ + if (virSetCloseExec(monfd) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to set monitor close-on-exec flag")); + goto error; + }
Alas, I also need to find time to make gnulib guarantee open(O_CLOEXEC).
+static int qemuAgentIOProcessData(qemuAgentPtr mon, + const char *data, + size_t len, + qemuAgentMessagePtr msg) +{ + int used = 0; +#if DEBUG_IO +# if DEBUG_RAW_IO + char *str1 = qemuAgentEscapeNonPrintable(data); + VIR_ERROR("[%s]", str1); + VIR_FREE(str1); +# else + VIR_DEBUG("Data %zu bytes [%s]", len, data); +# endif +#endif + + while (used < len) { + char *nl = strstr(data + used, LINE_ENDING);
Would strchr() be more efficient, since LINE_ENDING is a single character?
+ + if (nl) { + int got = nl - (data + used); + char *line = strndup(data + used, got); + if (!line) { + virReportOOMError(); + return -1; + } + used += got + strlen(LINE_ENDING); + line[got] = '\0'; /* kill \n */
The '\n' was already killed by virtue of the strndup(). This line is redundant.
+ if (qemuAgentIOProcessLine(mon, line, msg) < 0) { + VIR_FREE(line); + return -1; + } + + VIR_FREE(line);
Then again, do we really need to strndup(), or should we just modify data[] in place, before calling qemuAgentIOProcessLine(mon, data + used, got), and before incrementing used?
+#if DEBUG_IO +# if DEBUG_RAW_IO + char *str1 = qemuAgentEscapeNonPrintable(msg ? msg->txBuffer : ""); + char *str2 = qemuAgentEscapeNonPrintable(mon->buffer); + VIR_ERROR(_("Process %d %p %p [[[%s]]][[[%s]]]"), + (int)mon->bufferOffset, mon->msg, msg, str1, str2);
Why are we using %d and a cast to int, instead of %zu and mon->bufferOffset as-is?
+ VIR_FREE(str1); + VIR_FREE(str2); +# else + VIR_DEBUG("Process %d", (int)mon->bufferOffset);
And again.
+ +static int +qemuAgentCheckError(virJSONValuePtr cmd, + virJSONValuePtr reply)
Indentation.
+ +#if 0 +static int +qemuAgentHasError(virJSONValuePtr reply, + const char *klass) +{
Do we still need this block of code?
+ +static virJSONValuePtr ATTRIBUTE_SENTINEL +qemuAgentMakeCommand(const char *cmdname, + ...)
Indentation (and probably more instances, so I'll quit pointing them out).
+++ b/src/qemu/qemu_process.c @@ -107,6 +107,164 @@ qemuProcessRemoveDomainStatus(struct qemud_driver *driver, extern struct qemud_driver *qemu_driver;
/* + * This is a callback registered with a qemuAgentPtr instance, + * and to be invoked when the agent console hits an end of file + * condition, or error, thus indicating VM shutdown should be + * performed
Does agent shutdown really imply VM shutdown? I hope not - that should be reserved for just monitor EOF.
+static virDomainChrSourceDefPtr +qemuFindAgentConfig(virDomainDefPtr def) +{ + virDomainChrSourceDefPtr config = NULL; + int i; + + for (i = 0 ; i < def->nchannels ; i++) { + virDomainChrDefPtr channel = def->channels[i]; + + if (channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) + continue; + + if (STREQ(channel->target.name, "org.qemu.guest_agent.0")) { + config = &channel->source; + break; + } + }
This implementation requires users to add the agent in their XML; is there any way we can automate the use of an agent without doing this? Then again, it's not just that qemu has to be new enough to have agent support, but it's also that the guest has to have an agent installed. Maybe we should consider (as a followup, not for this patch) an XML shorthand that lets the user request use of an agent without having to specify full details (that is, without having to choose a patch for the socket, and without having to know the name "org.qemu.guest_agent.0" for the protocol of a channel). Overall, I'm looking forward to using the agent (I want to support a new flag to snapshot creation that uses the agent's ability to request file system queiscing from the guest). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 19.01.2012 23:18, Eric Blake wrote:
On 01/17/2012 04:44 AM, Michal Privoznik wrote:
There is now a standard QEMU guest agent that can be installed and given a virtio serial channel
<channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/>
Do we really want to be documenting a path in the libvirt-controlled hierarchy?
Or is this something that we should be automatically creating on the user's behalf if it is apparent that qemu supports it, rather than requiring the user to modify their XML to add it? At which point, it would _not_ be part of 'virsh dumpxml' output, but would only be visible in the /var/run/libvirt/qemu/dom.xml runtime state (similar to how <domstatus>/<monitor> is the chardev element automatically created for the monitor)?
<target type='virtio' name='org.qemu.guest_agent.0'/> </channel>
+++ b/src/qemu/qemu_process.c @@ -107,6 +107,164 @@ qemuProcessRemoveDomainStatus(struct qemud_driver *driver, extern struct qemud_driver *qemu_driver;
/* + * This is a callback registered with a qemuAgentPtr instance, + * and to be invoked when the agent console hits an end of file + * condition, or error, thus indicating VM shutdown should be + * performed
Does agent shutdown really imply VM shutdown? I hope not - that should be reserved for just monitor EOF.
Yes. You won't receive EOF if you close guest agent. But you'll get EOF when qemu closes the socket. Even during reboots this socket remains opened. So it is like monitor.
+static virDomainChrSourceDefPtr +qemuFindAgentConfig(virDomainDefPtr def) +{ + virDomainChrSourceDefPtr config = NULL; + int i; + + for (i = 0 ; i < def->nchannels ; i++) { + virDomainChrDefPtr channel = def->channels[i]; + + if (channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) + continue; + + if (STREQ(channel->target.name, "org.qemu.guest_agent.0")) { + config = &channel->source; + break; + } + }
This implementation requires users to add the agent in their XML; is there any way we can automate the use of an agent without doing this?
I am not sure we want this. Although I am not fully convinced by the opposite. Thing is, I don't want to enable something by default because users might have not noticed and things may break for them. On the other hand, users will definitely benefit from this feature so making it as easy as possible to enable is the right step. My initial thought is to have simple one line element: <guest_agent name="org.qemu.guest_agent.0"/> with name defaulting to "org.qemu.guest_agent.0" so simply inserting bare <guest_agent/> will turn everything on?
Then again, it's not just that qemu has to be new enough to have agent support, but it's also that the guest has to have an agent installed. Maybe we should consider (as a followup, not for this patch) an XML shorthand that lets the user request use of an agent without having to specify full details (that is, without having to choose a patch for the socket, and without having to know the name "org.qemu.guest_agent.0" for the protocol of a channel).
Overall, I'm looking forward to using the agent (I want to support a new flag to snapshot creation that uses the agent's ability to request file system queiscing from the guest).
Thanks for review and watch out for v2 :) (as soon as we settle down on XML part) Michal

On Fri, Jan 20, 2012 at 07:16:22PM +0100, Michal Privoznik wrote:
On 19.01.2012 23:18, Eric Blake wrote:
On 01/17/2012 04:44 AM, Michal Privoznik wrote:
There is now a standard QEMU guest agent that can be installed and given a virtio serial channel
<channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/>
[snip]
I am not sure we want this. Although I am not fully convinced by the opposite. Thing is, I don't want to enable something by default because users might have not noticed and things may break for them. On the other hand, users will definitely benefit from this feature so making it as easy as possible to enable is the right step. My initial thought is to have simple one line element: <guest_agent name="org.qemu.guest_agent.0"/>
with name defaulting to "org.qemu.guest_agent.0" so simply inserting bare <guest_agent/> will turn everything on?
No, we don't want to go about inventing new syntax for that - the very purpose of any <channel> element is to support a guest agent, so it is pointless creating another <guest_agent> element. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Thu, Jan 19, 2012 at 03:18:00PM -0700, Eric Blake wrote:
On 01/17/2012 04:44 AM, Michal Privoznik wrote:
There is now a standard QEMU guest agent that can be installed and given a virtio serial channel
<channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/>
Do we really want to be documenting a path in the libvirt-controlled hierarchy?
This is an artifact of the way we are doing the guest agent support today. If you look at the SPICE guest agent, there is a special channel type 'spicevmc' that is used, and QEMU deals with that internally. In the future, QEMU will deal with its own guest agent internally and thus we'll have a new channel type kinda like 'spicevmc' which does not expose any paths. For now though, we're using the externally managed agent for which we do want to expose the path IMHO. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Add a new API virDomainShutdownFlags and define: VIR_DOMAIN_SHUTDOWN_DEFAULT = 0, VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 << 0), VIR_DOMAIN_SHUTDOWN_GUEST_AGENT = (1 << 1), Also define some flags for the reboot API VIR_DOMAIN_REBOOT_DEFAULT = 0, VIR_DOMAIN_REBOOT_ACPI_POWER_BTN = (1 << 0), VIR_DOMAIN_REBOOT_GUEST_AGENT = (1 << 1), Although these two APIs currently have the same flags, using separate enums allows them to expand separately in the future. Add stub impls of the new API for all existing drivers --- include/libvirt/libvirt.h.in | 15 +++++++++ src/driver.h | 5 +++ src/esx/esx_driver.c | 11 ++++++- src/libvirt.c | 65 +++++++++++++++++++++++++++++++++++++++++- src/libvirt_public.syms | 4 ++ src/libxl/libxl_driver.c | 12 +++++++- src/openvz/openvz_driver.c | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 8 ++++- src/remote_protocol-structs | 5 +++ src/test/test_driver.c | 11 ++++++- src/uml/uml_driver.c | 11 ++++++- src/vbox/vbox_tmpl.c | 11 ++++++- src/vmware/vmware_driver.c | 1 + src/xen/xen_driver.c | 14 ++++++++- src/xenapi/xenapi_driver.c | 12 +++++++- 16 files changed, 177 insertions(+), 10 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e436f3c..27546af 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1148,7 +1148,22 @@ virDomainPtr virDomainLookupByUUID (virConnectPtr conn, virDomainPtr virDomainLookupByUUIDString (virConnectPtr conn, const char *uuid); +typedef enum { + VIR_DOMAIN_SHUTDOWN_DEFAULT = 0, + VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 << 0), + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT = (1 << 1), +} virDomainShutdownFlagValues; + int virDomainShutdown (virDomainPtr domain); +int virDomainShutdownFlags (virDomainPtr domain, + unsigned int flags); + +typedef enum { + VIR_DOMAIN_REBOOT_DEFAULT = 0, + VIR_DOMAIN_REBOOT_ACPI_POWER_BTN = (1 << 0), + VIR_DOMAIN_REBOOT_GUEST_AGENT = (1 << 1), +} virDomainRebootFlagValues; + int virDomainReboot (virDomainPtr domain, unsigned int flags); int virDomainReset (virDomainPtr domain, diff --git a/src/driver.h b/src/driver.h index 24636a4..6222bed 100644 --- a/src/driver.h +++ b/src/driver.h @@ -793,6 +793,10 @@ typedef int virTypedParameterPtr params, int *nparams, unsigned int flags); +typedef int + (*virDrvDomainShutdownFlags)(virDomainPtr domain, + unsigned int flags); + /** * _virDriver: @@ -829,6 +833,7 @@ struct _virDriver { virDrvDomainSuspend domainSuspend; virDrvDomainResume domainResume; virDrvDomainShutdown domainShutdown; + virDrvDomainShutdownFlags domainShutdownFlags; virDrvDomainReboot domainReboot; virDrvDomainReset domainReset; virDrvDomainDestroy domainDestroy; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 1e424eb..4ad7a37 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1886,7 +1886,7 @@ esxDomainResume(virDomainPtr domain) static int -esxDomainShutdown(virDomainPtr domain) +esxDomainShutdownFlags(virDomainPtr domain, unsigned int flags) { int result = -1; esxPrivate *priv = domain->conn->privateData; @@ -1894,6 +1894,8 @@ esxDomainShutdown(virDomainPtr domain) esxVI_String *propertyNameList = NULL; esxVI_VirtualMachinePowerState powerState; + virCheckFlags(0, -1); + if (esxVI_EnsureSession(priv->primary) < 0) { return -1; } @@ -1927,6 +1929,12 @@ esxDomainShutdown(virDomainPtr domain) } +static int +esxDomainShutdown(virDomainPtr domain) +{ + return esxDomainShutdownFlags(domain, 0); +} + static int esxDomainReboot(virDomainPtr domain, unsigned int flags) @@ -4960,6 +4968,7 @@ static virDriver esxDriver = { .domainSuspend = esxDomainSuspend, /* 0.7.0 */ .domainResume = esxDomainResume, /* 0.7.0 */ .domainShutdown = esxDomainShutdown, /* 0.7.0 */ + .domainShutdownFlags = esxDomainShutdownFlags, /* 0.9.10 */ .domainReboot = esxDomainReboot, /* 0.7.0 */ .domainDestroy = esxDomainDestroy, /* 0.7.0 */ .domainDestroyFlags = esxDomainDestroyFlags, /* 0.9.4 */ diff --git a/src/libvirt.c b/src/libvirt.c index a540424..9409a24 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -3106,14 +3106,77 @@ error: } /** + * virDomainShutdownFlags: + * @domain: a domain object + * @flags: bitwise-OR of virDomainShutdownFlagValues + * + * Shutdown a domain, the domain object is still usable thereafter but + * the domain OS is being stopped. Note that the guest OS may ignore the + * request. For guests that react to a shutdown request, the differences + * from virDomainDestroy() are that the guest's disk storage will be in a + * stable state rather than having the (virtual) power cord pulled, and + * this command returns as soon as the shutdown request is issued rather + * than blocking until the guest is no longer running. + * + * If the domain is transient and has any snapshot metadata (see + * virDomainSnapshotNum()), then that metadata will automatically + * be deleted when the domain quits. + * + * If @flags is set to zero, then the hypervisor will chose the + * method of shutdown it considers best. To have greater control + * pass one of the virDomainShutdownFlagValues. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainShutdownFlags(virDomainPtr domain, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if (domain->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + conn = domain->conn; + + if (conn->driver->domainShutdownFlags) { + int ret; + ret = conn->driver->domainShutdownFlags(domain, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + +/** * virDomainReboot: * @domain: a domain object - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virDomainRebootFlagValues * * Reboot a domain, the domain object is still usable there after but * the domain OS is being stopped for a restart. * Note that the guest OS may ignore the request. * + * If @flags is set to zero, then the hypervisor will chose the + * method of shutdown it considers best. To have greater control + * pass one of the virDomainRebootFlagValues. + * * Returns 0 in case of success and -1 in case of failure. */ int diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 4ca7216..e0cbdb4 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -516,4 +516,8 @@ LIBVIRT_0.9.9 { virDomainSetNumaParameters; } LIBVIRT_0.9.8; +LIBVIRT_0.9.10 { + global: + virDomainShutdownFlags; +} LIBVIRT_0.9.9; # .... define new API here using predicted next version number .... diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 0500ed0..e2b174e 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1412,13 +1412,15 @@ cleanup: } static int -libxlDomainShutdown(virDomainPtr dom) +libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { libxlDriverPrivatePtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; libxlDomainObjPrivatePtr priv; + virCheckFlags(0, -1); + libxlDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { @@ -1456,6 +1458,13 @@ cleanup: } static int +libxlDomainShutdown(virDomainPtr dom) +{ + return libxlDomainShutdownFlags(dom, 0); +} + + +static int libxlDomainReboot(virDomainPtr dom, unsigned int flags) { libxlDriverPrivatePtr driver = dom->conn->privateData; @@ -3877,6 +3886,7 @@ static virDriver libxlDriver = { .domainSuspend = libxlDomainSuspend, /* 0.9.0 */ .domainResume = libxlDomainResume, /* 0.9.0 */ .domainShutdown = libxlDomainShutdown, /* 0.9.0 */ + .domainShutdownFlags = libxlDomainShutdownFlags, /* 0.9.10 */ .domainReboot = libxlDomainReboot, /* 0.9.0 */ .domainDestroy = libxlDomainDestroy, /* 0.9.0 */ .domainDestroyFlags = libxlDomainDestroyFlags, /* 0.9.4 */ diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 03bf21a..b848a88 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1693,6 +1693,7 @@ static virDriver openvzDriver = { .domainSuspend = openvzDomainSuspend, /* 0.8.3 */ .domainResume = openvzDomainResume, /* 0.8.3 */ .domainShutdown = openvzDomainShutdown, /* 0.3.1 */ + .domainShutdownFlags = openvzDomainShutdownFlags, /* 0.9.10 */ .domainReboot = openvzDomainReboot, /* 0.3.1 */ .domainDestroy = openvzDomainShutdown, /* 0.3.1 */ .domainDestroyFlags = openvzDomainShutdownFlags, /* 0.9.4 */ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e28840b..f45a8fe 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4617,6 +4617,7 @@ static virDriver remote_driver = { .domainSuspend = remoteDomainSuspend, /* 0.3.0 */ .domainResume = remoteDomainResume, /* 0.3.0 */ .domainShutdown = remoteDomainShutdown, /* 0.3.0 */ + .domainShutdownFlags = remoteDomainShutdownFlags, /* 0.9.10 */ .domainReboot = remoteDomainReboot, /* 0.3.0 */ .domainReset = remoteDomainReset, /* 0.9.7 */ .domainDestroy = remoteDomainDestroy, /* 0.3.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index ca739ff..a9049ce 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2348,6 +2348,11 @@ struct remote_node_suspend_for_duration_args { unsigned int flags; }; +struct remote_domain_shutdown_flags_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + /*----- Protocol. -----*/ @@ -2653,7 +2658,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_NUMA_PARAMETERS = 254, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, /* autogen autogen */ - REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS = 258 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 2758315..430d8e4 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1832,6 +1832,10 @@ struct remote_node_suspend_for_duration_args { uint64_t duration; u_int flags; }; +struct remote_domain_shutdown_flags_args { + remote_nonnull_domain dom; + u_int flags; +}; enum remote_procedure { REMOTE_PROC_OPEN = 1, REMOTE_PROC_CLOSE = 2, @@ -2090,4 +2094,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, + REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS = 258, }; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 2a98e7e..f12112e 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1541,13 +1541,16 @@ cleanup: return ret; } -static int testShutdownDomain (virDomainPtr domain) +static int testShutdownDomainFlags(virDomainPtr domain, + unsigned int flags) { testConnPtr privconn = domain->conn->privateData; virDomainObjPtr privdom; virDomainEventPtr event = NULL; int ret = -1; + virCheckFlags(0, -1); + testDriverLock(privconn); privdom = virDomainFindByName(&privconn->domains, domain->name); @@ -1584,6 +1587,11 @@ cleanup: return ret; } +static int testShutdownDomain (virDomainPtr domain) +{ + return testShutdownDomainFlags(domain, 0); +} + /* Similar behaviour as shutdown */ static int testRebootDomain (virDomainPtr domain, unsigned int action ATTRIBUTE_UNUSED) @@ -5528,6 +5536,7 @@ static virDriver testDriver = { .domainSuspend = testPauseDomain, /* 0.1.1 */ .domainResume = testResumeDomain, /* 0.1.1 */ .domainShutdown = testShutdownDomain, /* 0.1.1 */ + .domainShutdownFlags = testShutdownDomainFlags, /* 0.9.10 */ .domainReboot = testRebootDomain, /* 0.1.1 */ .domainDestroy = testDestroyDomain, /* 0.1.1 */ .domainGetOSType = testGetOSType, /* 0.1.9 */ diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 671216e..a4cf945 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1513,12 +1513,15 @@ cleanup: } -static int umlDomainShutdown(virDomainPtr dom) { +static int umlDomainShutdownFlags(virDomainPtr dom, + unsigned int flags) { struct uml_driver *driver = dom->conn->privateData; virDomainObjPtr vm; char *info = NULL; int ret = -1; + virCheckFlags(0, -1); + umlDriverLock(driver); vm = virDomainFindByID(&driver->domains, dom->id); umlDriverUnlock(driver); @@ -1544,6 +1547,11 @@ cleanup: return ret; } +static int +umlDomainShutdown(virDomainPtr dom) +{ + return umlDomainShutdownFlags(dom, 0); +} static int umlDomainDestroyFlags(virDomainPtr dom, @@ -2533,6 +2541,7 @@ static virDriver umlDriver = { .domainLookupByUUID = umlDomainLookupByUUID, /* 0.5.0 */ .domainLookupByName = umlDomainLookupByName, /* 0.5.0 */ .domainShutdown = umlDomainShutdown, /* 0.5.0 */ + .domainShutdownFlags = umlDomainShutdownFlags, /* 0.9.10 */ .domainDestroy = umlDomainDestroy, /* 0.5.0 */ .domainDestroyFlags = umlDomainDestroyFlags, /* 0.9.4 */ .domainGetOSType = umlDomainGetOSType, /* 0.5.0 */ diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 22712d5..d720432 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -1606,7 +1606,8 @@ cleanup: return ret; } -static int vboxDomainShutdown(virDomainPtr dom) { +static int vboxDomainShutdownFlags(virDomainPtr dom, + unsigned int flags) { VBOX_OBJECT_CHECK(dom->conn, int, -1); IMachine *machine = NULL; vboxIID iid = VBOX_IID_INITIALIZER; @@ -1615,6 +1616,8 @@ static int vboxDomainShutdown(virDomainPtr dom) { PRBool isAccessible = PR_FALSE; nsresult rc; + virCheckFlags(0, -1); + vboxIIDFromUUID(&iid, dom->uuid); rc = VBOX_OBJECT_GET_MACHINE(iid.value, &machine); if (NS_FAILED(rc)) { @@ -1656,6 +1659,11 @@ cleanup: return ret; } +static int vboxDomainShutdown(virDomainPtr dom) { + return vboxDomainShutdownFlags(dom, 0); +} + + static int vboxDomainReboot(virDomainPtr dom, unsigned int flags) { VBOX_OBJECT_CHECK(dom->conn, int, -1); @@ -9112,6 +9120,7 @@ virDriver NAME(Driver) = { .domainSuspend = vboxDomainSuspend, /* 0.6.3 */ .domainResume = vboxDomainResume, /* 0.6.3 */ .domainShutdown = vboxDomainShutdown, /* 0.6.3 */ + .domainShutdownFlags = vboxDomainShutdownFlags, /* 0.9.10 */ .domainReboot = vboxDomainReboot, /* 0.6.3 */ .domainDestroy = vboxDomainDestroy, /* 0.6.3 */ .domainDestroyFlags = vboxDomainDestroyFlags, /* 0.9.4 */ diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index a9873ba..56e9d2d 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -980,6 +980,7 @@ static virDriver vmwareDriver = { .domainSuspend = vmwareDomainSuspend, /* 0.8.7 */ .domainResume = vmwareDomainResume, /* 0.8.7 */ .domainShutdown = vmwareDomainShutdown, /* 0.8.7 */ + .domainShutdownFlags = vmwareDomainShutdownFlags, /* 0.9.10 */ .domainReboot = vmwareDomainReboot, /* 0.8.7 */ .domainDestroy = vmwareDomainShutdown, /* 0.8.7 */ .domainDestroyFlags = vmwareDomainShutdownFlags, /* 0.9.4 */ diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 20671c0..5cbbecd 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -851,21 +851,30 @@ xenUnifiedDomainResume (virDomainPtr dom) } static int -xenUnifiedDomainShutdown (virDomainPtr dom) +xenUnifiedDomainShutdownFlags(virDomainPtr dom, + unsigned int flags) { GET_PRIVATE(dom->conn); int i; + virCheckFlags(0, -1); + for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) if (priv->opened[i] && drivers[i]->xenDomainShutdown && - drivers[i]->xenDomainShutdown (dom) == 0) + drivers[i]->xenDomainShutdown(dom) == 0) return 0; return -1; } static int +xenUnifiedDomainShutdown(virDomainPtr dom) +{ + return xenUnifiedDomainShutdownFlags(dom, 0); +} + +static int xenUnifiedDomainReboot (virDomainPtr dom, unsigned int flags) { GET_PRIVATE(dom->conn); @@ -2187,6 +2196,7 @@ static virDriver xenUnifiedDriver = { .domainSuspend = xenUnifiedDomainSuspend, /* 0.0.3 */ .domainResume = xenUnifiedDomainResume, /* 0.0.3 */ .domainShutdown = xenUnifiedDomainShutdown, /* 0.0.3 */ + .domainShutdownFlags = xenUnifiedDomainShutdownFlags, /* 0.9.10 */ .domainReboot = xenUnifiedDomainReboot, /* 0.1.0 */ .domainDestroy = xenUnifiedDomainDestroy, /* 0.0.3 */ .domainDestroyFlags = xenUnifiedDomainDestroyFlags, /* 0.9.4 */ diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 78137d4..68017bc 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -783,12 +783,15 @@ xenapiDomainResume (virDomainPtr dom) * Returns 0 on success or -1 in case of error */ static int -xenapiDomainShutdown (virDomainPtr dom) +xenapiDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { /* vm.clean_shutdown */ xen_vm vm; xen_vm_set *vms; xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session; + + virCheckFlags(0, -1); + if (xen_vm_get_by_name_label(session, &vms, dom->name) && vms->size > 0) { if (vms->size != 1) { xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, @@ -811,6 +814,12 @@ xenapiDomainShutdown (virDomainPtr dom) return -1; } +static int +xenapiDomainShutdown(virDomainPtr dom) +{ + return xenapiDomainShutdownFlags(dom, 0); +} + /* * xenapiDomainReboot * @@ -1928,6 +1937,7 @@ static virDriver xenapiDriver = { .domainSuspend = xenapiDomainSuspend, /* 0.8.0 */ .domainResume = xenapiDomainResume, /* 0.8.0 */ .domainShutdown = xenapiDomainShutdown, /* 0.8.0 */ + .domainShutdownFlags = xenapiDomainShutdownFlags, /* 0.9.10 */ .domainReboot = xenapiDomainReboot, /* 0.8.0 */ .domainDestroy = xenapiDomainDestroy, /* 0.8.0 */ .domainDestroyFlags = xenapiDomainDestroyFlags, /* 0.9.4 */ -- 1.7.3.4

On 01/17/2012 04:44 AM, Michal Privoznik wrote:
Add a new API virDomainShutdownFlags and define:
VIR_DOMAIN_SHUTDOWN_DEFAULT = 0, VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 << 0), VIR_DOMAIN_SHUTDOWN_GUEST_AGENT = (1 << 1),
Also define some flags for the reboot API
VIR_DOMAIN_REBOOT_DEFAULT = 0, VIR_DOMAIN_REBOOT_ACPI_POWER_BTN = (1 << 0), VIR_DOMAIN_REBOOT_GUEST_AGENT = (1 << 1),
Although these two APIs currently have the same flags, using separate enums allows them to expand separately in the future.
Fair enough.
Add stub impls of the new API for all existing drivers ---
I might have split this into several patches, but I think doing it in one go is okay since the stubs are trivial.
include/libvirt/libvirt.h.in | 15 +++++++++ src/driver.h | 5 +++ src/libvirt.c | 65 +++++++++++++++++++++++++++++++++++++++++- src/libvirt_public.syms | 4 ++
That is, I might have done part 1 (the public API),
src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 8 ++++- src/remote_protocol-structs | 5 +++
part 2 (the RPC),
src/esx/esx_driver.c | 11 ++++++- src/libxl/libxl_driver.c | 12 +++++++- src/openvz/openvz_driver.c | 1 + src/test/test_driver.c | 11 ++++++- src/uml/uml_driver.c | 11 ++++++- src/vbox/vbox_tmpl.c | 11 ++++++- src/vmware/vmware_driver.c | 1 + src/xen/xen_driver.c | 14 ++++++++- src/xenapi/xenapi_driver.c | 12 +++++++-
and part 3 (the stubs).
+++ b/include/libvirt/libvirt.h.in @@ -1148,7 +1148,22 @@ virDomainPtr virDomainLookupByUUID (virConnectPtr conn, virDomainPtr virDomainLookupByUUIDString (virConnectPtr conn, const char *uuid);
+typedef enum { + VIR_DOMAIN_SHUTDOWN_DEFAULT = 0, + VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 << 0), + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT = (1 << 1), +} virDomainShutdownFlagValues;
No documentation comments? Even a /** doc */ prior to the typedef that mentions virDomainShutdownFlagValues might be helpful to the doc generation process.
+++ b/src/libvirt.c @@ -3106,14 +3106,77 @@ error: }
/** + * virDomainShutdownFlags: + * @domain: a domain object + * @flags: bitwise-OR of virDomainShutdownFlagValues + * + * Shutdown a domain, the domain object is still usable thereafter but + * the domain OS is being stopped. Note that the guest OS may ignore the + * request. For guests that react to a shutdown request, the differences + * from virDomainDestroy() are that the guest's disk storage will be in a + * stable state rather than having the (virtual) power cord pulled, and + * this command returns as soon as the shutdown request is issued rather + * than blocking until the guest is no longer running. + * + * If the domain is transient and has any snapshot metadata (see + * virDomainSnapshotNum()), then that metadata will automatically + * be deleted when the domain quits. + * + * If @flags is set to zero, then the hypervisor will chose the + * method of shutdown it considers best. To have greater control + * pass one of the virDomainShutdownFlagValues.
Maybe mention the existing flag names: Use VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN to trigger a shutdown through an ACPI interrupt, and VIR_DOMAIN_SHUTDOWN_GUEST_AGENT to trigger a shutdown through a guest agent call (this requires that the domain have a <channel> device appropriately wired to a guest agent). Is it an error if multiple flags are requested at once?
+/** * virDomainReboot: * @domain: a domain object - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virDomainRebootFlagValues * * Reboot a domain, the domain object is still usable there after but * the domain OS is being stopped for a restart. * Note that the guest OS may ignore the request. * + * If @flags is set to zero, then the hypervisor will chose the + * method of shutdown it considers best. To have greater control + * pass one of the virDomainRebootFlagValues.
Again, mention the possible flag values, and that an agent request requires additional support from the XML.
+++ b/src/openvz/openvz_driver.c @@ -1693,6 +1693,7 @@ static virDriver openvzDriver = { .domainSuspend = openvzDomainSuspend, /* 0.8.3 */ .domainResume = openvzDomainResume, /* 0.8.3 */ .domainShutdown = openvzDomainShutdown, /* 0.3.1 */ + .domainShutdownFlags = openvzDomainShutdownFlags, /* 0.9.10 */
Hmm. Are we setting ourselves up for issues down the road by sharing shutdown and destroy implementations for openvz? But it's a pre-existing problem, not made worse by the patch, and okay as long as we reject all flag values.
+++ b/src/vmware/vmware_driver.c @@ -980,6 +980,7 @@ static virDriver vmwareDriver = { .domainSuspend = vmwareDomainSuspend, /* 0.8.7 */ .domainResume = vmwareDomainResume, /* 0.8.7 */ .domainShutdown = vmwareDomainShutdown, /* 0.8.7 */ + .domainShutdownFlags = vmwareDomainShutdownFlags, /* 0.9.10 */ .domainReboot = vmwareDomainReboot, /* 0.8.7 */ .domainDestroy = vmwareDomainShutdown, /* 0.8.7 */ .domainDestroyFlags = vmwareDomainShutdownFlags, /* 0.9.4 */
Same potential issue as openvz. Oh well, not worth worrying about today. ACK with documentation findings addressed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

This makes use of the QEMU guest agent to implement the virDomainShutdownFlags and virDomainReboot APIs. With no flags specified, it will prefer to use the agent, but fallback to ACPI. Explicit choice can be made by using a suitable flag * src/qemu/qemu_driver.c: Wire up use of agent --- src/qemu/qemu_driver.c | 107 ++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 86 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 712f1fc..0425018 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -47,6 +47,7 @@ #include "qemu_driver.h" +#include "qemu_agent.h" #include "qemu_conf.h" #include "qemu_capabilities.h" #include "qemu_command.h" @@ -1507,12 +1508,15 @@ cleanup: return ret; } - -static int qemuDomainShutdown(virDomainPtr dom) { +static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; qemuDomainObjPrivatePtr priv; + bool useAgent = false; + + virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN | + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1526,6 +1530,26 @@ static int qemuDomainShutdown(virDomainPtr dom) { goto cleanup; } + priv = vm->privateData; + + if ((flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) || + (!(flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) && + priv->agent)) + useAgent = true; + + if (useAgent) { + if (priv->agentError) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QEMU guest agent is not available due to an error")); + goto endjob; + } + if (!priv->agent) { + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + goto endjob; + } + } + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -1535,12 +1559,17 @@ static int qemuDomainShutdown(virDomainPtr dom) { goto endjob; } - qemuDomainSetFakeReboot(driver, vm, false); + if (useAgent) { + qemuDomainObjEnterAgent(driver, vm); + ret = qemuAgentShutdown(priv->agent, QEMU_AGENT_SHUTDOWN_POWERDOWN); + qemuDomainObjExitAgent(driver, vm); + } else { + qemuDomainSetFakeReboot(driver, vm, false); - priv = vm->privateData; - qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorSystemPowerdown(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + qemuDomainObjEnterMonitor(driver, vm); + ret = qemuMonitorSystemPowerdown(priv->mon); + qemuDomainObjExitMonitor(driver, vm); + } endjob: if (qemuDomainObjEndJob(driver, vm) == 0) @@ -1552,14 +1581,20 @@ cleanup: return ret; } +static int qemuDomainShutdown(virDomainPtr dom) { + return qemuDomainShutdownFlags(dom, 0); +} + static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; qemuDomainObjPrivatePtr priv; + bool useAgent = false; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN | + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT , -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1575,22 +1610,54 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) { priv = vm->privateData; - if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON)) { - if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) { + if ((flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) || + (!(flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) && + priv->agent)) + useAgent = true; + + if (useAgent) { + if (priv->agentError) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QEMU guest agent is not available due to an error")); + goto cleanup; + } + if (!priv->agent) { + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + goto cleanup; + } + } else { +#if HAVE_YAJL + if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON)) { + if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Reboot is not supported with this QEMU binary")); + goto cleanup; + } + } else { +#endif qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Reboot is not supported with this QEMU binary")); + _("Reboot is not supported without the JSON monitor")); goto cleanup; +#if HAVE_YAJL } +#endif + } - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; - if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto endjob; - } + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + if (useAgent) { + qemuDomainObjEnterAgent(driver, vm); + ret = qemuAgentShutdown(priv->agent, QEMU_AGENT_SHUTDOWN_REBOOT); + qemuDomainObjExitAgent(driver, vm); + } else { qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSystemPowerdown(priv->mon); qemuDomainObjExitMonitor(driver, vm); @@ -1601,9 +1668,6 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) { endjob: if (qemuDomainObjEndJob(driver, vm) == 0) vm = NULL; - } else { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Reboot is not supported without the JSON monitor")); } cleanup: @@ -11853,6 +11917,7 @@ static virDriver qemuDriver = { .domainSuspend = qemudDomainSuspend, /* 0.2.0 */ .domainResume = qemudDomainResume, /* 0.2.0 */ .domainShutdown = qemuDomainShutdown, /* 0.2.0 */ + .domainShutdownFlags = qemuDomainShutdownFlags, /* 0.9.7 */ .domainReboot = qemuDomainReboot, /* 0.9.3 */ .domainReset = qemuDomainReset, /* 0.9.7 */ .domainDestroy = qemuDomainDestroy, /* 0.2.0 */ -- 1.7.3.4

On 01/17/2012 04:44 AM, Michal Privoznik wrote:
This makes use of the QEMU guest agent to implement the virDomainShutdownFlags and virDomainReboot APIs. With no flags specified, it will prefer to use the agent, but fallback to ACPI. Explicit choice can be made by using a suitable flag
* src/qemu/qemu_driver.c: Wire up use of agent --- src/qemu/qemu_driver.c | 107 ++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 86 insertions(+), 21 deletions(-)
@@ -1526,6 +1530,26 @@ static int qemuDomainShutdown(virDomainPtr dom) { goto cleanup; }
+ priv = vm->privateData; + + if ((flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) || + (!(flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) && + priv->agent)) + useAgent = true;
Should we reject things if the user passes both flags? Or if not,
+ + if (useAgent) { + if (priv->agentError) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QEMU guest agent is not available due to an error")); + goto endjob; + } + if (!priv->agent) { + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + goto endjob; + } + }
if the user passes both flags, but the agent had an error or is not present, do we silently fall back to acpi?
@@ -1575,22 +1610,54 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) {
priv = vm->privateData;
- if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON)) { - if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) { + if ((flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) || + (!(flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) && + priv->agent)) + useAgent = true; + + if (useAgent) { + if (priv->agentError) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QEMU guest agent is not available due to an error")); + goto cleanup; + } + if (!priv->agent) { + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + goto cleanup; + }
Same questions.
@@ -11853,6 +11917,7 @@ static virDriver qemuDriver = { .domainSuspend = qemudDomainSuspend, /* 0.2.0 */ .domainResume = qemudDomainResume, /* 0.2.0 */ .domainShutdown = qemuDomainShutdown, /* 0.2.0 */ + .domainShutdownFlags = qemuDomainShutdownFlags, /* 0.9.7 */
0.9.10 -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Extend the 'shutdown' and 'reboot' methods so that they both accept a new argument --mode acpi|agent * tools/virsh.c: New args for shutdown/reboot * tools/virsh.pod: Document new args --- tools/virsh.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- tools/virsh.pod | 24 ++++++++++++++++-------- 2 files changed, 61 insertions(+), 10 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index c511e2a..f991604 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3827,6 +3827,7 @@ static const vshCmdInfo info_shutdown[] = { static const vshCmdOptDef opts_shutdown[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"mode", VSH_OT_STRING, VSH_OFLAG_NONE, N_("shutdown mode: acpi|agent")}, {NULL, 0, 0, NULL} }; @@ -3836,14 +3837,37 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; bool ret = true; const char *name; + const char *mode = NULL; + int flags = 0; + int rv; if (!vshConnectionUsability(ctl, ctl->conn)) return false; + if (vshCommandOptString(cmd, "mode", &mode) < 0) { + vshError(ctl, "%s", _("Invalid type")); + return false; + } + + if (mode) { + if (STREQ(mode, "acpi")) { + flags |= VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN; + } else if (STREQ(mode, "agent")) { + flags |= VIR_DOMAIN_SHUTDOWN_GUEST_AGENT; + } else { + vshError(ctl, _("Unknown mode %s value, expecting 'acpi' or 'agent'"), mode); + return false; + } + } + if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - if (virDomainShutdown(dom) == 0) { + if (flags) + rv = virDomainShutdownFlags(dom, flags); + else + rv = virDomainShutdown(dom); + if (rv == 0) { vshPrint(ctl, _("Domain %s is being shutdown\n"), name); } else { vshError(ctl, _("Failed to shutdown domain %s"), name); @@ -3865,6 +3889,7 @@ static const vshCmdInfo info_reboot[] = { static const vshCmdOptDef opts_reboot[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"mode", VSH_OT_STRING, VSH_OFLAG_NONE, N_("shutdown mode: acpi|agent")}, {NULL, 0, 0, NULL} }; @@ -3874,14 +3899,32 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; bool ret = true; const char *name; + const char *mode = NULL; + int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) return false; + if (vshCommandOptString(cmd, "mode", &mode) < 0) { + vshError(ctl, "%s", _("Invalid type")); + return false; + } + + if (mode) { + if (STREQ(mode, "acpi")) { + flags |= VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN; + } else if (STREQ(mode, "agent")) { + flags |= VIR_DOMAIN_SHUTDOWN_GUEST_AGENT; + } else { + vshError(ctl, _("Unknown mode %s value, expecting 'acpi' or 'agent'"), mode); + return false; + } + } + if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - if (virDomainReboot(dom, 0) == 0) { + if (virDomainReboot(dom, flags) == 0) { vshPrint(ctl, _("Domain %s is being rebooted\n"), name); } else { vshError(ctl, _("Failed to reboot domain %s"), name); diff --git a/tools/virsh.pod b/tools/virsh.pod index c88395b..856be43 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -805,7 +805,7 @@ If I<--live> is specified, set scheduler information of a running guest. If I<--config> is specified, affect the next boot of a persistent guest. If I<--current> is specified, affect the current guest state. -=item B<reboot> I<domain-id> +=item B<reboot> I<domain-id> [I<--mode acpi|agent>] Reboot a domain. This acts just as if the domain had the B<reboot> command run from the console. The command returns as soon as it has @@ -815,6 +815,10 @@ domain actually reboots. The exact behavior of a domain when it reboots is set by the I<on_reboot> parameter in the domain's XML definition. +By default the hypervisor will try to pick a suitable shutdown +method. To specify an alternative method, the I<--mode> parameter +can specify C<acpi> or C<agent>. + =item B<reset> I<domain-id> Reset a domain immediately without any guest shutdown. B<reset> @@ -1188,7 +1192,7 @@ The I<--maximum> flag controls the maximum number of virtual cpus that can be hot-plugged the next time the domain is booted. As such, it must only be used with the I<--config> flag, and not with the I<--live> flag. -=item B<shutdown> I<domain-id> +=item B<shutdown> I<domain-id> [I<--mode acpi|agent>] Gracefully shuts down a domain. This coordinates with the domain OS to perform graceful shutdown, so there is no guarantee that it will @@ -1203,6 +1207,10 @@ be lost once the guest stops running, but the snapshot contents still exist, and a new domain with the same name and UUID can restore the snapshot metadata with B<snapshot-create>. +By default the hypervisor will try to pick a suitable shutdown +method. To specify an alternative method, the I<--mode> parameter +can specify C<acpi> or C<agent>. + =item B<start> I<domain-name> [I<--console>] [I<--paused>] [I<--autodestroy>] [I<--bypass-cache>] [I<--force-boot>] @@ -1796,9 +1804,9 @@ Edit the XML configuration file for a storage pool. This is equivalent to: - virsh pool-dumpxml pool > pool.xml - vi pool.xml (or make changes with your other text editor) - virsh pool-define pool.xml +virsh pool-dumpxml pool > pool.xml +vi pool.xml (or make changes with your other text editor) +virsh pool-define pool.xml except that it does some error checking. @@ -1853,9 +1861,9 @@ pre-existing volume. B<Example> - virsh vol-dumpxml --pool storagepool1 appvolume1 > newvolume.xml - vi newvolume.xml (or make changes with your other text editor) - virsh vol-create differentstoragepool newvolume.xml +virsh vol-dumpxml --pool storagepool1 appvolume1 > newvolume.xml +vi newvolume.xml (or make changes with your other text editor) +virsh vol-create differentstoragepool newvolume.xml =item B<vol-create-from> I<pool-or-uuid> I<FILE> [I<--inputpool> I<pool-or-uuid>] I<vol-name-or-key-or-path> -- 1.7.3.4

On 01/17/2012 04:44 AM, Michal Privoznik wrote:
Extend the 'shutdown' and 'reboot' methods so that they both accept a new argument
--mode acpi|agent
* tools/virsh.c: New args for shutdown/reboot * tools/virsh.pod: Document new args --- tools/virsh.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- tools/virsh.pod | 24 ++++++++++++++++-------- 2 files changed, 61 insertions(+), 10 deletions(-)
@@ -1796,9 +1804,9 @@ Edit the XML configuration file for a storage pool.
This is equivalent to:
- virsh pool-dumpxml pool > pool.xml - vi pool.xml (or make changes with your other text editor) - virsh pool-define pool.xml +virsh pool-dumpxml pool > pool.xml +vi pool.xml (or make changes with your other text editor) +virsh pool-define pool.xml
except that it does some error checking.
@@ -1853,9 +1861,9 @@ pre-existing volume.
B<Example>
- virsh vol-dumpxml --pool storagepool1 appvolume1 > newvolume.xml - vi newvolume.xml (or make changes with your other text editor) - virsh vol-create differentstoragepool newvolume.xml +virsh vol-dumpxml --pool storagepool1 appvolume1 > newvolume.xml +vi newvolume.xml (or make changes with your other text editor) +virsh vol-create differentstoragepool newvolume.xml
These two whitespace-changing hunks look spurious. ACK to the rest of the patch. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

==== These patches are taken from here: https://www.redhat.com/archives/libvir-list/2011-October/msg00135.html I've just rebased and polished them. ==== The QEMU guest agent "/usr/bin/qemu-ga" has some handy functions for controlling the guest, not least, shutdown/reboot and filesystem freeze/thaw. In Fedora 15/16 the semantics of the ACPI power button have been changed to suspend-to-RAM which breaks our current shutdown implementation. By adding support for the agent we gain a more predictable way to shutdown / reboot guests. NB: the code currently has the same "flaw" as the monitor, in so much as we wait forever for a guest agent reply. We need to add a timeout ability to the agent code diff to v1: - Eric's review taken in Daniel P. Berrange (4): QEMU guest agent support Add new virDomainShutdownFlags API Wire up QEMU agent to reboot/shutdown APIs Allow choice of shutdown method via virsh include/libvirt/libvirt.h.in | 15 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/driver.h | 5 + src/esx/esx_driver.c | 11 +- src/libvirt.c | 68 +++- src/libvirt_public.syms | 4 + src/libxl/libxl_driver.c | 12 +- src/openvz/openvz_driver.c | 1 + src/qemu/qemu_agent.c | 1112 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 69 +++ src/qemu/qemu_domain.c | 97 ++++ src/qemu/qemu_domain.h | 22 + src/qemu/qemu_driver.c | 131 ++++- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_process.c | 188 +++++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 8 +- src/remote_protocol-structs | 5 + src/test/test_driver.c | 11 +- src/uml/uml_driver.c | 11 +- src/vbox/vbox_tmpl.c | 11 +- src/vmware/vmware_driver.c | 1 + src/xen/xen_driver.c | 14 +- src/xenapi/xenapi_driver.c | 12 +- tools/virsh.c | 47 ++- tools/virsh.pod | 12 +- 27 files changed, 1832 insertions(+), 40 deletions(-) create mode 100644 src/qemu/qemu_agent.c create mode 100644 src/qemu/qemu_agent.h -- 1.7.3.4

On 23.01.2012 15:48, Michal Privoznik wrote:
==== These patches are taken from here: https://www.redhat.com/archives/libvir-list/2011-October/msg00135.html
I've just rebased and polished them. ====
The QEMU guest agent "/usr/bin/qemu-ga" has some handy functions for controlling the guest, not least, shutdown/reboot and filesystem freeze/thaw.
In Fedora 15/16 the semantics of the ACPI power button have been changed to suspend-to-RAM which breaks our current shutdown implementation.
By adding support for the agent we gain a more predictable way to shutdown / reboot guests.
NB: the code currently has the same "flaw" as the monitor, in so much as we wait forever for a guest agent reply. We need to add a timeout ability to the agent code
diff to v1: - Eric's review taken in
Daniel P. Berrange (4): QEMU guest agent support Add new virDomainShutdownFlags API Wire up QEMU agent to reboot/shutdown APIs Allow choice of shutdown method via virsh
include/libvirt/libvirt.h.in | 15 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/driver.h | 5 + src/esx/esx_driver.c | 11 +- src/libvirt.c | 68 +++- src/libvirt_public.syms | 4 + src/libxl/libxl_driver.c | 12 +- src/openvz/openvz_driver.c | 1 + src/qemu/qemu_agent.c | 1112 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 69 +++ src/qemu/qemu_domain.c | 97 ++++ src/qemu/qemu_domain.h | 22 + src/qemu/qemu_driver.c | 131 ++++- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_process.c | 188 +++++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 8 +- src/remote_protocol-structs | 5 + src/test/test_driver.c | 11 +- src/uml/uml_driver.c | 11 +- src/vbox/vbox_tmpl.c | 11 +- src/vmware/vmware_driver.c | 1 + src/xen/xen_driver.c | 14 +- src/xenapi/xenapi_driver.c | 12 +- tools/virsh.c | 47 ++- tools/virsh.pod | 12 +- 27 files changed, 1832 insertions(+), 40 deletions(-) create mode 100644 src/qemu/qemu_agent.c create mode 100644 src/qemu/qemu_agent.h
Thanks Eric; I've changed the patches as you've suggested and pushed. Michal

There is now a standard QEMU guest agent that can be installed and given a virtio serial channel <channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/> <target type='virtio' name='org.qemu.guest_agent.0'/> </channel> The protocol that runs over the guest agent is JSON based and very similar to the JSON monitor. We can't use exactly the same code because there are some odd differences in the way messages and errors are structured. The qemu_agent.c file is based on a combination and simplification of qemu_monitor.c and qemu_monitor_json.c * src/qemu/qemu_agent.c, src/qemu/qemu_agent.h: Support for talking to the agent for shutdown * src/qemu/qemu_domain.c, src/qemu/qemu_domain.h: Add thread helpers for talking to the agent * src/qemu/qemu_process.c: Connect to agent whenever starting a guest * src/qemu/qemu_monitor_json.c: Make variable static --- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/qemu/qemu_agent.c | 1112 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 69 +++ src/qemu/qemu_domain.c | 97 ++++ src/qemu/qemu_domain.h | 22 + src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_process.c | 188 +++++++ 8 files changed, 1491 insertions(+), 1 deletions(-) create mode 100644 src/qemu/qemu_agent.c create mode 100644 src/qemu/qemu_agent.h diff --git a/po/POTFILES.in b/po/POTFILES.in index ca1db70..0126320 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -57,6 +57,7 @@ src/nwfilter/nwfilter_learnipaddr.c src/openvz/openvz_conf.c src/openvz/openvz_driver.c src/phyp/phyp_driver.c +src/qemu/qemu_agent.c src/qemu/qemu_bridge_filter.c src/qemu/qemu_capabilities.c src/qemu/qemu_cgroup.c diff --git a/src/Makefile.am b/src/Makefile.am index c459f2d..a44446f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -364,6 +364,7 @@ VBOX_DRIVER_EXTRA_DIST = \ vbox/vbox_XPCOMCGlue.c vbox/vbox_XPCOMCGlue.h QEMU_DRIVER_SOURCES = \ + qemu/qemu_agent.c qemu/qemu_agent.h \ qemu/qemu_capabilities.c qemu/qemu_capabilities.h\ qemu/qemu_command.c qemu/qemu_command.h \ qemu/qemu_domain.c qemu/qemu_domain.h \ diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c new file mode 100644 index 0000000..107cb6d --- /dev/null +++ b/src/qemu/qemu_agent.c @@ -0,0 +1,1112 @@ +/* + * qemu_agent.h: interaction with QEMU guest agent + * + * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> + +#include <sys/types.h> +#include <sys/socket.h> +#include <sys/un.h> +#include <poll.h> +#include <unistd.h> +#include <fcntl.h> +#include <string.h> +#include <sys/time.h> + +#include "qemu_agent.h" +#include "qemu_command.h" +#include "memory.h" +#include "logging.h" +#include "virterror_internal.h" +#include "json.h" +#include "virfile.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +#define LINE_ENDING "\n" + +#define DEBUG_IO 0 +#define DEBUG_RAW_IO 0 + +/* When you are the first to uncomment this, + * don't forget to uncomment the corresponding + * part in qemuAgentIOProcessEvent as well. + * +static struct { + const char *type; + void (*handler)(qemuAgentPtr mon, virJSONValuePtr data); +} eventHandlers[] = { +}; +*/ + +typedef struct _qemuAgentMessage qemuAgentMessage; +typedef qemuAgentMessage *qemuAgentMessagePtr; + +struct _qemuAgentMessage { + char *txBuffer; + int txOffset; + int txLength; + + /* Used by the JSON monitor to hold reply / error */ + char *rxBuffer; + int rxLength; + void *rxObject; + + /* True if rxBuffer / rxObject are ready, or a + * fatal error occurred on the monitor channel + */ + bool finished; +}; + + +struct _qemuAgent { + virMutex lock; /* also used to protect fd */ + virCond notify; + + int refs; + + int fd; + int watch; + + bool connectPending; + + virDomainObjPtr vm; + + qemuAgentCallbacksPtr cb; + + /* If there's a command being processed this will be + * non-NULL */ + qemuAgentMessagePtr 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 */ + virError lastError; +}; + +#if DEBUG_RAW_IO +# include <c-ctype.h> +static char * +qemuAgentEscapeNonPrintable(const char *text) +{ + int i; + virBuffer buf = VIR_BUFFER_INITIALIZER; + for (i = 0 ; text[i] != '\0' ; i++) { + if (text[i] == '\\') + virBufferAddLit(&buf, "\\\\"); + else if (c_isprint(text[i]) || text[i] == '\n' || + (text[i] == '\r' && text[i+1] == '\n')) + virBufferAddChar(&buf, text[i]); + else + virBufferAsprintf(&buf, "\\x%02x", text[i]); + } + return virBufferContentAndReset(&buf); +} +#endif + +void qemuAgentLock(qemuAgentPtr mon) +{ + virMutexLock(&mon->lock); +} + + +void qemuAgentUnlock(qemuAgentPtr mon) +{ + virMutexUnlock(&mon->lock); +} + + +static void qemuAgentFree(qemuAgentPtr mon) +{ + VIR_DEBUG("mon=%p", mon); + if (mon->cb && mon->cb->destroy) + (mon->cb->destroy)(mon, mon->vm); + ignore_value(virCondDestroy(&mon->notify)); + virMutexDestroy(&mon->lock); + VIR_FREE(mon->buffer); + VIR_FREE(mon); +} + +int qemuAgentRef(qemuAgentPtr mon) +{ + mon->refs++; + return mon->refs; +} + +int qemuAgentUnref(qemuAgentPtr mon) +{ + mon->refs--; + + if (mon->refs == 0) { + qemuAgentUnlock(mon); + qemuAgentFree(mon); + return 0; + } + + return mon->refs; +} + +static void +qemuAgentUnwatch(void *monitor) +{ + qemuAgentPtr mon = monitor; + + qemuAgentLock(mon); + if (qemuAgentUnref(mon) > 0) + qemuAgentUnlock(mon); +} + +static int +qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool *inProgress) +{ + struct sockaddr_un addr; + int monfd; + int timeout = 3; /* In seconds */ + int ret, i = 0; + + *inProgress = false; + + if ((monfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { + virReportSystemError(errno, + "%s", _("failed to create socket")); + return -1; + } + + if (virSetNonBlock(monfd) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to put monitor into non-blocking mode")); + goto error; + } + + if (virSetCloseExec(monfd) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to set monitor close-on-exec flag")); + goto error; + } + + memset(&addr, 0, sizeof(addr)); + addr.sun_family = AF_UNIX; + if (virStrcpyStatic(addr.sun_path, monitor) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Agent path %s too big for destination"), monitor); + goto error; + } + + do { + ret = connect(monfd, (struct sockaddr *) &addr, sizeof(addr)); + + if (ret == 0) + break; + + if ((errno == ENOENT || errno == ECONNREFUSED) && + virKillProcess(cpid, 0) == 0) { + /* ENOENT : Socket may not have shown up yet + * ECONNREFUSED : Leftover socket hasn't been removed yet */ + continue; + } + + if ((errno == EINPROGRESS) || + (errno == EAGAIN)) { + VIR_DEBUG("Connection attempt continuing in background"); + *inProgress = true; + ret = 0; + break; + } + + virReportSystemError(errno, "%s", + _("failed to connect to monitor socket")); + goto error; + + } while ((++i <= timeout*5) && (usleep(.2 * 1000000) <= 0)); + + if (ret != 0) { + virReportSystemError(errno, "%s", + _("monitor socket did not show up.")); + goto error; + } + + return monfd; + +error: + VIR_FORCE_CLOSE(monfd); + return -1; +} + +static int +qemuAgentOpenPty(const char *monitor) +{ + int monfd; + + if ((monfd = open(monitor, O_RDWR | O_NONBLOCK)) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to open monitor path %s"), monitor); + return -1; + } + + if (virSetCloseExec(monfd) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to set monitor close-on-exec flag")); + goto error; + } + + return monfd; + +error: + VIR_FORCE_CLOSE(monfd); + return -1; +} + + +static int +qemuAgentIOProcessEvent(qemuAgentPtr mon, + virJSONValuePtr obj) +{ + const char *type; + VIR_DEBUG("mon=%p obj=%p", mon, obj); + + type = virJSONValueObjectGetString(obj, "event"); + if (!type) { + VIR_WARN("missing event type in message"); + errno = EINVAL; + return -1; + } + +/* + for (i = 0 ; i < ARRAY_CARDINALITY(eventHandlers) ; i++) { + if (STREQ(eventHandlers[i].type, type)) { + virJSONValuePtr data = virJSONValueObjectGet(obj, "data"); + VIR_DEBUG("handle %s handler=%p data=%p", type, + eventHandlers[i].handler, data); + (eventHandlers[i].handler)(mon, data); + break; + } + } +*/ + return 0; +} + +static int +qemuAgentIOProcessLine(qemuAgentPtr mon, + const char *line, + qemuAgentMessagePtr msg) +{ + virJSONValuePtr obj = NULL; + int ret = -1; + + VIR_DEBUG("Line [%s]", line); + + if (!(obj = virJSONValueFromString(line))) + goto cleanup; + + if (obj->type != VIR_JSON_TYPE_OBJECT) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Parsed JSON reply '%s' isn't an object"), line); + goto cleanup; + } + + if (virJSONValueObjectHasKey(obj, "QMP") == 1) { + ret = 0; + } else if (virJSONValueObjectHasKey(obj, "event") == 1) { + ret = qemuAgentIOProcessEvent(mon, obj); + } else if (virJSONValueObjectHasKey(obj, "error") == 1 || + virJSONValueObjectHasKey(obj, "return") == 1) { + if (msg) { + msg->rxObject = obj; + msg->finished = 1; + obj = NULL; + ret = 0; + } else { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected JSON reply '%s'"), line); + } + } else { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown JSON reply '%s'"), line); + } + +cleanup: + virJSONValueFree(obj); + return ret; +} + +static int qemuAgentIOProcessData(qemuAgentPtr mon, + char *data, + size_t len, + qemuAgentMessagePtr msg) +{ + int used = 0; + int i = 0; +#if DEBUG_IO +# if DEBUG_RAW_IO + char *str1 = qemuAgentEscapeNonPrintable(data); + VIR_ERROR("[%s]", str1); + VIR_FREE(str1); +# else + VIR_DEBUG("Data %zu bytes [%s]", len, data); +# endif +#endif + + while (used < len) { + char *nl = strstr(data + used, LINE_ENDING); + + if (nl) { + int got = nl - (data + used); + for (i = 0; i < strlen(LINE_ENDING); i++) + data[used + got + i] = '\0'; + if (qemuAgentIOProcessLine(mon, data + used, msg) < 0) { + return -1; + } + used += got + strlen(LINE_ENDING); + } else { + break; + } + } + + VIR_DEBUG("Total used %d bytes out of %zd available in buffer", used, len); + return used; +} + +/* This method processes data that has been received + * from the monitor. Looking for async events and + * replies/errors. + */ +static int +qemuAgentIOProcess(qemuAgentPtr mon) +{ + int len; + qemuAgentMessagePtr msg = NULL; + + /* See if there's a message ready for reply; that is, + * one that has completed writing all its data. + */ + if (mon->msg && mon->msg->txOffset == mon->msg->txLength) + msg = mon->msg; + +#if DEBUG_IO +# if DEBUG_RAW_IO + char *str1 = qemuAgentEscapeNonPrintable(msg ? msg->txBuffer : ""); + char *str2 = qemuAgentEscapeNonPrintable(mon->buffer); + VIR_ERROR(_("Process %zu %p %p [[[%s]]][[[%s]]]"), + mon->bufferOffset, mon->msg, msg, str1, str2); + VIR_FREE(str1); + VIR_FREE(str2); +# else + VIR_DEBUG("Process %zu", mon->bufferOffset); +# endif +#endif + + len = qemuAgentIOProcessData(mon, + mon->buffer, mon->bufferOffset, + msg); + + if (len < 0) + 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; + } +#if DEBUG_IO + VIR_DEBUG("Process done %zu used %d", mon->bufferOffset, len); +#endif + if (msg && msg->finished) + virCondBroadcast(&mon->notify); + return len; +} + + +static int +qemuAgentIOConnect(qemuAgentPtr mon) +{ + int optval; + socklen_t optlen; + + VIR_DEBUG("Checking on background connection status"); + + mon->connectPending = false; + + optlen = sizeof(optval); + + if (getsockopt(mon->fd, SOL_SOCKET, SO_ERROR, + &optval, &optlen) < 0) { + virReportSystemError(errno, "%s", + _("Cannot check socket connection status")); + return -1; + } + + if (optval != 0) { + virReportSystemError(optval, "%s", + _("Cannot connect to agent socket")); + return -1; + } + + VIR_DEBUG("Agent is now connected"); + return 0; +} + +/* + * Called when the monitor is able to write data + * Call this function while holding the monitor lock. + */ +static int +qemuAgentIOWrite(qemuAgentPtr mon) +{ + int done; + + /* If no active message, or fully transmitted, then no-op */ + if (!mon->msg || mon->msg->txOffset == mon->msg->txLength) + return 0; + + done = safewrite(mon->fd, + mon->msg->txBuffer + mon->msg->txOffset, + mon->msg->txLength - mon->msg->txOffset); + + if (done < 0) { + if (errno == EAGAIN) + return 0; + + virReportSystemError(errno, "%s", + _("Unable to write to monitor")); + return -1; + } + mon->msg->txOffset += done; + return done; +} + +/* + * Called when the monitor has incoming data to read + * Call this function while holding the monitor lock. + * + * Returns -1 on error, or number of bytes read + */ +static int +qemuAgentIORead(qemuAgentPtr mon) +{ + size_t avail = mon->bufferLength - mon->bufferOffset; + int ret = 0; + + if (avail < 1024) { + if (VIR_REALLOC_N(mon->buffer, + mon->bufferLength + 1024) < 0) { + virReportOOMError(); + 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; + virReportSystemError(errno, "%s", + _("Unable to read from monitor")); + ret = -1; + break; + } + if (got == 0) + break; + + ret += got; + avail -= got; + mon->bufferOffset += got; + mon->buffer[mon->bufferOffset] = '\0'; + } + +#if DEBUG_IO + VIR_DEBUG("Now read %zu bytes of data", mon->bufferOffset); +#endif + + return ret; +} + + +static void qemuAgentUpdateWatch(qemuAgentPtr mon) +{ + int events = + VIR_EVENT_HANDLE_HANGUP | + VIR_EVENT_HANDLE_ERROR; + + if (mon->lastError.code == VIR_ERR_OK) { + events |= VIR_EVENT_HANDLE_READABLE; + + if (mon->msg && mon->msg->txOffset < mon->msg->txLength) + events |= VIR_EVENT_HANDLE_WRITABLE; + } + + virEventUpdateHandle(mon->watch, events); +} + + +static void +qemuAgentIO(int watch, int fd, int events, void *opaque) { + qemuAgentPtr mon = opaque; + bool error = false; + bool eof = false; + + /* lock access to the monitor and protect fd */ + qemuAgentLock(mon); + qemuAgentRef(mon); +#if DEBUG_IO + VIR_DEBUG("Agent %p I/O on watch %d fd %d events %d", mon, watch, fd, events); +#endif + + if (mon->fd != fd || mon->watch != watch) { + if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) + eof = true; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("event from unexpected fd %d!=%d / watch %d!=%d"), + mon->fd, fd, mon->watch, watch); + error = true; + } else if (mon->lastError.code != VIR_ERR_OK) { + if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) + eof = true; + error = true; + } else { + if (events & VIR_EVENT_HANDLE_WRITABLE) { + if (mon->connectPending) { + if (qemuAgentIOConnect(mon) < 0) + error = true; + } else { + if (qemuAgentIOWrite(mon) < 0) + error = true; + } + events &= ~VIR_EVENT_HANDLE_WRITABLE; + } + + if (!error && + events & VIR_EVENT_HANDLE_READABLE) { + int got = qemuAgentIORead(mon); + events &= ~VIR_EVENT_HANDLE_READABLE; + if (got < 0) { + error = true; + } else if (got == 0) { + eof = true; + } else { + /* Ignore hangup/error events if we read some data, to + * give time for that data to be consumed */ + events = 0; + + if (qemuAgentIOProcess(mon) < 0) + error = true; + } + } + + if (!error && + events & VIR_EVENT_HANDLE_HANGUP) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("End of file from monitor")); + eof = 1; + events &= ~VIR_EVENT_HANDLE_HANGUP; + } + + if (!error && !eof && + events & VIR_EVENT_HANDLE_ERROR) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid file descriptor while waiting for monitor")); + eof = 1; + events &= ~VIR_EVENT_HANDLE_ERROR; + } + if (!error && events) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unhandled event %d for monitor fd %d"), + events, mon->fd); + error = 1; + } + } + + if (error || eof) { + if (mon->lastError.code != VIR_ERR_OK) { + /* Already have an error, so clear any new error */ + virResetLastError(); + } else { + virErrorPtr err = virGetLastError(); + if (!err) + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Error while processing monitor IO")); + virCopyLastError(&mon->lastError); + virResetLastError(); + } + + VIR_DEBUG("Error on monitor %s", NULLSTR(mon->lastError.message)); + /* If IO process resulted in an error & we have a message, + * then wakeup that waiter */ + if (mon->msg && !mon->msg->finished) { + mon->msg->finished = 1; + virCondSignal(&mon->notify); + } + } + + qemuAgentUpdateWatch(mon); + + /* 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 (eof) { + void (*eofNotify)(qemuAgentPtr, virDomainObjPtr) + = mon->cb->eofNotify; + virDomainObjPtr vm = mon->vm; + + /* Make sure anyone waiting wakes up now */ + virCondSignal(&mon->notify); + if (qemuAgentUnref(mon) > 0) + qemuAgentUnlock(mon); + VIR_DEBUG("Triggering EOF callback"); + (eofNotify)(mon, vm); + } else if (error) { + void (*errorNotify)(qemuAgentPtr, virDomainObjPtr) + = mon->cb->errorNotify; + virDomainObjPtr vm = mon->vm; + + /* Make sure anyone waiting wakes up now */ + virCondSignal(&mon->notify); + if (qemuAgentUnref(mon) > 0) + qemuAgentUnlock(mon); + VIR_DEBUG("Triggering error callback"); + (errorNotify)(mon, vm); + } else { + if (qemuAgentUnref(mon) > 0) + qemuAgentUnlock(mon); + } +} + + +qemuAgentPtr +qemuAgentOpen(virDomainObjPtr vm, + virDomainChrSourceDefPtr config, + qemuAgentCallbacksPtr cb) +{ + qemuAgentPtr mon; + + if (!cb || !cb->eofNotify) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("EOF notify callback must be supplied")); + return NULL; + } + + if (VIR_ALLOC(mon) < 0) { + virReportOOMError(); + return NULL; + } + + if (virMutexInit(&mon->lock) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot initialize monitor mutex")); + VIR_FREE(mon); + return NULL; + } + if (virCondInit(&mon->notify) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot initialize monitor condition")); + virMutexDestroy(&mon->lock); + VIR_FREE(mon); + return NULL; + } + mon->fd = -1; + mon->refs = 1; + mon->vm = vm; + mon->cb = cb; + qemuAgentLock(mon); + + switch (config->type) { + case VIR_DOMAIN_CHR_TYPE_UNIX: + mon->fd = qemuAgentOpenUnix(config->data.nix.path, vm->pid, + &mon->connectPending); + break; + + case VIR_DOMAIN_CHR_TYPE_PTY: + mon->fd = qemuAgentOpenPty(config->data.file.path); + break; + + default: + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to handle monitor type: %s"), + virDomainChrTypeToString(config->type)); + goto cleanup; + } + + if (mon->fd == -1) + goto cleanup; + + if ((mon->watch = virEventAddHandle(mon->fd, + VIR_EVENT_HANDLE_HANGUP | + VIR_EVENT_HANDLE_ERROR | + VIR_EVENT_HANDLE_READABLE | + (mon->connectPending ? + VIR_EVENT_HANDLE_WRITABLE : + 0), + qemuAgentIO, + mon, qemuAgentUnwatch)) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to register monitor events")); + goto cleanup; + } + qemuAgentRef(mon); + + VIR_DEBUG("New mon %p fd =%d watch=%d", mon, mon->fd, mon->watch); + qemuAgentUnlock(mon); + + return mon; + +cleanup: + /* We don't want the 'destroy' callback invoked during + * cleanup from construction failure, because that can + * give a double-unref on virDomainObjPtr in the caller, + * so kill the callbacks now. + */ + mon->cb = NULL; + qemuAgentUnlock(mon); + qemuAgentClose(mon); + return NULL; +} + + +void qemuAgentClose(qemuAgentPtr mon) +{ + if (!mon) + return; + + VIR_DEBUG("mon=%p", mon); + + qemuAgentLock(mon); + + if (mon->fd >= 0) { + if (mon->watch) + virEventRemoveHandle(mon->watch); + VIR_FORCE_CLOSE(mon->fd); + } + + if (qemuAgentUnref(mon) > 0) + qemuAgentUnlock(mon); +} + + +static int qemuAgentSend(qemuAgentPtr mon, + qemuAgentMessagePtr msg) +{ + int ret = -1; + + /* Check whether qemu quit unexpectedly */ + if (mon->lastError.code != VIR_ERR_OK) { + VIR_DEBUG("Attempt to send command while error is set %s", + NULLSTR(mon->lastError.message)); + virSetError(&mon->lastError); + return -1; + } + + mon->msg = msg; + qemuAgentUpdateWatch(mon); + + while (!mon->msg->finished) { + if (virCondWait(&mon->notify, &mon->lock) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to wait on monitor condition")); + goto cleanup; + } + } + + if (mon->lastError.code != VIR_ERR_OK) { + VIR_DEBUG("Send command resulted in error %s", + NULLSTR(mon->lastError.message)); + virSetError(&mon->lastError); + goto cleanup; + } + + ret = 0; + +cleanup: + mon->msg = NULL; + qemuAgentUpdateWatch(mon); + + return ret; +} + + +static int +qemuAgentCommand(qemuAgentPtr mon, + virJSONValuePtr cmd, + virJSONValuePtr *reply) +{ + int ret = -1; + qemuAgentMessage msg; + char *cmdstr = NULL; + + *reply = NULL; + + memset(&msg, 0, sizeof msg); + + if (!(cmdstr = virJSONValueToString(cmd))) { + virReportOOMError(); + goto cleanup; + } + if (virAsprintf(&msg.txBuffer, "%s" LINE_ENDING, cmdstr) < 0) { + virReportOOMError(); + goto cleanup; + } + msg.txLength = strlen(msg.txBuffer); + + VIR_DEBUG("Send command '%s' for write", cmdstr); + + ret = qemuAgentSend(mon, &msg); + + VIR_DEBUG("Receive command reply ret=%d rxObject=%p", + ret, msg.rxObject); + + + if (ret == 0) { + if (!msg.rxObject) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing monitor reply object")); + ret = -1; + } else { + *reply = msg.rxObject; + } + } + +cleanup: + VIR_FREE(cmdstr); + VIR_FREE(msg.txBuffer); + + return ret; +} + + +/* Ignoring OOM in this method, since we're already reporting + * a more important error + * + * XXX see qerror.h for different klasses & fill out useful params + */ +static const char * +qemuAgentStringifyError(virJSONValuePtr error) +{ + const char *klass = virJSONValueObjectGetString(error, "class"); + const char *detail = NULL; + + /* The QMP 'desc' field is usually sufficient for our generic + * error reporting needs. + */ + if (klass) + detail = virJSONValueObjectGetString(error, "desc"); + + + if (!detail) + detail = "unknown QEMU command error"; + + return detail; +} + +static const char * +qemuAgentCommandName(virJSONValuePtr cmd) +{ + const char *name = virJSONValueObjectGetString(cmd, "execute"); + if (name) + return name; + else + return "<unknown>"; +} + +static int +qemuAgentCheckError(virJSONValuePtr cmd, + virJSONValuePtr reply) +{ + if (virJSONValueObjectHasKey(reply, "error")) { + virJSONValuePtr error = virJSONValueObjectGet(reply, "error"); + char *cmdstr = virJSONValueToString(cmd); + char *replystr = virJSONValueToString(reply); + + /* Log the full JSON formatted command & error */ + VIR_DEBUG("unable to execute QEMU command %s: %s", + cmdstr, replystr); + + /* Only send the user the command name + friendly error */ + if (!error) + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to execute QEMU command '%s'"), + qemuAgentCommandName(cmd)); + else + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to execute QEMU command '%s': %s"), + qemuAgentCommandName(cmd), + qemuAgentStringifyError(error)); + + VIR_FREE(cmdstr); + VIR_FREE(replystr); + return -1; + } else if (!virJSONValueObjectHasKey(reply, "return")) { + char *cmdstr = virJSONValueToString(cmd); + char *replystr = virJSONValueToString(reply); + + VIR_DEBUG("Neither 'return' nor 'error' is set in the JSON reply %s: %s", + cmdstr, replystr); + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to execute QEMU command '%s'"), + qemuAgentCommandName(cmd)); + VIR_FREE(cmdstr); + VIR_FREE(replystr); + return -1; + } + return 0; +} + +static virJSONValuePtr ATTRIBUTE_SENTINEL +qemuAgentMakeCommand(const char *cmdname, + ...) +{ + virJSONValuePtr obj; + virJSONValuePtr jargs = NULL; + va_list args; + char *key; + + va_start(args, cmdname); + + if (!(obj = virJSONValueNewObject())) + goto no_memory; + + if (virJSONValueObjectAppendString(obj, "execute", cmdname) < 0) + goto no_memory; + + while ((key = va_arg(args, char *)) != NULL) { + int ret; + char type; + + if (strlen(key) < 3) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("argument key '%s' is too short, missing type prefix"), + key); + goto error; + } + + /* Keys look like s:name the first letter is a type code */ + type = key[0]; + key += 2; + + if (!jargs && + !(jargs = virJSONValueNewObject())) + goto no_memory; + + /* This doesn't support maps/arrays. This hasn't + * proved to be a problem..... yet :-) */ + switch (type) { + case 's': { + char *val = va_arg(args, char *); + ret = virJSONValueObjectAppendString(jargs, key, val); + } break; + case 'i': { + int val = va_arg(args, int); + ret = virJSONValueObjectAppendNumberInt(jargs, key, val); + } break; + case 'u': { + unsigned int val = va_arg(args, unsigned int); + ret = virJSONValueObjectAppendNumberUint(jargs, key, val); + } break; + case 'I': { + long long val = va_arg(args, long long); + ret = virJSONValueObjectAppendNumberLong(jargs, key, val); + } break; + case 'U': { + /* qemu silently truncates numbers larger than LLONG_MAX, + * so passing the full range of unsigned 64 bit integers + * is not safe here. Pass them as signed 64 bit integers + * instead. + */ + long long val = va_arg(args, long long); + ret = virJSONValueObjectAppendNumberLong(jargs, key, val); + } break; + case 'd': { + double val = va_arg(args, double); + ret = virJSONValueObjectAppendNumberDouble(jargs, key, val); + } break; + case 'b': { + int val = va_arg(args, int); + ret = virJSONValueObjectAppendBoolean(jargs, key, val); + } break; + case 'n': { + ret = virJSONValueObjectAppendNull(jargs, key); + } break; + default: + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported data type '%c' for arg '%s'"), type, key - 2); + goto error; + } + if (ret < 0) + goto no_memory; + } + + if (jargs && + virJSONValueObjectAppend(obj, "arguments", jargs) < 0) + goto no_memory; + + va_end(args); + + return obj; + +no_memory: + virReportOOMError(); +error: + virJSONValueFree(obj); + virJSONValueFree(jargs); + va_end(args); + return NULL; +} + +VIR_ENUM_DECL(qemuAgentShutdownMode); + +VIR_ENUM_IMPL(qemuAgentShutdownMode, + QEMU_AGENT_SHUTDOWN_LAST, + "powerdown", "reboot", "halt"); + +int qemuAgentShutdown(qemuAgentPtr mon, + qemuAgentShutdownMode mode) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuAgentMakeCommand("guest-shutdown", + "s:mode", qemuAgentShutdownModeTypeToString(mode), + NULL); + if (!cmd) + return -1; + + ret = qemuAgentCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuAgentCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h new file mode 100644 index 0000000..93c2ae7 --- /dev/null +++ b/src/qemu/qemu_agent.h @@ -0,0 +1,69 @@ +/* + * qemu_agent.h: interaction with QEMU guest agent + * + * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + + +#ifndef __QEMU_AGENT_H__ +# define __QEMU_AGENT_H__ + +# include "internal.h" +# include "domain_conf.h" + +typedef struct _qemuAgent qemuAgent; +typedef qemuAgent *qemuAgentPtr; + +typedef struct _qemuAgentCallbacks qemuAgentCallbacks; +typedef qemuAgentCallbacks *qemuAgentCallbacksPtr; +struct _qemuAgentCallbacks { + void (*destroy)(qemuAgentPtr mon, + virDomainObjPtr vm); + void (*eofNotify)(qemuAgentPtr mon, + virDomainObjPtr vm); + void (*errorNotify)(qemuAgentPtr mon, + virDomainObjPtr vm); +}; + + +qemuAgentPtr qemuAgentOpen(virDomainObjPtr vm, + virDomainChrSourceDefPtr config, + qemuAgentCallbacksPtr cb); + +void qemuAgentLock(qemuAgentPtr mon); +void qemuAgentUnlock(qemuAgentPtr mon); + +int qemuAgentRef(qemuAgentPtr mon); +int qemuAgentUnref(qemuAgentPtr mon) ATTRIBUTE_RETURN_CHECK; + +void qemuAgentClose(qemuAgentPtr mon); + +typedef enum { + QEMU_AGENT_SHUTDOWN_POWERDOWN, + QEMU_AGENT_SHUTDOWN_REBOOT, + QEMU_AGENT_SHUTDOWN_HALT, + + QEMU_AGENT_SHUTDOWN_LAST, +} qemuAgentShutdownMode; + +int qemuAgentShutdown(qemuAgentPtr mon, + qemuAgentShutdownMode mode); + +#endif /* __QEMU_AGENT_H__ */ diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cfe0ece..d56e617 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -219,6 +219,10 @@ static void qemuDomainObjPrivateFree(void *data) VIR_ERROR(_("Unexpected QEMU monitor still active during domain deletion")); qemuMonitorClose(priv->mon); } + if (priv->agent) { + VIR_ERROR(_("Unexpected QEMU agent still active during domain deletion")); + qemuAgentClose(priv->agent); + } VIR_FREE(priv); } @@ -1025,6 +1029,99 @@ void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, qemuDomainObjExitMonitorInternal(driver, true, obj); } + + +static int +qemuDomainObjEnterAgentInternal(struct qemud_driver *driver, + bool driver_locked, + virDomainObjPtr obj) +{ + qemuDomainObjPrivatePtr priv = obj->privateData; + + qemuAgentLock(priv->agent); + qemuAgentRef(priv->agent); + ignore_value(virTimeMillisNow(&priv->agentStart)); + virDomainObjUnlock(obj); + if (driver_locked) + qemuDriverUnlock(driver); + + return 0; +} + +static void ATTRIBUTE_NONNULL(1) +qemuDomainObjExitAgentInternal(struct qemud_driver *driver, + bool driver_locked, + virDomainObjPtr obj) +{ + qemuDomainObjPrivatePtr priv = obj->privateData; + int refs; + + refs = qemuAgentUnref(priv->agent); + + if (refs > 0) + qemuAgentUnlock(priv->agent); + + if (driver_locked) + qemuDriverLock(driver); + virDomainObjLock(obj); + + priv->agentStart = 0; + if (refs == 0) { + priv->agent = NULL; + } +} + +/* + * obj must be locked before calling, qemud_driver must be unlocked + * + * To be called immediately before any QEMU agent API call + * Must have already either called qemuDomainObjBeginJob() and checked + * that the VM is still active; + * + * To be followed with qemuDomainObjExitAgent() once complete + */ +void qemuDomainObjEnterAgent(struct qemud_driver *driver, + virDomainObjPtr obj) +{ + ignore_value(qemuDomainObjEnterAgentInternal(driver, false, obj)); +} + +/* obj must NOT be locked before calling, qemud_driver must be unlocked + * + * Should be paired with an earlier qemuDomainObjEnterAgent() call + */ +void qemuDomainObjExitAgent(struct qemud_driver *driver, + virDomainObjPtr obj) +{ + qemuDomainObjExitAgentInternal(driver, false, obj); +} + +/* + * obj must be locked before calling, qemud_driver must be locked + * + * To be called immediately before any QEMU agent API call + * Must have already either called qemuDomainObjBeginJobWithDriver() and + * checked that the VM is still active; may not be used for nested async jobs. + * + * To be followed with qemuDomainObjExitAgentWithDriver() once complete + */ +void qemuDomainObjEnterAgentWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj) +{ + ignore_value(qemuDomainObjEnterAgentInternal(driver, true, obj)); +} + +/* obj must NOT be locked before calling, qemud_driver must be unlocked, + * and will be locked after returning + * + * Should be paired with an earlier qemuDomainObjEnterAgentWithDriver() call + */ +void qemuDomainObjExitAgentWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj) +{ + qemuDomainObjExitAgentInternal(driver, true, obj); +} + void qemuDomainObjEnterRemoteWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f40fa09..222d80e 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -27,6 +27,7 @@ # include "threads.h" # include "domain_conf.h" # include "qemu_monitor.h" +# include "qemu_agent.h" # include "qemu_conf.h" # include "bitmap.h" @@ -102,6 +103,11 @@ struct _qemuDomainObjPrivate { int monJSON; bool monError; unsigned long long monStart; + + qemuAgentPtr agent; + bool agentError; + unsigned long long agentStart; + bool gotShutdown; bool beingDestroyed; char *pidfile; @@ -192,6 +198,22 @@ int qemuDomainObjEnterMonitorAsync(struct qemud_driver *driver, void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + + +void qemuDomainObjEnterAgent(struct qemud_driver *driver, + virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +void qemuDomainObjExitAgent(struct qemud_driver *driver, + virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +void qemuDomainObjEnterAgentWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +void qemuDomainObjExitAgentWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + + void qemuDomainObjEnterRemoteWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 4a76fc0..11fdc0d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -59,7 +59,7 @@ static void qemuMonitorJSONHandleVNCInitialize(qemuMonitorPtr mon, virJSONValueP static void qemuMonitorJSONHandleVNCDisconnect(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleBlockJob(qemuMonitorPtr mon, virJSONValuePtr data); -struct { +static struct { const char *type; void (*handler)(qemuMonitorPtr mon, virJSONValuePtr data); } eventHandlers[] = { diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e16ca07..d22020b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -107,6 +107,164 @@ qemuProcessRemoveDomainStatus(struct qemud_driver *driver, extern struct qemud_driver *qemu_driver; /* + * This is a callback registered with a qemuAgentPtr instance, + * and to be invoked when the agent console hits an end of file + * condition, or error, thus indicating VM shutdown should be + * performed + */ +static void +qemuProcessHandleAgentEOF(qemuAgentPtr agent ATTRIBUTE_UNUSED, + virDomainObjPtr vm) +{ + struct qemud_driver *driver = qemu_driver; + qemuDomainObjPrivatePtr priv; + + VIR_DEBUG("Received EOF from agent on %p '%s'", vm, vm->def->name); + + qemuDriverLock(driver); + virDomainObjLock(vm); + + priv = vm->privateData; + + qemuAgentClose(agent); + priv->agent = NULL; + + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); +} + + +/* + * This is invoked when there is some kind of error + * parsing data to/from the agent. The VM can continue + * to run, but no further agent commands will be + * allowed + */ +static void +qemuProcessHandleAgentError(qemuAgentPtr agent ATTRIBUTE_UNUSED, + virDomainObjPtr vm) +{ + struct qemud_driver *driver = qemu_driver; + qemuDomainObjPrivatePtr priv; + + VIR_DEBUG("Received error from agent on %p '%s'", vm, vm->def->name); + + qemuDriverLock(driver); + virDomainObjLock(vm); + + priv = vm->privateData; + + priv->agentError = true; + + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); +} + +static void qemuProcessHandleAgentDestroy(qemuAgentPtr agent, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv; + + virDomainObjLock(vm); + priv = vm->privateData; + if (priv->agent == agent) + priv->agent = NULL; + if (virDomainObjUnref(vm) > 0) + virDomainObjUnlock(vm); +} + + +static qemuAgentCallbacks agentCallbacks = { + .destroy = qemuProcessHandleAgentDestroy, + .eofNotify = qemuProcessHandleAgentEOF, + .errorNotify = qemuProcessHandleAgentError, +}; + +static virDomainChrSourceDefPtr +qemuFindAgentConfig(virDomainDefPtr def) +{ + virDomainChrSourceDefPtr config = NULL; + int i; + + for (i = 0 ; i < def->nchannels ; i++) { + virDomainChrDefPtr channel = def->channels[i]; + + if (channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) + continue; + + if (STREQ(channel->target.name, "org.qemu.guest_agent.0")) { + config = &channel->source; + break; + } + } + + return config; +} + +static int +qemuConnectAgent(struct qemud_driver *driver, virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret = -1; + qemuAgentPtr agent = NULL; + virDomainChrSourceDefPtr config = qemuFindAgentConfig(vm->def); + + if (!config) + return 0; + + if (virSecurityManagerSetDaemonSocketLabel(driver->securityManager, + vm->def) < 0) { + VIR_ERROR(_("Failed to set security context for agent for %s"), + vm->def->name); + goto cleanup; + } + + /* Hold an extra reference because we can't allow 'vm' to be + * deleted while the agent is active */ + virDomainObjRef(vm); + + ignore_value(virTimeMillisNow(&priv->agentStart)); + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + + agent = qemuAgentOpen(vm, + config, + &agentCallbacks); + + qemuDriverLock(driver); + virDomainObjLock(vm); + priv->agentStart = 0; + + if (virSecurityManagerClearSocketLabel(driver->securityManager, + vm->def) < 0) { + VIR_ERROR(_("Failed to clear security context for agent for %s"), + vm->def->name); + goto cleanup; + } + + /* Safe to ignore value since ref count was incremented above */ + if (agent == NULL) + ignore_value(virDomainObjUnref(vm)); + + if (!virDomainObjIsActive(vm)) { + qemuAgentClose(agent); + goto cleanup; + } + priv->agent = agent; + + if (priv->agent == NULL) { + VIR_INFO("Failed to connect agent for %s", vm->def->name); + goto cleanup; + } + + ret = 0; + +cleanup: + return ret; +} + + +/* * 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 @@ -2691,6 +2849,14 @@ qemuProcessReconnect(void *opaque) if (qemuConnectMonitor(driver, obj) < 0) goto error; + /* Failure to connect to agent shouldn't be fatal */ + if (qemuConnectAgent(driver, obj) < 0) { + VIR_WARN("Cannot connect to QEMU guest agent for %s", + obj->def->name); + virResetLastError(); + priv->agentError = true; + } + if (qemuUpdateActivePciHostdevs(driver, obj->def) < 0) { goto error; } @@ -3258,6 +3424,14 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessWaitForMonitor(driver, vm, priv->qemuCaps, pos) < 0) goto cleanup; + /* Failure to connect to agent shouldn't be fatal */ + if (qemuConnectAgent(driver, vm) < 0) { + VIR_WARN("Cannot connect to QEMU guest agent for %s", + vm->def->name); + virResetLastError(); + priv->agentError = true; + } + VIR_DEBUG("Detecting VCPU PIDs"); if (qemuProcessDetectVcpuPIDs(driver, vm) < 0) goto cleanup; @@ -3460,6 +3634,12 @@ void qemuProcessStop(struct qemud_driver *driver, } } + if (priv->agent) { + qemuAgentClose(priv->agent); + priv->agent = NULL; + priv->agentError = false; + } + if (priv->mon) qemuMonitorClose(priv->mon); @@ -3713,6 +3893,14 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, if (qemuProcessWaitForMonitor(driver, vm, priv->qemuCaps, -1) < 0) goto cleanup; + /* Failure to connect to agent shouldn't be fatal */ + if (qemuConnectAgent(driver, vm) < 0) { + VIR_WARN("Cannot connect to QEMU guest agent for %s", + vm->def->name); + virResetLastError(); + priv->agentError = true; + } + VIR_DEBUG("Detecting VCPU PIDs"); if (qemuProcessDetectVcpuPIDs(driver, vm) < 0) goto cleanup; -- 1.7.3.4

On 01/23/2012 07:48 AM, Michal Privoznik wrote:
There is now a standard QEMU guest agent that can be installed and given a virtio serial channel
<channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/> <target type='virtio' name='org.qemu.guest_agent.0'/> </channel>
You also need to update docs/formatdomain.html.in to include this specific example. May I propose that you squash in this: diff --git i/docs/formatdomain.html.in w/docs/formatdomain.html.in index de9b480..6667bed 100644 --- i/docs/formatdomain.html.in +++ w/docs/formatdomain.html.in @@ -3017,6 +3017,10 @@ qemu-kvm -net nic,model=? /dev/null <channel type='pty'> <target type='virtio' name='arbitrary.virtio.serial.port.name'/> </channel> + <channel type='unix'> + <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/> + <target type='virtio' name='org.qemu.guest_agent.0'/> + </channel> <channel type='spicevmc'> <target type='virtio' name='com.redhat.spice.0'/> </channel> @@ -3045,7 +3049,11 @@ qemu-kvm -net nic,model=? /dev/null optional element <code>address</code> can tie the channel to a particular <code>type='virtio-serial'</code> controller, <a href="#elementsAddress">documented above</a>. - <span class="since">Since 0.7.7</span></dd> + With qemu, if <code>name</code> is "org.qemu.guest_agent.0", + then libvirt can interact with a guest agent installed in the + guest, for actions such as guest shutdown or file system quiescing. + <span class="since">Since 0.7.7, guest agent interaction + since 0.9.10</span></dd> <dt><code>spicevmc</code></dt> <dd>Paravirtualized SPICE channel. The domain must also have a I think the RNG already covers the addition, though, so we are good there, and that means that we can get by without any tests/ additions (we already have tests for virtio <channel> parsing), although it wouldn't hurt if you wanted to add some tests.
+struct _qemuAgent { + virMutex lock; /* also used to protect fd */ + virCond notify; + + int refs; + + int fd; + int watch; + + bool connectPending; + + virDomainObjPtr vm; + + qemuAgentCallbacksPtr cb; + + /* If there's a command being processed this will be + * non-NULL */ + qemuAgentMessagePtr msg; + + /* Buffer incoming data ready for Text/QMP monitor + * code to process & find message boundaries */
s,Text/QMP,Agent,
+ switch (config->type) { + case VIR_DOMAIN_CHR_TYPE_UNIX: + mon->fd = qemuAgentOpenUnix(config->data.nix.path, vm->pid, + &mon->connectPending); + break; + + case VIR_DOMAIN_CHR_TYPE_PTY: + mon->fd = qemuAgentOpenPty(config->data.file.path); + break;
Do we really want to support <channel type='pty'> for agent interaction, or just mandate that the guest agent must use <channel type='unix'>? I guess we support both for the monitor as well, in case we ever use qemu-attach to convert a running guest that used a pty monitor into a libvirt-managed guest. ACK with above fixes included. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Add a new API virDomainShutdownFlags and define: VIR_DOMAIN_SHUTDOWN_DEFAULT = 0, VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 << 0), VIR_DOMAIN_SHUTDOWN_GUEST_AGENT = (1 << 1), Also define some flags for the reboot API VIR_DOMAIN_REBOOT_DEFAULT = 0, VIR_DOMAIN_REBOOT_ACPI_POWER_BTN = (1 << 0), VIR_DOMAIN_REBOOT_GUEST_AGENT = (1 << 1), Although these two APIs currently have the same flags, using separate enums allows them to expand separately in the future. Add stub impls of the new API for all existing drivers --- include/libvirt/libvirt.h.in | 15 +++++++++ src/driver.h | 5 +++ src/esx/esx_driver.c | 11 ++++++- src/libvirt.c | 68 +++++++++++++++++++++++++++++++++++++++++- src/libvirt_public.syms | 4 ++ src/libxl/libxl_driver.c | 12 +++++++- src/openvz/openvz_driver.c | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 8 ++++- src/remote_protocol-structs | 5 +++ src/test/test_driver.c | 11 ++++++- src/uml/uml_driver.c | 11 ++++++- src/vbox/vbox_tmpl.c | 11 ++++++- src/vmware/vmware_driver.c | 1 + src/xen/xen_driver.c | 14 +++++++- src/xenapi/xenapi_driver.c | 12 +++++++- 16 files changed, 180 insertions(+), 10 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 958e5a6..381fcd2 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1200,7 +1200,22 @@ virDomainPtr virDomainLookupByUUID (virConnectPtr conn, virDomainPtr virDomainLookupByUUIDString (virConnectPtr conn, const char *uuid); +typedef enum { + VIR_DOMAIN_SHUTDOWN_DEFAULT = 0, /* hypervisor to choose */ + VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 << 0), /* Send ACPI event */ + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT = (1 << 1), /* Use guest agent */ +} virDomainShutdownFlagValues; + int virDomainShutdown (virDomainPtr domain); +int virDomainShutdownFlags (virDomainPtr domain, + unsigned int flags); + +typedef enum { + VIR_DOMAIN_REBOOT_DEFAULT = 0, /* hypervisor to choose */ + VIR_DOMAIN_REBOOT_ACPI_POWER_BTN = (1 << 0), /* Send ACPI event */ + VIR_DOMAIN_REBOOT_GUEST_AGENT = (1 << 1), /* Use guest agent */ +} virDomainRebootFlagValues; + int virDomainReboot (virDomainPtr domain, unsigned int flags); int virDomainReset (virDomainPtr domain, diff --git a/src/driver.h b/src/driver.h index 24636a4..6222bed 100644 --- a/src/driver.h +++ b/src/driver.h @@ -793,6 +793,10 @@ typedef int virTypedParameterPtr params, int *nparams, unsigned int flags); +typedef int + (*virDrvDomainShutdownFlags)(virDomainPtr domain, + unsigned int flags); + /** * _virDriver: @@ -829,6 +833,7 @@ struct _virDriver { virDrvDomainSuspend domainSuspend; virDrvDomainResume domainResume; virDrvDomainShutdown domainShutdown; + virDrvDomainShutdownFlags domainShutdownFlags; virDrvDomainReboot domainReboot; virDrvDomainReset domainReset; virDrvDomainDestroy domainDestroy; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 63cdba5..f5e1cc7 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1887,7 +1887,7 @@ esxDomainResume(virDomainPtr domain) static int -esxDomainShutdown(virDomainPtr domain) +esxDomainShutdownFlags(virDomainPtr domain, unsigned int flags) { int result = -1; esxPrivate *priv = domain->conn->privateData; @@ -1895,6 +1895,8 @@ esxDomainShutdown(virDomainPtr domain) esxVI_String *propertyNameList = NULL; esxVI_VirtualMachinePowerState powerState; + virCheckFlags(0, -1); + if (esxVI_EnsureSession(priv->primary) < 0) { return -1; } @@ -1928,6 +1930,12 @@ esxDomainShutdown(virDomainPtr domain) } +static int +esxDomainShutdown(virDomainPtr domain) +{ + return esxDomainShutdownFlags(domain, 0); +} + static int esxDomainReboot(virDomainPtr domain, unsigned int flags) @@ -4953,6 +4961,7 @@ static virDriver esxDriver = { .domainSuspend = esxDomainSuspend, /* 0.7.0 */ .domainResume = esxDomainResume, /* 0.7.0 */ .domainShutdown = esxDomainShutdown, /* 0.7.0 */ + .domainShutdownFlags = esxDomainShutdownFlags, /* 0.9.10 */ .domainReboot = esxDomainReboot, /* 0.7.0 */ .domainDestroy = esxDomainDestroy, /* 0.7.0 */ .domainDestroyFlags = esxDomainDestroyFlags, /* 0.9.4 */ diff --git a/src/libvirt.c b/src/libvirt.c index 7b8adf7..73eeb6b 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -3106,14 +3106,80 @@ error: } /** + * virDomainShutdownFlags: + * @domain: a domain object + * @flags: bitwise-OR of virDomainShutdownFlagValues + * + * Shutdown a domain, the domain object is still usable thereafter but + * the domain OS is being stopped. Note that the guest OS may ignore the + * request. For guests that react to a shutdown request, the differences + * from virDomainDestroy() are that the guest's disk storage will be in a + * stable state rather than having the (virtual) power cord pulled, and + * this command returns as soon as the shutdown request is issued rather + * than blocking until the guest is no longer running. + * + * If the domain is transient and has any snapshot metadata (see + * virDomainSnapshotNum()), then that metadata will automatically + * be deleted when the domain quits. + * + * If @flags is set to zero, then the hypervisor will chose the + * method of shutdown it considers best. To have greater control + * pass exactly one of the virDomainShutdownFlagValues. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainShutdownFlags(virDomainPtr domain, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if (domain->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + conn = domain->conn; + + if (conn->driver->domainShutdownFlags) { + int ret; + ret = conn->driver->domainShutdownFlags(domain, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + +/** * virDomainReboot: * @domain: a domain object - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virDomainRebootFlagValues * * Reboot a domain, the domain object is still usable there after but * the domain OS is being stopped for a restart. * Note that the guest OS may ignore the request. * + * If @flags is set to zero, then the hypervisor will chose the + * method of shutdown it considers best. To have greater control + * pass exactly one of the virDomainRebootFlagValues. + * + * To use guest agent (VIR_DOMAIN_REBOOT_GUEST_AGENT) the domain XML + * must have <channel> configured. + * * Returns 0 in case of success and -1 in case of failure. */ int diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 4ca7216..e0cbdb4 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -516,4 +516,8 @@ LIBVIRT_0.9.9 { virDomainSetNumaParameters; } LIBVIRT_0.9.8; +LIBVIRT_0.9.10 { + global: + virDomainShutdownFlags; +} LIBVIRT_0.9.9; # .... define new API here using predicted next version number .... diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index f7f45c7..41366e4 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1412,13 +1412,15 @@ cleanup: } static int -libxlDomainShutdown(virDomainPtr dom) +libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { libxlDriverPrivatePtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; libxlDomainObjPrivatePtr priv; + virCheckFlags(0, -1); + libxlDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { @@ -1456,6 +1458,13 @@ cleanup: } static int +libxlDomainShutdown(virDomainPtr dom) +{ + return libxlDomainShutdownFlags(dom, 0); +} + + +static int libxlDomainReboot(virDomainPtr dom, unsigned int flags) { libxlDriverPrivatePtr driver = dom->conn->privateData; @@ -3857,6 +3866,7 @@ static virDriver libxlDriver = { .domainSuspend = libxlDomainSuspend, /* 0.9.0 */ .domainResume = libxlDomainResume, /* 0.9.0 */ .domainShutdown = libxlDomainShutdown, /* 0.9.0 */ + .domainShutdownFlags = libxlDomainShutdownFlags, /* 0.9.10 */ .domainReboot = libxlDomainReboot, /* 0.9.0 */ .domainDestroy = libxlDomainDestroy, /* 0.9.0 */ .domainDestroyFlags = libxlDomainDestroyFlags, /* 0.9.4 */ diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 03bf21a..b848a88 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1693,6 +1693,7 @@ static virDriver openvzDriver = { .domainSuspend = openvzDomainSuspend, /* 0.8.3 */ .domainResume = openvzDomainResume, /* 0.8.3 */ .domainShutdown = openvzDomainShutdown, /* 0.3.1 */ + .domainShutdownFlags = openvzDomainShutdownFlags, /* 0.9.10 */ .domainReboot = openvzDomainReboot, /* 0.3.1 */ .domainDestroy = openvzDomainShutdown, /* 0.3.1 */ .domainDestroyFlags = openvzDomainShutdownFlags, /* 0.9.4 */ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e28840b..f45a8fe 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4617,6 +4617,7 @@ static virDriver remote_driver = { .domainSuspend = remoteDomainSuspend, /* 0.3.0 */ .domainResume = remoteDomainResume, /* 0.3.0 */ .domainShutdown = remoteDomainShutdown, /* 0.3.0 */ + .domainShutdownFlags = remoteDomainShutdownFlags, /* 0.9.10 */ .domainReboot = remoteDomainReboot, /* 0.3.0 */ .domainReset = remoteDomainReset, /* 0.9.7 */ .domainDestroy = remoteDomainDestroy, /* 0.3.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 514b0cc..6a20ae8 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2349,6 +2349,11 @@ struct remote_node_suspend_for_duration_args { unsigned int flags; }; +struct remote_domain_shutdown_flags_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + /*----- Protocol. -----*/ @@ -2654,7 +2659,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_NUMA_PARAMETERS = 254, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, /* autogen autogen */ - REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS = 258 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 2758315..430d8e4 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1832,6 +1832,10 @@ struct remote_node_suspend_for_duration_args { uint64_t duration; u_int flags; }; +struct remote_domain_shutdown_flags_args { + remote_nonnull_domain dom; + u_int flags; +}; enum remote_procedure { REMOTE_PROC_OPEN = 1, REMOTE_PROC_CLOSE = 2, @@ -2090,4 +2094,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, + REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS = 258, }; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index c0b2ca6..55b889b 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1542,13 +1542,16 @@ cleanup: return ret; } -static int testShutdownDomain (virDomainPtr domain) +static int testShutdownDomainFlags(virDomainPtr domain, + unsigned int flags) { testConnPtr privconn = domain->conn->privateData; virDomainObjPtr privdom; virDomainEventPtr event = NULL; int ret = -1; + virCheckFlags(0, -1); + testDriverLock(privconn); privdom = virDomainFindByName(&privconn->domains, domain->name); @@ -1585,6 +1588,11 @@ cleanup: return ret; } +static int testShutdownDomain (virDomainPtr domain) +{ + return testShutdownDomainFlags(domain, 0); +} + /* Similar behaviour as shutdown */ static int testRebootDomain (virDomainPtr domain, unsigned int action ATTRIBUTE_UNUSED) @@ -5523,6 +5531,7 @@ static virDriver testDriver = { .domainSuspend = testPauseDomain, /* 0.1.1 */ .domainResume = testResumeDomain, /* 0.1.1 */ .domainShutdown = testShutdownDomain, /* 0.1.1 */ + .domainShutdownFlags = testShutdownDomainFlags, /* 0.9.10 */ .domainReboot = testRebootDomain, /* 0.1.1 */ .domainDestroy = testDestroyDomain, /* 0.1.1 */ .domainGetOSType = testGetOSType, /* 0.1.9 */ diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 671216e..a4cf945 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1513,12 +1513,15 @@ cleanup: } -static int umlDomainShutdown(virDomainPtr dom) { +static int umlDomainShutdownFlags(virDomainPtr dom, + unsigned int flags) { struct uml_driver *driver = dom->conn->privateData; virDomainObjPtr vm; char *info = NULL; int ret = -1; + virCheckFlags(0, -1); + umlDriverLock(driver); vm = virDomainFindByID(&driver->domains, dom->id); umlDriverUnlock(driver); @@ -1544,6 +1547,11 @@ cleanup: return ret; } +static int +umlDomainShutdown(virDomainPtr dom) +{ + return umlDomainShutdownFlags(dom, 0); +} static int umlDomainDestroyFlags(virDomainPtr dom, @@ -2533,6 +2541,7 @@ static virDriver umlDriver = { .domainLookupByUUID = umlDomainLookupByUUID, /* 0.5.0 */ .domainLookupByName = umlDomainLookupByName, /* 0.5.0 */ .domainShutdown = umlDomainShutdown, /* 0.5.0 */ + .domainShutdownFlags = umlDomainShutdownFlags, /* 0.9.10 */ .domainDestroy = umlDomainDestroy, /* 0.5.0 */ .domainDestroyFlags = umlDomainDestroyFlags, /* 0.9.4 */ .domainGetOSType = umlDomainGetOSType, /* 0.5.0 */ diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 22712d5..d720432 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -1606,7 +1606,8 @@ cleanup: return ret; } -static int vboxDomainShutdown(virDomainPtr dom) { +static int vboxDomainShutdownFlags(virDomainPtr dom, + unsigned int flags) { VBOX_OBJECT_CHECK(dom->conn, int, -1); IMachine *machine = NULL; vboxIID iid = VBOX_IID_INITIALIZER; @@ -1615,6 +1616,8 @@ static int vboxDomainShutdown(virDomainPtr dom) { PRBool isAccessible = PR_FALSE; nsresult rc; + virCheckFlags(0, -1); + vboxIIDFromUUID(&iid, dom->uuid); rc = VBOX_OBJECT_GET_MACHINE(iid.value, &machine); if (NS_FAILED(rc)) { @@ -1656,6 +1659,11 @@ cleanup: return ret; } +static int vboxDomainShutdown(virDomainPtr dom) { + return vboxDomainShutdownFlags(dom, 0); +} + + static int vboxDomainReboot(virDomainPtr dom, unsigned int flags) { VBOX_OBJECT_CHECK(dom->conn, int, -1); @@ -9112,6 +9120,7 @@ virDriver NAME(Driver) = { .domainSuspend = vboxDomainSuspend, /* 0.6.3 */ .domainResume = vboxDomainResume, /* 0.6.3 */ .domainShutdown = vboxDomainShutdown, /* 0.6.3 */ + .domainShutdownFlags = vboxDomainShutdownFlags, /* 0.9.10 */ .domainReboot = vboxDomainReboot, /* 0.6.3 */ .domainDestroy = vboxDomainDestroy, /* 0.6.3 */ .domainDestroyFlags = vboxDomainDestroyFlags, /* 0.9.4 */ diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index a9873ba..56e9d2d 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -980,6 +980,7 @@ static virDriver vmwareDriver = { .domainSuspend = vmwareDomainSuspend, /* 0.8.7 */ .domainResume = vmwareDomainResume, /* 0.8.7 */ .domainShutdown = vmwareDomainShutdown, /* 0.8.7 */ + .domainShutdownFlags = vmwareDomainShutdownFlags, /* 0.9.10 */ .domainReboot = vmwareDomainReboot, /* 0.8.7 */ .domainDestroy = vmwareDomainShutdown, /* 0.8.7 */ .domainDestroyFlags = vmwareDomainShutdownFlags, /* 0.9.4 */ diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 520ec03..12d7eb0 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -851,21 +851,30 @@ xenUnifiedDomainResume (virDomainPtr dom) } static int -xenUnifiedDomainShutdown (virDomainPtr dom) +xenUnifiedDomainShutdownFlags(virDomainPtr dom, + unsigned int flags) { GET_PRIVATE(dom->conn); int i; + virCheckFlags(0, -1); + for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) if (priv->opened[i] && drivers[i]->xenDomainShutdown && - drivers[i]->xenDomainShutdown (dom) == 0) + drivers[i]->xenDomainShutdown(dom) == 0) return 0; return -1; } static int +xenUnifiedDomainShutdown(virDomainPtr dom) +{ + return xenUnifiedDomainShutdownFlags(dom, 0); +} + +static int xenUnifiedDomainReboot (virDomainPtr dom, unsigned int flags) { GET_PRIVATE(dom->conn); @@ -2187,6 +2196,7 @@ static virDriver xenUnifiedDriver = { .domainSuspend = xenUnifiedDomainSuspend, /* 0.0.3 */ .domainResume = xenUnifiedDomainResume, /* 0.0.3 */ .domainShutdown = xenUnifiedDomainShutdown, /* 0.0.3 */ + .domainShutdownFlags = xenUnifiedDomainShutdownFlags, /* 0.9.10 */ .domainReboot = xenUnifiedDomainReboot, /* 0.1.0 */ .domainDestroy = xenUnifiedDomainDestroy, /* 0.0.3 */ .domainDestroyFlags = xenUnifiedDomainDestroyFlags, /* 0.9.4 */ diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 78137d4..68017bc 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -783,12 +783,15 @@ xenapiDomainResume (virDomainPtr dom) * Returns 0 on success or -1 in case of error */ static int -xenapiDomainShutdown (virDomainPtr dom) +xenapiDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { /* vm.clean_shutdown */ xen_vm vm; xen_vm_set *vms; xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session; + + virCheckFlags(0, -1); + if (xen_vm_get_by_name_label(session, &vms, dom->name) && vms->size > 0) { if (vms->size != 1) { xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, @@ -811,6 +814,12 @@ xenapiDomainShutdown (virDomainPtr dom) return -1; } +static int +xenapiDomainShutdown(virDomainPtr dom) +{ + return xenapiDomainShutdownFlags(dom, 0); +} + /* * xenapiDomainReboot * @@ -1928,6 +1937,7 @@ static virDriver xenapiDriver = { .domainSuspend = xenapiDomainSuspend, /* 0.8.0 */ .domainResume = xenapiDomainResume, /* 0.8.0 */ .domainShutdown = xenapiDomainShutdown, /* 0.8.0 */ + .domainShutdownFlags = xenapiDomainShutdownFlags, /* 0.9.10 */ .domainReboot = xenapiDomainReboot, /* 0.8.0 */ .domainDestroy = xenapiDomainDestroy, /* 0.8.0 */ .domainDestroyFlags = xenapiDomainDestroyFlags, /* 0.9.4 */ -- 1.7.3.4

On 01/23/2012 07:48 AM, Michal Privoznik wrote:
Add a new API virDomainShutdownFlags and define:
VIR_DOMAIN_SHUTDOWN_DEFAULT = 0, VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 << 0), VIR_DOMAIN_SHUTDOWN_GUEST_AGENT = (1 << 1),
Also define some flags for the reboot API
VIR_DOMAIN_REBOOT_DEFAULT = 0, VIR_DOMAIN_REBOOT_ACPI_POWER_BTN = (1 << 0), VIR_DOMAIN_REBOOT_GUEST_AGENT = (1 << 1),
Although these two APIs currently have the same flags, using separate enums allows them to expand separately in the future.
Add stub impls of the new API for all existing drivers
--- +++ b/include/libvirt/libvirt.h.in @@ -1200,7 +1200,22 @@ virDomainPtr virDomainLookupByUUID (virConnectPtr conn, virDomainPtr virDomainLookupByUUIDString (virConnectPtr conn, const char *uuid);
+typedef enum { + VIR_DOMAIN_SHUTDOWN_DEFAULT = 0, /* hypervisor to choose */
/to choose/choice/
+ VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 << 0), /* Send ACPI event */ + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT = (1 << 1), /* Use guest agent */ +} virDomainShutdownFlagValues; + int virDomainShutdown (virDomainPtr domain); +int virDomainShutdownFlags (virDomainPtr domain, + unsigned int flags); + +typedef enum { + VIR_DOMAIN_REBOOT_DEFAULT = 0, /* hypervisor to choose */
and again.
+ * If @flags is set to zero, then the hypervisor will chose the
s/chose/choose/
+ * method of shutdown it considers best. To have greater control + * pass exactly one of the virDomainShutdownFlagValues.
Does it make sense to enforce the mutual exclusion of these flags here? For example, in various functions that take exactly one of VIR_DOMAIN_AFFECT_{LIVE,CONFIG}, we enforce the mutual exclusion in libvirt.c, so that hypervisor drivers don't have to repeat the check.
* virDomainReboot: * @domain: a domain object - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virDomainRebootFlagValues * * Reboot a domain, the domain object is still usable there after but * the domain OS is being stopped for a restart. * Note that the guest OS may ignore the request. * + * If @flags is set to zero, then the hypervisor will chose the
s/chose/choose/ I can live with this with just the spelling nits fixed, so: ACK However, if you want to post a v3 with the mutual exclusion check added to libvirt.c, that would also be reasonable. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

This makes use of the QEMU guest agent to implement the virDomainShutdownFlags and virDomainReboot APIs. With no flags specified, it will prefer to use the agent, but fallback to ACPI. Explicit choice can be made by using a suitable flag * src/qemu/qemu_driver.c: Wire up use of agent --- src/qemu/qemu_driver.c | 131 ++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 106 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 608e82a..66b955c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -47,6 +47,7 @@ #include "qemu_driver.h" +#include "qemu_agent.h" #include "qemu_conf.h" #include "qemu_capabilities.h" #include "qemu_command.h" @@ -1512,12 +1513,23 @@ cleanup: return ret; } - -static int qemuDomainShutdown(virDomainPtr dom) { +static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; qemuDomainObjPrivatePtr priv; + bool useAgent = false; + + virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN | + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1); + + if (flags & + (VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN & + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT)) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("Only one flag supported")); + return -1; + } qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1531,6 +1543,26 @@ static int qemuDomainShutdown(virDomainPtr dom) { goto cleanup; } + priv = vm->privateData; + + if ((flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) || + (!(flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) && + priv->agent)) + useAgent = true; + + if (useAgent) { + if (priv->agentError) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QEMU guest agent is not available due to an error")); + goto endjob; + } + if (!priv->agent) { + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + goto endjob; + } + } + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -1540,12 +1572,17 @@ static int qemuDomainShutdown(virDomainPtr dom) { goto endjob; } - qemuDomainSetFakeReboot(driver, vm, false); + if (useAgent) { + qemuDomainObjEnterAgent(driver, vm); + ret = qemuAgentShutdown(priv->agent, QEMU_AGENT_SHUTDOWN_POWERDOWN); + qemuDomainObjExitAgent(driver, vm); + } else { + qemuDomainSetFakeReboot(driver, vm, false); - priv = vm->privateData; - qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorSystemPowerdown(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + qemuDomainObjEnterMonitor(driver, vm); + ret = qemuMonitorSystemPowerdown(priv->mon); + qemuDomainObjExitMonitor(driver, vm); + } endjob: if (qemuDomainObjEndJob(driver, vm) == 0) @@ -1557,14 +1594,28 @@ cleanup: return ret; } +static int qemuDomainShutdown(virDomainPtr dom) { + return qemuDomainShutdownFlags(dom, 0); +} + static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; qemuDomainObjPrivatePtr priv; + bool useAgent = false; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN | + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT , -1); + + if (flags & + (VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN & + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT)) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("Only one flag supported")); + return -1; + } qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1580,37 +1631,66 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) { priv = vm->privateData; - if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON)) { - if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) { + if ((flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) || + (!(flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) && + priv->agent)) + useAgent = true; + + if (useAgent) { + if (priv->agentError) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QEMU guest agent is not available due to an error")); + goto cleanup; + } + if (!priv->agent) { + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + goto cleanup; + } + } else { +#if HAVE_YAJL + if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON)) { + if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Reboot is not supported with this QEMU binary")); + goto cleanup; + } + } else { +#endif qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Reboot is not supported with this QEMU binary")); + _("Reboot is not supported without the JSON monitor")); goto cleanup; +#if HAVE_YAJL } +#endif + } - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; - if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto endjob; - } + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + if (useAgent) { + qemuDomainObjEnterAgent(driver, vm); + ret = qemuAgentShutdown(priv->agent, QEMU_AGENT_SHUTDOWN_REBOOT); + qemuDomainObjExitAgent(driver, vm); + } else { qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSystemPowerdown(priv->mon); qemuDomainObjExitMonitor(driver, vm); if (ret == 0) qemuDomainSetFakeReboot(driver, vm, true); - - endjob: - if (qemuDomainObjEndJob(driver, vm) == 0) - vm = NULL; - } else { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Reboot is not supported without the JSON monitor")); } +endjob: + if (qemuDomainObjEndJob(driver, vm) == 0) + vm = NULL; + cleanup: if (vm) virDomainObjUnlock(vm); @@ -11627,6 +11707,7 @@ static virDriver qemuDriver = { .domainSuspend = qemudDomainSuspend, /* 0.2.0 */ .domainResume = qemudDomainResume, /* 0.2.0 */ .domainShutdown = qemuDomainShutdown, /* 0.2.0 */ + .domainShutdownFlags = qemuDomainShutdownFlags, /* 0.9.10 */ .domainReboot = qemuDomainReboot, /* 0.9.3 */ .domainReset = qemuDomainReset, /* 0.9.7 */ .domainDestroy = qemuDomainDestroy, /* 0.2.0 */ -- 1.7.3.4

On 01/23/2012 07:48 AM, Michal Privoznik wrote: [for some reason, this one didn't get threaded properly - bug in git send-email?]
This makes use of the QEMU guest agent to implement the virDomainShutdownFlags and virDomainReboot APIs. With no flags specified, it will prefer to use the agent, but fallback to ACPI. Explicit choice can be made by using a suitable flag
* src/qemu/qemu_driver.c: Wire up use of agent --- src/qemu/qemu_driver.c | 131 ++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 106 insertions(+), 25 deletions(-)
-static int qemuDomainShutdown(virDomainPtr dom) { +static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; qemuDomainObjPrivatePtr priv; + bool useAgent = false; + + virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN | + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1); + + if (flags & + (VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN & + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT)) {
Won't work. That statement is always false.
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("Only one flag supported")); + return -1; + }
Back to my comment in 2/4, I'd prefer that you move this into libvirt.c (so drivers don't have to duplicate it), where it should look like: /* At most one of these two flags should be set. */ if ((flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) && (flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT)) { virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto error; }
+ + if (flags & + (VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN & + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT)) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("Only one flag supported")); + return -1; + }
Another instance of broken logic that will always be false, and which I think belongs better in libvirt.c. I didn't see anything else wrong in the patch, so if you fix patch 2, then delete the two no-op hunks from patch 3, you have my ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Extend the 'shutdown' and 'reboot' methods so that they both accept a new argument --mode acpi|agent * tools/virsh.c: New args for shutdown/reboot * tools/virsh.pod: Document new args --- tools/virsh.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- tools/virsh.pod | 12 ++++++++++-- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index d635b56..5560988 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3850,6 +3850,7 @@ static const vshCmdInfo info_shutdown[] = { static const vshCmdOptDef opts_shutdown[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"mode", VSH_OT_STRING, VSH_OFLAG_NONE, N_("shutdown mode: acpi|agent")}, {NULL, 0, 0, NULL} }; @@ -3859,14 +3860,37 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; bool ret = true; const char *name; + const char *mode = NULL; + int flags = 0; + int rv; if (!vshConnectionUsability(ctl, ctl->conn)) return false; + if (vshCommandOptString(cmd, "mode", &mode) < 0) { + vshError(ctl, "%s", _("Invalid type")); + return false; + } + + if (mode) { + if (STREQ(mode, "acpi")) { + flags |= VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN; + } else if (STREQ(mode, "agent")) { + flags |= VIR_DOMAIN_SHUTDOWN_GUEST_AGENT; + } else { + vshError(ctl, _("Unknown mode %s value, expecting 'acpi' or 'agent'"), mode); + return false; + } + } + if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - if (virDomainShutdown(dom) == 0) { + if (flags) + rv = virDomainShutdownFlags(dom, flags); + else + rv = virDomainShutdown(dom); + if (rv == 0) { vshPrint(ctl, _("Domain %s is being shutdown\n"), name); } else { vshError(ctl, _("Failed to shutdown domain %s"), name); @@ -3888,6 +3912,7 @@ static const vshCmdInfo info_reboot[] = { static const vshCmdOptDef opts_reboot[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"mode", VSH_OT_STRING, VSH_OFLAG_NONE, N_("shutdown mode: acpi|agent")}, {NULL, 0, 0, NULL} }; @@ -3897,14 +3922,32 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; bool ret = true; const char *name; + const char *mode = NULL; + int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) return false; + if (vshCommandOptString(cmd, "mode", &mode) < 0) { + vshError(ctl, "%s", _("Invalid type")); + return false; + } + + if (mode) { + if (STREQ(mode, "acpi")) { + flags |= VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN; + } else if (STREQ(mode, "agent")) { + flags |= VIR_DOMAIN_SHUTDOWN_GUEST_AGENT; + } else { + vshError(ctl, _("Unknown mode %s value, expecting 'acpi' or 'agent'"), mode); + return false; + } + } + if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - if (virDomainReboot(dom, 0) == 0) { + if (virDomainReboot(dom, flags) == 0) { vshPrint(ctl, _("Domain %s is being rebooted\n"), name); } else { vshError(ctl, _("Failed to reboot domain %s"), name); diff --git a/tools/virsh.pod b/tools/virsh.pod index 67f93a9..72c6d8f 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -807,7 +807,7 @@ If I<--live> is specified, set scheduler information of a running guest. If I<--config> is specified, affect the next boot of a persistent guest. If I<--current> is specified, affect the current guest state. -=item B<reboot> I<domain-id> +=item B<reboot> I<domain-id> [I<--mode acpi|agent>] Reboot a domain. This acts just as if the domain had the B<reboot> command run from the console. The command returns as soon as it has @@ -817,6 +817,10 @@ domain actually reboots. The exact behavior of a domain when it reboots is set by the I<on_reboot> parameter in the domain's XML definition. +By default the hypervisor will try to pick a suitable shutdown +method. To specify an alternative method, the I<--mode> parameter +can specify C<acpi> or C<agent>. + =item B<reset> I<domain-id> Reset a domain immediately without any guest shutdown. B<reset> @@ -1190,7 +1194,7 @@ The I<--maximum> flag controls the maximum number of virtual cpus that can be hot-plugged the next time the domain is booted. As such, it must only be used with the I<--config> flag, and not with the I<--live> flag. -=item B<shutdown> I<domain-id> +=item B<shutdown> I<domain-id> [I<--mode acpi|agent>] Gracefully shuts down a domain. This coordinates with the domain OS to perform graceful shutdown, so there is no guarantee that it will @@ -1205,6 +1209,10 @@ be lost once the guest stops running, but the snapshot contents still exist, and a new domain with the same name and UUID can restore the snapshot metadata with B<snapshot-create>. +By default the hypervisor will try to pick a suitable shutdown +method. To specify an alternative method, the I<--mode> parameter +can specify C<acpi> or C<agent>. + =item B<start> I<domain-name> [I<--console>] [I<--paused>] [I<--autodestroy>] [I<--bypass-cache>] [I<--force-boot>] -- 1.7.3.4

The 23/01/12, Michal Privoznik wrote:
+By default the hypervisor will try to pick a suitable shutdown +method. To specify an alternative method, the I<--mode> parameter +can specify C<acpi> or C<agent>. +
What's the "suitable shutdown method", BTW? If I don't give the --mode option, it is not clear which method will be used. Or did I missed something? -- Nicolas Sebrecht

On Mon, Jan 23, 2012 at 04:41:43PM +0100, Nicolas Sebrecht wrote:
The 23/01/12, Michal Privoznik wrote:
+By default the hypervisor will try to pick a suitable shutdown +method. To specify an alternative method, the I<--mode> parameter +can specify C<acpi> or C<agent>. +
What's the "suitable shutdown method", BTW? If I don't give the --mode option, it is not clear which method will be used.
That is intentional, because the choice of what is used as the default option is hypervisor depedendant and may change at will Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

The 23/01/12, Daniel P. Berrange wrote:
On Mon, Jan 23, 2012 at 04:41:43PM +0100, Nicolas Sebrecht wrote:
The 23/01/12, Michal Privoznik wrote:
+By default the hypervisor will try to pick a suitable shutdown +method. To specify an alternative method, the I<--mode> parameter +can specify C<acpi> or C<agent>. +
What's the "suitable shutdown method", BTW? If I don't give the --mode option, it is not clear which method will be used.
That is intentional, because the choice of what is used as the default option is hypervisor depedendant and may change at will
Thank you for the information but I think my point still applies. I mean, what are the options/conditions which can change this "hypervisor dependent default option which may change at will"? A compilation option? A qemu option? A libvirt option? The current documentation is a bit cryptic for the user unless we document _how_ is defined the "suitable shutdown method". Exposing what is the current enabled method (if possible) could be interesting, too. -- Nicolas Sebrecht

On 01/23/2012 07:48 AM, Michal Privoznik wrote:
Extend the 'shutdown' and 'reboot' methods so that they both accept a new argument
--mode acpi|agent
* tools/virsh.c: New args for shutdown/reboot * tools/virsh.pod: Document new args --- tools/virsh.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- tools/virsh.pod | 12 ++++++++++-- 2 files changed, 55 insertions(+), 4 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel P. Berrange
-
Eric Blake
-
Michal Privoznik
-
Nicolas Sebrecht