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

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

From: "Daniel P. Berrange" <berrange@redhat.com> 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 exaclty the same code because there are some odd differences in the way messages and errors are strucutured. 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 --- src/Makefile.am | 1 + src/qemu/qemu_agent.c | 1135 ++++++++++++++++++++++++++++++++++++++++++ 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 | 187 +++++++ 7 files changed, 1512 insertions(+), 1 deletions(-) create mode 100644 src/qemu/qemu_agent.c create mode 100644 src/qemu/qemu_agent.h diff --git a/src/Makefile.am b/src/Makefile.am index 738ee91..f27d714 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -339,6 +339,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..41bd696 --- /dev/null +++ b/src/qemu/qemu_agent.c @@ -0,0 +1,1135 @@ +/* + * qemu_agent.h: interaction with QEMU guest agent + * + * Copyright (C) 2006-2011 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 + +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); + if (virCondDestroy(&mon->notify) < 0) + {} + 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; + } + + 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")); + VIR_FORCE_CLOSE(monfd); + return -1; + } + + return monfd; +} + + +static int +qemuAgentIOProcessEvent(qemuAgentPtr mon, + virJSONValuePtr obj) +{ + const char *type; + int i; + 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 & whether its ready for its reply + * ie whether its completed writing all its data */ + if (mon->msg && mon->msg->txOffset == mon->msg->txLength) + msg = mon->msg; + +#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, the no-op */ + if (!mon->msg || mon->msg->txOffset == mon->msg->txLength) + return 0; + + done = write(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 (virSetCloseExec(mon->fd) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to set monitor close-on-exec flag")); + 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 quited 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 supports 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..c52f122 --- /dev/null +++ b/src/qemu/qemu_agent.h @@ -0,0 +1,69 @@ +/* + * qemu_agent.h: interaction with QEMU guest agent + * + * Copyright (C) 2006-2011 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 65f721a..e1e1445 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -238,6 +238,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); } @@ -1038,6 +1042,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(virTimeMs(&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 d9f323c..cf55254 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" @@ -109,6 +110,11 @@ struct _qemuDomainObjPrivate { int monJSON; bool monError; unsigned long long monStart; + + qemuAgentPtr agent; + bool agentError; + unsigned long long agentStart; + bool gotShutdown; char *pidfile; @@ -198,6 +204,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 c4f8360..effc1d3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -58,7 +58,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 a7fe86c..3b2a801 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -106,6 +106,163 @@ 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) < 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(virTimeMs(&priv->agentStart)); + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + + agent = qemuAgentOpen(vm, + config, + &agentCallbacks); + + qemuDriverLock(driver); + virDomainObjLock(vm); + priv->agentStart = 0; + + if (virSecurityManagerClearSocketLabel(driver->securityManager, vm) < 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 @@ -2602,6 +2759,14 @@ qemuProcessReconnect(void *opaque) if (qemuConnectMonitor(driver, obj) < 0) goto error; + /* Failure to connect to agent shouldn't be made to 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; } @@ -3152,6 +3317,14 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessWaitForMonitor(driver, vm, priv->qemuCaps, pos) < 0) goto cleanup; + /* Failure to connect to agent shouldn't be made to 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; @@ -3354,6 +3527,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); @@ -3607,6 +3786,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 made to 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.6

On 10/05/2011 11:31 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
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>
And the next step would be to have virt-manager/virt-install automatically set this up when creating new domains.
The protocol that runs over the guest agent is JSON based and very similar to the JSON monitor. We can't use exaclty the same
s/exaclty/exactly/
code because there are some odd differences in the way messages and errors are strucutured. The qemu_agent.c file is based on
s/strucutured/structured/
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 --- src/Makefile.am | 1 + src/qemu/qemu_agent.c | 1135 ++++++++++++++++++++++++++++++++++++++++++ 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 | 187 +++++++ 7 files changed, 1512 insertions(+), 1 deletions(-) create mode 100644 src/qemu/qemu_agent.c create mode 100644 src/qemu/qemu_agent.h
We'll see how far I get in this review. At any rate, this patch should be independently back-portable to Fedora 16 without requiring a rebase, although the same cannot be said of patches 2-4.
+ +static struct { + const char *type; + void (*handler)(qemuAgentPtr mon, virJSONValuePtr data); +} eventHandlers[] = { +};
What good is a 0-element initializer? Should we nuke this struct until we actually have an event that we need to handle?
+ +static void qemuAgentFree(qemuAgentPtr mon) +{ + VIR_DEBUG("mon=%p", mon); + if (mon->cb&& mon->cb->destroy) + (mon->cb->destroy)(mon, mon->vm); + if (virCondDestroy(&mon->notify)< 0) + {}
I'd write this as: ignore_value(virCondDestroy(&mon->notify)); I know it was just copy-and-paste from old code that predated our use of ignore_value.
+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) {
Someday, I'll get gnulib to support SOCK_NONBLOCK, so we can do this in fewer kernel calls with newer kernels...
+ +static int +qemuAgentIOProcessEvent(qemuAgentPtr mon, + virJSONValuePtr obj) +{ + const char *type; + int i; + 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++) {
I guess you really are using a 0-element array. But does that compile correctly on all C99 compilers, or should we also be commenting out this function as long as we lack agent events?
+ +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);
Doesn't this trip up 'make syntax-check', since VIR_ERROR shouldn't need to translate its string?
+/* 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& whether its ready for its reply + * ie whether its completed writing all its data */
Copy-and-paste, but the grammar here is awful. How about: 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);
Unbalanced: four [[[[, but only three ]]]
+/* + * 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, the no-op */
s/the/then/
+ + 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 (virSetCloseExec(mon->fd)< 0) {
Why do we set nonblock in qemuAgentOpen{Unix,Pty}, but delay SetCloseExec to here? This seems like it might be worth moving closer to where the fds are opened (that, and someday I want to teach gnulib to guarantee that SOCK_CLOEXEC works for socket()...)
+ +static int qemuAgentSend(qemuAgentPtr mon, + qemuAgentMessagePtr msg) +{ + int ret = -1; + + /* Check whether qemu quited unexpectedly */
s/quited/quit/
+ + +#if 0 +static int +qemuAgentHasError(virJSONValuePtr reply, + const char *klass)
Do we need to be adding #if 0 stuff here, or can you trim this completely out?
+ + /* This doesn't supports maps/arrays. This hasn't
s/supports/support/
* 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 @@ -2602,6 +2759,14 @@ qemuProcessReconnect(void *opaque) if (qemuConnectMonitor(driver, obj)< 0) goto error;
+ /* Failure to connect to agent shouldn't be made to be fatal */
s/be made to be/be/
+ 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; } @@ -3152,6 +3317,14 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessWaitForMonitor(driver, vm, priv->qemuCaps, pos)< 0) goto cleanup;
+ /* Failure to connect to agent shouldn't be made to be fatal */
again
@@ -3607,6 +3786,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 made to be fatal */
and again ACK with nits fixed - and looks like most of the nits are due to copy and pasting of working code, rather than implementing new code from scratch. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange@redhat.com> 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 seperately 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 | 59 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/libxl/libxl_driver.c | 12 ++++++++- src/openvz/openvz_driver.c | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 9 +++++- src/test/test_driver.c | 11 +++++++- src/uml/uml_driver.c | 11 +++++++- src/vbox/vbox_tmpl.c | 10 ++++++- src/vmware/vmware_driver.c | 1 + src/xen/xen_driver.c | 13 ++++++++- src/xenapi/xenapi_driver.c | 11 +++++++- 15 files changed, 162 insertions(+), 9 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index bd7a0f7..dba2099 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1028,7 +1028,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 f85a1b1..86857d5 100644 --- a/src/driver.h +++ b/src/driver.h @@ -725,6 +725,10 @@ typedef int (*virDrvDomainBlockPull)(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +typedef int + (*virDrvDomainShutdownFlags)(virDomainPtr domain, + unsigned int flags); + /** * _virDriver: @@ -881,6 +885,7 @@ struct _virDriver { virDrvDomainGetBlockJobInfo domainGetBlockJobInfo; virDrvDomainBlockJobSetSpeed domainBlockJobSetSpeed; virDrvDomainBlockPull domainBlockPull; + virDrvDomainShutdownFlags domainShutdownFlags; }; typedef int diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 3f26557..b206239 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1870,7 +1870,7 @@ esxDomainResume(virDomainPtr domain) static int -esxDomainShutdown(virDomainPtr domain) +esxDomainShutdownFlags(virDomainPtr domain, unsigned int flags) { int result = -1; esxPrivate *priv = domain->conn->privateData; @@ -1878,6 +1878,8 @@ esxDomainShutdown(virDomainPtr domain) esxVI_String *propertyNameList = NULL; esxVI_VirtualMachinePowerState powerState; + virCheckFlags(0, -1); + if (esxVI_EnsureSession(priv->primary) < 0) { return -1; } @@ -1911,6 +1913,12 @@ esxDomainShutdown(virDomainPtr domain) } +static int +esxDomainShutdown(virDomainPtr domain) +{ + return esxDomainShutdownFlags(domain, 0); +} + static int esxDomainReboot(virDomainPtr domain, unsigned int flags) @@ -4829,6 +4837,7 @@ static virDriver esxDriver = { .domainSuspend = esxDomainSuspend, /* 0.7.0 */ .domainResume = esxDomainResume, /* 0.7.0 */ .domainShutdown = esxDomainShutdown, /* 0.7.0 */ + .domainShutdownFlags = esxDomainShutdownFlags, /* 0.9.7 */ .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 9080b2f..1607b99 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2970,6 +2970,65 @@ error: } /** + * virDomainShutdownFlags: + * @domain: a domain object + * @flags: optional flags + * + * 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 guests 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 shutdown flags + * + * 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 for the reboot operation, not used yet diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index afea29b..4186fd8 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -493,6 +493,7 @@ LIBVIRT_0.9.7 { global: virDomainReset; virDomainSnapshotGetParent; + virDomainShutdownFlags; } LIBVIRT_0.9.5; # .... define new API here using predicted next version number .... diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index d324632..da1031c 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1440,13 +1440,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) { @@ -1484,6 +1486,13 @@ cleanup: } static int +libxlDomainShutdown(virDomainPtr dom) +{ + return libxlDomainShutdownFlags(dom, 0); +} + + +static int libxlDomainReboot(virDomainPtr dom, unsigned int flags) { libxlDriverPrivatePtr driver = dom->conn->privateData; @@ -3895,6 +3904,7 @@ static virDriver libxlDriver = { .domainSuspend = libxlDomainSuspend, /* 0.9.0 */ .domainResume = libxlDomainResume, /* 0.9.0 */ .domainShutdown = libxlDomainShutdown, /* 0.9.0 */ + .domainShutdownFlags = libxlDomainShutdownFlags, /* 0.9.7 */ .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 69ff444..32e1bfe 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1689,6 +1689,7 @@ static virDriver openvzDriver = { .domainSuspend = openvzDomainSuspend, /* 0.8.3 */ .domainResume = openvzDomainResume, /* 0.8.3 */ .domainShutdown = openvzDomainShutdown, /* 0.3.1 */ + .domainShutdownFlags = openvzDomainShutdownFlags, /* 0.9.7 */ .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 83f4f3c..eaf8bbd 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4312,6 +4312,7 @@ static virDriver remote_driver = { .domainSuspend = remoteDomainSuspend, /* 0.3.0 */ .domainResume = remoteDomainResume, /* 0.3.0 */ .domainShutdown = remoteDomainShutdown, /* 0.3.0 */ + .domainShutdownFlags = remoteDomainShutdownFlags, /* 0.9.7 */ .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 c8a92fd..34e4b2a 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2234,6 +2234,12 @@ struct remote_domain_get_control_info_ret { /* insert@1 */ unsigned hyper stateTime; }; +struct remote_domain_shutdown_flags_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -2525,7 +2531,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_SPEED = 242, /* autogen autogen */ REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244, /* autogen autogen */ - REMOTE_PROC_DOMAIN_RESET = 245 /* autogen autogen */ + REMOTE_PROC_DOMAIN_RESET = 245, /* autogen autogen */ + REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS = 246 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b3e24b4..3efed78 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1533,13 +1533,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); @@ -1576,6 +1579,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) @@ -5578,6 +5586,7 @@ static virDriver testDriver = { .domainSuspend = testPauseDomain, /* 0.1.1 */ .domainResume = testResumeDomain, /* 0.1.1 */ .domainShutdown = testShutdownDomain, /* 0.1.1 */ + .domainShutdownFlags = testShutdownDomainFlags, /* 0.9.7 */ .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 2b7219a..5a90031 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1385,12 +1385,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); @@ -1416,6 +1419,11 @@ cleanup: return ret; } +static int +umlDomainShutdown(virDomainPtr dom) +{ + return umlDomainShutdownFlags(dom, 0); +} static int umlDomainDestroyFlags(virDomainPtr dom, @@ -2425,6 +2433,7 @@ static virDriver umlDriver = { .domainLookupByUUID = umlDomainLookupByUUID, /* 0.5.0 */ .domainLookupByName = umlDomainLookupByName, /* 0.5.0 */ .domainShutdown = umlDomainShutdown, /* 0.5.0 */ + .domainShutdownFlags = umlDomainShutdownFlags, /* 0.9.7 */ .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 8c53f1f..efa5dd6 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -1590,7 +1590,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; @@ -1599,6 +1600,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)) { @@ -1640,6 +1643,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); diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index b2cfdce..6dba82f 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -974,6 +974,7 @@ static virDriver vmwareDriver = { .domainSuspend = vmwareDomainSuspend, /* 0.8.7 */ .domainResume = vmwareDomainResume, /* 0.8.7 */ .domainShutdown = vmwareDomainShutdown, /* 0.8.7 */ + .domainShutdownFlags = vmwareDomainShutdownFlags, /* 0.9.7 */ .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 9c96fca..51d83af 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -855,21 +855,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); diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 80a706a..49b29b8 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -773,12 +773,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, @@ -801,6 +804,12 @@ xenapiDomainShutdown (virDomainPtr dom) return -1; } +static int +xenapiDomainShutdown(virDomainPtr dom) +{ + return xenapiDomainShutdownFlags(dom, 0); +} + /* * xenapiDomainReboot * -- 1.7.6

On 10/05/2011 11:31 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
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 seperately in the future.
s/seperately/separately/
Add stub impls of the new API for all existing drivers
well, except for qemu in the next patch :)
* _virDriver: @@ -881,6 +885,7 @@ struct _virDriver { virDrvDomainGetBlockJobInfo domainGetBlockJobInfo; virDrvDomainBlockJobSetSpeed domainBlockJobSetSpeed; virDrvDomainBlockPull domainBlockPull; + virDrvDomainShutdownFlags domainShutdownFlags; };
Should we list this driver callback next to domainShutdown, to match the fact that most hypervisors now list the two callbacks side-by-side? I don't know if gcc can generate slightly better code when the initializers appear in the same order in the hypervisors as they are declared in this struct.
/** + * virDomainShutdownFlags: + * @domain: a domain object + * @flags: optional flags + * + * 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 guests disk storage will be in a
s/guests/guest's/ (probably copy and paste of an existing typo)
+ * 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 shutdown flags
Should we list the flag names here, or at least the enum name so that the html docs will have a hyperlink to the flag names?
+++ b/src/libvirt_public.syms @@ -493,6 +493,7 @@ LIBVIRT_0.9.7 { global: virDomainReset; virDomainSnapshotGetParent; + virDomainShutdownFlags;
I know, sorting doesn't matter here... :)
+++ b/src/openvz/openvz_driver.c @@ -1689,6 +1689,7 @@ static virDriver openvzDriver = { .domainSuspend = openvzDomainSuspend, /* 0.8.3 */ .domainResume = openvzDomainResume, /* 0.8.3 */ .domainShutdown = openvzDomainShutdown, /* 0.3.1 */ + .domainShutdownFlags = openvzDomainShutdownFlags, /* 0.9.7 */
You obviously aren't compiling openvz, as this is missing the new function openvzDomainShutdownFlags.
+++ b/src/vmware/vmware_driver.c @@ -974,6 +974,7 @@ static virDriver vmwareDriver = { .domainSuspend = vmwareDomainSuspend, /* 0.8.7 */ .domainResume = vmwareDomainResume, /* 0.8.7 */ .domainShutdown = vmwareDomainShutdown, /* 0.8.7 */ + .domainShutdownFlags = vmwareDomainShutdownFlags, /* 0.9.7 */
Same goes for you not compiling vmware.
+++ b/src/xenapi/xenapi_driver.c @@ -773,12 +773,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, @@ -801,6 +804,12 @@ xenapiDomainShutdown (virDomainPtr dom) return -1; }
+static int +xenapiDomainShutdown(virDomainPtr dom) +{ + return xenapiDomainShutdownFlags(dom, 0); +} +
Here, you have the opposite problem - you forgot to register xenapiDomainShutdownFlags in the list of driver callbacks. The fixes seem trivial enough, so Conditional ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 83f4f3c..eaf8bbd 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4312,6 +4312,7 @@ static virDriver remote_driver = { .domainSuspend = remoteDomainSuspend, /* 0.3.0 */ .domainResume = remoteDomainResume, /* 0.3.0 */ .domainShutdown = remoteDomainShutdown, /* 0.3.0 */ + .domainShutdownFlags = remoteDomainShutdownFlags, /* 0.9.7 */ .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 c8a92fd..34e4b2a 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2234,6 +2234,12 @@ struct remote_domain_get_control_info_ret { /* insert@1 */ unsigned hyper stateTime; };
+struct remote_domain_shutdown_flags_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + + /*----- Protocol. -----*/
/* Define the program number, protocol version and procedure numbers here. */ @@ -2525,7 +2531,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_SPEED = 242, /* autogen autogen */ REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244, /* autogen autogen */ - REMOTE_PROC_DOMAIN_RESET = 245 /* autogen autogen */ + REMOTE_PROC_DOMAIN_RESET = 245, /* autogen autogen */ + REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS = 246 /* autogen autogen */
/* * Notice how the entries are grouped in sets of 10 ?
I think it does not works well if target is localhost or not, because remoteDomainShutdownFlags() function is not implemented. So, do you have a plan ? MATSUDA Daiki

On 10/19/2011 06:22 PM, MATSUDA, Daiki wrote:
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 83f4f3c..eaf8bbd 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4312,6 +4312,7 @@ static virDriver remote_driver = { .domainSuspend = remoteDomainSuspend, /* 0.3.0 */ .domainResume = remoteDomainResume, /* 0.3.0 */ .domainShutdown = remoteDomainShutdown, /* 0.3.0 */ + .domainShutdownFlags = remoteDomainShutdownFlags, /* 0.9.7 */
+ REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS = 246 /* autogen autogen */
/* * Notice how the entries are grouped in sets of 10 ?
I think it does not works well if target is localhost or not, because remoteDomainShutdownFlags() function is not implemented. So, do you have a plan ?
The notation "autogen autogen" after REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS _is_ what implements remoteDomainShutdownFlags(). See src/rpc/genprotocol.pl. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange@redhat.com> 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 | 118 +++++++++++++++++++++++++++++++++++------------ 1 files changed, 88 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 276dc06..8660b37 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" @@ -1484,12 +1485,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); @@ -1503,6 +1507,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; @@ -1512,12 +1536,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) @@ -1529,14 +1558,17 @@ 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; -#if HAVE_YAJL qemuDomainObjPrivatePtr priv; -#endif + bool useAgent = false; virCheckFlags(0, -1); @@ -1552,42 +1584,67 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) { goto cleanup; } -#if HAVE_YAJL 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 { -#endif - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Reboot is not supported without the JSON monitor")); -#if HAVE_YAJL } -#endif + +endjob: + if (qemuDomainObjEndJob(driver, vm) == 0) + vm = NULL; cleanup: if (vm) @@ -10439,6 +10496,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.6

On 10/05/2011 11:31 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
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
For backport purposes, should we split this into two patches? 1. wire up the agent with no user control, 2. wire up virDomainShutdownFlags for additional control over the agent. Then patch 1 can be pulled across backports to platforms that cannot rebase due to API reasons, but want to try out the agent (I'm thinking specifically of F16).
@@ -1552,42 +1584,67 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) { goto cleanup; }
-#if HAVE_YAJL 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) ||
Hmm, since the use of the agent requires JSON, and since JSON requires compilation with HAVE_YAJL, are we introducing problems here for compilation on a system without yajl?
+ 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 { -#endif
In particular, the qemuMonitorSystemPowerdown call used to be protected by HAVE_YAJL and now it is not. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange@redhat.com> 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 | 42 ++++++++++++++++++++++++++++++++++++++++-- tools/virsh.pod | 12 ++++++++++-- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 3a17971..e84f60a 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3127,6 +3127,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_DATA, 0, N_("shutdown mode: acpi|agent")}, {NULL, 0, 0, NULL} }; @@ -3136,14 +3137,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); @@ -3165,6 +3189,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_DATA, 0, N_("shutdown mode: acpi|agent")}, {NULL, 0, 0, NULL} }; @@ -3174,14 +3199,27 @@ 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 (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 be81afc..7006025 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -780,7 +780,7 @@ migrated to another host. Get the maximum migration bandwidth (in Mbps) for a domain. -=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 @@ -790,6 +790,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> @@ -1074,7 +1078,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 @@ -1089,6 +1093,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.6

On 10/05/2011 11:31 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
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 | 42 ++++++++++++++++++++++++++++++++++++++++-- tools/virsh.pod | 12 ++++++++++-- 2 files changed, 50 insertions(+), 4 deletions(-)
You remembered the docs. Thanks :) ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

(2011/10/06 2:31), Daniel P. Berrange wrote:
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
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
The QEMU guest agent has other function FS Freeze. But your patches does not support it. Do you have a plan ? In addition, the QEMU guest agent requires specified socket. Virt IO Console, too. But unfortunately libvirt does not support to xml tags to give socket name options to QEMU for the QEMU guest agent and Virt IO Console. MATSUDA Daiki

On Thu, Oct 20, 2011 at 09:39:20AM +0900, MATSUDA, Daiki wrote:
(2011/10/06 2:31), Daniel P. Berrange wrote:
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
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
The QEMU guest agent has other function FS Freeze. But your patches does not support it. Do you have a plan ?
I expect Eric will add FS freeze support as part of his ongoing effort to improve snapshotting.
In addition, the QEMU guest agent requires specified socket. Virt IO Console, too. But unfortunately libvirt does not support to xml tags to give socket name options to QEMU for the QEMU guest agent and Virt IO Console.
Err, yes we do. <channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/> <target type='virtio' name='org.qemu.guest_agent.0'/> </channel> Or for the console <console type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.console'/> <target type='virtio'/> </channel> though you really want to use type=pty for consoles, so that 'virsh console' works correctly. 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 :|

In addition, the QEMU guest agent requires specified socket. Virt IO Console, too. But unfortunately libvirt does not support to xml tags to give socket name options to QEMU for the QEMU guest agent and Virt IO Console.
Err, yes we do.
<channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/> <target type='virtio' name='org.qemu.guest_agent.0'/> </channel>
Or for the console
<console type='unix'> <source mode='bind'
Yes, I confirmed that it can create the socket for guest agent and communicate to guest. path='/var/lib/libvirt/qemu/f16x86_64.console'/>
<target type='virtio'/> </channel>
though you really want to use type=pty for consoles, so that 'virsh
console'
works correctly.
Daniel
But It is not enough. Because I use the socket for VirtIO console, i.e. gives the option '-device virtconsole,chardev=...,name=foo' for qemu. And I read the source code, but found funny... In src/conf/domain_conf.h, struct _virDomainChrDef is struct _virDomainChrDef { ... union { int port; /* parallel, serial, console */ virSocketAddrPtr addr; /* guestfwd */ char *name; /* virtio */ } target; ... It is written that virtio must use char *name. But in docs/schemas/domaincommon.rng and others, VirtIO Console use only int port. Thoug I do not understand that which should be used, I attaches the patch for using *name. P.S. I can not use GIT, because live in restricted network. MATSUDA Daiki

On Mon, Dec 12, 2011 at 10:18:23AM +0900, MATSUDA, Daiki wrote:
In addition, the QEMU guest agent requires specified socket. Virt IO Console, too. But unfortunately libvirt does not support to xml tags to give socket name options to QEMU for the QEMU guest agent and Virt IO Console.
Err, yes we do.
<channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/> <target type='virtio' name='org.qemu.guest_agent.0'/> </channel>
Yes, I confirmed that it can create the socket for guest agent and communicate to guest.
Or for the console
<console type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.console'/> <target type='virtio'/> </channel>
though you really want to use type=pty for consoles, so that 'virsh console' works correctly.
Daniel
But It is not enough. Because I use the socket for VirtIO console, i.e. gives the option '-device virtconsole,chardev=...,name=foo' for qemu.
And I read the source code, but found funny... In src/conf/domain_conf.h, struct _virDomainChrDef is struct _virDomainChrDef { ... union { int port; /* parallel, serial, console */ virSocketAddrPtr addr; /* guestfwd */ char *name; /* virtio */ } target; ... It is written that virtio must use char *name.
But in docs/schemas/domaincommon.rng and others, VirtIO Console use only int port.
Thoug I do not understand that which should be used, I attaches the patch for using *name.
Neither is really relevant for virtio-console, it just provides one or more interactive console for admins. The name is only relevant when coming to create virtio serial channels for non-interactive use. eg the <channel> elements. 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 :|

(2011/12/12 22:58), Daniel P. Berrange wrote:
On Mon, Dec 12, 2011 at 10:18:23AM +0900, MATSUDA, Daiki wrote:
In addition, the QEMU guest agent requires specified socket. Virt IO Console, too. But unfortunately libvirt does not support to xml tags to give socket name options to QEMU for the QEMU guest agent and Virt IO Console.
Err, yes we do.
<channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/> <target type='virtio' name='org.qemu.guest_agent.0'/> </channel>
Yes, I confirmed that it can create the socket for guest agent and communicate to guest.
Or for the console
<console type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.console'/> <target type='virtio'/> </channel>
though you really want to use type=pty for consoles, so that 'virsh console' works correctly.
Daniel
But It is not enough. Because I use the socket for VirtIO console, i.e. gives the option '-device virtconsole,chardev=...,name=foo' for qemu.
And I read the source code, but found funny... In src/conf/domain_conf.h, struct _virDomainChrDef is struct _virDomainChrDef { ... union { int port; /* parallel, serial, console */ virSocketAddrPtr addr; /* guestfwd */ char *name; /* virtio */ } target; ... It is written that virtio must use char *name.
But in docs/schemas/domaincommon.rng and others, VirtIO Console use only int port.
Thoug I do not understand that which should be used, I attaches the patch for using *name.
Neither is really relevant for virtio-console, it just provides one or more interactive console for admins.
The name is only relevant when coming to create virtio serial channels for non-interactive use. eg the<channel> elements.
Daniel
My simple requirement is to give qemu the option such as -device virtio-serial \ -chardev socket,path=/tmp/foo,server,nowait,id=foo \ -device virtconsole,chardev=foo,name=org.fedoraproject.console.foo (e.g. http://fedoraproject.org/wiki/Features/VirtioSerial it has a little mistake not virtioconsole but virtconsole.) But current libvirt source code does not accept name=... with virtconsole. Because 'virtconsole' is only added on qemuBuildVirtioSerialPortDevStr() in src/qemu/qemu_command.c when dev->deviceType is VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE and 'name=' is only added on some function when dev->deviceType is VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL. Similarly it is impossible to use <channel> instead of <console> for virtconsole because deviceType is set as VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL. So, if it is possible to take 2 options port and name for <console> virtio, could you modify from union to struct on struct _vifDomainChrDef in src/conf/domain_conf.h ? MATSUDA Daiki

(2011/12/13 13:02), MATSUDA, Daiki wrote:
(2011/12/12 22:58), Daniel P. Berrange wrote:
On Mon, Dec 12, 2011 at 10:18:23AM +0900, MATSUDA, Daiki wrote:
In addition, the QEMU guest agent requires specified socket. Virt IO Console, too. But unfortunately libvirt does not support to xml tags to give socket name options to QEMU for the QEMU guest agent and Virt IO Console.
Err, yes we do.
<channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/> <target type='virtio' name='org.qemu.guest_agent.0'/> </channel>
Yes, I confirmed that it can create the socket for guest agent and communicate to guest.
Or for the console
<console type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.console'/> <target type='virtio'/> </channel>
though you really want to use type=pty for consoles, so that 'virsh console' works correctly.
Daniel
But It is not enough. Because I use the socket for VirtIO console, i.e. gives the option '-device virtconsole,chardev=...,name=foo' for qemu.
And I read the source code, but found funny... In src/conf/domain_conf.h, struct _virDomainChrDef is struct _virDomainChrDef { ... union { int port; /* parallel, serial, console */ virSocketAddrPtr addr; /* guestfwd */ char *name; /* virtio */ } target; ... It is written that virtio must use char *name.
But in docs/schemas/domaincommon.rng and others, VirtIO Console use only int port.
Thoug I do not understand that which should be used, I attaches the patch for using *name.
Neither is really relevant for virtio-console, it just provides one or more interactive console for admins.
The name is only relevant when coming to create virtio serial channels for non-interactive use. eg the<channel> elements.
Daniel
My simple requirement is to give qemu the option such as -device virtio-serial \ -chardev socket,path=/tmp/foo,server,nowait,id=foo \ -device virtconsole,chardev=foo,name=org.fedoraproject.console.foo (e.g. http://fedoraproject.org/wiki/Features/VirtioSerial it has a little mistake not virtioconsole but virtconsole.)
But current libvirt source code does not accept name=... with virtconsole. Because 'virtconsole' is only added on qemuBuildVirtioSerialPortDevStr() in src/qemu/qemu_command.c when dev->deviceType is VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE and 'name=' is only added on some function when dev->deviceType is VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL.
Similarly it is impossible to use <channel> instead of <console> for virtconsole because deviceType is set as VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL.
So, if it is possible to take 2 options port and name for <console> virtio, could you modify from union to struct on struct _vifDomainChrDef in src/conf/domain_conf.h ?
MATSUDA Daiki
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
I have modified the patch to change from union to struct for struct _vifDomainChrDef. At least, port and name can exist togethter. MATSUDA Daiki diff -uNrp libvirt-0.9.8.orig/docs/schemas/domaincommon.rng libvirt-0.9.8/docs/schemas/domaincommon.rng --- libvirt-0.9.8.orig/docs/schemas/domaincommon.rng 2011-12-08 11:29:49.000000000 +0900 +++ libvirt-0.9.8/docs/schemas/domaincommon.rng 2011-12-15 08:51:27.829971955 +0900 @@ -1814,6 +1814,9 @@ <optional> <attribute name="port"/> </optional> + <optional> + <attribute name="name"/> + </optional> </interleave> </element> </define> diff -uNrp libvirt-0.9.8.orig/src/conf/domain_conf.c libvirt-0.9.8/src/conf/domain_conf.c --- libvirt-0.9.8.orig/src/conf/domain_conf.c 2011-12-08 11:29:49.000000000 +0900 +++ libvirt-0.9.8/src/conf/domain_conf.c 2011-12-15 08:51:27.839971932 +0900 @@ -1156,22 +1156,10 @@ void virDomainChrDefFree(virDomainChrDef if (!def) return; - switch (def->deviceType) { - case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL: - switch (def->targetType) { - case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: - VIR_FREE(def->target.addr); - break; - - case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: - VIR_FREE(def->target.name); - break; - } - break; - - default: - break; - } + if (def->target.addr) + VIR_FREE(def->target.addr); + if (def->target.name) + VIR_FREE(def->target.name); virDomainChrSourceDefClear(&def->source); virDomainDeviceInfoClear(&def->info); @@ -3969,6 +3957,8 @@ virDomainChrDefParseTargetXML(virCapsPtr break; default: + def->target.name = virXMLPropString(cur, "name"); + portStr = virXMLPropString(cur, "port"); if (portStr == NULL) { /* Set to negative value to indicate we should set it later */ @@ -3983,6 +3973,7 @@ virDomainChrDefParseTargetXML(virCapsPtr goto error; } def->target.port = port; + break; } @@ -10406,10 +10397,14 @@ virDomainChrDefFormat(virBufferPtr buf, case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE: virBufferAsprintf(buf, - " <target type='%s' port='%d'/>\n", + " <target type='%s'", virDomainChrTargetTypeToString(def->deviceType, - def->targetType), - def->target.port); + def->targetType)); + if (def->target.port != -1) + virBufferAsprintf(buf, " port='%d'", def->target.port); + if (def->target.name) + virBufferAsprintf(buf, " name='%s'", def->target.name); + virBufferAsprintf(buf, "/>\n"); break; default: diff -uNrp libvirt-0.9.8.orig/src/conf/domain_conf.h libvirt-0.9.8/src/conf/domain_conf.h --- libvirt-0.9.8.orig/src/conf/domain_conf.h 2011-12-08 11:29:49.000000000 +0900 +++ libvirt-0.9.8/src/conf/domain_conf.h 2011-12-15 08:51:27.830961800 +0900 @@ -693,10 +693,10 @@ typedef virDomainChrDef *virDomainChrDef struct _virDomainChrDef { int deviceType; int targetType; - union { - int port; /* parallel, serial, console */ + struct { + int port; /* parallel, serial, console, virtio console */ virSocketAddrPtr addr; /* guestfwd */ - char *name; /* virtio */ + char *name; /* virtio, virtio console */ } target; virDomainChrSourceDef source; diff -uNrp libvirt-0.9.8.orig/src/qemu/qemu_command.c libvirt-0.9.8/src/qemu/qemu_command.c --- libvirt-0.9.8.orig/src/qemu/qemu_command.c 2011-12-02 12:59:50.000000000 +0900 +++ libvirt-0.9.8/src/qemu/qemu_command.c 2011-12-15 08:51:27.832971775 +0900 @@ -3061,8 +3061,7 @@ qemuBuildVirtioSerialPortDevStr(virDomai qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC))) { virBufferAsprintf(&buf, ",chardev=char%s,id=%s", dev->info.alias, dev->info.alias); - if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && - dev->target.name) { + if (dev->target.name) { virBufferAsprintf(&buf, ",name=%s", dev->target.name); } } else { diff -uNrp libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-mix.args libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-mix.args --- libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-mix.args 1970-01-01 09:00:00.000000000 +0900 +++ libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-mix.args 2011-12-15 08:51:27.842971912 +0900 @@ -0,0 +1,12 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \ +socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \ +chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \ +virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 -hda \ +/dev/HostVG/QEMUGuest1 -chardev pty,id=charserial0 \ +-device isa-serial,chardev=charserial0,id=serial0 -chardev pty,id=charconsole1 \ +-device virtconsole,chardev=charconsole1,id=console1,name=bar1 -chardev \ +pty,id=charconsole2 -device virtconsole,chardev=charconsole2,id=console2,name=foo2 \ +-chardev pty,id=charconsole3 -device virtconsole,chardev=charconsole3,\ +id=console3,name=bar3 -usb -device virtio-balloon-pci,id=balloon0,\ +bus=pci.0,addr=0x4 diff -uNrp libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-mix.xml libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-mix.xml --- libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-mix.xml 1970-01-01 09:00:00.000000000 +0900 +++ libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-mix.xml 2011-12-15 08:51:27.842971912 +0900 @@ -0,0 +1,41 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu cpuset='1-4,8-20,525'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <controller type='virtio-serial' index='0'/> + <serial type='pty'> + <target port='0'/> + </serial> + <console type='pty'> + <target type='serial' port='0' name='foo0'/> + </console> + <console type='pty'> + <target type='virtio' name='bar1'/> + </console> + <console type='pty'> + <target type='virtio' name='foo2' port='2'/> + </console> + <console type='pty'> + <target type='virtio' port='3' name='bar3'/> + </console> + <memballoon model='virtio'/> + </devices> +</domain> diff -uNrp libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-name.args libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-name.args --- libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-name.args 1970-01-01 09:00:00.000000000 +0900 +++ libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-name.args 2011-12-15 08:51:27.834971875 +0900 @@ -0,0 +1,12 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \ +socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \ +chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \ +virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 -hda \ +/dev/HostVG/QEMUGuest1 -chardev pty,id=charserial0 \ +-device isa-serial,chardev=charserial0,id=serial0 -chardev pty,id=charconsole1 \ +-device virtconsole,chardev=charconsole1,id=console1,name=bar1 -chardev \ +pty,id=charconsole2 -device virtconsole,chardev=charconsole2,id=console2,name=foo2 \ +-chardev pty,id=charconsole3 -device virtconsole,chardev=charconsole3,\ +id=console3,name=bar3 -usb -device virtio-balloon-pci,id=balloon0,\ +bus=pci.0,addr=0x4 diff -uNrp libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-name.xml libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-name.xml --- libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-name.xml 1970-01-01 09:00:00.000000000 +0900 +++ libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many-name.xml 2011-12-15 08:51:27.834971875 +0900 @@ -0,0 +1,41 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu cpuset='1-4,8-20,525'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <controller type='virtio-serial' index='0'/> + <serial type='pty'> + <target port='0'/> + </serial> + <console type='pty'> + <target type='serial' name='foo0'/> + </console> + <console type='pty'> + <target type='virtio' name='bar1'/> + </console> + <console type='pty'> + <target type='virtio' name='foo2'/> + </console> + <console type='pty'> + <target type='virtio' name='bar3'/> + </console> + <memballoon model='virtio'/> + </devices> +</domain> diff -uNrp libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-mix.args libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-mix.args --- libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-mix.args 1970-01-01 09:00:00.000000000 +0900 +++ libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-mix.args 2011-12-15 08:51:27.841971785 +0900 @@ -0,0 +1,8 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \ +socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \ +chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \ +virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 -hda \ +/dev/HostVG/QEMUGuest1 -chardev pty,id=charconsole0 -device virtconsole,\ +chardev=charconsole0,id=console0,name=foo -usb -device virtio-balloon-pci,id=balloon0,\ +bus=pci.0,addr=0x4 diff -uNrp libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-mix.xml libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-mix.xml --- libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-mix.xml 1970-01-01 09:00:00.000000000 +0900 +++ libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-mix.xml 2011-12-15 08:51:27.841971785 +0900 @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu cpuset='1-4,8-20,525'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <console type='pty'> + <target type='virtio' port='0' name='foo'/> + </console> + </devices> +</domain> diff -uNrp libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-name.args libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-name.args --- libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-name.args 1970-01-01 09:00:00.000000000 +0900 +++ libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-name.args 2011-12-15 08:51:27.841971785 +0900 @@ -0,0 +1,8 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \ +socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \ +chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \ +virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 -hda \ +/dev/HostVG/QEMUGuest1 -chardev pty,id=charconsole0 -device virtconsole,\ +chardev=charconsole0,id=console0,name=foo -usb -device virtio-balloon-pci,id=balloon0,\ +bus=pci.0,addr=0x4 diff -uNrp libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-name.xml libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-name.xml --- libvirt-0.9.8.orig/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-name.xml 1970-01-01 09:00:00.000000000 +0900 +++ libvirt-0.9.8/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-name.xml 2011-12-15 08:51:27.841971785 +0900 @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu cpuset='1-4,8-20,525'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <console type='pty'> + <target type='virtio' name='foo'/> + </console> + </devices> +</domain> diff -uNrp libvirt-0.9.8.orig/tests/qemuxml2argvtest.c libvirt-0.9.8/tests/qemuxml2argvtest.c --- libvirt-0.9.8.orig/tests/qemuxml2argvtest.c 2011-12-02 12:59:56.000000000 +0900 +++ libvirt-0.9.8/tests/qemuxml2argvtest.c 2011-12-15 08:51:27.843971916 +0900 @@ -539,6 +539,14 @@ mymain(void) QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); DO_TEST("console-virtio-many", false, QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); + DO_TEST("console-virtio-name", false, + QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); + DO_TEST("console-virtio-many-name", false, + QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); + DO_TEST("console-virtio-mix", false, + QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); + DO_TEST("console-virtio-many-mix", false, + QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); DO_TEST("channel-spicevmc", false, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_SPICE, QEMU_CAPS_CHARDEV_SPICEVMC);

On 12/11/2011 06:18 PM, MATSUDA, Daiki wrote:
P.S. I can not use GIT, because live in restricted network.
Not necessarily true. Git, being a distributed version control system, is great at letting you copy a repository via non-networked means between systems that can't share a network. For example, you can tar up the .git directory from a machine that has libvirt.git checked out, burn the tarball to a CD, sneakernet the CD to your other machine, and unpack things on your firewalled devel machine for an initial setup of a full git repository; then commands like 'git bundle' can be used to provide incremental synchronization between your machine with network access and your machine where you are developing. Granted, it's not as easy as network access, but at least it remains possible (which is more than can be said of any centralized system like CVS). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
MATSUDA, Daiki