- Devel - libvirt List Archives

[libvirt] [PATCH] Add iptables rule to fixup DHCP response checksum.
by Laine Stump 02 Aug '10

02 Aug '10

From: Laine Stump <laine(a)redhat.com> (I don't know whether or not we want to commit this upstream yet - the proposed iptables and kernel module backend for the changes have been posted but not yet committed upstream. On the other hand, the new libvirt code ends up simply printing a warning message if the necessary iptables support isn't yet in place.) This patch attempts to take advantage of a newly added netfilter module to correct for a problem with some guest DHCP client implementations when used in conjunction with a DHCP server run on the host systems with packet checksum offloading enabled. The problem is that, when the guest uses a RAW socket to read the DHCP response packets, the checksum hasn't yet been fixed by the IP stack, so it is incorrect. The fix implemented here is to add a rule to the POSTROUTING chain of the mangle table in iptables that fixes up the checksum for packets on the virtual network's bridge that are destined for the bootpc port (ie "dhcpc", ie port 68) port on the guest. Only very new versions of iptables will have this support (it has been submitted upstream, but not yet committed), so a failure to add this rule only results in a warning message. The iptables patch is here: http://patchwork.ozlabs.org/patch/58525/ A corresponding kernel module patch is also required (the backend of the iptables patch) and has been submitted, but I don't have the details for that (I tested using a pre-built image I received from the developer, Michael Tsirkin). --- src/libvirt_private.syms | 2 + src/network/bridge_driver.c | 17 ++++++++++ src/util/iptables.c | 71 +++++++++++++++++++++++++++++++++++++++++++ src/util/iptables.h | 6 ++++ 4 files changed, 96 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 778ceb1..d81d4cf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -328,6 +328,7 @@ iptablesAddForwardAllowRelatedIn; iptablesAddForwardMasquerade; iptablesAddForwardRejectIn; iptablesAddForwardRejectOut; +iptablesAddOutputFixUdpChecksum; iptablesAddTcpInput; iptablesAddUdpInput; iptablesContextFree; @@ -339,6 +340,7 @@ iptablesRemoveForwardAllowRelatedIn; iptablesRemoveForwardMasquerade; iptablesRemoveForwardRejectIn; iptablesRemoveForwardRejectOut; +iptablesRemoveOutputFixUdpChecksum; iptablesRemoveTcpInput; iptablesRemoveUdpInput; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 72255c1..c4480ff 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -781,6 +781,19 @@ networkAddIptablesRules(struct network_driver *driver, !networkAddRoutingIptablesRules(driver, network)) goto err8; + /* If we are doing local DHCP service on this network, attempt to + * add a rule that will fixup the checksum of DHCP response + * packets back to the guests (but report failure without + * aborting, since not all iptables implementations support it). + */ + + if ((network->def->ipAddress || network->def->nranges) && + (iptablesAddOutputFixUdpChecksum(driver->iptables, + network->def->bridge, 68) != 0)) { + VIR_WARN("Could not add rule to fixup DHCP response checksums " + "on network '%s'", network->def->name); + } + return 1; err8: @@ -811,6 +824,10 @@ networkAddIptablesRules(struct network_driver *driver, static void networkRemoveIptablesRules(struct network_driver *driver, virNetworkObjPtr network) { + if (network->def->ipAddress || network->def->nranges) { + iptablesRemoveOutputFixUdpChecksum(driver->iptables, + network->def->bridge, 68); + } if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE) { if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) { iptablesRemoveForwardMasquerade(driver->iptables, diff --git a/src/util/iptables.c b/src/util/iptables.c index d06b857..9b888e5 100644 --- a/src/util/iptables.c +++ b/src/util/iptables.c @@ -60,6 +60,7 @@ struct _iptablesContext iptRules *input_filter; iptRules *forward_filter; iptRules *nat_postrouting; + iptRules *mangle_postrouting; }; static void @@ -188,6 +189,9 @@ iptablesContextNew(void) if (!(ctx->nat_postrouting = iptRulesNew("nat", "POSTROUTING"))) goto error; + if (!(ctx->mangle_postrouting = iptRulesNew("mangle", "POSTROUTING"))) + goto error; + return ctx; error: @@ -210,6 +214,8 @@ iptablesContextFree(iptablesContext *ctx) iptRulesFree(ctx->forward_filter); if (ctx->nat_postrouting) iptRulesFree(ctx->nat_postrouting); + if (ctx->mangle_postrouting) + iptRulesFree(ctx->mangle_postrouting); VIR_FREE(ctx); } @@ -753,3 +759,68 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx, { return iptablesForwardMasquerade(ctx, network, physdev, REMOVE); } + + +static int +iptablesOutputFixUdpChecksum(iptablesContext *ctx, + const char *iface, + int port, + int action) +{ + char portstr[32]; + + snprintf(portstr, sizeof(portstr), "%d", port); + portstr[sizeof(portstr) - 1] = '\0'; + + return iptablesAddRemoveRule(ctx->mangle_postrouting, + action, + "--out-interface", iface, + "--protocol", "udp", + "--destination-port", portstr, + "--jump", "CHECKSUM", "--checksum-fill", + NULL); +} + +/** + * iptablesAddOutputFixUdpChecksum: + * @ctx: pointer to the IP table context + * @iface: the interface name + * @port: the UDP port to match + * + * Add an rule to the mangle table's POSTROUTING chain that fixes up the + * checksum of packets with the given destination @port. + * the given @iface interface for TCP packets. + * + * Returns 0 in case of success or an error code in case of error. + * (NB: if the system's iptables does not support checksum mangling, + * this will return an error, which should be ignored.) + */ + +int +iptablesAddOutputFixUdpChecksum(iptablesContext *ctx, + const char *iface, + int port) +{ + return iptablesOutputFixUdpChecksum(ctx, iface, port, ADD); +} + +/** + * iptablesRemoveOutputFixUdpChecksum: + * @ctx: pointer to the IP table context + * @iface: the interface name + * @port: the UDP port of the rule to remove + * + * Removes the checksum fixup rule that was previous added with + * iptablesAddOutputFixUdpChecksum. + * + * Returns 0 in case of success or an error code in case of error + * (again, if iptables doesn't support checksum fixup, this will + * return an error, which should be ignored) + */ +int +iptablesRemoveOutputFixUdpChecksum(iptablesContext *ctx, + const char *iface, + int port) +{ + return iptablesOutputFixUdpChecksum(ctx, iface, port, REMOVE); +} diff --git a/src/util/iptables.h b/src/util/iptables.h index 7d55a6d..5c2e553 100644 --- a/src/util/iptables.h +++ b/src/util/iptables.h @@ -89,5 +89,11 @@ int iptablesAddForwardMasquerade (iptablesContext *ctx, int iptablesRemoveForwardMasquerade (iptablesContext *ctx, const char *network, const char *physdev); +int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx, + const char *iface, + int port); +int iptablesRemoveOutputFixUdpChecksum (iptablesContext *ctx, + const char *iface, + int port); #endif /* __QEMUD_IPTABLES_H__ */ -- 1.7.1.1

2 3

[libvirt] [PATCH] qemu: kill some dead stores
by Eric Blake 02 Aug '10

02 Aug '10

Spotted by clang. The qemuConnectMonitor one is an outright bug, the other two are cosmetic. * src/qemu/qemu_monitor.c (qemuMonitorClose): Kill dead store. * src/qemu/qemu_driver.c (qemudDomainSaveImageStartVM): Likewise. (qemuConnectMonitor): Don't lose error status. --- src/qemu/qemu_driver.c | 2 -- src/qemu/qemu_monitor.c | 4 +--- 2 files changed, 1 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 098f4da..57b8271 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1416,7 +1416,6 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) ret = qemuMonitorSetCapabilities(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); - ret = 0; error: if (ret < 0) qemuMonitorClose(priv->mon); @@ -6535,7 +6534,6 @@ qemudDomainSaveImageStartVM(virConnectPtr conn, wait_ret = qemudDomainSaveImageClose(fd, read_pid, &status); fd = -1; if (read_pid != -1) { - read_pid = -1; if (wait_ret == -1) { virReportSystemError(errno, _("failed to wait for process reading '%s'"), diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index f1494ff..2366fdb 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -685,8 +685,6 @@ cleanup: void qemuMonitorClose(qemuMonitorPtr mon) { - int refs; - if (!mon) return; @@ -706,7 +704,7 @@ void qemuMonitorClose(qemuMonitorPtr mon) mon->closed = 1; } - if ((refs = qemuMonitorUnref(mon)) > 0) + if (qemuMonitorUnref(mon) > 0) qemuMonitorUnlock(mon); } -- 1.7.2

2 4

[libvirt] [PATCH] Check that virsh -d argument is numeric
by Daniel Veillard 02 Aug '10

02 Aug '10

Having been bitten one more time by the use of -d to pass the hypervisor URI instead of -c (confusion coming from CVS using -d to specify the root), I suggest to drop atoi and use the function with checking and error out with proper explanation instead of silently failing ! paphio:~/libvirt -> /usr/bin/virsh -d qemu+ssh://test2/system list Id Name State ---------------------------------- paphio:~/libvirt -> tools/virsh -d qemu+ssh://test2/system list error: option -d take a numeric argument paphio:~/libvirt -> tools/virsh -d 5 list command: "list " Id Name State ---------------------------------- paphio:~/libvirt -> Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

3 6

[libvirt] [PATCH] Fix the ACS checking in the PCI code.
by Chris Lalancette 02 Aug '10

02 Aug '10

When trying to assign a PCI device to a guest, we have to check that all bridges upstream of that device support ACS. That means that we have to find the parent bridge of the current device, check for ACS, then find the parent bridge of that device, check for ACS, etc. As it currently stands, the code to do this iterates through all PCI devices on the system, looking for a device that has a range of busses that included the current device's bus. That check is not restrictive enough, though. Depending on how we iterated through the list of PCI devices, we could first find the *topmost* bridge in the system; since it necessarily had a range of busses including the current device's bus, we would only ever check the topmost bridge, and not check any of the intermediate bridges. Note that this also caused a fairly serious bug in the secondary bus reset code, where we could erroneously find and reset the topmost bus instead of the inner bus. This patch changes pciGetParentDevice() so that it first checks if a bridge device's secondary bus exactly matches the bus of the device we are looking for. If it does, we've found the correct parent bridge and we are done. If it does not, then we check to see if this bridge device's busses *include* the bus of the device we care about. If so, we mark this bridge device as best, and go on. If we later find another bridge device whose busses include this device, but is more restrictive, then we free up the previous best and mark the new one as best. This algorithm ensures that in the normal case we find the direct parent, but in the case that the parent bridge secondary bus is not exactly the same as the device, we still find the correct bridge. This patch was tested by me on a 4-port NIC with a bridge without ACS (where assignment failed), a 4-port NIC with a bridge with ACS (where assignment succeeded), and a 2-port NIC with no bridges (where assignment succeeded). Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/util/pci.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++-------- 1 files changed, 66 insertions(+), 12 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index 26d55b8..f2890bd 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -283,6 +283,7 @@ pciIterDevices(pciIterPredicate predicate, DIR *dir; struct dirent *entry; int ret = 0; + int rc; *matched = NULL; @@ -322,11 +323,20 @@ pciIterDevices(pciIterPredicate predicate, break; } - if (predicate(dev, check, data)) { + rc = predicate(dev, check, data); + if (rc < 0) { + /* the predicate returned an error, bail */ + pciFreeDevice(check); + ret = -1; + break; + } + else if (rc == 1) { VIR_DEBUG("%s %s: iter matched on %s", dev->id, dev->name, check->name); *matched = check; + ret = 1; break; } + pciFreeDevice(check); } closedir(dir); @@ -510,10 +520,11 @@ pciBusContainsActiveDevices(pciDevice *dev, /* Is @check the parent of @dev ? */ static int -pciIsParent(pciDevice *dev, pciDevice *check, void *data ATTRIBUTE_UNUSED) +pciIsParent(pciDevice *dev, pciDevice *check, void *data) { uint16_t device_class; uint8_t header_type, secondary, subordinate; + pciDevice **best = data; if (dev->domain != check->domain) return 0; @@ -533,16 +544,54 @@ pciIsParent(pciDevice *dev, pciDevice *check, void *data ATTRIBUTE_UNUSED) VIR_DEBUG("%s %s: found parent device %s", dev->id, dev->name, check->name); - /* No, it's superman! */ - return (dev->bus >= secondary && dev->bus <= subordinate); + /* if the secondary bus exactly equals the device's bus, then we found + * the direct parent. No further work is necessary + */ + if (dev->bus == secondary) + return 1; + + /* otherwise, SRIOV allows VFs to be on different busses then their PFs. + * In this case, what we need to do is look for the "best" match; i.e. + * the most restrictive match that still satisfies all of the conditions. + */ + if (dev->bus > secondary && dev->bus <= subordinate) { + if (*best == NULL) { + *best = pciGetDevice(check->domain, check->bus, check->slot, + check->function); + if (*best == NULL) + return -1; + } + else { + /* OK, we had already recorded a previous "best" match for the + * parent. See if the current device is more restrictive than the + * best, and if so, make it the new best + */ + if (secondary > pciRead8(*best, PCI_SECONDARY_BUS)) { + pciFreeDevice(*best); + *best = pciGetDevice(check->domain, check->bus, check->slot, + check->function); + if (*best == NULL) + return -1; + } + } + } + + return 0; } -static pciDevice * -pciGetParentDevice(pciDevice *dev) +static int +pciGetParentDevice(pciDevice *dev, pciDevice **parent) { - pciDevice *parent = NULL; - pciIterDevices(pciIsParent, dev, &parent, NULL); - return parent; + pciDevice *best = NULL; + int ret; + + *parent = NULL; + ret = pciIterDevices(pciIsParent, dev, parent, &best); + if (ret == 1) + pciFreeDevice(best); + else if (ret == 0) + *parent = best; + return ret; } /* Secondary Bus Reset is our sledgehammer - it resets all @@ -570,7 +619,8 @@ pciTrySecondaryBusReset(pciDevice *dev, } /* Find the parent bus */ - parent = pciGetParentDevice(dev); + if (pciGetParentDevice(dev, &parent) < 0) + return -1; if (!parent) { pciReportError(VIR_ERR_NO_SUPPORT, _("Failed to find parent device for %s"), @@ -1377,7 +1427,8 @@ pciDeviceIsBehindSwitchLackingACS(pciDevice *dev) { pciDevice *parent; - parent = pciGetParentDevice(dev); + if (pciGetParentDevice(dev, &parent) < 0) + return -1; if (!parent) { /* if we have no parent, and this is the root bus, ACS doesn't come * into play since devices on the root bus can't P2P without going @@ -1400,6 +1451,7 @@ pciDeviceIsBehindSwitchLackingACS(pciDevice *dev) do { pciDevice *tmp; int acs; + int ret; acs = pciDeviceDownstreamLacksACS(parent); @@ -1412,8 +1464,10 @@ pciDeviceIsBehindSwitchLackingACS(pciDevice *dev) } tmp = parent; - parent = pciGetParentDevice(parent); + ret = pciGetParentDevice(parent, &parent); pciFreeDevice(tmp); + if (ret < 0) + return -1; } while (parent); return 0; -- 1.7.1.1

3 3

[libvirt] [PATCH] Free up memballoon def.
by Chris Lalancette 02 Aug '10

02 Aug '10

Forgetting to do this was causing a memory leak. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/conf/domain_conf.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ca4bc6e..1ddea0a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -768,6 +768,8 @@ void virDomainDefFree(virDomainDefPtr def) virDomainWatchdogDefFree(def->watchdog); + virDomainMemballoonDefFree(def->memballoon); + virSecurityLabelDefFree(def); virCPUDefFree(def->cpu); -- 1.7.2

2 2

[libvirt] [PATCH] Don't put a semicolon on the end of a VIR_ENUM_IMPL.
by Chris Lalancette 02 Aug '10

02 Aug '10

Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/conf/domain_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 04c417e..ca4bc6e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -99,7 +99,7 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "none", "pci", "drive", - "virtio-serial"); + "virtio-serial") VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, "block", -- 1.7.2

2 2

[libvirt] [PATCH] Fix a bogus warning when parsing <hostdev>
by Chris Lalancette 02 Aug '10

02 Aug '10

When parsing hostdev, the following message would be emitted: 10:17:19.052: error : virDomainHostdevDefParseXML:3748 : internal error unknown node alias However, alias is appropriately parsed in virDomainDeviceInfoParseXML anyway. Disable the error message in the initial XML parsing loop. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/conf/domain_conf.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8721dd1..04c417e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3723,6 +3723,9 @@ virDomainHostdevDefParseXML(const xmlNodePtr node, goto error; } } else if (xmlStrEqual(cur->name, BAD_CAST "address")) { + /* address is parsed as part of virDomainDeviceInfoParseXML */ + } else if (xmlStrEqual(cur->name, BAD_CAST "alias")) { + /* alias is parsed as part of virDomainDeviceInfoParseXML */ } else { virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("unknown node %s"), cur->name); -- 1.7.2

2 2

[libvirt] What should I do when I want to update the kvm for using libvirt in thel 5.4
by 姚远 02 Aug '10

02 Aug '10

My linux is rhel 5.4. In it, the version of kvm is 83 and the qemu is 0.9.1. Virt-manager and libvirt use "/usr/libexec/qemu-kvm" to manage VMs. Now, I want to use the higher version of kvm, so I download qemu-kvm-0.12.4.tar.gz. tar xvf qemu-kvm-0.12.4.tar.gz cd qemu-kvm-0.12.4 ./configure --prefix=/usr/ make make install Then, qemu-system-x86_86 is appeared, but the executalbe file "qemu-kvm" is not generated. Virt-manager an libvirt do not use "qemu-system-x86_86", they only use "qemu-kvm". What should I do when I want to update kvm for using libvirt in thel 5.4. Maybe I can compile a new "qemu-kvm" to substitute the old one. Thank you for your reply! Best wishes!

1 0

[libvirt] default storage pool will not start
by Albert Hopkins 01 Aug '10

01 Aug '10

Long ago I had a VM that used a qcow image that was in the default directory-based storage pool. Since then the VM and the image have been deleted. But now libvirt will not start without the image file: Traceback (most recent call last): File "/usr/share/virt-manager/virtManager/host.py", line 586, in start_pool pool.start() File "/usr/share/virt-manager/virtManager/storagepool.py", line 67, in start self.pool.create(0) File "/usr/lib64/python2.6/site-packages/libvirt.py", line 1039, in create if ret == -1: raise libvirtError ('virStoragePoolCreate() failed', pool=self) libvirtError: cannot open volume '/var/lib/libvirt/images/gstable.qcow2': No such file or directory If I simply "touch" the file, then the pool starts. But I don't want it around. No other vm's reference this file. I've tried deleting and re-creating the pool. I've tried removing all the files in the pool, removing the directory, mkdir'ing the directory and then re-creating the pool, but it still will not start without that file. I've grepped /etc/libvirt for the existence of that filename but it doesn't exist. For the life of me I can't figure out where libvirt is getting this information from and why it thinks it needs this file.

2 3

[libvirt] [PATCH] Remove duplicate </p> from downloads.html.in
by Matthias Bolte 01 Aug '10

01 Aug '10

--- Just pushing this trivial patch. docs/downloads.html.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/docs/downloads.html.in b/docs/downloads.html.in index 04fd16c..6872272 100644 --- a/docs/downloads.html.in +++ b/docs/downloads.html.in @@ -69,7 +69,7 @@ <code>make install</code> </p> <p>At that point you may have to rerun ldconfig or a similar utility to - update your list of installed shared libs.</p> + update your list of installed shared libs. </p> <h2>Building from a source code checkout</h2> -- 1.7.0.4

1 0

[libvirt] OpenBSD crash after "shutdown vm"
by Armando 01 Aug '10

01 Aug '10

Good evening all, firstly I'd like to thank you for your wonderful job: it works nicely and simplifies a lot my life :) after saying that, I'd like to expose you a "bug" (not sure if I can call it this way): 1. scenario: as host I've configured a Ubuntu 10.04 with KVM + libvirt + LVM; I have also configured 4 virtual machines, three of wich are OpenBSDs and the other one is Linux (Fedora 13). 2. issue: when I issue a shutdown to a OpenBSD guest using "virsh shutdown vm" what happens is that the guest machine crashes. To be more precise it seems a kernel panic becuase the vnc session through KVM still works as the machine is running but every digit I issue does not appear on the server shell. Also the ssh sessions die; After having a chat in the #libvirt channel I tried doing this both with acpibtn enabled (on OpenBSD is the default behaviour) and also with acpibtn disabled (using the command "boot -c" and then "disable acpi" at boot time in the OpenBSD server). In both cases the server crashes At this point I have found an alternative way to automatically shut down the machine (using ssh) but it would be nice to understand way this happens. Is this a well-known issue? Is there a way to deeply debug it? Thanks a lot! 0x412E

1 0

[libvirt] [PATCH] Update ID after stopping a domain
by Matthias Bolte 30 Jul '10

30 Jul '10

--- src/esx/esx_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 2 ++ src/vbox/vbox_tmpl.c | 1 + src/xenapi/xenapi_driver.c | 1 + 5 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 964a3a5..913420c 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1620,6 +1620,7 @@ esxDomainDestroy(virDomainPtr domain) goto cleanup; } + domain->id = -1; result = 0; cleanup: diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 98381fb..c46f3a7 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -531,6 +531,7 @@ static int openvzDomainShutdown(virDomainPtr dom) { vm->def->id = -1; vm->state = VIR_DOMAIN_SHUTOFF; + dom->id = -1; ret = 0; cleanup: diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index e4afc5a..7143933 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3688,6 +3688,8 @@ phypDomainDestroy(virDomainPtr dom) if (phypUUIDTable_RemLpar(dom->conn, dom->id) == -1) goto err; + dom->id = -1; + VIR_FREE(cmd); VIR_FREE(ret); return 0; diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 0a91c7f..31fec67 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -1587,6 +1587,7 @@ static int vboxDomainDestroy(virDomainPtr dom) { } #endif VBOX_RELEASE(console); + dom->id = -1; ret = 0; } data->vboxSession->vtbl->Close(data->vboxSession); diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 2262cef..fb3c91d 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -830,6 +830,7 @@ xenapiDomainDestroy (virDomainPtr dom) return -1; } xen_vm_set_free(vms); + dom->id = -1; return 0; } if (vms) xen_vm_set_free(vms); -- 1.7.0.4

2 2

[libvirt] [PATCH] openvzDomainCreateWithFlags: set domain id to the correct value
by Jean-Baptiste Rouault 30 Jul '10

30 Jul '10

I'm not sure if it's ok or not to modify dom->id as I did here. Regards, Jean-Baptiste

2 2

[libvirt] [PATCH] storage: kill dead stores
by Eric Blake 30 Jul '10

30 Jul '10

Found by clang. Clang complained that virStorageBackendProbeTarget could dereference NULL if backingStoreFormat was NULL, but since all callers passed a valid pointer, I added attributes instead of null checks. * src/storage/storage_backend.c (virStorageBackendQEMUImgBackingFormat): Kill dead store. * src/storage/storage_backend_fs.c (virStorageBackendProbeTarget): Likewise. Skip null checks, by adding attributes. --- Thankfully, the null dereference scenario noted by clang was never triggered in the code, which is good since it was introduced as part of fixing a CVE. src/storage/storage_backend.c | 3 +-- src/storage/storage_backend_fs.c | 34 +++++++++++++++------------------- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index d989743..1fe7ba6 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -581,7 +581,6 @@ static int virStorageBackendQEMUImgBackingFormat(const char *qemuimg) int newstdout = -1; char *help = NULL; enum { MAX_HELP_OUTPUT_SIZE = 1024*8 }; - int len; char *start; char *end; char *tmp; @@ -591,7 +590,7 @@ static int virStorageBackendQEMUImgBackingFormat(const char *qemuimg) &child, -1, &newstdout, NULL, VIR_EXEC_CLEAR_CAPS) < 0) goto cleanup; - if ((len = virFileReadLimFD(newstdout, MAX_HELP_OUTPUT_SIZE, &help)) < 0) { + if (virFileReadLimFD(newstdout, MAX_HELP_OUTPUT_SIZE, &help) < 0) { virReportSystemError(errno, _("Unable to read '%s -h' output"), qemuimg); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 0761bd8..b6b8fdd 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -48,7 +48,7 @@ #define VIR_FROM_THIS VIR_FROM_STORAGE -static int +static int ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) virStorageBackendProbeTarget(virStorageVolTargetPtr target, char **backingStore, int *backingStoreFormat, @@ -59,10 +59,8 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target, int fd, ret; virStorageFileMetadata meta; - if (backingStore) - *backingStore = NULL; - if (backingStoreFormat) - *backingStoreFormat = VIR_STORAGE_FILE_AUTO; + *backingStore = NULL; + *backingStoreFormat = VIR_STORAGE_FILE_AUTO; if (encryption) *encryption = NULL; @@ -75,7 +73,7 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target, allocation, capacity)) < 0) { close(fd); - return -1; + return ret; } memset(&meta, 0, sizeof(meta)); @@ -95,20 +93,19 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target, close(fd); if (meta.backingStore) { - if (backingStore) { - *backingStore = meta.backingStore; - meta.backingStore = NULL; - if (meta.backingStoreFormat == VIR_STORAGE_FILE_AUTO) { - if ((*backingStoreFormat = virStorageFileProbeFormat(*backingStore)) < 0) { - close(fd); - goto cleanup; - } - } else { - *backingStoreFormat = meta.backingStoreFormat; + *backingStore = meta.backingStore; + meta.backingStore = NULL; + if (meta.backingStoreFormat == VIR_STORAGE_FILE_AUTO) { + if ((*backingStoreFormat + = virStorageFileProbeFormat(*backingStore)) < 0) { + close(fd); + goto cleanup; } } else { - VIR_FREE(meta.backingStore); + *backingStoreFormat = meta.backingStoreFormat; } + } else { + VIR_FREE(meta.backingStore); } if (capacity && meta.capacity) @@ -139,8 +136,7 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target, return 0; cleanup: - if (backingStore) - VIR_FREE(*backingStore); + VIR_FREE(*backingStore); return -1; } -- 1.7.2

2 2

[libvirt] [PATCH] network: kill dead store
by Eric Blake 30 Jul '10

30 Jul '10

* src/network/bridge_driver.c (networkDefine): Kill dead store. --- src/network/bridge_driver.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index b7ee880..217cdcc 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1490,14 +1490,13 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { virNetworkDefPtr def; virNetworkObjPtr network = NULL; virNetworkPtr ret = NULL; - int dupNet; networkDriverLock(driver); if (!(def = virNetworkDefParseString(xml))) goto cleanup; - if ((dupNet = virNetworkObjIsDuplicate(&driver->networks, def, 0)) < 0) + if (virNetworkObjIsDuplicate(&driver->networks, def, 0) < 0) goto cleanup; if (virNetworkSetBridgeName(&driver->networks, def, 1)) -- 1.7.2

2 2

[libvirt] [PATCH] xenapi: Update ID after starting a domain
by Matthias Bolte 30 Jul '10

30 Jul '10

--- src/xenapi/xenapi_driver.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index e385648..2262cef 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1459,6 +1459,7 @@ xenapiDomainCreateWithFlags (virDomainPtr dom, unsigned int flags) xen_vm_set *vms; xen_vm vm; xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session; + int64_t domid = -1; virCheckFlags(0, -1); @@ -1475,6 +1476,10 @@ xenapiDomainCreateWithFlags (virDomainPtr dom, unsigned int flags) xen_vm_set_free(vms); return -1; } + + xen_vm_get_domid(session, &domid, vm); + dom->id = domid; + xen_vm_set_free(vms); } else { if (vms) xen_vm_set_free(vms); -- 1.7.0.4

2 2

[libvirt] [PATCH] esx: Update ID after starting a domain
by Matthias Bolte 30 Jul '10

30 Jul '10

--- src/esx/esx_driver.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index eb64556..964a3a5 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2483,6 +2483,7 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) esxVI_ObjectContent *virtualMachine = NULL; esxVI_String *propertyNameList = NULL; esxVI_VirtualMachinePowerState powerState; + int id = -1; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; @@ -2497,8 +2498,8 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) esxVI_LookupVirtualMachineByUuidAndPrepareForTask (priv->primary, domain->uuid, propertyNameList, &virtualMachine, priv->autoAnswer) < 0 || - esxVI_GetVirtualMachinePowerState(virtualMachine, - &powerState) < 0) { + esxVI_GetVirtualMachinePowerState(virtualMachine, &powerState) < 0 || + esxVI_GetVirtualMachineIdentity(virtualMachine, &id, NULL, NULL) < 0) { goto cleanup; } @@ -2521,6 +2522,7 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) goto cleanup; } + domain->id = id; result = 0; cleanup: -- 1.7.0.4

2 2

[libvirt] [PATCH] Fix DMI uuid parsing.
by Chris Lalancette 30 Jul '10

30 Jul '10

valgrind was complaining that virUUIDParse was depending on an uninitialized value. Indeed it was; virSetHostUUIDStr() didn't initialize the dmiuuid buffer to 0's, meaning that anything after the string read from /sys was uninitialized. Clear out the dmiuuid buffer before use, and make sure to always leave a \0 at the end. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/util/uuid.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/util/uuid.c b/src/util/uuid.c index f188148..9cafc2a 100644 --- a/src/util/uuid.c +++ b/src/util/uuid.c @@ -286,7 +286,8 @@ virSetHostUUIDStr(const char *uuid) return EEXIST; if (!uuid) { - if (!getDMISystemUUID(dmiuuid, sizeof(dmiuuid))) { + memset(dmiuuid, 0, sizeof(dmiuuid)); + if (!getDMISystemUUID(dmiuuid, sizeof(dmiuuid) - 1)) { if (!virUUIDParse(dmiuuid, host_uuid)) return 0; } -- 1.7.1.1

2 2

[libvirt] [PATCH] Do not activate boot=on on devices when not using KVM
by Daniel Veillard 30 Jul '10

30 Jul '10

Basically the 'boot=on' boot selection device is something present in KVM but not in upstream QEmu, as a result if we boot a QEmu domain without KVM acceleration we must disable boot=on ... even if the front end kvm binary expose that capability in the help page. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2 2

[libvirt] [PATCH] build: distribute libvirt_qemu.syms
by Eric Blake 30 Jul '10

30 Jul '10

* src/Makefile.am (EXTRA_DIST): Ensure 'make distcheck' and 'rpmbuild' can reproduce a build. * daemon/Makefile.am (DAEMON_SOURCES): Likewise. --- The recent qemu monitor commands were incomplete. daemon/Makefile.am | 4 ++++ src/Makefile.am | 4 +++- 2 files changed, 7 insertions(+), 1 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 6769bab..963d64f 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -10,6 +10,10 @@ DAEMON_SOURCES = \ remote_dispatch_table.h \ remote_dispatch_args.h \ remote_dispatch_ret.h \ + qemu_dispatch_prototypes.h \ + qemu_dispatch_table.h \ + qemu_dispatch_args.h \ + qemu_dispatch_ret.h \ ../src/remote/remote_protocol.c \ ../src/remote/qemu_protocol.c diff --git a/src/Makefile.am b/src/Makefile.am index 64d618b..a66eb2a 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -162,7 +162,8 @@ REMOTE_DRIVER_SOURCES = \ remote/qemu_protocol.c \ remote/qemu_protocol.h -EXTRA_DIST += remote/remote_protocol.x remote/rpcgen_fix.pl +EXTRA_DIST += remote/remote_protocol.x remote/qemu_protocol.x \ + remote/rpcgen_fix.pl # Ensure that we don't change the struct or member names or member ordering # in remote_protocol.x The embedded perl below needs a few comments, and @@ -1074,6 +1075,7 @@ libvirt_qemu_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_QEMU_SYMBOL_FILE) \ $(AM_LDFLAGS) libvirt_qemu_la_CFLAGS = $(AM_CFLAGS) libvirt_qemu_la_LIBADD = libvirt.la $(CYGWIN_EXTRA_LIBADD) +EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE) libexec_PROGRAMS = -- 1.7.2

3 3

[libvirt] [PATCH] esx: Switch from name to number checks in the subdrivers
by Matthias Bolte 30 Jul '10

30 Jul '10

--- src/esx/esx_device_monitor.c | 2 +- src/esx/esx_interface_driver.c | 2 +- src/esx/esx_network_driver.c | 2 +- src/esx/esx_nwfilter_driver.c | 2 +- src/esx/esx_secret_driver.c | 2 +- src/esx/esx_storage_driver.c | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/esx/esx_device_monitor.c b/src/esx/esx_device_monitor.c index 5887ba0..d559f96 100644 --- a/src/esx/esx_device_monitor.c +++ b/src/esx/esx_device_monitor.c @@ -44,7 +44,7 @@ esxDeviceOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { - if (STRNEQ(conn->driver->name, "ESX")) { + if (conn->driver->no != VIR_DRV_ESX) { return VIR_DRV_OPEN_DECLINED; } diff --git a/src/esx/esx_interface_driver.c b/src/esx/esx_interface_driver.c index d58e365..4bac3d5 100644 --- a/src/esx/esx_interface_driver.c +++ b/src/esx/esx_interface_driver.c @@ -44,7 +44,7 @@ esxInterfaceOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { - if (STRNEQ(conn->driver->name, "ESX")) { + if (conn->driver->no != VIR_DRV_ESX) { return VIR_DRV_OPEN_DECLINED; } diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index 52f5e65..a64bb8e 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -44,7 +44,7 @@ esxNetworkOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { - if (STRNEQ(conn->driver->name, "ESX")) { + if (conn->driver->no != VIR_DRV_ESX) { return VIR_DRV_OPEN_DECLINED; } diff --git a/src/esx/esx_nwfilter_driver.c b/src/esx/esx_nwfilter_driver.c index cc2e73b..a9d046d 100644 --- a/src/esx/esx_nwfilter_driver.c +++ b/src/esx/esx_nwfilter_driver.c @@ -43,7 +43,7 @@ esxNWFilterOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { - if (STRNEQ(conn->driver->name, "ESX")) { + if (conn->driver->no != VIR_DRV_ESX) { return VIR_DRV_OPEN_DECLINED; } diff --git a/src/esx/esx_secret_driver.c b/src/esx/esx_secret_driver.c index 2d54b3f..1ae7ddc 100644 --- a/src/esx/esx_secret_driver.c +++ b/src/esx/esx_secret_driver.c @@ -42,7 +42,7 @@ static virDrvOpenStatus esxSecretOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { - if (STRNEQ(conn->driver->name, "ESX")) { + if (conn->driver->no != VIR_DRV_ESX) { return VIR_DRV_OPEN_DECLINED; } diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index 44d7d85..9f25e02 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -45,7 +45,7 @@ esxStorageOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { - if (STRNEQ(conn->driver->name, "ESX")) { + if (conn->driver->no != VIR_DRV_ESX) { return VIR_DRV_OPEN_DECLINED; } -- 1.7.0.4

2 2

[libvirt] [PATCH] esx: Improve blocked task detection and fix race condition
by Matthias Bolte 30 Jul '10

30 Jul '10

esxVI_WaitForTaskCompletion can take a UUID to lookup the corresponding domain and check if the current task for it is blocked by a question. It calls another function to do this: esxVI_LookupAndHandleVirtualMachineQuestion looks up the VirtualMachine and checks for a question. If there is a question it calls esxVI_HandleVirtualMachineQuestion to handle it. If there was no question or it has been answered the call to esxVI_LookupAndHandleVirtualMachineQuestion returns 0. If any error occurred during the lookup and answering process -1 is returned. The problem with this is, that -1 is also returned when there was no error but the question could not be answered. So esxVI_WaitForTaskCompletion cannot distinguish between this two situations and reports that a question is blocking the task even when there was actually another problem. This inherent problem didn't surface until vSphere 4.1 when you try to define a new domain. The driver tries to lookup the domain that is just in the process of being registered. There seems to be some kind of race condition and the driver manages to issue a lookup command before the ESX server was able to register the domain. This used to work before. Due to the return value problem described above the driver reported a false error message in that case. To solve this esxVI_WaitForTaskCompletion now takes an additional occurrence parameter that describes whether or not to expect the domain to be existent. Also add a new parameter to esxVI_LookupAndHandleVirtualMachineQuestion that allows to distinguish if the call returned -1 because of an actual error or because the question could not be answered. --- src/esx/esx_driver.c | 17 ++++++++++++++- src/esx/esx_vi.c | 53 ++++++++++++++++++++++++++++++++++++------------- src/esx/esx_vi.h | 7 ++++- src/esx/esx_vmx.c | 4 +- 4 files changed, 61 insertions(+), 20 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index f9affea..eb64556 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1409,6 +1409,7 @@ 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) { goto cleanup; } @@ -1462,6 +1463,7 @@ esxDomainResume(virDomainPtr domain) if (esxVI_PowerOnVM_Task(priv->primary, virtualMachine->obj, NULL, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, + esxVI_Occurrence_RequiredItem, priv->autoAnswer, &taskInfoState) < 0) { goto cleanup; } @@ -1607,8 +1609,9 @@ esxDomainDestroy(virDomainPtr domain) } if (esxVI_PowerOffVM_Task(ctx, virtualMachine->obj, &task) < 0 || - esxVI_WaitForTaskCompletion(ctx, task, domain->uuid, priv->autoAnswer, - &taskInfoState) < 0) { + esxVI_WaitForTaskCompletion(ctx, task, domain->uuid, + esxVI_Occurrence_RequiredItem, + priv->autoAnswer, &taskInfoState) < 0) { goto cleanup; } @@ -1724,6 +1727,7 @@ esxDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) if (esxVI_ReconfigVM_Task(priv->primary, virtualMachine->obj, spec, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, + esxVI_Occurrence_RequiredItem, priv->autoAnswer, &taskInfoState) < 0) { goto cleanup; } @@ -1775,6 +1779,7 @@ esxDomainSetMemory(virDomainPtr domain, unsigned long memory) if (esxVI_ReconfigVM_Task(priv->primary, virtualMachine->obj, spec, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, + esxVI_Occurrence_RequiredItem, priv->autoAnswer, &taskInfoState) < 0) { goto cleanup; } @@ -2104,6 +2109,7 @@ esxDomainSetVcpus(virDomainPtr domain, unsigned int nvcpus) if (esxVI_ReconfigVM_Task(priv->primary, virtualMachine->obj, spec, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, + esxVI_Occurrence_RequiredItem, priv->autoAnswer, &taskInfoState) < 0) { goto cleanup; } @@ -2505,6 +2511,7 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) if (esxVI_PowerOnVM_Task(priv->primary, virtualMachine->obj, NULL, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, + esxVI_Occurrence_RequiredItem, priv->autoAnswer, &taskInfoState) < 0) { goto cleanup; } @@ -2702,6 +2709,7 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml ATTRIBUTE_UNUSED) datastoreRelatedPath, NULL, esxVI_Boolean_False, resourcePool, hostSystem->obj, &task) < 0 || esxVI_WaitForTaskCompletion(priv->host, task, def->uuid, + esxVI_Occurrence_OptionalItem, priv->autoAnswer, &taskInfoState) < 0) { goto cleanup; } @@ -3069,6 +3077,7 @@ esxDomainSetSchedulerParameters(virDomainPtr domain, if (esxVI_ReconfigVM_Task(priv->primary, virtualMachine->obj, spec, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, + esxVI_Occurrence_RequiredItem, priv->autoAnswer, &taskInfoState) < 0) { goto cleanup; } @@ -3223,6 +3232,7 @@ esxDomainMigratePerform(virDomainPtr domain, esxVI_VirtualMachinePowerState_Undefined, &task) < 0 || esxVI_WaitForTaskCompletion(priv->vCenter, task, domain->uuid, + esxVI_Occurrence_RequiredItem, priv->autoAnswer, &taskInfoState) < 0) { goto cleanup; } @@ -3465,6 +3475,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, esxVI_Boolean_True, esxVI_Boolean_False, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, + esxVI_Occurrence_RequiredItem, priv->autoAnswer, &taskInfoState) < 0) { goto cleanup; } @@ -3721,6 +3732,7 @@ esxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) if (esxVI_RevertToSnapshot_Task(priv->primary, snapshotTree->snapshot, NULL, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, snapshot->domain->uuid, + esxVI_Occurrence_RequiredItem, priv->autoAnswer, &taskInfoState) < 0) { goto cleanup; } @@ -3775,6 +3787,7 @@ esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) if (esxVI_RemoveSnapshot_Task(priv->primary, snapshotTree->snapshot, removeChildren, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, snapshot->domain->uuid, + esxVI_Occurrence_RequiredItem, priv->autoAnswer, &taskInfoState) < 0) { goto cleanup; } diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 69ba7a9..98b8bcd 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -2217,6 +2217,7 @@ esxVI_LookupVirtualMachineByUuidAndPrepareForTask esxVI_String *completePropertyNameList = NULL; esxVI_VirtualMachineQuestionInfo *questionInfo = NULL; esxVI_TaskInfo *pendingTaskInfoList = NULL; + esxVI_Boolean blocked = esxVI_Boolean_Undefined; if (esxVI_String_DeepCopyList(&completePropertyNameList, propertyNameList) < 0 || @@ -2235,7 +2236,8 @@ esxVI_LookupVirtualMachineByUuidAndPrepareForTask if (questionInfo != NULL && esxVI_HandleVirtualMachineQuestion(ctx, (*virtualMachine)->obj, - questionInfo, autoAnswer) < 0) { + questionInfo, autoAnswer, + &blocked) < 0) { goto cleanup; } @@ -2528,7 +2530,9 @@ esxVI_LookupPendingTaskInfoListByVirtualMachine int esxVI_LookupAndHandleVirtualMachineQuestion(esxVI_Context *ctx, const unsigned char *uuid, - esxVI_Boolean autoAnswer) + esxVI_Occurrence occurrence, + esxVI_Boolean autoAnswer, + esxVI_Boolean *blocked) { int result = -1; esxVI_ObjectContent *virtualMachine = NULL; @@ -2538,17 +2542,22 @@ esxVI_LookupAndHandleVirtualMachineQuestion(esxVI_Context *ctx, if (esxVI_String_AppendValueToList(&propertyNameList, "runtime.question") < 0 || esxVI_LookupVirtualMachineByUuid(ctx, uuid, propertyNameList, - &virtualMachine, - esxVI_Occurrence_RequiredItem) < 0 || - esxVI_GetVirtualMachineQuestionInfo(virtualMachine, - &questionInfo) < 0) { + &virtualMachine, occurrence) < 0) { goto cleanup; } - if (questionInfo != NULL && - esxVI_HandleVirtualMachineQuestion(ctx, virtualMachine->obj, - questionInfo, autoAnswer) < 0) { - goto cleanup; + if (virtualMachine != NULL) { + if (esxVI_GetVirtualMachineQuestionInfo(virtualMachine, + &questionInfo) < 0) { + goto cleanup; + } + + if (questionInfo != NULL && + esxVI_HandleVirtualMachineQuestion(ctx, virtualMachine->obj, + questionInfo, autoAnswer, + blocked) < 0) { + goto cleanup; + } } result = 0; @@ -2700,7 +2709,7 @@ int esxVI_HandleVirtualMachineQuestion (esxVI_Context *ctx, esxVI_ManagedObjectReference *virtualMachine, esxVI_VirtualMachineQuestionInfo *questionInfo, - esxVI_Boolean autoAnswer) + esxVI_Boolean autoAnswer, esxVI_Boolean *blocked) { int result = -1; esxVI_ElementDescription *elementDescription = NULL; @@ -2709,6 +2718,13 @@ esxVI_HandleVirtualMachineQuestion int answerIndex = 0; char *possibleAnswers = NULL; + if (blocked == NULL || *blocked != esxVI_Boolean_Undefined) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + *blocked = esxVI_Boolean_False; + if (questionInfo->choice->choiceInfo != NULL) { for (elementDescription = questionInfo->choice->choiceInfo; elementDescription != NULL; @@ -2742,6 +2758,8 @@ esxVI_HandleVirtualMachineQuestion _("Pending question blocks virtual machine execution, " "question is '%s', no possible answers"), questionInfo->text); + + *blocked = esxVI_Boolean_True; goto cleanup; } else if (answerChoice == NULL) { ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, @@ -2749,6 +2767,8 @@ esxVI_HandleVirtualMachineQuestion "question is '%s', possible answers are %s, but no " "default answer is specified"), questionInfo->text, possibleAnswers); + + *blocked = esxVI_Boolean_True; goto cleanup; } @@ -2774,6 +2794,7 @@ esxVI_HandleVirtualMachineQuestion questionInfo->text); } + *blocked = esxVI_Boolean_True; goto cleanup; } @@ -2795,6 +2816,7 @@ int esxVI_WaitForTaskCompletion(esxVI_Context *ctx, esxVI_ManagedObjectReference *task, const unsigned char *virtualMachineUuid, + esxVI_Occurrence virtualMachineOccurrence, esxVI_Boolean autoAnswer, esxVI_TaskInfoState *finalState) { @@ -2810,6 +2832,7 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, esxVI_PropertyChange *propertyChange = NULL; esxVI_AnyType *propertyValue = NULL; esxVI_TaskInfoState state = esxVI_TaskInfoState_Undefined; + esxVI_Boolean blocked = esxVI_Boolean_Undefined; esxVI_TaskInfo *taskInfo = NULL; version = strdup(""); @@ -2850,7 +2873,8 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, if (virtualMachineUuid != NULL) { if (esxVI_LookupAndHandleVirtualMachineQuestion - (ctx, virtualMachineUuid, autoAnswer) < 0) { + (ctx, virtualMachineUuid, virtualMachineOccurrence, + autoAnswer, &blocked) < 0) { /* * FIXME: Disable error reporting here, so possible errors from * esxVI_LookupTaskInfoByTask() and esxVI_CancelTask() @@ -2861,12 +2885,13 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, } if (taskInfo->cancelable == esxVI_Boolean_True) { - if (esxVI_CancelTask(ctx, task) < 0) { + if (esxVI_CancelTask(ctx, task) < 0 && + blocked == esxVI_Boolean_True) { VIR_ERROR0(_("Cancelable task is blocked by an " "unanswered question but cancelation " "failed")); } - } else { + } else if (blocked == esxVI_Boolean_True) { VIR_ERROR0(_("Non-cancelable task is blocked by an " "unanswered question")); } diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index 9eb5094..ef9c6d9 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -367,7 +367,9 @@ int esxVI_LookupPendingTaskInfoListByVirtualMachine int esxVI_LookupAndHandleVirtualMachineQuestion(esxVI_Context *ctx, const unsigned char *uuid, - esxVI_Boolean autoAnswer); + esxVI_Occurrence occurrence, + esxVI_Boolean autoAnswer, + esxVI_Boolean *blocked); int esxVI_LookupRootSnapshotTreeList (esxVI_Context *ctx, const unsigned char *virtualMachineUuid, @@ -382,11 +384,12 @@ int esxVI_HandleVirtualMachineQuestion (esxVI_Context *ctx, esxVI_ManagedObjectReference *virtualMachine, esxVI_VirtualMachineQuestionInfo *questionInfo, - esxVI_Boolean autoAnswer); + esxVI_Boolean autoAnswer, esxVI_Boolean *blocked); int esxVI_WaitForTaskCompletion(esxVI_Context *ctx, esxVI_ManagedObjectReference *task, const unsigned char *virtualMachineUuid, + esxVI_Occurrence virtualMachineOccurrence, esxVI_Boolean autoAnswer, esxVI_TaskInfoState *finalState); diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c index c63b159..ee7afb2 100644 --- a/src/esx/esx_vmx.c +++ b/src/esx/esx_vmx.c @@ -811,8 +811,8 @@ esxVMX_AutodetectSCSIControllerModel(esxVI_Context *ctx, /* Search datastore for file */ if (esxVI_SearchDatastore_Task(ctx, hostDatastoreBrowser, datastorePath, searchSpec, &task) < 0 || - esxVI_WaitForTaskCompletion(ctx, task, NULL, esxVI_Boolean_False, - &taskInfoState) < 0) { + esxVI_WaitForTaskCompletion(ctx, task, NULL, esxVI_Occurrence_None, + esxVI_Boolean_False, &taskInfoState) < 0) { goto cleanup; } -- 1.7.0.4

2 2

[libvirt] [PATCH] esx: Fix freeing of heterogeneous lists
by Matthias Bolte 30 Jul '10

30 Jul '10

Always call the free function of the base type. The base type function then dynamically dispatches the call to the free function for the actual type. --- src/esx/esx_vi_generator.py | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 82bc9b6..44c3493 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -684,7 +684,10 @@ class Object: source += "{\n" if self.features & Object.FEATURE__LIST: - source += " esxVI_%s_Free(&item->_next);\n\n" % self.name + if self.extends is not None: + source += " esxVI_%s_Free((esxVI_%s **)&item->_next);\n\n" % (self.extends, self.extends) + else: + source += " esxVI_%s_Free(&item->_next);\n\n" % self.name source += self.generate_free_code() @@ -701,7 +704,10 @@ class Object: source += "{\n" if self.features & Object.FEATURE__LIST: - source += " esxVI_%s_Free(&item->_next);\n\n" % self.name + if self.extends is not None: + source += " esxVI_%s_Free((esxVI_%s **)&item->_next);\n\n" % (self.extends, self.extends) + else: + source += " esxVI_%s_Free(&item->_next);\n\n" % self.name source += self.generate_free_code() @@ -1126,11 +1132,11 @@ additional_object_features = { "DatastoreInfo" : Object.FEATURE__AN "FileInfo" : Object.FEATURE__DYNAMIC_CAST, "FileQuery" : Object.FEATURE__DYNAMIC_CAST, "HostCpuIdInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__LIST, - "HostDatastoreBrowserSearchResults" : Object.FEATURE__ANY_TYPE, + "HostDatastoreBrowserSearchResults" : Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE, "ManagedObjectReference" : Object.FEATURE__ANY_TYPE, "ObjectContent" : Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST, "PerfCounterInfo" : Object.FEATURE__LIST, - "PerfEntityMetric" : Object.FEATURE__LIST | Object.FEATURE__DYNAMIC_CAST, + "PerfEntityMetric" : Object.FEATURE__LIST | Object.FEATURE__DYNAMIC_CAST, "PerfQuerySpec" : Object.FEATURE__LIST, "PerfMetricIntSeries" : Object.FEATURE__DYNAMIC_CAST, "PropertyFilterSpec" : Object.FEATURE__LIST, -- 1.7.0.4

2 2

[libvirt] [PATCH 0/2] build: fix bugs detected by newer gcc
by Eric Blake 30 Jul '10

30 Jul '10

Rawhide's gcc 4.5.0 got smarter in relation to Fedora 13's gcc 4.4.4, and exposed a real bug (introduced by commit ed9c14a7 this January) as well as extra noise that is easy enough to silence. The new warnings looked like: esx/esx_vi_types.c:1231:1: warning: logical 'or' of collectively exhaustive tests is always true [-Wlogical-op] And the esx_vi_types.c case is proof why it is a bad idea to ever stick -Werror in a spec file - upgrade the compiler, and something that previously built silently can now fail to build; in contrast, using -Werror during development is great as it forces you to think about the issue, and in the case of xen, fix a real bug. Eric Blake (2): xen: fix logic bug esx: silence spurious compiler warning src/esx/esx_vi_types.c | 6 ++++-- src/xen/xend_internal.c | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) -- 1.7.2

3 7

[libvirt] "make rpm" failure?
by Justin Clift 30 Jul '10

30 Jul '10

Hi Eric, Have you tried a "make rpm" with libvirt git head since your recent "build: fix VPATH builds" commit? commit 4018a026b2b4b50bfc5e4c35dabb828ebf6cc3f7 Author: Eric Blake <eblake(a)redhat.com> Date: Mon Jul 26 10:38:30 2010 -0600 build: fix VPATH builds After the recent libvirt-qemu library addition, VPATH builds fail ... The commit comment indicates it should fix an error where ld can't find "libvirt_qemu.syms". I'm still getting that the error though: $ make rpm ... CCLD libvirt-qemu.la /usr/bin/ld: cannot open linker script file ./libvirt_qemu.syms: No such file or directory collect2: ld returned 1 exit status make[4]: *** [libvirt-qemu.la] Error 1 make[4]: *** Waiting for unfinished jobs.... make[4]: Leaving directory `/home/jc/rpmbuild/BUILD/libvirt-0.8.2/src' make[3]: *** [all] Error 2 make[3]: Leaving directory `/home/jc/rpmbuild/BUILD/libvirt-0.8.2/src' make[2]: *** [all-recursive] Error 1 make[2]: Leaving directory `/home/jc/rpmbuild/BUILD/libvirt-0.8.2' make[1]: *** [all] Error 2 make[1]: Leaving directory `/home/jc/rpmbuild/BUILD/libvirt-0.8.2' error: Bad exit status from /var/tmp/rpm-tmp.xiUlH6 (%build) In configure.ac, where it's defined as: LIBVIRT_QEMU_SYMBOL_FILE='$(srcdir)/libvirt_qemu.syms' The $srcdir seems to be getting turned into a ".", so that this happens: [jc@host1 libvirt]$ grep -r "libvirt_qemu.syms" . ./gnulib/lib/Makefile:LIBVIRT_QEMU_SYMBOL_FILE = ./libvirt_qemu.syms ./gnulib/tests/Makefile:LIBVIRT_QEMU_SYMBOL_FILE = ./libvirt_qemu.syms ./src/Makefile:LIBVIRT_QEMU_SYMBOL_FILE = ./libvirt_qemu.syms ./tests/xencapsdata/Makefile:LIBVIRT_QEMU_SYMBOL_FILE = ./libvirt_qemu.syms ./tests/xml2sexprdata/Makefile:LIBVIRT_QEMU_SYMBOL_FILE = ./libvirt_qemu.syms ./tests/sexpr2xmldata/Makefile:LIBVIRT_QEMU_SYMBOL_FILE = ./libvirt_qemu.syms ./tests/xmconfigdata/Makefile:LIBVIRT_QEMU_SYMBOL_FILE = ./libvirt_qemu.syms ./tests/Makefile:LIBVIRT_QEMU_SYMBOL_FILE = ./libvirt_qemu.syms ./tests/confdata/Makefile:LIBVIRT_QEMU_SYMBOL_FILE = ./libvirt_qemu.syms ./include/Makefile:LIBVIRT_QEMU_SYMBOL_FILE = ./libvirt_qemu.syms ./include/libvirt/Makefile:LIBVIRT_QEMU_SYMBOL_FILE = ./libvirt_qemu.syms ./Makefile:LIBVIRT_QEMU_SYMBOL_FILE = ./libvirt_qemu.syms ./proxy/Makefile:LIBVIRT_QEMU_SYMBOL_FILE = ./libvirt_qemu.syms ./python/tests/Makefile:LIBVIRT_QEMU_SYMBOL_FILE = ./libvirt_qemu.syms It looks like the $srcdir needs to be resolved to an absolute path, rather than a relative one, but I have no idea how in autoconf stuff. Any ideas off the top of your head? :) Regards and best wishes, Justin Clift

2 1