- Devel - libvirt List Archives

[libvirt] [PATCH] daemon: plug a memory leak
by Eric Blake 13 Dec '10

13 Dec '10

* daemon/libvirtd.c (qemudFreeClient): Avoid a leak. (qemudDispatchServer): Avoid null dereference. --- I keep finding more of these. daemon/libvirtd.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 791b3dc..2914f2f 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1410,7 +1410,7 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket /* Client is running as root, so disable auth */ if (uid == 0) { VIR_INFO(_("Turn off polkit auth for privileged client pid %d from %s"), - pid, addrstr); + pid, client->addrstr); client->auth = REMOTE_AUTH_NONE; } } @@ -1451,7 +1451,7 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket } else { PROBE(CLIENT_TLS_FAIL, "fd=%d", client->fd); VIR_ERROR(_("TLS handshake failed for client %s: %s"), - addrstr, gnutls_strerror (ret)); + client->addrstr, gnutls_strerror (ret)); goto error; } } @@ -2283,6 +2283,7 @@ static void qemudFreeClient(struct qemud_client *client) { if (client->conn) virConnectClose(client->conn); virMutexDestroy(&client->lock); + VIR_FREE(client->addrstr); VIR_FREE(client); } -- 1.7.3.2

3 3

[libvirt] [PATCH] qemu: Introduce two new job types
by Osier Yang 13 Dec '10

13 Dec '10

Currently, all of domain "save/dump/managed save/migration" use the same function "qemudDomainWaitForMigrationComplete" to wait the job finished, but the error messages are all about "migration", e.g. when a domain saving job is canceled by user, "migration was cancled by client" will be throwed as an error message, which will be confused for user. As a solution, intoduce two new job types(QEMU_JOB_SAVE, QEMU_JOB_DUMP), and set "priv->jobActive" to "QEMU_JOB_SAVE" before saving, to "QEMU_JOB_DUMP" before dumping, so that we could get the real job type in "qemudDomainWaitForMigrationComplete", and give more clear message further. * src/qemu/qemu_driver.c --- src/qemu/qemu_driver.c | 39 ++++++++++++++++++++++++++++++--------- 1 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 19ce9a6..aed48f4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -104,6 +104,8 @@ enum qemuDomainJob { QEMU_JOB_UNSPECIFIED, QEMU_JOB_MIGRATION_OUT, QEMU_JOB_MIGRATION_IN, + QEMU_JOB_SAVE, + QEMU_JOB_DUMP, }; enum qemuDomainJobSignals { @@ -5389,21 +5391,36 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull }; struct timeval now; int rc; + const char *job; + + switch (priv->jobActive) { + case QEMU_JOB_MIGRATION_OUT: + job = "migration"; + break; + case QEMU_JOB_SAVE: + job = "domain saving"; + break; + case QEMU_JOB_DUMP: + job = "domain core dump"; + break; + default: + job = "job"; + } if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit during migration")); + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("guest unexpectedly quit during %s"), job); goto cleanup; } if (priv->jobSignals & QEMU_JOB_SIGNAL_CANCEL) { priv->jobSignals ^= QEMU_JOB_SIGNAL_CANCEL; - VIR_DEBUG0("Cancelling migration at client request"); + VIR_DEBUG("Cancelling %s at client request", job); qemuDomainObjEnterMonitorWithDriver(driver, vm); rc = qemuMonitorMigrateCancel(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); if (rc < 0) { - VIR_WARN0("Unable to cancel migration"); + VIR_WARN("Unable to cancel %s", job); } } else if (priv->jobSignals & QEMU_JOB_SIGNAL_SUSPEND) { priv->jobSignals ^= QEMU_JOB_SIGNAL_SUSPEND; @@ -5427,8 +5444,8 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr * guest to die */ if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit during migration")); + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("guest unexpectedly quit during %s"), job); goto cleanup; } @@ -5459,7 +5476,7 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE: priv->jobInfo.type = VIR_DOMAIN_JOB_NONE; qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Migration is not active")); + _("%s is not active"), job); break; case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE: @@ -5480,13 +5497,13 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr case QEMU_MONITOR_MIGRATION_STATUS_ERROR: priv->jobInfo.type = VIR_DOMAIN_JOB_FAILED; qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Migration unexpectedly failed")); + _("%s unexpectedly failed"), job); break; case QEMU_MONITOR_MIGRATION_STATUS_CANCELLED: priv->jobInfo.type = VIR_DOMAIN_JOB_CANCELLED; qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Migration was cancelled by client")); + _("%s was cancelled by client"), job); break; } @@ -5606,6 +5623,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, goto endjob; } + priv->jobActive = QEMU_JOB_SAVE; + memset(&priv->jobInfo, 0, sizeof(priv->jobInfo)); priv->jobInfo.type = VIR_DOMAIN_JOB_UNBOUNDED; @@ -6198,6 +6217,8 @@ static int qemudDomainCoreDump(virDomainPtr dom, goto endjob; } + priv->jobActive = QEMU_JOB_DUMP; + /* Migrate will always stop the VM, so the resume condition is independent of whether the stop command is issued. */ resume = (vm->state == VIR_DOMAIN_RUNNING); -- 1.7.3.2

2 4

[libvirt] ANNOUNCE: ruby-libvirt bindings 0.3.0
by Chris Lalancette 13 Dec '10

13 Dec '10

All, I'm pleased to announce the release of 0.3.0 of the ruby-libvirt bindings. This release has a number of updates: * Implementation of Libvirt::open_auth, Libvirt::event_register_impl * Updated Connect class, implementing conn.compare_cpu, conn.baseline_cpu, conn.domain_event_register_any, conn.domain_event_deregister_any, conn.domain_event_register, conn.domain_event_deregister, and conn.create_domain_xml. * Updated Domain class, implementing dom.get_vcpus, dom.update_device, dom.scheduler_type, dom.scheduler_parameters, dom.scheduler_parameters=, dom.num_vcpus, dom.vcpus_flags=, and dom.qemu_monitor_command. * Updated Interface class, implementing interface.free * Many potential memory leaks have been fixed. * Many bugfixes. * Documentation update of many methods, including all of the lookup methods that were missing before. With the above list, almost all of the APIs up to and including libvirt 0.8.6 are covered. There are a few that I have not yet had time to implement; you can look at the list in the README file in the source tree to see which ones are missing. The tarballs and source RPMs are here: http://libvirt.org/ruby/download/ The git repository is here: http://libvirt.org/git/?p=ruby-libvirt.git;a=summary The documentation for all of the APIs are here: http://libvirt.org/ruby/api/index.html I've uploaded some examples on how to use the bindings: http://libvirt.org/ruby/documentation.html I've also pushed a new gem to rubygems.org; you should be able to use 'gem install ruby-libvirt' (if you haven't installed them before) or 'gem update ruby-libvirt' (if you have them installed already). If you have time, and are interested in the bindings, please give these new ones a whirl and let me know if they work for you. Note that release 0.3.0 is intended to be fully backwards compatible with release 0.2.0 and 0.1.0. If you find that they are not, or find any other problems with the ruby bindings, please report it to the libvirt development mailing list (and CC me) so that I can take a look at them. Thanks, -- Chris Lalancette

1 0

[libvirt] [PATCH] Convert dhcpStartDhcpDaemon from virRun to virCommand
by Laine Stump 11 Dec '10

11 Dec '10

This is pretty straightforward - even though dnsmasq gets daemonized and uses a pid file, those things are both handled by the dnsmasq binary itself. And libvirt doesn't need any of the output of the dnsmasq command either, so we just setup the args and call virRun(). Mainly it was just a (mostly) mechanical job of replacing the APPEND_ARG() macro (and some other *printfs()) with virCommandAddArg*(). --- src/network/bridge_driver.c | 238 +++++++++++++++---------------------------- 1 files changed, 80 insertions(+), 158 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 62639e4..9802222 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -51,6 +51,7 @@ #include "event.h" #include "buf.h" #include "util.h" +#include "command.h" #include "memory.h" #include "uuid.h" #include "iptables.h" @@ -390,25 +391,15 @@ networkSaveDnsmasqHostsfile(virNetworkObjPtr network, static int networkBuildDnsmasqArgv(virNetworkObjPtr network, const char *pidfile, - const char ***argv) { - int i, len, r; + virCommandPtr cmd) { + int r, ret = -1; int nbleases = 0; - char *pidfileArg; - char buf[1024]; - unsigned int ranges; - char *ipAddr; - - /* - * For static-only DHCP, i.e. with no range but at least one host element, - * we have to add a special --dhcp-range option to enable the service in - * dnsmasq. - */ - ranges = network->def->nranges; - if (!ranges && network->def->nhosts) - ranges = 1; + char *bridgeaddr; + if (!(bridgeaddr = virSocketFormatAddr(&network->def->ipAddress))) + goto cleanup; /* - * NB, be careful about syntax for dnsmasq options in long format + * NB, be careful about syntax for dnsmasq options in long format. * * If the flag has a mandatory argument, it can be given using * either syntax: @@ -422,66 +413,27 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, * --foo=bar * * It is hard to determine whether a flag is optional or not, - * without reading the dnsmasq source :-( The manpages is not - * very explicit on this + * without reading the dnsmasq source :-( The manpage is not + * very explicit on this. */ - len = - 1 + /* dnsmasq */ - 1 + /* --strict-order */ - 1 + /* --bind-interfaces */ - (network->def->domain?2:0) + /* --domain name */ - 2 + /* --pid-file /var/run/libvirt/network/$NAME.pid */ - 2 + /* --conf-file "" */ - /*2 + *//* --interface virbr0 */ - 2 + /* --except-interface lo */ - 2 + /* --listen-address 10.0.0.1 */ - (2 * ranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */ - /* --dhcp-lease-max=xxx if needed */ - (network->def->nranges ? 1 : 0) + - /* --dhcp-no-override if needed */ - (ranges ? 1 : 0) + - /* --dhcp-hostsfile=/var/lib/dnsmasq/$NAME.hostsfile */ - (network->def->nhosts > 0 ? 1 : 0) + - /* --enable-tftp --tftp-root /srv/tftp */ - (network->def->tftproot ? 3 : 0) + - /* --dhcp-boot pxeboot.img[,,12.34.56.78] */ - (network->def->bootfile ? 2 : 0) + - 1; /* NULL */ - - if (VIR_ALLOC_N(*argv, len) < 0) - goto no_memory; - -#define APPEND_ARG(v, n, s) do { \ - if (!((v)[(n)] = strdup(s))) \ - goto no_memory; \ - } while (0) - -#define APPEND_ARG_LIT(v, n, s) \ - (v)[(n)] = s - - i = 0; - - APPEND_ARG(*argv, i++, DNSMASQ); - /* * Needed to ensure dnsmasq uses same algorithm for processing * multiple namedriver entries in /etc/resolv.conf as GLibC. */ - APPEND_ARG(*argv, i++, "--strict-order"); - APPEND_ARG(*argv, i++, "--bind-interfaces"); + virCommandAddArg(cmd, "--strict-order"); + virCommandAddArg(cmd, "--bind-interfaces"); if (network->def->domain) { - APPEND_ARG(*argv, i++, "--domain"); - APPEND_ARG(*argv, i++, network->def->domain); + virCommandAddArg(cmd, "--domain"); + virCommandAddArg(cmd, network->def->domain); } - if (virAsprintf(&pidfileArg, "--pid-file=%s", pidfile) < 0) - goto no_memory; - APPEND_ARG_LIT(*argv, i++, pidfileArg); + virCommandAddArgPair(cmd, "--pid-file", pidfile); - APPEND_ARG(*argv, i++, "--conf-file="); - APPEND_ARG(*argv, i++, ""); + /* *no* conf file */ + virCommandAddArg(cmd, "--conf-file="); + virCommandAddArg(cmd, ""); /* * XXX does not actually work, due to some kind of @@ -489,166 +441,140 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, * interface. A sleep(10) makes it work, but that's * clearly not practical * - * APPEND_ARG(*argv, i++, "--interface"); - * APPEND_ARG(*argv, i++, network->def->bridge); + * virCommandAddArg(cmd, "--interface"); + * virCommandAddArg(cmd, network->def->bridge); */ - APPEND_ARG(*argv, i++, "--listen-address"); - if (!(ipAddr = virSocketFormatAddr(&network->def->ipAddress))) - goto error; - APPEND_ARG_LIT(*argv, i++, ipAddr); + virCommandAddArg(cmd, "--listen-address"); + virCommandAddArg(cmd, bridgeaddr); - APPEND_ARG(*argv, i++, "--except-interface"); - APPEND_ARG(*argv, i++, "lo"); + virCommandAddArg(cmd, "--except-interface"); + virCommandAddArg(cmd, "lo"); for (r = 0 ; r < network->def->nranges ; r++) { char *saddr = virSocketFormatAddr(&network->def->ranges[r].start); if (!saddr) - goto error; + goto cleanup; char *eaddr = virSocketFormatAddr(&network->def->ranges[r].end); if (!eaddr) { VIR_FREE(saddr); - goto error; + goto cleanup; } - char *range; - int rc = virAsprintf(&range, "%s,%s", saddr, eaddr); + virCommandAddArg(cmd, "--dhcp-range"); + virCommandAddArgFormat(cmd, "%s,%s", saddr, eaddr); VIR_FREE(saddr); VIR_FREE(eaddr); - if (rc < 0) - goto no_memory; - APPEND_ARG(*argv, i++, "--dhcp-range"); - APPEND_ARG_LIT(*argv, i++, range); nbleases += virSocketGetRange(&network->def->ranges[r].start, &network->def->ranges[r].end); } + /* + * For static-only DHCP, i.e. with no range but at least one host element, + * we have to add a special --dhcp-range option to enable the service in + * dnsmasq. + */ if (!network->def->nranges && network->def->nhosts) { - char *ipaddr = virSocketFormatAddr(&network->def->ipAddress); - if (!ipaddr) - goto error; - char *range; - int rc = virAsprintf(&range, "%s,static", ipaddr); - VIR_FREE(ipaddr); - if (rc < 0) - goto no_memory; - - APPEND_ARG(*argv, i++, "--dhcp-range"); - APPEND_ARG_LIT(*argv, i++, range); + virCommandAddArg(cmd, "--dhcp-range"); + virCommandAddArgFormat(cmd, "%s,static", bridgeaddr); } if (network->def->nranges > 0) { - snprintf(buf, sizeof(buf), "--dhcp-lease-max=%d", nbleases); - APPEND_ARG(*argv, i++, buf); + virCommandAddArgFormat(cmd, "--dhcp-lease-max=%d", nbleases); } - if (ranges) - APPEND_ARG(*argv, i++, "--dhcp-no-override"); + if (network->def->nranges || network->def->nhosts) + virCommandAddArg(cmd, "--dhcp-no-override"); if (network->def->nhosts > 0) { - dnsmasqContext *dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR); - char *hostsfileArg; - - if (dctx == NULL) - goto no_memory; + dnsmasqContext *dctx = dnsmasqContextNew(network->def->name, + DNSMASQ_STATE_DIR); + if (dctx == NULL) { + virReportOOMError(); + goto cleanup; + } if (networkSaveDnsmasqHostsfile(network, dctx, false)) { - if (virAsprintf(&hostsfileArg, "--dhcp-hostsfile=%s", dctx->hostsfile->path) < 0) { - dnsmasqContextFree(dctx); - goto no_memory; - } - APPEND_ARG_LIT(*argv, i++, hostsfileArg); + virCommandAddArgPair(cmd, "--dhcp-hostsfile", + dctx->hostsfile->path); } - dnsmasqContextFree(dctx); } if (network->def->tftproot) { - APPEND_ARG(*argv, i++, "--enable-tftp"); - APPEND_ARG(*argv, i++, "--tftp-root"); - APPEND_ARG(*argv, i++, network->def->tftproot); + virCommandAddArg(cmd, "--enable-tftp"); + virCommandAddArg(cmd, "--tftp-root"); + virCommandAddArg(cmd, network->def->tftproot); } if (network->def->bootfile) { - char *ipaddr = NULL; + + virCommandAddArg(cmd, "--dhcp-boot"); if (VIR_SOCKET_HAS_ADDR(&network->def->bootserver)) { - if (!(ipaddr = virSocketFormatAddr(&network->def->bootserver))) - goto error; - } - char *boot; - int rc = virAsprintf(&boot, "%s%s%s", - network->def->bootfile, - ipaddr ? ",," : "", - ipaddr ? ipaddr : ""); - VIR_FREE(ipaddr); - if (rc < 0) - goto no_memory; + char *bootserver = virSocketFormatAddr(&network->def->bootserver); - APPEND_ARG(*argv, i++, "--dhcp-boot"); - APPEND_ARG_LIT(*argv, i++, boot); + if (!bootserver) + goto cleanup; + virCommandAddArgFormat(cmd, "%s%s%s", + network->def->bootfile, ",,", bootserver); + VIR_FREE(bootserver); + } else { + virCommandAddArg(cmd, network->def->bootfile); + } } -#undef APPEND_ARG - - return 0; - - no_memory: - virReportOOMError(); - error: - if (*argv) { - for (i = 0; (*argv)[i]; i++) - VIR_FREE((*argv)[i]); - VIR_FREE(*argv); - } - return -1; + ret = 0; +cleanup: + VIR_FREE(bridgeaddr); + return ret; } static int dhcpStartDhcpDaemon(virNetworkObjPtr network) { - const char **argv; - char *pidfile; - int ret = -1, i, err; + virCommandPtr cmd = NULL; + char *pidfile = NULL; + int ret = -1, err; network->dnsmasqPid = -1; if (!VIR_SOCKET_IS_FAMILY(&network->def->ipAddress, AF_INET)) { networkReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot start dhcp daemon without IPv4 address for server")); - return -1; + goto cleanup; } if ((err = virFileMakePath(NETWORK_PID_DIR)) != 0) { virReportSystemError(err, _("cannot create directory %s"), NETWORK_PID_DIR); - return -1; + goto cleanup; } if ((err = virFileMakePath(NETWORK_STATE_DIR)) != 0) { virReportSystemError(err, _("cannot create directory %s"), NETWORK_STATE_DIR); - return -1; + goto cleanup; } if (!(pidfile = virFilePid(NETWORK_PID_DIR, network->def->name))) { virReportOOMError(); - return -1; + goto cleanup; } - argv = NULL; - if (networkBuildDnsmasqArgv(network, pidfile, &argv) < 0) { + cmd = virCommandNew(DNSMASQ); + if (networkBuildDnsmasqArgv(network, pidfile, cmd) < 0) { VIR_FREE(pidfile); - return -1; + goto cleanup; } - if (virRun(argv, NULL) < 0) + if (virCommandRun(cmd, NULL) < 0) goto cleanup; /* - * There really is no race here - when dnsmasq daemonizes, - * its leader process stays around until its child has - * actually written its pidfile. So by time virRun exits - * it has waitpid'd and guaranteed the proess has started - * and written a pid + * There really is no race here - when dnsmasq daemonizes, its + * leader process stays around until its child has actually + * written its pidfile. So by time virCommandRun exits it has + * waitpid'd and guaranteed the proess has started and written a + * pid */ if (virFileReadPid(NETWORK_PID_DIR, network->def->name, @@ -656,13 +582,9 @@ dhcpStartDhcpDaemon(virNetworkObjPtr network) goto cleanup; ret = 0; - cleanup: VIR_FREE(pidfile); - for (i = 0; argv[i]; i++) - VIR_FREE(argv[i]); - VIR_FREE(argv); - + virCommandFree(cmd); return ret; } -- 1.7.3.2

3 4

[libvirt] [PATCH 0/4] more memory leak patches
by Eric Blake 10 Dec '10

10 Dec '10

More in the ongoing battle to make libvirtd not so leaky. Also related, and still waiting on a review for: virExec (and thus virCommand) have undefined behavior https://www.redhat.com/archives/libvir-list/2010-December/msg00242.html avoid matchpathcon memory leak (although libselinux 2.0.97 finally patched that leak upstream) https://www.redhat.com/archives/libvir-list/2010-December/msg00001.html The leak in qemu_conf covered in patch 3 was introduced with the conversion to virCommand; if you like patch 4, I'd rather combine those into one patch (but patch 3 is a minimal plug). Eric Blake (4): tests: plug memory leaks conf: plug memory leaks qemu: plug memory leak command: ease use with virBuffer src/conf/domain_conf.c | 10 ++++++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_conf.c | 42 +++++++++--------------------------------- src/util/command.c | 43 +++++++++++++++++++++++++++++++++++++++++++ src/util/command.h | 17 +++++++++++++++++ tests/qemuxml2argvtest.c | 3 +-- 6 files changed, 82 insertions(+), 35 deletions(-) -- 1.7.3.2

2 11

[libvirt] [PATCH] virExec: avoid undefined behavior
by Eric Blake 10 Dec '10

10 Dec '10

* src/util/util.c (__virExec): Don't use FD_ISSET on out-of-bounds fd. --- Noticed this one by inspection, while investigating https://bugzilla.redhat.com/show_bug.cgi?id=659855 Don't know if it's the root cause of the crash in that bug, though. src/util/util.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 79ca5d3..1b5bc68 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -570,8 +570,7 @@ __virExec(const char *const*argv, i != null && i != childout && i != childerr && - (!keepfd || - !FD_ISSET(i, keepfd))) { + (!keepfd || (i < FD_SETSIZE && !FD_ISSET(i, keepfd)))) { tmpfd = i; VIR_FORCE_CLOSE(tmpfd); } -- 1.7.3.2

2 2

[libvirt] [PATCH] esx: Refactor storage pool type lookup into a function
by Matthias Bolte 10 Dec '10

10 Dec '10

--- src/esx/esx_storage_driver.c | 130 ++++++++++++++++++------------------------ 1 files changed, 56 insertions(+), 74 deletions(-) diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index 83b53fb..9165f7a 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -49,6 +49,58 @@ verify(MD5_DIGEST_SIZE == VIR_UUID_BUFLEN); +static int +esxStoragePoolLookupType(esxVI_Context *ctx, const char *poolName, + int *poolType) +{ + int result = -1; + esxVI_String *propertyNameList = NULL; + esxVI_ObjectContent *datastore = NULL; + esxVI_DynamicProperty *dynamicProperty = NULL; + esxVI_DatastoreInfo *datastoreInfo = NULL; + + if (esxVI_String_AppendValueToList(&propertyNameList, "info") < 0 || + esxVI_LookupDatastoreByName(ctx, poolName, propertyNameList, &datastore, + esxVI_Occurrence_RequiredItem) < 0) { + goto cleanup; + } + + for (dynamicProperty = datastore->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, "info")) { + if (esxVI_DatastoreInfo_CastFromAnyType(dynamicProperty->val, + &datastoreInfo) < 0) { + goto cleanup; + } + + break; + } + } + + if (esxVI_LocalDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { + *poolType = VIR_STORAGE_POOL_DIR; + } else if (esxVI_NasDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { + *poolType = VIR_STORAGE_POOL_NETFS; + } else if (esxVI_VmfsDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { + *poolType = VIR_STORAGE_POOL_FS; + } else { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + _("DatastoreInfo has unexpected type")); + goto cleanup; + } + + result = 0; + + cleanup: + esxVI_String_Free(&propertyNameList); + esxVI_ObjectContent_Free(&datastore); + esxVI_DatastoreInfo_Free(&datastoreInfo); + + return result; +} + + + static virDrvOpenStatus esxStorageOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, @@ -908,10 +960,6 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, { virStorageVolPtr volume = NULL; esxPrivate *priv = pool->conn->storagePrivateData; - esxVI_String *propertyNameList = NULL; - esxVI_ObjectContent *datastore = NULL; - esxVI_DynamicProperty *dynamicProperty = NULL; - esxVI_DatastoreInfo *datastoreInfo = NULL; virStoragePoolDef poolDef; virStorageVolDefPtr def = NULL; char *tmp; @@ -938,36 +986,8 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, return NULL; } - /* Lookup storage pool type */ - if (esxVI_String_AppendValueToList(&propertyNameList, "info") < 0 || - esxVI_LookupDatastoreByName(priv->primary, pool->name, - propertyNameList, &datastore, - esxVI_Occurrence_RequiredItem) < 0) { - goto cleanup; - } - - for (dynamicProperty = datastore->propSet; dynamicProperty != NULL; - dynamicProperty = dynamicProperty->_next) { - if (STREQ(dynamicProperty->name, "info")) { - if (esxVI_DatastoreInfo_CastFromAnyType(dynamicProperty->val, - &datastoreInfo) < 0) { - goto cleanup; - } - - break; - } - } - - if (esxVI_LocalDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { - poolDef.type = VIR_STORAGE_POOL_DIR; - } else if (esxVI_NasDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { - poolDef.type = VIR_STORAGE_POOL_NETFS; - } else if (esxVI_VmfsDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { - poolDef.type = VIR_STORAGE_POOL_FS; - } else { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", - _("DatastoreInfo has unexpected type")); - goto cleanup; + if (esxStoragePoolLookupType(priv->primary, pool->name, &poolDef.type) < 0) { + return NULL; } /* Parse config */ @@ -1140,9 +1160,6 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, virtualDiskSpec->adapterType = NULL; } - esxVI_String_Free(&propertyNameList); - esxVI_ObjectContent_Free(&datastore); - esxVI_DatastoreInfo_Free(&datastoreInfo); virStorageVolDefFree(def); VIR_FREE(unescapedDatastorePath); VIR_FREE(unescapedDirectoryName); @@ -1216,10 +1233,6 @@ static char * esxStorageVolumeDumpXML(virStorageVolPtr volume, unsigned int flags) { esxPrivate *priv = volume->conn->storagePrivateData; - esxVI_String *propertyNameList = NULL; - esxVI_ObjectContent *datastore = NULL; - esxVI_DynamicProperty *dynamicProperty = NULL; - esxVI_DatastoreInfo *datastoreInfo = NULL; virStoragePoolDef pool; char *datastorePath = NULL; esxVI_FileInfo *fileInfo = NULL; @@ -1238,36 +1251,8 @@ esxStorageVolumeDumpXML(virStorageVolPtr volume, unsigned int flags) return NULL; } - /* Lookup storage pool type */ - if (esxVI_String_AppendValueToList(&propertyNameList, "info") < 0 || - esxVI_LookupDatastoreByName(priv->primary, volume->pool, - propertyNameList, &datastore, - esxVI_Occurrence_RequiredItem) < 0) { - goto cleanup; - } - - for (dynamicProperty = datastore->propSet; dynamicProperty != NULL; - dynamicProperty = dynamicProperty->_next) { - if (STREQ(dynamicProperty->name, "info")) { - if (esxVI_DatastoreInfo_CastFromAnyType(dynamicProperty->val, - &datastoreInfo) < 0) { - goto cleanup; - } - - break; - } - } - - if (esxVI_LocalDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { - pool.type = VIR_STORAGE_POOL_DIR; - } else if (esxVI_NasDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { - pool.type = VIR_STORAGE_POOL_NETFS; - } else if (esxVI_VmfsDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { - pool.type = VIR_STORAGE_POOL_FS; - } else { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", - _("DatastoreInfo has unexpected type")); - goto cleanup; + if (esxStoragePoolLookupType(priv->primary, volume->pool, &pool.type) < 0) { + return NULL; } /* Lookup file info */ @@ -1320,9 +1305,6 @@ esxStorageVolumeDumpXML(virStorageVolPtr volume, unsigned int flags) xml = virStorageVolDefFormat(&pool, &def); cleanup: - esxVI_String_Free(&propertyNameList); - esxVI_ObjectContent_Free(&datastore); - esxVI_DatastoreInfo_Free(&datastoreInfo); VIR_FREE(datastorePath); esxVI_FileInfo_Free(&fileInfo); VIR_FREE(def.key); -- 1.7.0.4

2 2

[libvirt] [PATCH] esx: Improve error reporting for failed tasks
by Matthias Bolte 10 Dec '10

10 Dec '10

Instead of just reporting that a task failed get the localized message from the TaskInfo error and include it in the reported error message. Implement minimal deserialization support for the MethodFault type in order to obtain the actual fault type. For example, this changes the reported error message when trying to create a volume with zero size from Could not create volume to Could not create volume: InvalidArgument - A specified parameter was not correct. Not perfect yet, but better than before. --- src/esx/esx_driver.c | 118 +++++++++++++++++++++++++++++----------- src/esx/esx_storage_driver.c | 8 ++- src/esx/esx_vi.c | 53 ++++++++++++++++-- src/esx/esx_vi.h | 3 +- src/esx/esx_vi_generator.input | 8 +++- src/esx/esx_vi_generator.py | 2 + src/esx/esx_vi_types.c | 51 +++++++++++++++++- src/esx/esx_vi_types.h | 25 +++++++++ 8 files changed, 227 insertions(+), 41 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 8ea6219..03a7be3 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1706,6 +1706,7 @@ esxDomainSuspend(virDomainPtr domain) esxVI_VirtualMachinePowerState powerState; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { return -1; @@ -1729,12 +1730,14 @@ esxDomainSuspend(virDomainPtr domain) if (esxVI_SuspendVM_Task(priv->primary, virtualMachine->obj, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not suspend domain")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not suspend domain: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -1744,6 +1747,7 @@ esxDomainSuspend(virDomainPtr domain) esxVI_ObjectContent_Free(&virtualMachine); esxVI_String_Free(&propertyNameList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -1760,6 +1764,7 @@ esxDomainResume(virDomainPtr domain) esxVI_VirtualMachinePowerState powerState; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { return -1; @@ -1783,12 +1788,14 @@ esxDomainResume(virDomainPtr domain) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not resume domain")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not resume domain: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -1798,6 +1805,7 @@ esxDomainResume(virDomainPtr domain) esxVI_ObjectContent_Free(&virtualMachine); esxVI_String_Free(&propertyNameList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -1901,6 +1909,7 @@ esxDomainDestroy(virDomainPtr domain) esxVI_VirtualMachinePowerState powerState; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; if (priv->vCenter != NULL) { ctx = priv->vCenter; @@ -1930,12 +1939,14 @@ esxDomainDestroy(virDomainPtr domain) if (esxVI_PowerOffVM_Task(ctx, virtualMachine->obj, &task) < 0 || esxVI_WaitForTaskCompletion(ctx, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not destroy domain")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not destroy domain: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -1946,6 +1957,7 @@ esxDomainDestroy(virDomainPtr domain) esxVI_ObjectContent_Free(&virtualMachine); esxVI_String_Free(&propertyNameList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -2028,6 +2040,7 @@ esxDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) esxVI_VirtualMachineConfigSpec *spec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { return -1; @@ -2048,13 +2061,15 @@ esxDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not set max-memory to %lu kilobytes"), memory); + _("Could not set max-memory to %lu kilobytes: %s"), memory, + taskInfoErrorMessage); goto cleanup; } @@ -2064,6 +2079,7 @@ esxDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineConfigSpec_Free(&spec); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -2079,6 +2095,7 @@ esxDomainSetMemory(virDomainPtr domain, unsigned long memory) esxVI_VirtualMachineConfigSpec *spec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { return -1; @@ -2100,13 +2117,15 @@ esxDomainSetMemory(virDomainPtr domain, unsigned long memory) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not set memory to %lu kilobytes"), memory); + _("Could not set memory to %lu kilobytes: %s"), memory, + taskInfoErrorMessage); goto cleanup; } @@ -2116,6 +2135,7 @@ esxDomainSetMemory(virDomainPtr domain, unsigned long memory) esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineConfigSpec_Free(&spec); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -2394,6 +2414,7 @@ esxDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus, esxVI_VirtualMachineConfigSpec *spec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; if (flags != VIR_DOMAIN_VCPU_LIVE) { ESX_ERROR(VIR_ERR_INVALID_ARG, _("unsupported flags: (0x%x)"), flags); @@ -2438,13 +2459,15 @@ esxDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not set number of virtual CPUs to %d"), nvcpus); + _("Could not set number of virtual CPUs to %d: %s"), nvcpus, + taskInfoErrorMessage); goto cleanup; } @@ -2454,6 +2477,7 @@ esxDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus, esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineConfigSpec_Free(&spec); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -2828,6 +2852,7 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) int id = -1; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; virCheckFlags(0, -1); @@ -2855,12 +2880,14 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not start domain")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not start domain: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -2871,6 +2898,7 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) esxVI_ObjectContent_Free(&virtualMachine); esxVI_String_Free(&propertyNameList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -2907,6 +2935,7 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml) esxVI_ManagedObjectReference *resourcePool = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; virDomainPtr domain = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { @@ -3066,12 +3095,14 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, def->uuid, esxVI_Occurrence_OptionalItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not define domain")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not define domain: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -3100,6 +3131,7 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml) esxVI_ObjectContent_Free(&hostSystem); esxVI_ManagedObjectReference_Free(&resourcePool); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return domain; } @@ -3340,6 +3372,7 @@ esxDomainSetSchedulerParameters(virDomainPtr domain, esxVI_SharesInfo *sharesInfo = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; int i; if (esxVI_EnsureSession(priv->primary) < 0) { @@ -3434,13 +3467,15 @@ esxDomainSetSchedulerParameters(virDomainPtr domain, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not change scheduler parameters")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Could not change scheduler parameters: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -3450,6 +3485,7 @@ esxDomainSetSchedulerParameters(virDomainPtr domain, esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineConfigSpec_Free(&spec); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -3504,6 +3540,7 @@ esxDomainMigratePerform(virDomainPtr domain, esxVI_Event *eventList = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; if (priv->vCenter == NULL) { ESX_ERROR(VIR_ERR_INVALID_ARG, "%s", @@ -3601,14 +3638,16 @@ esxDomainMigratePerform(virDomainPtr domain, &task) < 0 || esxVI_WaitForTaskCompletion(priv->vCenter, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not migrate domain, migration task finished with " - "an error")); + "an error: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -3619,6 +3658,7 @@ esxDomainMigratePerform(virDomainPtr domain, esxVI_ObjectContent_Free(&virtualMachine); esxVI_Event_Free(&eventList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -3783,6 +3823,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, esxVI_VirtualMachineSnapshotTree *snapshotTreeParent = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; virDomainSnapshotPtr snapshot = NULL; virCheckFlags(0, NULL); @@ -3820,12 +3861,14 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, esxVI_Boolean_False, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create snapshot")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not create snapshot: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -3836,6 +3879,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return snapshot; } @@ -4058,6 +4102,7 @@ esxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) esxVI_VirtualMachineSnapshotTree *snapshotTreeParent = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; virCheckFlags(0, -1); @@ -4077,13 +4122,15 @@ esxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, snapshot->domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not revert to snapshot '%s'"), snapshot->name); + _("Could not revert to snapshot '%s': %s"), snapshot->name, + taskInfoErrorMessage); goto cleanup; } @@ -4092,6 +4139,7 @@ esxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) cleanup: esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -4109,6 +4157,7 @@ esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) esxVI_Boolean removeChildren = esxVI_Boolean_False; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, -1); @@ -4132,13 +4181,15 @@ esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) removeChildren, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, snapshot->domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not delete snapshot '%s'"), snapshot->name); + _("Could not delete snapshot '%s': %s"), snapshot->name, + taskInfoErrorMessage); goto cleanup; } @@ -4147,6 +4198,7 @@ esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) cleanup: esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -4163,6 +4215,7 @@ esxDomainSetMemoryParameters(virDomainPtr domain, virMemoryParameterPtr params, esxVI_VirtualMachineConfigSpec *spec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; int i; virCheckFlags(0, -1); @@ -4199,13 +4252,15 @@ esxDomainSetMemoryParameters(virDomainPtr domain, virMemoryParameterPtr params, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not change memory parameters")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Could not change memory parameters: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -4215,6 +4270,7 @@ esxDomainSetMemoryParameters(virDomainPtr domain, virMemoryParameterPtr params, esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineConfigSpec_Free(&spec); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index 72e0d7a..83b53fb 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -926,6 +926,7 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, esxVI_FileBackedVirtualDiskSpec *virtualDiskSpec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; char *uuid_string = NULL; char *key = NULL; @@ -1092,12 +1093,14 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, esxVI_VirtualDiskSpec_DynamicCast(virtualDiskSpec), &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, NULL, esxVI_Occurrence_None, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create volume")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not create volume: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -1151,6 +1154,7 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, esxVI_FileInfo_Free(&fileInfo); esxVI_FileBackedVirtualDiskSpec_Free(&virtualDiskSpec); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); VIR_FREE(uuid_string); VIR_FREE(key); diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 9c96650..74a2a42 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -3009,6 +3009,7 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, esxVI_FloppyImageFileQuery *floppyImageFileQuery = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; esxVI_TaskInfo *taskInfo = NULL; esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL; @@ -3128,13 +3129,15 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, datastorePathWithoutFileName, searchSpec, &task) < 0 || esxVI_WaitForTaskCompletion(ctx, task, NULL, esxVI_Occurrence_None, - esxVI_Boolean_False, &taskInfoState) < 0) { + esxVI_Boolean_False, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not serach in datastore '%s'"), datastoreName); + _("Could not search in datastore '%s': %s"), + datastoreName, taskInfoErrorMessage); goto cleanup; } @@ -3179,6 +3182,7 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, esxVI_ManagedObjectReference_Free(&hostDatastoreBrowser); esxVI_HostDatastoreBrowserSearchSpec_Free(&searchSpec); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); esxVI_TaskInfo_Free(&taskInfo); esxVI_HostDatastoreBrowserSearchResults_Free(&searchResults); @@ -3203,6 +3207,7 @@ esxVI_LookupDatastoreContentByDatastoreName char *datastorePath = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; esxVI_TaskInfo *taskInfo = NULL; if (searchResultsList == NULL || *searchResultsList != NULL) { @@ -3269,13 +3274,15 @@ esxVI_LookupDatastoreContentByDatastoreName datastorePath, searchSpec, &task) < 0 || esxVI_WaitForTaskCompletion(ctx, task, NULL, esxVI_Occurrence_None, - esxVI_Boolean_False, &taskInfoState) < 0) { + esxVI_Boolean_False, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not serach in datastore '%s'"), datastoreName); + _("Could not serach in datastore '%s': %s"), + datastoreName, taskInfoErrorMessage); goto cleanup; } @@ -3294,6 +3301,7 @@ esxVI_LookupDatastoreContentByDatastoreName esxVI_HostDatastoreBrowserSearchSpec_Free(&searchSpec); VIR_FREE(datastorePath); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); esxVI_TaskInfo_Free(&taskInfo); return result; @@ -3472,7 +3480,8 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, const unsigned char *virtualMachineUuid, esxVI_Occurrence virtualMachineOccurrence, esxVI_Boolean autoAnswer, - esxVI_TaskInfoState *finalState) + esxVI_TaskInfoState *finalState, + char **errorMessage) { int result = -1; esxVI_ObjectSpec *objectSpec = NULL; @@ -3489,6 +3498,11 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, esxVI_Boolean blocked = esxVI_Boolean_Undefined; esxVI_TaskInfo *taskInfo = NULL; + if (errorMessage == NULL || *errorMessage != NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + version = strdup(""); if (version == NULL) { @@ -3609,6 +3623,35 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, goto cleanup; } + if (*finalState != esxVI_TaskInfoState_Success) { + if (esxVI_LookupTaskInfoByTask(ctx, task, &taskInfo)) { + goto cleanup; + } + + if (taskInfo->error == NULL) { + *errorMessage = strdup(_("Unknown error")); + + if (*errorMessage == NULL) { + virReportOOMError(); + goto cleanup; + } + } else if (taskInfo->error->localizedMessage == NULL) { + *errorMessage = strdup(taskInfo->error->fault->_actualType); + + if (*errorMessage == NULL) { + virReportOOMError(); + goto cleanup; + } + } else { + if (virAsprintf(errorMessage, "%s - %s", + taskInfo->error->fault->_actualType, + taskInfo->error->localizedMessage) < 0) { + virReportOOMError(); + goto cleanup; + } + } + } + result = 0; cleanup: diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index 26779ad..20e6fb5 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -429,7 +429,8 @@ int esxVI_WaitForTaskCompletion(esxVI_Context *ctx, const unsigned char *virtualMachineUuid, esxVI_Occurrence virtualMachineOccurrence, esxVI_Boolean autoAnswer, - esxVI_TaskInfoState *finalState); + esxVI_TaskInfoState *finalState, + char **errorMessage); int esxVI_ParseHostCpuIdInfo(esxVI_ParsedHostCpuIdInfo *parsedHostCpuIdInfo, esxVI_HostCpuIdInfo *hostCpuIdInfo); diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input index 2ee513a..991ce8a 100644 --- a/src/esx/esx_vi_generator.input +++ b/src/esx/esx_vi_generator.input @@ -311,6 +311,12 @@ object LocalDatastoreInfo extends DatastoreInfo end +object LocalizedMethodFault + MethodFault fault r + String localizedMessage o +end + + object NasDatastoreInfo extends DatastoreInfo HostNasVolume nas o end @@ -494,7 +500,7 @@ object TaskInfo TaskInfoState state r Boolean cancelled r Boolean cancelable r - LocalizedMethodFault error i + LocalizedMethodFault error o AnyType result o Int progress o TaskReason reason i diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 4593379..4a8a9dc 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -1139,6 +1139,7 @@ predefined_objects = ["AnyType", "Long", "String", "DateTime", + "MethodFault", "ManagedObjectReference"] @@ -1172,6 +1173,7 @@ additional_object_features = { "DatastoreHostMount" : Object.FEATURE__DE removed_object_features = { "DynamicProperty" : Object.FEATURE__SERIALIZE, + "LocalizedMethodFault" : Object.FEATURE__SERIALIZE, "ObjectContent" : Object.FEATURE__SERIALIZE, "ObjectUpdate" : Object.FEATURE__SERIALIZE, "PropertyChange" : Object.FEATURE__SERIALIZE, diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index ad45483..a70e1e0 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -701,6 +701,9 @@ esxVI_Type_ToString(esxVI_Type type) case esxVI_Type_Fault: return "Fault"; + case esxVI_Type_MethodFault: + return "MethodFault"; + case esxVI_Type_ManagedObjectReference: return "ManagedObjectReference"; @@ -741,6 +744,8 @@ esxVI_Type_FromString(const char *type) return esxVI_Type_DateTime; } else if (STREQ(type, "Fault")) { return esxVI_Type_Fault; + } else if (STREQ(type, "MethodFault")) { + return esxVI_Type_MethodFault; } else if (STREQ(type, "ManagedObjectReference")) { return esxVI_Type_ManagedObjectReference; } else if (STREQ(type, "Datacenter")) { @@ -751,7 +756,6 @@ esxVI_Type_FromString(const char *type) return esxVI_Type_HostSystem; } - #include "esx_vi_types.generated.typefromstring" else { @@ -1447,6 +1451,51 @@ ESX_VI__TEMPLATE__DESERIALIZE(Fault, /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * VI Type: MethodFault + */ + +/* esxVI_MethodFault_Alloc */ +ESX_VI__TEMPLATE__ALLOC(MethodFault); + +/* esxVI_MethodFault_Free */ +ESX_VI__TEMPLATE__FREE(MethodFault, +{ + VIR_FREE(item->_actualType); +}) + +int +esxVI_MethodFault_Deserialize(xmlNodePtr node, esxVI_MethodFault **methodFault) +{ + if (methodFault == NULL || *methodFault != NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + if (esxVI_MethodFault_Alloc(methodFault) < 0) { + return -1; + } + + (*methodFault)->_actualType = + (char *)xmlGetNsProp(node, BAD_CAST "type", + BAD_CAST "http://www.w3.org/2001/XMLSchema-instance"); + + if ((*methodFault)->_actualType == NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + _("MethodFault is missing 'type' property")); + goto failure; + } + + return 0; + + failure: + esxVI_MethodFault_Free(methodFault); + + return -1; +} + + + +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * VI Type: ManagedObjectReference */ diff --git a/src/esx/esx_vi_types.h b/src/esx/esx_vi_types.h index 901eda0..64bf2dc 100644 --- a/src/esx/esx_vi_types.h +++ b/src/esx/esx_vi_types.h @@ -48,6 +48,7 @@ typedef struct _esxVI_DateTime esxVI_DateTime; */ typedef struct _esxVI_Fault esxVI_Fault; +typedef struct _esxVI_MethodFault esxVI_MethodFault; typedef struct _esxVI_ManagedObjectReference esxVI_ManagedObjectReference; typedef struct _esxVI_Datacenter esxVI_Datacenter; typedef struct _esxVI_ComputeResource esxVI_ComputeResource; @@ -71,6 +72,7 @@ enum _esxVI_Type { esxVI_Type_Long, esxVI_Type_DateTime, esxVI_Type_Fault, + esxVI_Type_MethodFault, esxVI_Type_ManagedObjectReference, esxVI_Type_Datacenter, esxVI_Type_ComputeResource, @@ -279,6 +281,29 @@ int esxVI_Fault_Deserialize(xmlNodePtr node, esxVI_Fault **fault); /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * VI Type: MethodFault + */ + +/* + * FIXME: This is just a minimal implementation of the MethodFault type. + * A complete implementation would require to implement dozens of + * extending types too. + */ +struct _esxVI_MethodFault { + esxVI_MethodFault *_unused; /* optional */ + esxVI_Type _type; /* required */ + + char *_actualType; /* required */ +}; + +int esxVI_MethodFault_Alloc(esxVI_MethodFault **methodfault); +void esxVI_MethodFault_Free(esxVI_MethodFault **methodFault); +int esxVI_MethodFault_Deserialize(xmlNodePtr node, + esxVI_MethodFault **methodFault); + + + +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * VI Type: ManagedObjectReference */ -- 1.7.0.4

2 2

[libvirt] [PATCH] command: plug memory leak
by Eric Blake 10 Dec '10

10 Dec '10

* src/util/command.c (virCommandFree): Free data from virCommandSetInputBuffer. --- src/util/command.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index 089e0bd..5e2b19a 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -1198,6 +1198,7 @@ virCommandFree(virCommandPtr cmd) } } + VIR_FREE(cmd->inbuf); VIR_FORCE_CLOSE(cmd->outfd); VIR_FORCE_CLOSE(cmd->errfd); -- 1.7.3.2

3 3

[libvirt] [Patch v3] Add VMware Workstation and Player driver
by Jean-Baptiste Rouault 10 Dec '10

10 Dec '10

* Changes since v2: remove unnecessary mutex locks use vmrun reset soft convert popen to virCommand remove duplicate errors various other fixes

3 6

[libvirt] [v2] qemu: Set domain def as updated and transient if changes
by Osier Yang 10 Dec '10

10 Dec '10

As qemu driver doesn't allow to make changes on persistent domain configuration via "attach/detach/update device", and all the changes made on the running domain configuration should not be persistent across next boot (without the need of restarting libvirtd), so: 1) Set the running domain def as transient, and restore the domain configuration to original configuration when shutdown. 2) Set the running domain def as updated, and reset it as not updated when shutdown. Also for "live VCPU set", it doesn't change the persistent domain configuration, so, we also set the running domain def as updated and transient, and restore the original def when shutdown. * src/qemu/qemu_driver.c --- src/qemu/qemu_driver.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 47 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 19ce9a6..a3d87eb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4429,11 +4429,18 @@ retry: VIR_FREE(priv->vcpupids); priv->nvcpupids = 0; + /* Restore original domain def, so that changes on running domain def + * will not be persistent across next boot. + */ if (vm->newDef) { virDomainDefFree(vm->def); vm->def = vm->newDef; vm->def->id = -1; vm->newDef = NULL; + + /* Now set domain def as not updated */ + if (vm->updated) + vm->updated = 0; } if (orig_err) { @@ -6473,7 +6480,17 @@ qemudDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, break; case VIR_DOMAIN_VCPU_LIVE: + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) { + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + + goto endjob; + } + ret = qemudDomainHotplugVcpus(vm, nvcpus); + + if (ret == 0) + vm->updated = 1; break; case VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG: @@ -8819,6 +8836,13 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto endjob; } + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) { + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + + goto endjob; + } + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, VIR_DOMAIN_XML_INACTIVE); if (dev == NULL) @@ -8916,6 +8940,9 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto endjob; } + if (ret == 0) + vm->updated = 1; + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; @@ -9066,6 +9093,13 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, goto endjob; } + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) { + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + + goto endjob; + } + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, VIR_DOMAIN_XML_INACTIVE); if (dev == NULL) @@ -9126,6 +9160,9 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, break; } + if (ret == 0) + vm->updated = 1; + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; @@ -9786,6 +9823,13 @@ static int qemudDomainDetachDevice(virDomainPtr dom, goto endjob; } + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) { + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + + goto endjob; + } + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, VIR_DOMAIN_XML_INACTIVE); if (dev == NULL) @@ -9828,6 +9872,9 @@ static int qemudDomainDetachDevice(virDomainPtr dom, "%s", _("This type of device cannot be hot unplugged")); } + if (ret == 0) + vm->updated = 1; + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; -- 1.7.3.2

3 5

[libvirt] building fails with 0.8.6 and snapshot
by Dominik Klein 10 Dec '10

10 Dec '10

hi. i am trying to build 0.8.6 but this fails due to the lack of macvtap enabled in my kernel. i won't use it, so i don't care, but even if i disable it, build won't work. apparently, thats a known bug, according to https://www.redhat.com/archives/libvir-list/2010-December/msg00034.html so now i am trying to build a snapshot but it also fails during "make rpm", specifically during make check in "tests". commandtest.c fails and I cannot find out why. here's the log: http://pastebin.com/rVPNDpS6 maybe someone can help me out? environment is opensuse 11.3 64 bit regards dominik

3 5

[libvirt] [PATCH v6 0/4] Support of auto-dump on watchdog event in libvirtd
by Hu Tao 10 Dec '10

10 Dec '10

This patch series adds a new watchdog action `dump' which lets libvirtd can do auto-dump when receiving a watchdog event from qemu guest. In order to make the function work, there must be a watchdog device added to guest, and guest must have a watchdog daemon running, for example, /etc/init.d/watchdog start or auto-started on boot. Changes: v6: - remove struct qemud_worker and qemud_server.job, qemud_server.nworkers, qemud_server.nactiveworkers, qemud_server.workers. v5: - qemu_driver is passed into threadpool as opaque parameter rather than visit the global qemu_driver in worker function - same situation as above of server in libvirtd.c - also list auto_dump_path in src/qemu/libvirtd_qemu.aug and src/qemu/test_libvirtd_qemu.aug - check return value of qemuDomainObjEndJob for safety v4: - updated threadpool to follow libvirt naming style, use appropriate internals APIs, and hide the struct definitions from the header (by Daniel) - fix an error that qemuDomainObjBeginJobWithDriver() get lost in qemuDomainCoreDump() - use thread pool in libvirtd (qemud worker) v3: - let default auto-dump dir be /var/lib/libvirt/qemu/dump Hu Tao (4): threadpool impl Add a new function doCoreDump Add a watchdog action `dump' Using threadpool API to manage qemud worker cfg.mk | 1 + daemon/libvirtd.c | 187 +++++--------------------------- daemon/libvirtd.h | 16 +--- src/Makefile.am | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 6 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 5 + src/qemu/qemu_conf.c | 16 +++- src/qemu/qemu_conf.h | 5 + src/qemu/qemu_driver.c | 228 ++++++++++++++++++++++++++++---------- src/qemu/test_libvirtd_qemu.aug | 2 + src/util/threadpool.c | 231 +++++++++++++++++++++++++++++++++++++++ src/util/threadpool.h | 48 ++++++++ 15 files changed, 517 insertions(+), 232 deletions(-) create mode 100644 src/util/threadpool.c create mode 100644 src/util/threadpool.h -- 1.7.3 -- Thanks, Hu Tao

4 18

[libvirt] [PATCH 0/8] Initial integration of lock managers
by Daniel P. Berrange 10 Dec '10

10 Dec '10

This patch series is the bare minimum framework required to integrate with external lock managers. This is a fleshing out of the original proposal in http://www.redhat.com/archives/libvir-list/2010-September/msg00167.html What's in this series * Major refactor of the security drivers to simplify code and clarify interaction with lock maanger * Low level internal lock manager plugin API for locking objects and their resources * High level internal lock manager API for locking virtual machines and their disks * Simple 'no op' plugin impl to stub out all the code * Re-ordering of QEMU startup to add a handshake between libvirtd & the child process, immediately prior to exec(). This allows acquiring locks in the child, before the parent libvirtd process then re-labels disks * The public API header for 3rd parties to implement new lock manager plugins against * Some basic documentation What's not in this series * A standard 'fcntl' based plugin * Enhanced migration process to transfer lock state * Hooks to re-validate the lock at time of VM resume

3 22

[libvirt] [PATCH] spec: do not start libvirt-guests if that service is off
by Dan Kenigsberg 10 Dec '10

10 Dec '10

starting a service during rpm installation is impolite. It is even worse if done during upgrade, for a service that was explicitly turned off. --- libvirt.spec.in | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 48453d7..e0cab78 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -700,9 +700,11 @@ fi /sbin/ldconfig /sbin/chkconfig --add libvirt-guests if [ $1 -ge 1 ]; then - # this doesn't do anything but allowing for libvirt-guests to be - # stopped on the first shutdown - /sbin/service libvirt-guests start > /dev/null 2>&1 || true + if /sbin/chkconfig --list libvirt-guests | /bin/grep -q :on ; then + # this doesn't do anything but allowing for libvirt-guests to be + # stopped on the first shutdown + /sbin/service libvirt-guests start > /dev/null 2>&1 || true + fi fi %postun client -p /sbin/ldconfig -- 1.7.3.2

2 1

[libvirt] [PATCH] add network disk support
by MORITA Kazutaka 09 Dec '10

09 Dec '10

This patch adds network disk support to libvirt/QEMU. The currently supported protcols are nbd, rbd, and sheepdog. The XML syntax is like this: <disk type="network" device="disk"> <driver name="qemu" type="raw" /> <source protocol='rbd|sheepdog|nbd' name="...some image identifier..."> <host name="mon1.example.org" port="6000"> <host name="mon2.example.org" port="6000"> <host name="mon3.example.org" port="6000"> </source> <target dev="vda" bus="virtio" /> </disk> Signed-off-by: MORITA Kazutaka <morita.kazutaka(a)lab.ntt.co.jp> --- This patch addresses the discussion on https://www.redhat.com/archives/libvir-list/2010-November/msg00759.html Josh mentioned that the monitor hostnames of RBD can be set through the environment variables, but I couldn't find any documentations about it, so the monitors are not set in this patch. I hope someone who is familiar with RBD implements it. I appreciate any feedback. Thanks, Kazutaka docs/schemas/domain.rng | 31 +++++++++ src/conf/domain_conf.c | 68 +++++++++++++++++-- src/conf/domain_conf.h | 20 ++++++ src/qemu/qemu_conf.c | 169 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 279 insertions(+), 9 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index fb44335..81f4004 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -612,6 +612,37 @@ <ref name="diskspec"/> </interleave> </group> + <group> + <attribute name="type"> + <value>network</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <attribute name="protocol"> + <choice> + <value>nbd</value> + <value>rbd</value> + <value>sheepdog</value> + </choice> + </attribute> + <attribute name="name"/> + <zeroOrMore> + <element name="host"> + <attribute name="name"> + <ref name="genericName"/> + </attribute> + <attribute name="port"> + <ref name="unsignedInt"/> + </attribute> + </element> + </zeroOrMore> + <empty/> + </element> + </optional> + <ref name="diskspec"/> + </interleave> + </group> <ref name="diskspec"/> </choice> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3f14cee..b9dbc61 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -113,7 +113,8 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, "block", "file", - "dir") + "dir", + "network") VIR_ENUM_IMPL(virDomainDiskDevice, VIR_DOMAIN_DISK_DEVICE_LAST, "disk", @@ -142,6 +143,11 @@ VIR_ENUM_IMPL(virDomainDiskErrorPolicy, VIR_DOMAIN_DISK_ERROR_POLICY_LAST, "ignore", "enospace") +VIR_ENUM_IMPL(virDomainDiskProtocol, VIR_DOMAIN_DISK_PROTOCOL_LAST, + "nbd", + "rbd", + "sheepdog") + VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, "ide", "fdc", @@ -508,6 +514,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->serial); VIR_FREE(def->src); + VIR_FREE(def->hosts); VIR_FREE(def->dst); VIR_FREE(def->driverName); VIR_FREE(def->driverType); @@ -1574,13 +1581,15 @@ virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, int flags) { virDomainDiskDefPtr def; - xmlNodePtr cur; + xmlNodePtr cur, host; char *type = NULL; char *device = NULL; char *driverName = NULL; char *driverType = NULL; char *source = NULL; char *target = NULL; + char *protocol = NULL; + virDomainDiskHostDefPtr hosts = NULL; char *bus = NULL; char *cachetag = NULL; char *error_policy = NULL; @@ -1607,7 +1616,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { - if ((source == NULL) && + if ((source == NULL && hosts == NULL) && (xmlStrEqual(cur->name, BAD_CAST "source"))) { switch (def->type) { @@ -1620,6 +1629,30 @@ virDomainDiskDefParseXML(virCapsPtr caps, case VIR_DOMAIN_DISK_TYPE_DIR: source = virXMLPropString(cur, "dir"); break; + case VIR_DOMAIN_DISK_TYPE_NETWORK: + protocol = virXMLPropString(cur, "protocol"); + if (protocol == NULL) { + virDomainReportError(VIR_ERR_XML_ERROR, + "%s", _("missing protocol type")); + break; + } + def->protocol = virDomainDiskProtocolTypeFromString(protocol); + source = virXMLPropString(cur, "name"); + host = cur->children; + while (host != NULL) { + if (host->type == XML_ELEMENT_NODE && + xmlStrEqual(host->name, BAD_CAST "host")) { + if (VIR_REALLOC_N(hosts, def->nhosts + 1) < 0) { + virReportOOMError(); + goto error; + } + hosts[def->nhosts].name = virXMLPropString(host, "name"); + hosts[def->nhosts].port = virXMLPropString(host, "port"); + def->nhosts++; + } + host = host->next; + } + break; default: virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected disk type %s"), @@ -1685,7 +1718,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, /* Only CDROM and Floppy devices are allowed missing source path * to indicate no media present */ - if (source == NULL && + if (source == NULL && hosts == NULL && def->device != VIR_DOMAIN_DISK_DEVICE_CDROM && def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { virDomainReportError(VIR_ERR_NO_SOURCE, @@ -1791,6 +1824,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, source = NULL; def->dst = target; target = NULL; + def->hosts = hosts; + hosts = NULL; def->driverName = driverName; driverName = NULL; def->driverType = driverType; @@ -1819,6 +1854,8 @@ cleanup: VIR_FREE(type); VIR_FREE(target); VIR_FREE(source); + VIR_FREE(hosts); + VIR_FREE(protocol); VIR_FREE(device); VIR_FREE(driverType); VIR_FREE(driverName); @@ -5887,7 +5924,7 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferVSprintf(buf, "/>\n"); } - if (def->src) { + if (def->src || def->nhosts > 0) { switch (def->type) { case VIR_DOMAIN_DISK_TYPE_FILE: virBufferEscapeString(buf, " <source file='%s'/>\n", @@ -5901,6 +5938,27 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " <source dir='%s'/>\n", def->src); break; + case VIR_DOMAIN_DISK_TYPE_NETWORK: + virBufferVSprintf(buf, " <source protocol='%s'", + virDomainDiskProtocolTypeToString(def->protocol)); + if (def->src) { + virBufferEscapeString(buf, " name='%s'", def->src); + } + if (def->nhosts == 0) { + virBufferVSprintf(buf, "/>\n"); + } else { + int i; + + virBufferVSprintf(buf, ">\n"); + for (i = 0; i < def->nhosts; i++) { + virBufferEscapeString(buf, " <host name='%s'", + def->hosts[i].name); + virBufferEscapeString(buf, " port='%s'/>\n", + def->hosts[i].port); + } + virBufferVSprintf(buf, " </source>\n"); + } + break; default: virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected disk type %s"), diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 899b19f..6c97289 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -120,6 +120,7 @@ enum virDomainDiskType { VIR_DOMAIN_DISK_TYPE_BLOCK, VIR_DOMAIN_DISK_TYPE_FILE, VIR_DOMAIN_DISK_TYPE_DIR, + VIR_DOMAIN_DISK_TYPE_NETWORK, VIR_DOMAIN_DISK_TYPE_LAST }; @@ -164,6 +165,21 @@ enum virDomainDiskErrorPolicy { VIR_DOMAIN_DISK_ERROR_POLICY_LAST }; +enum virDomainDiskProtocol { + VIR_DOMAIN_DISK_PROTOCOL_NBD, + VIR_DOMAIN_DISK_PROTOCOL_RBD, + VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG, + + VIR_DOMAIN_DISK_PROTOCOL_LAST +}; + +typedef struct _virDomainDiskHostDef virDomainDiskHostDef; +typedef virDomainDiskHostDef *virDomainDiskHostDefPtr; +struct _virDomainDiskHostDef { + char *name; + char *port; +}; + /* Stores the virtual disk configuration */ typedef struct _virDomainDiskDef virDomainDiskDef; typedef virDomainDiskDef *virDomainDiskDefPtr; @@ -173,6 +189,9 @@ struct _virDomainDiskDef { int bus; char *src; char *dst; + int protocol; + int nhosts; + virDomainDiskHostDefPtr hosts; char *driverName; char *driverType; char *serial; @@ -1237,6 +1256,7 @@ VIR_ENUM_DECL(virDomainDiskDevice) VIR_ENUM_DECL(virDomainDiskBus) VIR_ENUM_DECL(virDomainDiskCache) VIR_ENUM_DECL(virDomainDiskErrorPolicy) +VIR_ENUM_DECL(virDomainDiskProtocol) VIR_ENUM_DECL(virDomainController) VIR_ENUM_DECL(virDomainControllerModel) VIR_ENUM_DECL(virDomainFS) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 35caccc..63abd75 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2714,7 +2714,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, break; } - if (disk->src) { + if (disk->src || disk->nhosts > 0) { if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { /* QEMU only supports magic FAT format for now */ if (disk->driverType && @@ -2733,6 +2733,24 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, virBufferVSprintf(&opt, "file=fat:floppy:%s,", disk->src); else virBufferVSprintf(&opt, "file=fat:%s,", disk->src); + } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + switch (disk->protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + virBufferVSprintf(&opt, "file=nbd:%s:%s,", + disk->hosts->name, disk->hosts->port); + break; + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + virBufferVSprintf(&opt, "file=rbd:%s,", disk->src); + break; + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + if (disk->nhosts > 0) + virBufferVSprintf(&opt, "file=sheepdog:%s:%s:%s,", + disk->hosts->name, disk->hosts->port, + disk->src); + else + virBufferVSprintf(&opt, "file=sheepdog:%s,", disk->src); + break; + } } else { virBufferVSprintf(&opt, "file=%s,", disk->src); } @@ -4722,6 +4740,24 @@ int qemudBuildCommandLine(virConnectPtr conn, snprintf(file, PATH_MAX, "fat:floppy:%s", disk->src); else snprintf(file, PATH_MAX, "fat:%s", disk->src); + } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + switch (disk->protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + snprintf(file, PATH_MAX, "nbd:%s:%s,", + disk->hosts->name, disk->hosts->port); + break; + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + snprintf(file, PATH_MAX, "rbd:%s,", disk->src); + break; + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + if (disk->nhosts > 0) + snprintf(file, PATH_MAX, "sheepdog:%s:%s:%s,", + disk->hosts->name, disk->hosts->port, + disk->src); + else + snprintf(file, PATH_MAX, "sheepdog:%s,", disk->src); + break; + } } else { snprintf(file, PATH_MAX, "%s", disk->src); } @@ -5794,7 +5830,67 @@ qemuParseCommandLineDisk(virCapsPtr caps, values[i] = NULL; if (STRPREFIX(def->src, "/dev/")) def->type = VIR_DOMAIN_DISK_TYPE_BLOCK; - else + else if (STRPREFIX(def->src, "nbd:")) { + char *host, *port; + + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + host = def->src + strlen("nbd:"); + port = strchr(host, ':'); + if (!port) { + def = NULL; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse nbd filename '%s'"), def->src); + goto cleanup; + } + *port++ = '\0'; + if (VIR_ALLOC(def->hosts) < 0) { + virReportOOMError(); + goto cleanup; + } + def->nhosts = 1; + def->hosts->name = strdup(host); + def->hosts->port = strdup(port); + + VIR_FREE(def->src); + def->src = NULL; + } else if (STRPREFIX(def->src, "rbd:")) { + char *p = def->src; + + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->src = strdup(p + strlen("rbd:")); + + VIR_FREE(p); + } else if (STRPREFIX(def->src, "sheepdog:")) { + char *p = def->src; + char *port, *vdi; + + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->src = strdup(p + strlen("sheepdog:")); + + /* def->src must be [vdiname] or [host]:[port]:[vdiname] */ + port = strchr(def->src, ':'); + if (port) { + *port++ = '\0'; + vdi = strchr(port, ':'); + if (!vdi) { + def = NULL; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse sheepdog filename '%s'"), p); + goto cleanup; + } + *vdi++ = '\0'; + if (VIR_ALLOC(def->hosts) < 0) { + virReportOOMError(); + goto cleanup; + } + def->nhosts = 1; + def->hosts->name = def->src; + def->hosts->port = strdup(port); + def->src = strdup(vdi); + } + + VIR_FREE(p); + } else def->type = VIR_DOMAIN_DISK_TYPE_FILE; } else { def->type = VIR_DOMAIN_DISK_TYPE_FILE; @@ -6731,7 +6827,19 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, if (STRPREFIX(val, "/dev/")) disk->type = VIR_DOMAIN_DISK_TYPE_BLOCK; - else + else if (STRPREFIX(val, "nbd:")) { + disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_NBD; + val += strlen("nbd:"); + } else if (STRPREFIX(val, "rbd:")) { + disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; + val += strlen("rbd:"); + } else if (STRPREFIX(val, "sheepdog:")) { + disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG; + val += strlen("sheepdog:"); + } else disk->type = VIR_DOMAIN_DISK_TYPE_FILE; if (STREQ(arg, "-cdrom")) { disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; @@ -6751,7 +6859,60 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, disk->dst = strdup(arg + 1); } disk->src = strdup(val); - if (!disk->src || + + if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + char *host, *port; + + switch (disk->protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + host = disk->src; + port = strchr(host, ':'); + if (!port) { + def = NULL; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse nbd filename '%s'"), disk->src); + goto error; + } + *port++ = '\0'; + if (VIR_ALLOC(disk->hosts) < 0) { + virReportOOMError(); + goto error; + } + disk->nhosts = 1; + disk->hosts->name = host; + disk->hosts->port = strdup(port); + disk->src = NULL; + break; + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + break; + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + /* disk->src must be [vdiname] or [host]:[port]:[vdiname] */ + port = strchr(disk->src, ':'); + if (port) { + char *vdi; + + *port++ = '\0'; + vdi = strchr(port, ':'); + if (!vdi) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse sheepdog filename '%s'"), val); + goto error; + } + *vdi++ = '\0'; + if (VIR_ALLOC(disk->hosts) < 0) { + virReportOOMError(); + goto error; + } + disk->nhosts = 1; + disk->hosts->name = disk->src; + disk->hosts->port = strdup(port); + disk->src = strdup(vdi); + } + break; + } + } + + if (!(disk->src || disk->nhosts > 0) || !disk->dst) { virDomainDiskDefFree(disk); goto no_memory; -- 1.7.1

4 11

[libvirt] [PATCH] correct the signal's name
by Wen Congyang 09 Dec '10

09 Dec '10

The signal's name is wrong... Signed-off-by: Wen Congyang <wency(a)cn.fujitsu.com> --- tools/console.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/console.c b/tools/console.c index c2971cf..e126320 100644 --- a/tools/console.c +++ b/tools/console.c @@ -365,10 +365,10 @@ int vshRunConsole(virDomainPtr dom, const char *devname) } /* Restore original signal handlers */ - signal(SIGQUIT, old_sigpipe); - signal(SIGQUIT, old_sighup); - signal(SIGQUIT, old_sigint); - signal(SIGQUIT, old_sigterm); + signal(SIGPIPE, old_sigpipe); + signal(SIGHUP, old_sighup); + signal(SIGINT, old_sigint); + signal(SIGTERM, old_sigterm); signal(SIGQUIT, old_sigquit); resettty: -- 1.7.1

2 1

[libvirt] [PATCH] Fix memory leak in virCommandRun()
by Hu Tao 09 Dec '10

09 Dec '10

--- src/util/command.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index c0520ec..473d1fc 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -956,6 +956,8 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) cmd->errbuf = NULL; } + VIR_FREE(outbuf); + VIR_FREE(errbuf); return ret; } -- 1.7.3

2 3

[libvirt] [PATCH] qemu: Distinguish between domain shutdown and crash
by Jiri Denemark 09 Dec '10

09 Dec '10

When we get an EOF event on monitor connection, it may be a result of either crash or graceful shutdown. QEMU which supports async events (i.e., we are talking to it using JSON monitor) emits SHUTDOWN event on graceful shutdown. In case we don't get this event by the time monitor connection is closed, we assume the associated domain crashed. --- src/qemu/qemu_driver.c | 23 +++++++++++++++++++++++ 1 files changed, 23 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4e65f3b..ab42a6d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -129,6 +129,7 @@ struct _qemuDomainObjPrivate { virDomainChrDefPtr monConfig; int monJSON; int monitor_warned; + bool gotShutdown; int nvcpupids; int *vcpupids; @@ -919,11 +920,19 @@ qemuHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, int hasError) { struct qemud_driver *driver = qemu_driver; virDomainEventPtr event = NULL; + qemuDomainObjPrivatePtr priv; VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name); virDomainObjLock(vm); + priv = vm->privateData; + if (!hasError && priv->monJSON && !priv->gotShutdown) { + VIR_DEBUG("Monitor connection to '%s' closed without SHUTDOWN event; " + "assuming the domain crashed", vm->def->name); + hasError = 1; + } + event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, hasError ? @@ -1120,6 +1129,18 @@ qemuHandleDomainReset(qemuMonitorPtr mon ATTRIBUTE_UNUSED, static int +qemuHandleDomainShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm) +{ + virDomainObjLock(vm); + ((qemuDomainObjPrivatePtr) vm->privateData)->gotShutdown = true; + virDomainObjUnlock(vm); + + return 0; +} + + +static int qemuHandleDomainStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainObjPtr vm) { @@ -1382,6 +1403,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .destroy = qemuHandleMonitorDestroy, .eofNotify = qemuHandleMonitorEOF, .diskSecretLookup = findVolumeQcowPassphrase, + .domainShutdown = qemuHandleDomainShutdown, .domainStop = qemuHandleDomainStop, .domainReset = qemuHandleDomainReset, .domainRTCChange = qemuHandleDomainRTCChange, @@ -3997,6 +4019,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, priv->monJSON = 0; priv->monitor_warned = 0; + priv->gotShutdown = false; if ((ret = virFileDeletePid(driver->stateDir, vm->def->name)) != 0) { virReportSystemError(ret, -- 1.7.3.3

2 2

[libvirt] Create VMWare ESXi domain via virsh error(error: this function is not supported by the connection driver: virDomainCreateXML)
by mop amg 09 Dec '10

09 Dec '10

Hi, Dear. I have two questions ask for help: my virt-manager and libvirt version is : linux-vaan:~ # rpm -q libvirt libvirt-0.8.5-1.2.i586 linux-vaan:~ # uname -a Linux linux-vaan 2.6.34-12-desktop #1 SMP PREEMPT 2010-06-29 02:39:08 +0200 i686 i386 GNU/Linux linux-vaan:~ # rpm -q virt-manager virt-manager-0.8.5-3.1.i586 linux-vaan:~ # cat /etc/SuSE-release openSUSE 11.3 (i586) VERSION = 11.3 virsh # version Compiled against library: libvir 0.8.5 Using library: libvir 0.8.5 Using API: ESX 0.8.5 Running hypervisor: ESX 4.1.0 *question 1 : * When I try to create a vmware esxi domain from virsh like this : create --file /work/dom1.x there is a error message : error: Failed to create domain from /work/dom1.xml error: this function is not supported by the connection driver: virDomainCreateXML it seems dosen't support create esxi domain, but the http://libvirt.org/hvsupport.html show that is supported after ≥ 0.7.0 or create via virt-install linux-vaan:~ # virt-install --connect=esx:// root(a)192.168.8.162/?no_verify=1 -n rhel5 -r 512 --vcpus=1 --nodisks bridge=vmk0 --network bridge=vmk0 --pxe Enter root's password for 192.168.8.162: Starting install... ERROR this function is not supported by the connection driver: virDomainCreateXML how to fix it ? *dom1.xml* <domain type='vmware'> <name>dom1</name> <memory>524288</memory> <currentMemory>524288</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686'>hvm</type> </os> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <disk type='file' device='disk'> <source file='[datastore1] rhel5/rhel5_2.vmdk'/> <target dev='sda' bus='scsi'/> <address type='drive' controller='0' bus='0' unit='0'/> </disk> <controller type='scsi' index='0' model='lsilogic'/> <interface type='bridge'> <mac address='00:0c:29:51:f1:1c'/> <source bridge='VM Network'/> </interface> </devices> </domain> *question 2:* vol-clone failed virsh # vol-clone --pool datastore1 rhel5/rhel5.vmdk rhel5/rhel5_2.vmdk error: Failed to clone vol from rhel5/rhel5.vmdk error: this function is not supported by the connection driver: virStorageVolCreateXMLFrom thanks for help! best regards!

3 2

[libvirt] [PATCH] qemu: Set domain def as updated and transient if changes
by Osier Yang 09 Dec '10

09 Dec '10

As qemu driver doesn't allow to make changes on persistent domain configuration via "attach/detach/update device", and all the changes made on the running domain configuration should not be persistent across next boot (without the need of restarting libvirtd), so: 1) Set the running domain def as transient, and restore the domain configuration to original configuration when shutdown. 2) Set the running domain def as updated, and reset it as not updated when shutdown. Also for "live VCPU set", it doesn't change the persistent domain configuration, so, we also set the running domain def as updated and transient, and restore the original def when shutdown. * src/qemu/qemu_driver.c (qemudDomainSetVcpusFlags, qemudDomainAttachDevice, qemuDomainUpdateDeviceFlags, qemudDomainDetachDevice) --- src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 45 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1b86b5e..7825e2b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4369,11 +4369,18 @@ retry: VIR_FREE(priv->vcpupids); priv->nvcpupids = 0; + /* Restore original domain def, so that changes on running domain def + * will not be persistent across next boot + */ if (vm->newDef) { virDomainDefFree(vm->def); vm->def = vm->newDef; vm->def->id = -1; vm->newDef = NULL; + + /* Now set domain def as not updated */ + if (vm->updated) + vm->updated = 0; } if (orig_err) { @@ -6353,6 +6360,17 @@ qemudDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (flags & VIR_DOMAIN_VCPU_CONFIG) ret = virDomainSaveConfig(driver->configDir, persistentDef); + if (flags & VIR_DOMAIN_VCPU_LIVE) { + /* Set running domain def as updated */ + if (ret == 0) { + vm->updated = 1; + + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + } + } + endjob: if (qemuDomainObjEndJob(vm) == 0) vm = NULL; @@ -8781,6 +8799,15 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto endjob; } + /* Set running domain def as updated */ + if (ret == 0) { + vm->updated = 1; + + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + } + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; @@ -8991,6 +9018,15 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, break; } + /* Set running domain def as updated */ + if (ret == 0) { + vm->updated = 1; + + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + } + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; @@ -9665,6 +9701,15 @@ static int qemudDomainDetachDevice(virDomainPtr dom, "%s", _("This type of device cannot be hot unplugged")); } + /* Set running domain def as updated */ + if (ret == 0) { + vm->updated = 1; + + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + } + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; -- 1.7.3.2

2 2

[libvirt] [PATCH] qemu: use virAsprintf instead of PATH_MAX
by Eric Blake 08 Dec '10

08 Dec '10

* src/qemu/qemu_driver.c (qemudLogFD, qemudLogReadFD) (qemudStartup, qemudGetProcessInfo) (qemudDomainDetachPciDiskDevice) (qemudDomainDetachSCSIDiskDevice): Use heap instead of stack. --- src/qemu/qemu_driver.c | 72 ++++++++++++++++++++++++------------------------ 1 files changed, 36 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 75f4563..6739b89 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -744,13 +744,11 @@ static int qemuCgroupControllerActive(struct qemud_driver *driver, static int qemudLogFD(struct qemud_driver *driver, const char* name, bool append) { - char logfile[PATH_MAX]; + char *logfile; mode_t logmode; - int ret, fd = -1; + int fd = -1; - if ((ret = snprintf(logfile, sizeof(logfile), "%s/%s.log", - driver->logDir, name)) - < 0 || ret >= sizeof(logfile)) { + if (virAsprintf(&logfile, "%s/%s.log", driver->logDir, name) < 0) { virReportOOMError(); return -1; } @@ -766,8 +764,10 @@ qemudLogFD(struct qemud_driver *driver, const char* name, bool append) virReportSystemError(errno, _("failed to create logfile %s"), logfile); + VIR_FREE(logfile); return -1; } + VIR_FREE(logfile); if (virSetCloseExec(fd) < 0) { virReportSystemError(errno, "%s", _("Unable to set VM logfile close-on-exec flag")); @@ -781,30 +781,28 @@ qemudLogFD(struct qemud_driver *driver, const char* name, bool append) static int qemudLogReadFD(const char* logDir, const char* name, off_t pos) { - char logfile[PATH_MAX]; + char *logfile; mode_t logmode = O_RDONLY; - int ret, fd = -1; + int fd = -1; - if ((ret = snprintf(logfile, sizeof(logfile), "%s/%s.log", logDir, name)) - < 0 || ret >= sizeof(logfile)) { + if (virAsprintf(&logfile, "%s/%s.log", logDir, name) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("failed to build logfile name %s/%s.log"), logDir, name); return -1; } - if ((fd = open(logfile, logmode)) < 0) { virReportSystemError(errno, _("failed to create logfile %s"), logfile); - return -1; + goto cleanup; } if (virSetCloseExec(fd) < 0) { virReportSystemError(errno, "%s", _("Unable to set VM logfile close-on-exec flag")); VIR_FORCE_CLOSE(fd); - return -1; + goto cleanup; } if (pos < 0 || lseek(fd, pos, SEEK_SET) < 0) { virReportSystemError(pos < 0 ? 0 : errno, @@ -812,6 +810,9 @@ qemudLogReadFD(const char* logDir, const char* name, off_t pos) (long long) pos, logfile); VIR_FORCE_CLOSE(fd); } + +cleanup: + VIR_FREE(logfile); return fd; } @@ -1721,7 +1722,7 @@ cleanup: static int qemudStartup(int privileged) { char *base = NULL; - char driverConf[PATH_MAX]; + char *driverConf = NULL; int rc; virConnectPtr conn = NULL; @@ -1850,14 +1851,9 @@ qemudStartup(int privileged) { /* Configuration paths are either ~/.libvirt/qemu/... (session) or * /etc/libvirt/qemu/... (system). */ - if (snprintf (driverConf, sizeof(driverConf), "%s/qemu.conf", base) == -1) - goto out_of_memory; - driverConf[sizeof(driverConf)-1] = '\0'; - - if (virAsprintf(&qemu_driver->configDir, "%s/qemu", base) == -1) - goto out_of_memory; - - if (virAsprintf(&qemu_driver->autostartDir, "%s/qemu/autostart", base) == -1) + if (virAsprintf(&driverConf, "%s/qemu.conf", base) < 0 || + virAsprintf(&qemu_driver->configDir, "%s/qemu", base) < 0 || + virAsprintf(&qemu_driver->autostartDir, "%s/qemu/autostart", base) < 0) goto out_of_memory; VIR_FREE(base); @@ -1872,6 +1868,7 @@ qemudStartup(int privileged) { if (qemudLoadDriverConfig(qemu_driver, driverConf) < 0) { goto error; } + VIR_FREE(driverConf); if (qemudSecurityInit(qemu_driver) < 0) goto error; @@ -1984,6 +1981,7 @@ error: if (conn) virConnectClose(conn); VIR_FREE(base); + VIR_FREE(driverConf); qemudShutdown(); return -1; } @@ -4552,18 +4550,20 @@ cleanup: } -static int qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, int pid, int tid) { - char proc[PATH_MAX]; +static int +qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, int pid, + int tid) { + char *proc; FILE *pidinfo; unsigned long long usertime, systime; int cpu; int ret; if (tid) - ret = snprintf(proc, sizeof(proc), "/proc/%d/task/%d/stat", pid, tid); + ret = virAsprintf(&proc, "/proc/%d/task/%d/stat", pid, tid); else - ret = snprintf(proc, sizeof(proc), "/proc/%d/stat", pid); - if (ret >= (int)sizeof(proc)) { + ret = virAsprintf(&proc, "/proc/%d/stat", pid); + if (ret < 0) { errno = E2BIG; return -1; } @@ -4574,8 +4574,10 @@ static int qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, int pi *cpuTime = 0; if (lastCpu) *lastCpu = 0; + VIR_FREE(proc); return 0; } + VIR_FREE(proc); /* See 'man proc' for information about what all these fields are. We're * only interested in a very few of them */ @@ -9033,7 +9035,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup = NULL; - char drivestr[PATH_MAX]; + char *drivestr = NULL; i = qemudFindDisk(vm->def, dev->data.disk->dst); @@ -9063,10 +9065,8 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, /* build the actual drive id string as the disk->info.alias doesn't * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */ - if ((ret = snprintf(drivestr, sizeof(drivestr), "%s%s", - QEMU_DRIVE_HOST_PREFIX, - detach->info.alias)) - < 0 || ret >= sizeof(drivestr)) { + if (virAsprintf(&drivestr, QEMU_DRIVE_HOST_PREFIX "%s", + detach->info.alias) < 0) { virReportOOMError(); goto cleanup; } @@ -9124,6 +9124,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, ret = 0; cleanup: + VIR_FREE(drivestr); return ret; } @@ -9136,7 +9137,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup = NULL; - char drivestr[PATH_MAX]; + char *drivestr = NULL; i = qemudFindDisk(vm->def, dev->data.disk->dst); @@ -9165,10 +9166,8 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, /* build the actual drive id string as the disk->info.alias doesn't * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */ - if ((ret = snprintf(drivestr, sizeof(drivestr), "%s%s", - QEMU_DRIVE_HOST_PREFIX, - detach->info.alias)) - < 0 || ret >= sizeof(drivestr)) { + if (virAsprintf(&drivestr, QEMU_DRIVE_HOST_PREFIX "%s", + detach->info.alias) < 0) { virReportOOMError(); goto cleanup; } @@ -9207,6 +9206,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, cleanup: virCgroupFree(&cgroup); + VIR_FREE(drivestr); return ret; } -- 1.7.3.2

1 1

[libvirt] [PATCH v5] qemu: call drive_del in DetachPciDiskDevice
by Ryan Harper 08 Dec '10

08 Dec '10

Currently libvirt doesn't confirm whether the guest has responded to the disk removal request. In some cases this can leave the guest with continued access to the device while the mgmt layer believes that it has been removed. With a recent qemu monitor command[1] we can deterministically revoke a guests access to the disk (on the QEMU side) to ensure no futher access is permitted. This patch adds support for the drive_del() command and introduces it in the disk removal paths. If the guest is running in a QEMU without this command we currently explicitly check for unknown command/CommandNotFound and log the issue. If QEMU supports the command we issue the drive_del command after we attempt to remove the device. The guest may respond and remove the block device before we get to attempt to call drive_del. In that case, we explicitly check for 'Device not found' from the monitor indicating that the target drive was auto-deleted upon guest responds to the device removal notification. 1. http://thread.gmane.org/gmane.comp.emulators.qemu/84745 Signed-off-by: Ryan Harper <ryanh(a)us.ibm.com> --- Changes since v4: - removed PATH_MAX, use virAsprintf() - moved drivestr allocation before call to EnterMonitor Changes since v3: - Renamed DriveUnplug -> DriveDel, use drive_del monitor cmd. - Moved invocation to after DelDevice and guest notification. - Handle the case where drive is auto-deleted before we call DriveDel by catching and ignoring 'Device not found' error. - Simplified DriveDel invocation; no need to check return codes as the monitor implementations handle all failure case and logs or ignores as needed. Changes since v2: - use VIR_ERROR to report when unplug command not found Changes since v1: - return > 0 when command isn't present, < 0 on command failure - detect when drive_unplug command isn't present and log error instead of failing entire command src/qemu/qemu_driver.c | 28 +++++++++++++++++++++ src/qemu/qemu_monitor.c | 19 ++++++++++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 38 +++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 ++ src/qemu/qemu_monitor_text.c | 54 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 3 ++ 7 files changed, 148 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e7b37e1..a6a7b8d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9037,6 +9037,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup = NULL; + char *drivestr = NULL; i = qemudFindDisk(vm->def, dev->data.disk->dst); @@ -9064,6 +9065,14 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, goto cleanup; } + /* build the actual drive id string as the disk->info.alias doesn't + * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */ + if (virAsprintf(&drivestr, "%s%s", + QEMU_DRIVE_HOST_PREFIX, detach->info.alias) < 0) { + virReportOOMError(); + goto cleanup; + } + qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { @@ -9077,6 +9086,10 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, goto cleanup; } } + + /* disconnect guest from host device */ + qemuMonitorDriveDel(priv->mon, drivestr); + qemuDomainObjExitMonitorWithDriver(driver, vm); qemuDomainDiskAudit(vm, detach, NULL, "detach", ret >= 0); @@ -9104,6 +9117,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, ret = 0; cleanup: + VIR_FREE(drivestr); return ret; } @@ -9116,6 +9130,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup = NULL; + char *drivestr = NULL; i = qemudFindDisk(vm->def, dev->data.disk->dst); @@ -9142,11 +9157,23 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, } } + /* build the actual drive id string as the disk->info.alias doesn't + * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */ + if (virAsprintf(&drivestr, "%s%s", + QEMU_DRIVE_HOST_PREFIX, detach->info.alias) < 0) { + virReportOOMError(); + goto cleanup; + } + qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(vm); goto cleanup; } + + /* disconnect guest from host device */ + qemuMonitorDriveDel(priv->mon, drivestr); + qemuDomainObjExitMonitorWithDriver(driver, vm); qemuDomainDiskAudit(vm, detach, NULL, "detach", ret >= 0); @@ -9170,6 +9197,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, ret = 0; cleanup: + VIR_FREE(drivestr); virCgroupFree(&cgroup); return ret; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2366fdb..80adba4 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1781,6 +1781,25 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, return ret; } +int qemuMonitorDriveDel(qemuMonitorPtr mon, + const char *drivestr) +{ + DEBUG("mon=%p drivestr=%s", mon, drivestr); + int ret; + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (mon->json) + ret = qemuMonitorJSONDriveDel(mon, drivestr); + else + ret = qemuMonitorTextDriveDel(mon, drivestr); + return ret; +} + int qemuMonitorDelDevice(qemuMonitorPtr mon, const char *devalias) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 7d09145..8cda43b 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -381,6 +381,9 @@ int qemuMonitorDelDevice(qemuMonitorPtr mon, int qemuMonitorAddDrive(qemuMonitorPtr mon, const char *drivestr); +int qemuMonitorDriveDel(qemuMonitorPtr mon, + const char *drivestr); + int qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d2c6f0a..a380ab2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2244,6 +2244,44 @@ int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, } +int qemuMonitorJSONDriveDel(qemuMonitorPtr mon, + const char *drivestr) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + DEBUG("JSONDriveDel drivestr=%s", drivestr); + cmd = qemuMonitorJSONMakeCommand("drive_del", + "s:id", drivestr, + NULL); + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) { + /* See if drive_del isn't supported */ + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + VIR_ERROR0(_("deleting disk is not supported. " + "This may leak data if disk is reassigned")); + ret = 1; + goto cleanup; + } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) { + /* NB: device not found errors mean the drive was + * auto-deleted and we ignore the error */ + ret = 0; + } else { + ret = qemuMonitorJSONCheckError(cmd, reply); + } + } + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 94806c1..82671c7 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -188,6 +188,9 @@ int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, const char *drivestr); +int qemuMonitorJSONDriveDel(qemuMonitorPtr mon, + const char *drivestr); + int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7f15008..483ceb0 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2285,6 +2285,7 @@ int qemuMonitorTextDelDevice(qemuMonitorPtr mon, goto cleanup; } + DEBUG("TextDelDevice devalias=%s", devalias); if (qemuMonitorCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("cannot detach %s device"), devalias); @@ -2391,6 +2392,59 @@ cleanup: return ret; } +/* Attempts to remove a host drive. + * Returns 1 if unsupported, 0 if ok, and -1 on other failure */ +int qemuMonitorTextDriveDel(qemuMonitorPtr mon, + const char *drivestr) +{ + char *cmd = NULL; + char *reply = NULL; + char *safedev; + int ret = -1; + DEBUG("TextDriveDel drivestr=%s", drivestr); + + if (!(safedev = qemuMonitorEscapeArg(drivestr))) { + virReportOOMError(); + goto cleanup; + } + + if (virAsprintf(&cmd, "drive_del %s", safedev) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot delete %s drive"), drivestr); + goto cleanup; + } + + if (strstr(reply, "unknown command:")) { + VIR_ERROR0(_("deleting drive is not supported. " + "This may leak data if disk is reassigned")); + ret = 1; + goto cleanup; + + /* (qemu) drive_del wark + * Device 'wark' not found */ + } else if (STRPREFIX(reply, "Device '") && (strstr(reply, "not found"))) { + /* NB: device not found errors mean the drive was auto-deleted and we + * ignore the error */ + ret = 0; + } else if (STRNEQ(reply, "")) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("deleting %s drive failed: %s"), drivestr, reply); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(cmd); + VIR_FREE(reply); + VIR_FREE(safedev); + return ret; +} int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index c017509..3a88f87 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -186,6 +186,9 @@ int qemuMonitorTextDelDevice(qemuMonitorPtr mon, int qemuMonitorTextAddDrive(qemuMonitorPtr mon, const char *drivestr); +int qemuMonitorTextDriveDel(qemuMonitorPtr mon, + const char *drivestr); + int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase); -- 1.6.3.3 -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ryanh(a)us.ibm.com

3 4

[libvirt] [PATCH v3] qemu: call drive_unplug in DetachPciDiskDevice
by Ryan Harper 08 Dec '10

08 Dec '10

Currently libvirt doesn't confirm whether the guest has responded to the disk removal request. In some cases this can leave the guest with continued access to the device while the mgmt layer believes that it has been removed. With a recent qemu monitor command[1] we can deterministically revoke a guests access to the disk (on the QEMU side) to ensure no futher access is permitted. This patch adds support for the drive_unplug() command and introduces it in the disk removal paths. There is some discussion to be had about how to handle the case where the guest is running in a QEMU without this command (and the fact that we currently don't have a way of detecting what monitor commands are available). Changes since v2: - use VIR_ERROR to report when unplug command not found Changes since v1: - return > 0 when command isn't present, < 0 on command failure - detect when drive_unplug command isn't present and log error instead of failing entire command Signed-off-by: Ryan Harper <ryanh(a)us.ibm.com> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index abd8e9d..615427a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8646,6 +8646,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup = NULL; + char drivestr[PATH_MAX]; i = qemudFindDisk(vm->def, dev->data.disk->dst); @@ -8673,13 +8674,36 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, goto cleanup; } + /* build the actual drive id string as the disk->info.alias doesn't + * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */ + if ((ret = snprintf(drivestr, sizeof(drivestr), "%s%s", + QEMU_DRIVE_HOST_PREFIX, + detach->info.alias)) + < 0 || ret >= sizeof(drivestr)) { + virReportOOMError(); + goto cleanup; + } + qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ret = qemuMonitorDriveUnplug(priv->mon, drivestr); + DEBUG("DriveUnplug ret=%d", ret); + /* ret > 0 indicates unplug isn't supported, issue will be logged */ + if (ret < 0) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(vm); goto cleanup; } } else { + ret = qemuMonitorDriveUnplug(priv->mon, drivestr); + /* ret > 0 indicates unplug isn't supported, issue will be logged */ + if (ret < 0) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } if (qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci) < 0) { qemuDomainObjExitMonitor(vm); @@ -8723,6 +8747,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup = NULL; + char drivestr[PATH_MAX]; i = qemudFindDisk(vm->def, dev->data.disk->dst); @@ -8749,7 +8774,22 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, } } + /* build the actual drive id string as the disk->info.alias doesn't + * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */ + if ((ret = snprintf(drivestr, sizeof(drivestr), "%s%s", + QEMU_DRIVE_HOST_PREFIX, + detach->info.alias)) + < 0 || ret >= sizeof(drivestr)) { + virReportOOMError(); + goto cleanup; + } + qemuDomainObjEnterMonitorWithDriver(driver, vm); + /* ret > 0 indicates unplug isn't supported, issue will be logged */ + if (qemuMonitorDriveUnplug(priv->mon, drivestr) < 0) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(vm); goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2366fdb..285381d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1781,6 +1781,25 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, return ret; } +int qemuMonitorDriveUnplug(qemuMonitorPtr mon, + const char *drivestr) +{ + DEBUG("mon=%p drivestr=%s", mon, drivestr); + int ret; + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (mon->json) + ret = qemuMonitorJSONDriveUnplug(mon, drivestr); + else + ret = qemuMonitorTextDriveUnplug(mon, drivestr); + return ret; +} + int qemuMonitorDelDevice(qemuMonitorPtr mon, const char *devalias) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 48f4c20..bfe3641 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -381,6 +381,9 @@ int qemuMonitorDelDevice(qemuMonitorPtr mon, int qemuMonitorAddDrive(qemuMonitorPtr mon, const char *drivestr); +int qemuMonitorDriveUnplug(qemuMonitorPtr mon, + const char *drivestr); + int qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d3ab25f..b11fd4f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2243,6 +2243,39 @@ int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, } +int qemuMonitorJSONDriveUnplug(qemuMonitorPtr mon, + const char *drivestr) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + DEBUG("JSONDriveUnplug drivestr=%s", drivestr); + cmd = qemuMonitorJSONMakeCommand("drive_unplug", + "s:id", drivestr, + NULL); + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) { + /* See if drive_unplug isn't supported */ + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + VIR_ERROR0(_("unplugging disk is not supported. " + "This may leak data if disk is reassigned")); + ret = 1; + goto cleanup; + } + ret = qemuMonitorJSONCheckError(cmd, reply); + } + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 94806c1..6a8692e 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -188,6 +188,9 @@ int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, const char *drivestr); +int qemuMonitorJSONDriveUnplug(qemuMonitorPtr mon, + const char *drivestr); + int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 69971a6..6e5bc95 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2380,6 +2380,52 @@ cleanup: return ret; } +/* Attempts to unplug a drive. Returns 1 if unsupported, 0 if ok, and -1 on + * other failure */ +int qemuMonitorTextDriveUnplug(qemuMonitorPtr mon, + const char *drivestr) +{ + char *cmd = NULL; + char *reply = NULL; + char *safedev; + int ret = -1; + DEBUG("TextDriveUnplug drivestr=%s", drivestr); + + if (!(safedev = qemuMonitorEscapeArg(drivestr))) { + virReportOOMError(); + goto cleanup; + } + + if (virAsprintf(&cmd, "drive_unplug %s", safedev) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot unplug %s drive"), drivestr); + goto cleanup; + } + + if (strstr(reply, "unknown command:")) { + VIR_ERROR0(_("unplugging disk is not supported. " + "This may leak data if disk is reassigned")); + ret = 1; + goto cleanup; + } else if (STRNEQ(reply, "")) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("unplugging %s drive failed: %s"), drivestr, reply); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(cmd); + VIR_FREE(reply); + VIR_FREE(safedev); + return ret; +} int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index c017509..8355ce8 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -186,6 +186,9 @@ int qemuMonitorTextDelDevice(qemuMonitorPtr mon, int qemuMonitorTextAddDrive(qemuMonitorPtr mon, const char *drivestr); +int qemuMonitorTextDriveUnplug(qemuMonitorPtr mon, + const char *drivestr); + int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase); -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ryanh(a)us.ibm.com

3 4

[libvirt] [PATCH] build: find xdr headers on cygwin
by Eric Blake 08 Dec '10

08 Dec '10

* configure.ac (XDR_CFLAGS): Define when needed. * src/Makefile.am (libvirt_driver_remote_la_CFLAGS): Use it. --- On cygwin, './autogen.sh' still tries to enable --with-libvirtd, which also enables networking, but that fails to compile because bridge.c assumes <linux/param.h> and friends exist. Also, vbox_XPCOMCGlue.c fails to compile because it can't figure out an appropriate DYNLIB_NAME; I don't know if anyone has ported a vbox shared library to cygwin (where it would probably be named cygVBoxXPCMC.dll). But with this patch, './autogen.sh --without-libvirtd --without-vbox' successfully compiles. It still fails several tests in 'make check', which I hope to have time to look at next. configure.ac | 24 ++++++++++++++++++++++++ src/Makefile.am | 2 +- 2 files changed, 25 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index dde2cde..1c31e73 100644 --- a/configure.ac +++ b/configure.ac @@ -333,6 +333,25 @@ if test x"$with_remote" = x"yes" || test x"$with_libvirtd" = x"yes"; then dnl check for cygwin's variation in xdr function names AC_CHECK_FUNCS([xdr_u_int64_t],[],[],[#include <rpc/xdr.h>]) + + dnl Cygwin requires -I/usr/include/tirpc for <rpc/rpc.h> + old_CFLAGS=$CFLAGS + AC_CACHE_CHECK([where to find <rpc/rpc.h>], [lv_cv_xdr_cflags], [ + for CFLAGS in '' '-I/usr/include/tirpc' 'missing'; do + if test x"$CFLAGS" = xmissing; then + lv_cv_xdr_cflags=missing; break + fi + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <rpc/rpc.h> + ]])], [lv_cv_xdr_cflags=${CFLAGS:-none}; break]) + done + ]) + CFLAGS=$old_CFLAGS + case $lv_cv_xdr_cflags in + none) XDR_CFLAGS= ;; + missing) AC_MSG_ERROR([Unable to find <rpc/rpc.h>]) ;; + *) XDR_CFLAGS=$lv_cv_xdr_cflags ;; + esac + AC_SUBST([XDR_CFLAGS]) fi @@ -2415,6 +2434,11 @@ AC_MSG_NOTICE([ nl: $LIBNL_CFLAGS $LIBNL_LIBS]) else AC_MSG_NOTICE([ nl: no]) fi +if test "$with_remote" = "yes" || test "$with_libvirtd" = "yes" ; then +AC_MSG_NOTICE([ xdr: $XDR_CFLAGS]) +else +AC_MSG_NOTICE([ xdr: no]) +fi AC_MSG_NOTICE([]) AC_MSG_NOTICE([Test suite]) AC_MSG_NOTICE([]) diff --git a/src/Makefile.am b/src/Makefile.am index 0923d60..baa1bfd 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -473,7 +473,7 @@ libvirt_la_BUILT_LIBADD += libvirt_driver_remote.la endif libvirt_driver_remote_la_CFLAGS = \ $(GNUTLS_CFLAGS) \ - $(SASL_CFLAGS) \ + $(SASL_CFLAGS) $(XDR_CFLAGS) \ -I@top_srcdir@/src/conf \ $(AM_CFLAGS) libvirt_driver_remote_la_LDFLAGS = $(AM_LDFLAGS) -- 1.7.3.2

1 0

[libvirt] [PATCH] qemud: fix memory leak in io error events
by Anthony Liguori 08 Dec '10

08 Dec '10

The extra data isn't being free()'d for IO error events that have a reason. Signed-off-by: Anthony Liguori <aliguori(a)us.ibm.com> --- I wasn't able to test this because the build is broken for me. I won't be able to test it in the field for a couple days. The problem we're seeing is a rather fast memory leak that exhausts all system memory. I believe the source of the leak is that our underlying storage is throwing an I/O error and libvirt is not properly freeing the resulting IO error event object. Because the storage is constantly generating errors and the guest is constantly reading, memory is just consumed until the system is exhausted. diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index e88aafe..5f086bd 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -472,6 +472,7 @@ void virDomainEventFree(virDomainEventPtr event) return; switch (event->eventID) { + case VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON: case VIR_DOMAIN_EVENT_ID_IO_ERROR: VIR_FREE(event->data.ioError.srcPath); VIR_FREE(event->data.ioError.devAlias); -- 1.7.0.4

3 2

[libvirt] [PATCH] qemu: Enable disabled debug messages
by Jiri Denemark 08 Dec '10

08 Dec '10

--- src/qemu/qemu_conf.c | 2 +- src/qemu/qemu_conf.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index d81e6cc..08c084b 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1449,7 +1449,7 @@ int qemudParseHelpStr(const char *qemu, *flags = qemudComputeCmdFlags(help, *version, *is_kvm, *kvm_version); - qemudDebug("Version %u.%u.%u, cooked version %u, flags %u", + qemudDebug("Version %u.%u.%u, cooked version %u, flags 0x%llx", major, minor, micro, *version, *flags); if (*kvm_version) qemudDebug("KVM version %d detected", *kvm_version); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index a1556cb..50938f5 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -44,7 +44,7 @@ # include "macvtap.h" # include "command.h" -# define qemudDebug(fmt, ...) do {} while(0) +# define qemudDebug DEBUG # define QEMUD_CPUMASK_LEN CPU_SETSIZE -- 1.7.3.2

3 6

[libvirt] [PATCH] qemu: Only build devstr when needs (attach PCI controller)
by Osier Yang 08 Dec '10

08 Dec '10

- qemudDomainAttachPciControllerDevice: Don't build "devstr" if "-device" of qemu is not available, as "devstr" will only be used by "qemuMonitorAddDevice", which depends on "-device" argument of qemu is supported. - "qemudDomainSaveImageOpen": Fix indent problem. * src/qemu/qemu_driver.c --- src/qemu/qemu_driver.c | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 54e9dcb..4ef186f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6844,10 +6844,10 @@ static int qemudDomainSaveImageClose(int fd, pid_t read_pid, int *status) static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) qemudDomainSaveImageOpen(struct qemud_driver *driver, - const char *path, - virDomainDefPtr *ret_def, - struct qemud_save_header *ret_header, - pid_t *ret_read_pid) + const char *path, + virDomainDefPtr *ret_def, + struct qemud_save_header *ret_header, + pid_t *ret_read_pid) { int fd; pid_t read_pid = -1; @@ -7902,11 +7902,11 @@ static int qemudDomainAttachPciControllerDevice(struct qemud_driver *driver, goto cleanup; if (qemuAssignDeviceControllerAlias(controller) < 0) goto cleanup; - } - if (!(devstr = qemuBuildControllerDevStr(controller))) { - virReportOOMError(); - goto cleanup; + if (!(devstr = qemuBuildControllerDevStr(controller))) { + virReportOOMError(); + goto cleanup; + } } if (VIR_REALLOC_N(vm->def->controllers, vm->def->ncontrollers+1) < 0) { -- 1.7.3.2

2 1

[libvirt] [PATCH] virsh: fix a typo in the memtune help description
by Justin Clift 08 Dec '10

08 Dec '10

--- tools/virsh.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 6c48a62..4e37f2d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2894,8 +2894,9 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd) * "memtune" command */ static const vshCmdInfo info_memtune[] = { - {"help", N_("Get/Set memory paramters")}, - {"desc", N_("Get/Set the current memory paramters for the guest domain.\n" \ + {"help", N_("Get or set memory parameters")}, + {"desc", N_("Get or set the current memory parameters for a guest" \ + " domain.\n" \ " To get the memory parameters use following command: \n\n" \ " virsh # memtune <domain>")}, {NULL, NULL} -- 1.7.3.2

3 4

[libvirt] [RFC] cgroups net_cls controller implementation
by D. Herrendoerfer 08 Dec '10

08 Dec '10

This a basic implemantation to support the net_cls feature of cgroups. It adds the setting of a net_cls.classid value to the existing cgroups setup in the qemu driver. The classid is specified in the qemu.conf file. This enables the use of the tc utility to manage traffic from/to vitual machines based on the setting combination of classid and network interface. Signed-off-by: D.Herrendoerfer <d.herrendoerfer [at] herrendoerfer [dot] name > src/libvirt_private.syms | 1 + src/qemu/qemu.conf | 6 +++++- src/qemu/qemu_conf.c | 7 ++++++- src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 12 ++++++++++++ src/util/cgroup.c | 18 +++++++++++++++++- src/util/cgroup.h | 3 +++ 7 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f251c94..771911e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -80,6 +80,7 @@ virCgroupSetFreezerState; virCgroupSetMemory; virCgroupSetMemoryHardLimit; virCgroupSetMemorySoftLimit; +virCgroupSetNetworkClassID; virCgroupSetSwapHardLimit; diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index f4f965e..591d8dc 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -157,7 +157,7 @@ # can be mounted in different locations. libvirt will detect # where they are located. # -# cgroup_controllers = [ "cpu", "devices", "memory" ] +# cgroup_controllers = [ "cpu", "devices", "memory", "net_cls" ] # This is the basic set of devices allowed / required by # all virtual machines. @@ -175,6 +175,10 @@ # "/dev/rtc", "/dev/hpet", "/dev/net/tun", #] +# This is the default classid that will be assigned +# to all virtual machines. +# cgroup_net_cls_classid = 4096 + # The default format for Qemu/KVM guest save images is raw; that is, the # memory from the domain is dumped out directly to a file. If you have diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 7cd0603..46ac040 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -326,7 +326,8 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, driver->cgroupControllers = (1 << VIR_CGROUP_CONTROLLER_CPU) | (1 << VIR_CGROUP_CONTROLLER_DEVICES) | - (1 << VIR_CGROUP_CONTROLLER_MEMORY); + (1 << VIR_CGROUP_CONTROLLER_MEMORY) | + (1 << VIR_CGROUP_CONTROLLER_NETWORK); } for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { if (driver->cgroupControllers & (1 << i)) { @@ -364,6 +365,10 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, driver->cgroupDeviceACL[i] = NULL; } + p = virConfGetValue (conf, "cgroup_net_cls_classid"); + CHECK_TYPE ("cgroup_net_cls_classid", VIR_CONF_LONG); + if (p) driver->cgroupNetClsClassid = p->l; + p = virConfGetValue (conf, "save_image_format"); CHECK_TYPE ("save_image_format", VIR_CONF_STRING); if (p && p->str) { diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index aba64d6..961c6cd 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -119,6 +119,7 @@ struct qemud_driver { virCgroupPtr cgroup; int cgroupControllers; char **cgroupDeviceACL; + int cgroupNetClsClassid; virDomainObjList domains; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1a7c1ad..42448b5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -186,6 +186,8 @@ static int qemudVMFiltersInstantiate(virConnectPtr conn, static struct qemud_driver *qemu_driver = NULL; +#include "interface.h" + static void *qemuDomainObjPrivateAlloc(void) { @@ -3597,6 +3599,16 @@ static int qemuSetupCgroup(struct qemud_driver *driver, vm->def->name); } + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_NETWORK)) { + + if (driver->cgroupNetClsClassid != 0) { + rc = virCgroupSetNetworkClassID(cgroup, driver- >cgroupNetClsClassid); + if (rc != 0) { + VIR_WARN("Cannot set net_cls.classid for: %s", + vm->def->name); + } + } + } done: virCgroupFree(&cgroup); return 0; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index 2758a8f..a2ed0ed 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -37,7 +37,7 @@ VIR_ENUM_IMPL(virCgroupController, VIR_CGROUP_CONTROLLER_LAST, "cpu", "cpuacct", "cpuset", "memory", "devices", - "freezer"); + "freezer", "net_cls"); struct virCgroupController { int type; @@ -851,6 +851,22 @@ int virCgroupForDomain(virCgroupPtr driver ATTRIBUTE_UNUSED, #endif /** + * virCgroupSetNetworkClassID: + * + * @group: The cgroup to change memory for + * @classid: The classID number + * + * Returns: 0 on success + */ +int virCgroupSetNetworkClassID(virCgroupPtr group, unsigned long classid) +{ + return virCgroupSetValueU64(group, + VIR_CGROUP_CONTROLLER_NETWORK, + "net_cls.classid", + classid); +} + +/** * virCgroupSetMemory: * * @group: The cgroup to change memory for diff --git a/src/util/cgroup.h b/src/util/cgroup.h index 9e1c61f..9626e82 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -22,6 +22,7 @@ enum { VIR_CGROUP_CONTROLLER_MEMORY, VIR_CGROUP_CONTROLLER_DEVICES, VIR_CGROUP_CONTROLLER_FREEZER, + VIR_CGROUP_CONTROLLER_NETWORK, VIR_CGROUP_CONTROLLER_LAST }; @@ -40,6 +41,8 @@ int virCgroupForDomain(virCgroupPtr driver, int virCgroupAddTask(virCgroupPtr group, pid_t pid); +int virCgroupSetNetworkClassID(virCgroupPtr group, unsigned long classid); + int virCgroupSetMemory(virCgroupPtr group, unsigned long kb); int virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb);

3 4

[libvirt] cancel cloning
by Zdenek Styblik 08 Dec '10

08 Dec '10

Hello, please, is there some way how to cancel VM cloning in progress? To be more specific, to cancel volume allocation. And since we are on the topic, why is volume allocation so slow even for VM that's shut off? Cloning has been initiated via virt-tools resp. virt-manager. Thank you for reply, Zdenek -- Zdenek Styblik Net/Linux admin OS TurnovFree.net email: stybla(a)turnovfree.net jabber: stybla(a)jabber.turnovfree.net

1 1

[libvirt] [PATCH] docs: updated virsh command reference download links to new version
by Justin Clift 08 Dec '10

08 Dec '10

The new version lists every virsh command, its description, and the version of libvirt where it became available. --- docs/virshcmdref.html.in | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/virshcmdref.html.in b/docs/virshcmdref.html.in index 0a19ca7..352c6b3 100644 --- a/docs/virshcmdref.html.in +++ b/docs/virshcmdref.html.in @@ -49,21 +49,21 @@ <ul> <li> Standard HTML format, multiple pages - (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html…">.tar.gz</a>) - (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html…">.tar.bz2</a>) - (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html…">.zip</a>) + (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html…">.tar.gz</a>) + (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html…">.tar.bz2</a>) + (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html…">.zip</a>) </li> <li> HTML format, one long page - (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html…">.tar.gz</a>) - (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html…">.tar.bz2</a>) - (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html…">.zip</a>) + (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html…">.tar.gz</a>) + (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html…">.tar.bz2</a>) + (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html…">.zip</a>) </li> <li> - <a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0.pdf">PDF format</a> + <a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1.pdf">PDF format</a> </li> <li> - <a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0.epub">ePub format</a> + <a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1.epub">ePub format</a> </li> </ul> -- 1.7.3.2

1 1

[libvirt] [PATCH] configure: improve misleading libnl missing error message
by Justin Clift 08 Dec '10

08 Dec '10

This fixes a misleading error message saying the libnl package needs to be installed, when it's really the libnl-devel package needing to be installed. --- configure.ac | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index d26bc68..dde2cde 100644 --- a/configure.ac +++ b/configure.ac @@ -2215,7 +2215,7 @@ LIBNL_LIBS="" if test "$with_macvtap" = "yes"; then PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ ], [ - AC_MSG_ERROR([libnl >= $LIBNL_REQUIRED is required for macvtap support]) + AC_MSG_ERROR([libnl-devel >= $LIBNL_REQUIRED is required for macvtap support]) ]) fi -- 1.7.3.2

2 2

[libvirt] [PATCH] maint: update to latest gnulib
by Eric Blake 08 Dec '10

08 Dec '10

* .gnulib: Update to latest, for at least a stdint.h fix * src/storage/storage_driver.c (storageVolumeZeroSparseFile) (storageWipeExtent): Use better type, although it still triggers spurious -Wformat warning on MacOS's gcc. --- * .gnulib 48b1a1a...6491120 (47): > unistr/u8-strcmp: Avoid collision with libc function on Solaris 11. > Update internal documentation. > Put more information about failed tests into the test return codes. > Update for Solaris 11 2010-11. > nproc: Relax license. > maint: restore executable bit > utimecmp: fine-grained src to nearby coarse-grained dest > strerror_r-posix: Fix autoconf test. > Tests for module 'getdomainname'. > getdomainname: Use the system function when possible. > sys_socket: Ensure ss_family field on AIX. > readline: Improve configure output. > *printf-posix: Detect a bug on Solaris 10/x86. > Fix link error when module libunistring-optional is in use. > regex: Mention link dependencies. > ftoastr: Fix compilation error on Solaris. > Oops, fix typo in last commit. > autoupdate > autoupdate > getloadavg: Update documentation. > sys_socket: Fix test whether the functions are declared. > getpass: Make sure to get system declaration on some platforms. > iconv-h: Fix test-iconv-h-c++ failure on Solaris 11 2010-11. > ftoastr: comment fix > stdint: port to GCC 4.3 + OSX + Octave > doc: Corrections regarding MacOS X 10.4 and 10.5. > autoupdate > Uninstall ".bin" files installed by relocwrapper. > Update for NetBSD 5.0. > Update for HP-UX 11.23 and HP-UX 11.31. > Update for MacOS X 10.5. > bootstrap: add bootstrap_sync option. > Oops, fix incomplete ChangeLog entry. > Ensure that <features.h> is included before __GLIBC__ is tested. > memmem: Fix autoconf test. > Port to uClibc. > nproc: Fix condition. > Fix a comment. * lib/vasnprintf.c (VASNPRINTF): Fix comment. > ftoastr: don't assume snprintf > test-rename.h: fix compilation failure > maint.mk: add a URL discussing the no-@acronym policy > ftoastr: depend on snprintf, improve comments > ftoastr: port to hosts lacking strtof and strtold > c-strtold: Avoid link error on AIX 7. > intprops: new macro INT_BITS_STRLEN_BOUND > ftoastr: new module, for lossless conversion of floats to short strings > autoupdate .gnulib | 2 +- src/storage/storage_driver.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.gnulib b/.gnulib index 48b1a1a..6491120 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 48b1a1ae7d5b0f1494aabd4c8a02fbfcec531026 +Subproject commit 64911207854610668b480939469282fdaeb96f74 diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 0f099f0..67d043b 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1575,7 +1575,7 @@ storageVolumeZeroSparseFile(virStorageVolDefPtr vol, virReportSystemError(errno, _("Failed to truncate volume with " "path '%s' to %ju bytes"), - vol->target.path, (intmax_t)size); + vol->target.path, (uintmax_t)size); } out: @@ -1597,13 +1597,13 @@ storageWipeExtent(virStorageVolDefPtr vol, size_t write_size = 0; VIR_DEBUG("extent logical start: %ju len: %ju", - (intmax_t)extent_start, (intmax_t)extent_length); + (uintmax_t)extent_start, (uintmax_t)extent_length); if ((ret = lseek(fd, extent_start, SEEK_SET)) < 0) { virReportSystemError(errno, _("Failed to seek to position %ju in volume " "with path '%s'"), - (intmax_t)extent_start, vol->target.path); + (uintmax_t)extent_start, vol->target.path); goto out; } -- 1.7.3.2

2 2

[libvirt] [PATCH] Moved the nodeinfo command to the 'host' help keyword group
by Justin Clift 08 Dec '10

08 Dec '10

--- tools/virsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 9ce3202..6c48a62 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -10062,7 +10062,6 @@ static const vshCmdDef nodedevCmds[] = { {"nodedev-list", cmdNodeListDevices, opts_node_list_devices, info_node_list_devices}, {"nodedev-reattach", cmdNodeDeviceReAttach, opts_node_device_reattach, info_node_device_reattach}, {"nodedev-reset", cmdNodeDeviceReset, opts_node_device_reset, info_node_device_reset}, - {"nodeinfo", cmdNodeinfo, NULL, info_nodeinfo}, {NULL, NULL, NULL, NULL} }; @@ -10127,6 +10126,7 @@ static const vshCmdDef hostAndHypervisorCmds[] = { {"connect", cmdConnect, opts_connect, info_connect}, {"freecell", cmdFreecell, opts_freecell, info_freecell}, {"hostname", cmdHostname, NULL, info_hostname}, + {"nodeinfo", cmdNodeinfo, NULL, info_nodeinfo}, {"qemu-monitor-command", cmdQemuMonitorCommand, opts_qemu_monitor_command, info_qemu_monitor_command}, {"uri", cmdURI, NULL, info_uri}, {NULL, NULL, NULL, NULL} -- 1.7.3.2

2 2

[libvirt] [PATCH] openvz: convert popen to virCommand
by Eric Blake 08 Dec '10

08 Dec '10

popen must be matched with pclose (not fclose), or it will leak resources. Furthermore, it is a lousy interface when it comes to signal handling. We're much better off using our decent command wrapper. * src/openvz/openvz_conf.c (openvzLoadDomains, openvzGetVEID): Replace popen with virCommand usage. --- src/openvz/openvz_conf.c | 54 +++++++++++++++++++++++---------------------- 1 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 863af93..9d2689a 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -51,6 +51,7 @@ #include "util.h" #include "nodeinfo.h" #include "files.h" +#include "command.h" #define VIR_FROM_THIS VIR_FROM_OPENVZ @@ -433,26 +434,26 @@ openvzFreeDriver(struct openvz_driver *driver) int openvzLoadDomains(struct openvz_driver *driver) { - FILE *fp; int veid, ret; char status[16]; char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainObjPtr dom = NULL; char temp[50]; + char *outbuf = NULL; + char *line; + virCommandPtr cmd = NULL; if (openvzAssignUUIDs() < 0) return -1; - if ((fp = popen(VZLIST " -a -ovpsid,status -H 2>/dev/null", "r")) == NULL) { - openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("popen failed")); - return -1; - } - - while (!feof(fp)) { - if (fscanf(fp, "%d %s\n", &veid, status) != 2) { - if (feof(fp)) - break; + cmd = virCommandNewArgList(VZLIST, "-a", "-ovpsid,status", "-H", NULL); + virCommandSetOutputBuffer(cmd, &outbuf); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + line = *outbuf ? outbuf : NULL; + while (line) { + if (sscanf(line, "%d %s\n", &veid, status) != 2) { openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to parse vzlist output")); goto cleanup; @@ -526,9 +527,14 @@ int openvzLoadDomains(struct openvz_driver *driver) { virDomainObjUnlock(dom); dom = NULL; + + line = strchr(line, '\n'); + if (line) + line++; } - VIR_FORCE_FCLOSE(fp); + virCommandFree(cmd); + VIR_FREE(outbuf); return 0; @@ -536,7 +542,8 @@ int openvzLoadDomains(struct openvz_driver *driver) { virReportOOMError(); cleanup: - VIR_FORCE_FCLOSE(fp); + virCommandFree(cmd); + VIR_FREE(outbuf); if (dom) virDomainObjUnref(dom); return -1; @@ -978,27 +985,22 @@ static int openvzAssignUUIDs(void) */ int openvzGetVEID(const char *name) { - char *cmd; + virCommandPtr cmd; + char *outbuf; int veid; - FILE *fp; bool ok; - if (virAsprintf(&cmd, "%s %s -ovpsid -H", VZLIST, name) < 0) { - openvzError(VIR_ERR_INTERNAL_ERROR, "%s", - _("virAsprintf failed")); + cmd = virCommandNewArgList(VZLIST, name, "-ovpsid", "-H", NULL); + virCommandSetOutputBuffer(cmd, &outbuf); + if (virCommandRun(cmd, NULL) < 0) { + virCommandFree(cmd); return -1; } - fp = popen(cmd, "r"); - VIR_FREE(cmd); - - if (fp == NULL) { - openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("popen failed")); - return -1; - } + virCommandFree(cmd); + ok = sscanf(outbuf, "%d\n", &veid) == 1; + VIR_FREE(outbuf); - ok = fscanf(fp, "%d\n", &veid ) == 1; - VIR_FORCE_FCLOSE(fp); if (ok && veid >= 0) return veid; -- 1.7.3.2

2 7

[libvirt] [PATCH 1/2] virCommand: document behavior on no output
by Eric Blake 07 Dec '10

07 Dec '10

Option 1: This patch (all callers have to worry about NULL buffers, but checking for output is a simple pointer check). Option 2: Guarantee that outbuf/errbuf are allocated, even if to the empty string. Caller always has to free the result, and empty output check requires checking if *outbuf=='\0'. Personally, I prefer option 2. Thoughts? * docs/internals/command.html.in: Update documentation. * src/util/command.c (virCommandSetOutputBuffer) (virCommandSetErrorBuffer) Guarantee NULL buffer on no output. --- docs/internals/command.html.in | 4 ++-- src/util/command.c | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in index c4fa800..ea9ec64 100644 --- a/docs/internals/command.html.in +++ b/docs/internals/command.html.in @@ -349,8 +349,8 @@ <p> Once the command has finished executing, these buffers - will contain the output. It is the callers responsibility - to free these buffers. + will contain the output, or be NULL if there was no output. It + is the callers responsibility to free these buffers. </p> <h3><a name="directory">Setting working directory</a></h3> diff --git a/src/util/command.c b/src/util/command.c index aa43f76..1923799 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -549,6 +549,7 @@ virCommandSetOutputBuffer(virCommandPtr cmd, char **outbuf) return; } + VIR_FREE(*outbuf); cmd->outbuf = outbuf; cmd->outfdptr = &cmd->outfd; } @@ -569,6 +570,7 @@ virCommandSetErrorBuffer(virCommandPtr cmd, char **errbuf) return; } + VIR_FREE(*errbuf); cmd->errbuf = errbuf; cmd->errfdptr = &cmd->errfd; } -- 1.7.3.2

5 8

[libvirt] [PATCH] command: enforce fd vs. buffer considerations
by Eric Blake 07 Dec '10

07 Dec '10

* docs/internals/command.html.in: Better documentation of buffer vs. fd considerations. * src/util/command.c (virCommandRunAsync): Reject raw execution with string io. (virCommandRun): Reject execution with user-specified fds not visiting a regular file. --- Perhaps we need to relax the fstat check to permit block devices in addition to regular files; but that can be a later patch if needed. docs/internals/command.html.in | 35 ++++++++++++++++++++++++++--------- src/util/command.c | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in index c4fa800..259e68e 100644 --- a/docs/internals/command.html.in +++ b/docs/internals/command.html.in @@ -250,7 +250,7 @@ allow an open file handle to be passed into the child, while controlling whether that handle remains open in the parent or guaranteeing that the handle will be closed in the parent after - either virCommandRun or virCommandFree. + virCommandRun, virCommandRunAsync, or virCommandFree. </p> <pre> @@ -266,9 +266,16 @@ With this, both file descriptors sharedfd and childfd in the current process remain open as the same file descriptors in the child. Meanwhile, after the child is spawned, sharedfd remains - open in the parent, while childfd is closed. For stdin/out/err - it is usually necessary to map a file handle. To attach file - descriptor 7 in the current process to stdin in the child: + open in the parent, while childfd is closed. + </p> + + <p> + For stdin/out/err it is sometimes necessary to map a file + handle. If a mapped file handle is a pipe fed or consumed by + the caller, then the caller should use virCommandDaemonize or + virCommandRunAsync rather than virCommandRun to avoid deadlock + (mapping a regular file is okay with virCommandRun). To attach + file descriptor 7 in the current process to stdin in the child: </p> <pre> @@ -322,11 +329,21 @@ <h3><a name="buffers">Feeding & capturing strings to/from the child</a></h3> <p> - Often dealing with file handles for stdin/out/err - is unnecessarily complex. It is possible to specify - a string buffer to act as the data source for the - child's stdin, if there are no embedded NUL bytes, - and if the command will be run with virCommandRun: + Often dealing with file handles for stdin/out/err is + unnecessarily complex; an alternative is to let virCommandRun + perform the I/O and interact via string buffers. Use of a buffer + only works with virCommandRun, and cannot be mixed with pipe + file descriptors. That is, the choice is generally between + managing all I/O in the caller (any fds not specified are tied + to /dev/null), or letting virCommandRun manage all I/O via + strings (unspecified stdin is tied to /dev/null, and unspecified + output streams get logged but are otherwise discarded). + </p> + + <p> + It is possible to specify a string buffer to act as the data + source for the child's stdin, if there are no embedded NUL + bytes, and if the command will be run with virCommandRun: </p> <pre> diff --git a/src/util/command.c b/src/util/command.c index c0520ec..d1d8f6d 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -25,6 +25,7 @@ #include <stdarg.h> #include <stdbool.h> #include <stdlib.h> +#include <sys/stat.h> #include <sys/wait.h> #include "command.h" @@ -872,6 +873,9 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) char *outbuf = NULL; char *errbuf = NULL; int infd[2]; + struct stat st; + bool string_io; + bool async_io = false; if (!cmd ||cmd->has_error == ENOMEM) { virReportOOMError(); @@ -883,6 +887,28 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) return -1; } + /* Avoid deadlock, by requiring that any open fd not under our + * control must be visiting a regular file, or that we are + * daemonized and no string io is required. */ + string_io = cmd->inbuf || cmd->outbuf || cmd->errbuf; + if (cmd->infd != -1 && + (fstat(cmd->infd, &st) < 0 || !S_ISREG(st.st_mode))) + async_io = true; + if (cmd->outfdptr && cmd->outfdptr != &cmd->outfd && + (*cmd->outfdptr == -1 || + fstat(*cmd->outfdptr, &st) < 0 || !S_ISREG(st.st_mode))) + async_io = true; + if (cmd->errfdptr && cmd->errfdptr != &cmd->errfd && + (*cmd->errfdptr == -1 || + fstat(*cmd->errfdptr, &st) < 0 || !S_ISREG(st.st_mode))) + async_io = true; + if (async_io ? (!(cmd->flags & VIR_EXEC_DAEMON) || string_io) + : ((cmd->flags & VIR_EXEC_DAEMON) && string_io)) { + virCommandError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid use of command API")); + return -1; + } + /* If we have an input buffer, we need * a pipe to feed the data to the child */ if (cmd->inbuf) { @@ -921,7 +947,7 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) return -1; } - if (cmd->inbuf || cmd->outbuf || cmd->errbuf) + if (string_io) ret = virCommandProcessIO(cmd); if (virCommandWait(cmd, exitstatus) < 0) @@ -1001,6 +1027,15 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) return -1; } + /* Buffer management can only be requested via virCommandRun. */ + if ((cmd->inbuf && cmd->infd == -1) || + (cmd->outbuf && cmd->outfdptr != &cmd->outfd) || + (cmd->errbuf && cmd->errfdptr != &cmd->errfd)) { + virCommandError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid use of command API")); + return -1; + } + if (cmd->pid != -1) { virCommandError(VIR_ERR_INTERNAL_ERROR, _("command is already running as pid %d"), -- 1.7.3.2

2 2

[libvirt] Just in case it's useful: virsh commands, their version, and their commits
by Justin Clift 07 Dec '10

07 Dec '10

Hi all, Just put a (temporary) page on the wiki showing all of the virsh commands, the version of libvirt each was introduced in, and the commit that did it. http://wiki.libvirt.org/page/VirshCmdAdditionCommits It's very "raw data" style, and at some point in the next week the info will get transferred somewhere better (Virsh Command Reference). But figured other people out there might have a use for it (no idea). Regards and best wishes, Justin Clift

1 0

[libvirt] [PATCH 0/4] Fix up the mess in commandtest
by Jiri Denemark 07 Dec '10

07 Dec '10

commandtest is quite messy with lots of memory leaks, some filedescriptor leaks and few other small issues. This patches fixes them. Jiri Denemark (4): tests: Fix code formating in commandtest tests: Fix leaks in commandtest tests: Fix commandtest in VPATH build tests: Fix detection of expected error tests/commandtest.c | 282 +++++++++++++++++++++++++++++++++------------------ 1 files changed, 184 insertions(+), 98 deletions(-) -- 1.7.3.2

3 16

[libvirt] [PATCH] virsh: Remove redundant commands in group domain
by Osier Yang 07 Dec '10

07 Dec '10

"echo" is already in group "virsh", "freecell" and "hostname" are already in group "host", so remove them from group "domain" * tools/virsh.c --- tools/virsh.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 441fd77..9ce3202 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -9956,10 +9956,7 @@ static const vshCmdDef domManagementCmds[] = { {"domxml-to-native", cmdDomXMLToNative, opts_domxmltonative, info_domxmltonative}, {"dump", cmdDump, opts_dump, info_dump}, {"dumpxml", cmdDumpXML, opts_dumpxml, info_dumpxml}, - {"echo", cmdEcho, opts_echo, info_echo}, {"edit", cmdEdit, opts_edit, info_edit}, - {"freecell", cmdFreecell, opts_freecell, info_freecell}, - {"hostname", cmdHostname, NULL, info_hostname}, {"managedsave", cmdManagedSave, opts_managedsave, info_managedsave}, {"managedsave-remove", cmdManagedSaveRemove, opts_managedsaveremove, info_managedsaveremove}, {"maxvcpus", cmdMaxvcpus, opts_maxvcpus, info_maxvcpus}, -- 1.7.3.2

3 2

[libvirt] RFC: Setting a permanent MAC address for virtual network bridges
by Laine Stump 07 Dec '10

07 Dec '10

This is in reference to the following bugs that are filed against RHEL, but applicable to all Linux distros: https://bugzilla.redhat.com/show_bug.cgi?id=560994 https://bugzilla.redhat.com/show_bug.cgi?id=609463 "MAC address of virtual network bridge changes when TAP devices are added/removed" The problem is that the MAC address of a bridge that hasn't had its address specifically set, will be set to the MAC address of the first interface attached to it. If that interface later goes away, it will be set to the MAC address of another attached interface. Daniel's comments in the bug 1) state that the MAC address of a bridge device must match the MAC address of one of the attached interfaces, or no traffic will pass, and 2) suggest creating a dummy interface that is created and used purely to set a fixed MAC address that won't go away when any guest is destroyed (because it's not associated with any guest. I did some quick experimentation and found that: 1) It is possible to set a bridge's MAC address to something that isn't the address of an attached interface (although the range of numbers seems to be limited, at least when setting it using "ip link set virbr0 address xx:xx:xx:xx:xx:xx - anything starting with less than "B2" was rejected). 2) When a bridge's MAC address is set in that way, as Daniel says, no traffic passes. (Why is that, anyway?) 3) If I create a tap device, set a MAC address on it, attach it to the bridge, and make sure the bridge is assigned that MAC address, traffic does pass, and when guests are destroyed and restarted, the MAC address of the bridge doesn't change. So it looks like this is a viable way to solve the problem. Before digging into it, though, I thought I should ask if there are any alternate ideas, or problems that anyone sees with this approach. Also, I'm wondering where the name and MAC address of the dummy interfaces should come from, and whether I need to worry about storing them in the network XML. So far, this is what I think on those topics: 1) the MAC address can be autogenerated if it's not in the XML, but needs to be saved in the XML so that it's persistent across libvirtd restarts. 2) the dummy interface name can just be some derivation of the bridge device name ("virbr0-mac"?), and probably doesn't need to be saved in the XML. (Also, I just noticed the code that sets the first byte of the MAC address of all tap interfaces being attached to virtual network bridges to 0xFE. What's the story behind that? Does the bridge always take the numerically lowest MAC that's currently connected?)

2 2

[libvirt] [Patch 0/2] Add new feature into live migration
by Wen Congyang 07 Dec '10

07 Dec '10

If the memory of guest OS is changed constantly, the live migration can not be ended ever for ever. We can use the command 'virsh migrate-setmaxdowntime' to control the live migration. But the value of maxdowntime is diffcult to calculate because it depends on the transfer speed of network and constantly changing memroy size. We need a easy way to control the live migration. This patch set add the support of auto cold migration fallback on timeout. With this patch set, when we migrate the guest OS, we can specify a timeout. If the live migration timeouts, the migration will fallback to cold migration. TODO: 1. The timer for Windows has not been implemented. Test of this patchset: Env: a. The size of guest OS's memory: 1GB b. The transfer speed of network: about 100Mb/s c. The size of constantly changing memory: more than 900MB 1. migrate without timeout # virsh migrate --live RHEL6RC qemu+ssh://<dest IP>/system tcp://<dest IP>:49152 The migration does not end after 12 hours. 2. migrate with timeout(30 minutes): # date Tue Dec 7 10:00:13 CST 2010 # virsh migrate --live --timeout 1800 RHEL6RC qemu+ssh://10.167.225.67/system tcp:10.167.225.67:49152 # date Tue Dec 7 10:43:28 CST 2010 Wen Congyang (2): timer impl auto cold migration fallback at timeout configure.ac | 4 ++ src/Makefile.am | 6 ++- src/libvirt.c | 2 + src/libvirt_private.syms | 6 ++ src/util/timer.c | 116 ++++++++++++++++++++++++++++++++++++++++++++ src/util/timer.h | 37 ++++++++++++++ src/util/timer_linux.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.c | 53 ++++++++++++++++++++ 8 files changed, 343 insertions(+), 2 deletions(-) create mode 100644 src/util/timer.c create mode 100644 src/util/timer.h create mode 100644 src/util/timer_linux.c

1 2

[libvirt] Virsh commands in two help groups?
by Justin Clift 07 Dec '10

07 Dec '10

Hi Osier, Just noticed that some of the virsh commands are now listed in two groups: freecell: listed in both domain and host groups hostname: listed in both domain and host groups echo: listed in both domain and virsh groups Wondering if that's on purpose, or if it needs to be changed? The "echo" one seems a bit wrong in the domain group, but not sure about the others. :)

2 1

[libvirt] [PATCH v4 RESEND 0/4] Support of auto-dump on watchdog event in libvirtd
by Hu Tao 07 Dec '10

07 Dec '10

This patch series adds a new watchdog action `dump' which lets libvirtd can do auto-dump when receiving a watchdog event from qemu guest. In order to make the function work, there must be a watchdog device added to guest, and guest must have a watchdog daemon running, for example, /etc/init.d/watchdog start or auto-started on boot. Changes: v4: - updated threadpool to follow libvirt naming style, use appropriate internals APIs, and hide the struct definitions from the header (by Daniel) - fix an error that qemuDomainObjBeginJobWithDriver() get lost in qemuDomainCoreDump() - use thread pool in libvirtd (qemud worker) v3: - let default auto-dump dir be /var/lib/libvirt/qemu/dump Hu Tao (4): threadpool impl Add a new function doCoreDump Add a watchdog action `dump' Using threadpool API to manage qemud worker cfg.mk | 1 + daemon/libvirtd.c | 172 ++++++---------------------------- daemon/libvirtd.h | 4 + src/Makefile.am | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 6 + src/qemu/qemu.conf | 5 + src/qemu/qemu_conf.c | 16 +++- src/qemu/qemu_conf.h | 5 + src/qemu/qemu_driver.c | 227 +++++++++++++++++++++++++++++++++----------- src/util/threadpool.c | 235 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/threadpool.h | 49 ++++++++++ 13 files changed, 521 insertions(+), 202 deletions(-) create mode 100644 src/util/threadpool.c create mode 100644 src/util/threadpool.h -- 1.7.3 -- Thanks, Hu Tao

3 15

[libvirt] [PATCH] Fix funny off-by-one error in clock-variable
by Dan Kenigsberg 06 Dec '10

06 Dec '10

Humans consider January as month #1, while gmtime_r(3) calls it month #0. While fixing it, render qemu's rtc parameter with leading zeros, as is more commonplace. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=660194 --- src/qemu/qemu_conf.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 83c0f83..973e95b 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -3510,9 +3510,9 @@ qemuBuildClockArgStr(virDomainClockDefPtr def) now += def->data.adjustment; gmtime_r(&now, &nowbits); - virBufferVSprintf(&buf, "base=%d-%d-%dT%d:%d:%d", + virBufferVSprintf(&buf, "base=%d-%d-%dT%02d:%02d:%02d", nowbits.tm_year + 1900, - nowbits.tm_mon, + nowbits.tm_mon + 1, nowbits.tm_mday, nowbits.tm_hour, nowbits.tm_min, -- 1.7.3.2

2 1

[libvirt] Syntax error "Bad fd number" when stopping libvirt-guests
by Laurent Léonard 06 Dec '10

06 Dec '10

Hi, When libvirt-guests is being stopped, I get the following message: $Running guests on default URI: test-vm $Suspending guests on default URI... $Suspending test-vm: /etc/init.d/libvirt-guests: 340: Syntax error: Bad fd number The size of RAM already dumped should be displayed instead. The attached patch fixes that issue. -- Laurent Léonard

2 1

[libvirt] [PATCH] util: Fix bug which will cause libvirtd crash
by Osier Yang 06 Dec '10

06 Dec '10

"virCommandRun": if "cmd->outbuf" or "cmd->errbuf" is NULL, libvirtd will be crashed when trying to start a qemu domain (which invokes "virCommandRun"), it caused by we try to use "*cmd->outbuf" and "*cmd->errbuf" regardless of cmd->outbuf or cmd->errbuf is NULL. * src/util/command.c (virCommandRun) --- src/util/command.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index aa43f76..38d462b 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -931,8 +931,8 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) ret = -1; VIR_DEBUG("Result stdout: '%s' stderr: '%s'", - NULLSTR(*cmd->outbuf), - NULLSTR(*cmd->errbuf)); + cmd->outbuf ? NULLSTR(*cmd->outbuf) : "(null)", + cmd->errbuf ? NULLSTR(*cmd->errbuf) : "(null)"); /* Reset any capturing, in case caller runs * this identical command again */ -- 1.7.3.2

2 1

[libvirt] "shadows a global declaration" warnings in virsh
by Justin Clift 06 Dec '10

06 Dec '10

Hi all, Started seeing these warnings when compiling virsh: virsh.c: In function 'cmdRunConsole': virsh.c:735: warning: declaration of 'devname' shadows a global declaration [-Wshadow] /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow] virsh.c: In function 'cmdConsole': virsh.c:765: warning: declaration of 'devname' shadows a global declaration [-Wshadow] /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow] virsh.c: In function 'vshReadlineInit': virsh.c:11575: warning: assignment discards qualifiers from pointer target type Haven't looked into it yet as they're only warnings. Anyone know if they're important enough to look into? Regards and best wishes, Justin Clift

7 18

[libvirt] [PATCH] Implement virVMOperationType{To|From}String independent from WITH_MACVTAP
by Matthias Bolte 06 Dec '10

06 Dec '10

As this symbols are exported independent from WITH_MACVTAP. --- src/util/macvtap.c | 33 +++++++++++++++++++-------------- 1 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 2aa319c..4345d97 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -27,12 +27,13 @@ #include <config.h> +#include <stdint.h> + #if WITH_MACVTAP || WITH_VIRTUALPORT # include <stdio.h> # include <errno.h> # include <fcntl.h> -# include <stdint.h> # include <c-ctype.h> # include <sys/socket.h> # include <sys/ioctl.h> @@ -44,10 +45,15 @@ # include <netlink/msg.h> -# include "util.h" +#endif /* WITH_MACVTAP || WITH_VIRTUALPORT */ + +#include "util.h" +#include "macvtap.h" + +#if WITH_MACVTAP || WITH_VIRTUALPORT + # include "memory.h" # include "logging.h" -# include "macvtap.h" # include "interface.h" # include "conf/domain_conf.h" # include "virterror_internal.h" @@ -77,17 +83,6 @@ # define LLDPAD_PID_FILE "/var/run/lldpad.pid" -VIR_ENUM_IMPL(virVMOperation, VIR_VM_OP_LAST, - "create", - "save", - "restore", - "destroy", - "migrate out", - "migrate in start", - "migrate in finish", - "no-op") - - enum virVirtualPortOp { ASSOCIATE = 0x1, DISASSOCIATE = 0x2, @@ -1609,3 +1604,13 @@ vpDisassociatePortProfileId(const char *macvtap_ifname, } #endif /* WITH_MACVTAP || WITH_VIRTUALPORT */ + +VIR_ENUM_IMPL(virVMOperation, VIR_VM_OP_LAST, + "create", + "save", + "restore", + "destroy", + "migrate out", + "migrate in start", + "migrate in finish", + "no-op") -- 1.7.0.4

3 5