- Devel - libvirt List Archives

[libvirt] QMP Capability Detection Issues with GIT version of QEMU
by Viktor Mihajlovski 23 Nov '12

23 Nov '12

I ran into trouble today trying to use the GIT level of QEMU. In a nutshell: the capability detection with QMP is failing and the fallback using -help isn't working with the GIT level probably due to help text reformatting. The failure reason is that QEMU cannot bind to the QMP monitor socket in the /var/lib/libvirt/qemu directory. That's because the child process is stripped of all capabilities and this directory is chown'ed to qemu:qemu by the QEMU driver. Note that this is failing with the release QEMU as well, with the difference that the fallback is working there. I am willing to provide a patch, however I'd like to get feedback on the approach to use: 1. Add back Linux capabilities CAP_DAC_OVERRIDE, CAP_DAC_READ_SEARCH to allow QEMU to bind to the monitor socket. This seems to be hacky/dodging the problem. 2. Use a separate directory for the QMP probing instance of QEMU. 3. Run the QMP QEMU under the configured qemu user. This would be my favorite. 4. Other ideas? -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

2 3

[libvirt] [PATCH] Correct include-password option for domdisplay
by Martin Kletzander 23 Nov '12

23 Nov '12

The 'virsh domdisplay' command is able to display the password configured for spice, but it was missing for vnc type graphics. This is just a simple patch for that to work properly. --- tools/virsh-domain.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cc47383..18aa869 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7073,6 +7073,23 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) if (STREQ(scheme[iter], "vnc")) { /* VNC protocol handlers take their port number as 'port' - 5900 */ port -= 5900; + + if (vshCommandOptBool(cmd, "include-password")) { + /* Create our XPATH lookup for the SPICE password */ + virAsprintf(&xpath, + "string(/domain/devices/graphics" + "[@type='%s']/@passwd)", + scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the SPICE password */ + passwd = virXPathString(xpath, ctxt); + VIR_FREE(xpath); + } + } else if (STREQ(scheme[iter], "spice")) { /* Create our XPATH lookup for the SPICE TLS Port */ virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" -- 1.8.0

5 10

[libvirt] FYI: Upgrade from pc-0.12 / pc-014 vs. savevm section "kvmclock"
by Philipp Hahn 23 Nov '12

23 Nov '12

Hello, I'm using libvirt to manage my KVM instances. I created a VM with qemu-kvm-0.12, later upgraded to qemu-kvm-0.14 and took a snapshot using libvirt. As the original VM was created with qemu-kvm-0.12, libvirt stored "pc-0.12" with its XML data. Now I upgraded to qemu-kvm-1.1.2, where reverting to that snapshot fails with a the message > Unknown savevm section or instance 'kvmclock' 0 > load of migration failed I tracked that down to hw/pc_piix.c:590 where > static QEMUMachine pc_machine_v0_12 = { > .name = "pc-0.12", > .desc = "Standard PC", > .init = pc_init_pci_no_kvmclock, is defined. If I change .init to pc_init_pci, I'm able to load the old snapshot. I think this is because kvm-0.14 always created the kvmclock device, while that was only later changed (0ec329da) to be created "on demand" for pc-0.14s onward. So the snapshot is no longer a pure pc-0.12, but some pc-0.12+something, which qemu-kvm-1.1.2 refuses to load. Now that I know what the problem is I just want to inform others, who might experience the same problem. My workaround is a patched qemu-kvm, where I changed that .init mentioned above, since an additional enabled kvmclock does not seem to do any harm. If someone has a better fix, I'm open to suggestions. Sincerely Philipp -- Philipp Hahn Open Source Software Engineer hahn(a)univention.de Univention GmbH be open. fon: +49 421 22 232- 0 Mary-Somerville-Str.1 D-28359 Bremen fax: +49 421 22 232-99 http://www.univention.de/

1 0

[libvirt] [PATCH] node_device: Fix the improper logic when enumerating SRIOV VF
by Osier Yang 23 Nov '12

23 Nov '12

pciGetVirtualFunctions returns 0 even if there is no "virtfn" entry under the device sysfs path. And pciGetVirtualFunctions returns -1 when it tries to get the PCI config space of the VF, however, with keeping the the VFs already detected. That's why udevProcessPCI and gather_pci_cap use logic like: if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions) || data->pci_dev.num_virtual_functions > 0) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; to tag the PCI device with "virtual_function" cap. This results in a VF will aslo get "virtual_function" cap. This patch fixes it by: * Ignoring the VF which has failure of getting PCI config space of the device (given that the succesfully detected VFs are kept , it makes sense to not give up on the failure of one VF too) with a warning, so pciGetVirtualFunctions will always return 0 except out of memory. * Free the allocated *virtual_functions when out of memory And thus the logic can be changed to: /* Out of memory */ if (pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions) < 0) goto out; else if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions) && (data->pci_dev.num_virtual_functions > 0)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; This also fixes a memory leak in the old codes (it realloc the *virtual_functions first, and doesn't shrink it when it fails on getting the PCI config space). --- src/node_device/node_device_hal.c | 13 +++++++++-- src/node_device/node_device_udev.c | 11 +++++++-- src/util/pci.c | 37 +++++++++++++++++++++++------------ 3 files changed, 42 insertions(+), 19 deletions(-) diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index ff73db0..69e6ebf 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -151,10 +151,17 @@ static int gather_pci_cap(LibHalContext *ctx, const char *udi, if (!pciGetPhysicalFunction(sysfs_path, &d->pci_dev.physical_function)) d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; - if (!pciGetVirtualFunctions(sysfs_path, &d->pci_dev.virtual_functions, - &d->pci_dev.num_virtual_functions) || - d->pci_dev.num_virtual_functions > 0) + if (!pciGetVirtualFunctions(sysfs_path, + &d->pci_dev.virtual_functions, + &d->pci_dev.num_virtual_functions) < 0) { + VIR_FREE(sysfs_path); + return -1; + } else if (!pciGetVirtualFunctions(sysfs_path, + &d->pci_dev.virtual_functions, + &d->pci_dev.num_virtual_functions) && + (d->pci_dev.num_virtual_functions > 0)) d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; + } VIR_FREE(sysfs_path); } diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index acd78f2..72b8850 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -484,9 +484,14 @@ static int udevProcessPCI(struct udev_device *device, if (!pciGetPhysicalFunction(syspath, &data->pci_dev.physical_function)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; - if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, - &data->pci_dev.num_virtual_functions) || - data->pci_dev.num_virtual_functions > 0) + /* Out of memory */ + if (pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, + &data->pci_dev.num_virtual_functions) < 0) + goto out; + else if (!pciGetVirtualFunctions(syspath, + &data->pci_dev.virtual_functions, + &data->pci_dev.num_virtual_functions) && + (data->pci_dev.num_virtual_functions > 0)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; ret = 0; diff --git a/src/util/pci.c b/src/util/pci.c index d1ad121..3b4d96a 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1930,6 +1930,7 @@ pciGetVirtualFunctions(const char *sysfs_path, unsigned int *num_virtual_functions) { int ret = -1; + int i; DIR *dir = NULL; struct dirent *entry = NULL; char *device_link = NULL; @@ -1952,6 +1953,7 @@ pciGetVirtualFunctions(const char *sysfs_path, *num_virtual_functions = 0; while ((entry = readdir(dir))) { if (STRPREFIX(entry->d_name, "virtfn")) { + struct pci_config_address *config_addr = NULL; if (virBuildPath(&device_link, sysfs_path, entry->d_name) == -1) { virReportOOMError(); @@ -1960,24 +1962,23 @@ pciGetVirtualFunctions(const char *sysfs_path, VIR_DEBUG("Number of virtual functions: %d", *num_virtual_functions); - if (VIR_REALLOC_N(*virtual_functions, - (*num_virtual_functions) + 1) != 0) { - virReportOOMError(); - VIR_FREE(device_link); - goto out; - } if (pciGetPciConfigAddressFromSysfsDeviceLink(device_link, - &((*virtual_functions)[*num_virtual_functions])) != + &config_addr) != SRIOV_FOUND) { - /* We should not get back SRIOV_NOT_FOUND in this - * case, so if we do, it's an error. */ - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to get SR IOV function from device " - "link '%s'"), device_link); + VIR_WARN("Failed to get SRIOV function from device " + "link '%s'", device_link); VIR_FREE(device_link); - goto out; + continue; } else { + if (VIR_ALLOC_N(*virtual_functions, + *num_virtual_functions + 1) < 0) { + virReportOOMError(); + VIR_FREE(config_addr); + goto out; + } + + (*virtual_functions)[*num_virtual_functions] = config_addr; (*num_virtual_functions)++; } VIR_FREE(device_link); @@ -1985,8 +1986,18 @@ pciGetVirtualFunctions(const char *sysfs_path, } ret = 0; + goto cleanup; out: + if (*virtual_functions) { + for (i = 0; i < *num_virtual_functions; i++) + VIR_FREE((*virtual_functions)[i]); + VIR_FREE(*virtual_functions); + } + +cleanup: + if (device_link) + VIR_FREE(device_link); if (dir) closedir(dir); -- 1.7.7.6

1 0

[libvirt] [PATCH 0/3] IPv6 enhancements plus dnsmasq conf-file
by Gene Czarcinski 23 Nov '12

23 Nov '12

The three patches are submitted with "one" because there is a dependency between them. That is, patch 2 (DHCPv6) assumes that patch 1 (guest-to-guest IPV6) has been applied and patch 3 (dnsmasq conf-file) assumes that patch 2 has been applied. While other files are also changed, most of the changes occur in src/network/bridge_driver.c and result in the conflicts. Yes, patch 1 was previously submitted as a stand-alone patch but it became obvious that it was not really standalone. This submittal is a "re-do" of my previous submittal which involved similar changes. Specifically, the order of the patches have been changed so that the conf-file changes are after the DHCPv6 changes. In addition, some code has been cleaned up. For example, there is no need to modify the radvd configuration since both DHCPv6 and dnsmasq's handling of RA are both keyed to dnsmasq version 2.64 or later. Gene Czarcinski (3): v1: allow guest to guest IPv6 without gateway definition v7.1 add support for DHCPv6 v7.2: put dnsmasq parameters into conf-file docs/formatnetwork.html.in | 126 +++++- src/conf/network_conf.c | 100 +++-- src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 461 +++++++++++++++------ src/network/bridge_driver.h | 7 +- src/util/dnsmasq.c | 9 +- tests/networkxml2argvdata/dhcp6-network.argv | 17 + tests/networkxml2argvdata/dhcp6-network.xml | 16 + tests/networkxml2argvdata/isolated-network.argv | 25 +- tests/networkxml2argvdata/nat-network-dhcp6.argv | 20 + tests/networkxml2argvdata/nat-network-dhcp6.xml | 26 ++ .../networkxml2argvdata/nat-network-dns-hosts.argv | 15 +- .../nat-network-dns-srv-record-minimal.argv | 37 +- .../nat-network-dns-srv-record.argv | 37 +- .../nat-network-dns-txt-record.argv | 31 +- tests/networkxml2argvdata/nat-network.argv | 29 +- tests/networkxml2argvdata/netboot-network.argv | 29 +- .../networkxml2argvdata/netboot-proxy-network.argv | 26 +- .../routed-network-dhcphost.argv | 15 + .../routed-network-dhcphost.xml | 19 + tests/networkxml2argvdata/routed-network.argv | 13 +- tests/networkxml2argvtest.c | 46 +- 22 files changed, 790 insertions(+), 315 deletions(-) create mode 100644 tests/networkxml2argvdata/dhcp6-network.argv create mode 100644 tests/networkxml2argvdata/dhcp6-network.xml create mode 100644 tests/networkxml2argvdata/nat-network-dhcp6.argv create mode 100644 tests/networkxml2argvdata/nat-network-dhcp6.xml create mode 100644 tests/networkxml2argvdata/routed-network-dhcphost.argv create mode 100644 tests/networkxml2argvdata/routed-network-dhcphost.xml -- 1.7.11.7

1 5

[libvirt] [PATCHv2] qemu: Stop recursive detection of image chains when an image is missing
by Peter Krempa 22 Nov '12

22 Nov '12

Commit e0c469e58b93f852a72265919703cb6abd3779f8 that fixes the detection of image chain wasn't complete. Iteration through the backing image chain has to stop at the last existing image if some of the images are missing otherwise the backing chain that is cached contains entries with paths being set to NULL resulting to: error: Unable to allow access for disk path (null): Bad address Fortunately stat() is kind enough not to crash when it's presented with a NULL argument. At least on Linux. --- Left meta->backingStoreRaw in place to mark broken chains, added a comment and a warning. --- src/util/storage_file.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 2249212..4417404 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -728,8 +728,13 @@ virStorageFileGetMetadataFromBuf(int format, meta->backingStore = absolutePathFromBaseFile(path, backing); if (meta->backingStore == NULL) { /* the backing file is (currently) unavailable, treat this - * file as standalone */ + * file as standalone: + * backingStoreRaw is kept to mark broken image chains */ + meta->backingStoreIsFile = false; backingFormat = VIR_STORAGE_FILE_NONE; + VIR_WARN("Backing file '%s' of image '%s' is missing.", + meta->backingStoreRaw, path); + } } VIR_FREE(backing); -- 1.8.0

2 2

[libvirt] [PATCH] build: trivial fix error: implicit declaration of function 'malloc'
by Natanael Copa 22 Nov '12

22 Nov '12

Fixes this error when building with -Werror on Alpine Linux: util/processinfo.c: In function 'virProcessInfoSetAffinity': util/processinfo.c:52:5: error: implicit declaration of function 'malloc' [-Werror=implicit-function-declaration] Signed-off-by: Natanael Copa <ncopa(a)alpinelinux.org> --- src/util/processinfo.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/processinfo.c b/src/util/processinfo.c index c3648d4..b8c60eb 100644 --- a/src/util/processinfo.c +++ b/src/util/processinfo.c @@ -21,6 +21,7 @@ #include <config.h> +#include <stdlib.h> #include <sched.h> #include "processinfo.h" -- 1.8.0

3 5

[libvirt] RFC: Enable unprivileged SG_IO
by Osier Yang 22 Nov '12

22 Nov '12

Hi, http://lwn.net/Articles/524720/ introduces new sysfs knob (unpriv_sgio) for SCSI device to allow the unprivileged SG_IO. I don't have solid thought on how the libvirt interface should be yet. It shouldn't be a XML entry of disk device, as the device can be shared by multiple guests, and the configuration should be kept same for all of them, having a XML entry for it will make things a mess. What Paolo suggested is to add an entry in qemu.conf, just like "cgroup_device_acl": sgio_device_acl = [ "/dev/sda" ] When libvirtd starting, set the sysfs knob "unpriv_sgio" of the devices listed to 1, and 0 when libvirtd exists. I don't quite agree with this approach, as entries in qemu.conf generally should be configuration for the whole qemu driver, however, the SG_IO setting is at the device layer, or not higher than guest layer. What I'm thinking about is to have a public API to tune the knob independantly with domain/driver, that means it's up to management apps to manage the knob's value, setting it to 1 before domain(s) starting, and 0 when no domain is using it. Any thoughts? Regards, Osier

3 4

[libvirt] [PATCH v2] conf: Report sensible error for invalid disk name
by Martin Kletzander 22 Nov '12

22 Nov '12

The error "... but the cause is unknown" appeared for XMLs similar to this: <disk type='file' device='cdrom'> <driver name='qemu' type='raw'/> <source file='/dev/zero'/> <target dev='sr0'/> </disk> Notice unsupported disk type (for the driver), but also no address specified. The first part is not a problem and we should not abort immediately because of that, but the combination with the address unknown was causing an unspecified error. While fixing this, I added an error to one place where this return value was not managed properly. --- v2: - Error moved from virDiskNameToIndex(a)util/util.c to virDomainDiskDefAssignAddress(a)conf/domain_conf.c - One more error added into qemuParseCommandLine(a)qemu/qemu_command.c src/conf/domain_conf.c | 6 +++++- src/qemu/qemu_command.c | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ed21f0f..3a1be02 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3063,8 +3063,12 @@ int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) { int idx = virDiskNameToIndex(def->dst); - if (idx < 0) + if (idx < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Unknown disk name '%s' and no address specified"), + def->dst); return -1; + } switch (def->bus) { case VIR_DOMAIN_DISK_BUS_SCSI: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 22bb209..097de9b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8342,8 +8342,12 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, !disk->dst) goto no_memory; - if (virDomainDiskDefAssignAddress(caps, disk) < 0) + if (virDomainDiskDefAssignAddress(caps, disk) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot assign address for device name '%s'"), + disk->dst); goto error; + } if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) goto no_memory; -- 1.8.0

2 2

[libvirt] [PATCH v3 UPDATED 0/2] Qemu/Gluster support in Libvirt
by Harsh Prateek Bora 22 Nov '12

22 Nov '12

Changelog: v3 updated: - Fix other network backends (nbd, rbd, sheepdog) to initialize new members (transport, socket) of _virDomainDiskHostDef appropriately so that garbage values doesnt break the argv2xml tests. v3: - RNG schema updated as required for unix transport [Paolo] - introduced another new attribute 'socket' for unix transport [Paolo] - Uses virURIFormat and virURIParse for URI parsing. [danpb] - updated documentation as required. [Jirka] v2: - Addressed review comments by Jiri - Updated patcheset as per new URI spec Ref: http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg05199.html v1: - Initial prototype Harsh Prateek Bora (2): Qemu/Gluster: Add Gluster protocol as supported network disk formats. tests: Add tests for gluster protocol based network disks support docs/formatdomain.html.in | 24 ++- docs/schemas/domaincommon.rng | 35 ++++- src/conf/domain_conf.c | 72 +++++++-- src/conf/domain_conf.h | 12 ++ src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 168 ++++++++++++++++++++- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-disk-drive-network-gluster.args | 1 + .../qemuxml2argv-disk-drive-network-gluster.xml | 35 +++++ tests/qemuxml2argvtest.c | 2 + 10 files changed, 323 insertions(+), 29 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml -- 1.7.11.7

6 10

[libvirt] [PATCH] qemu: Stop recursive detection of image chains when an image is missing
by Peter Krempa 22 Nov '12

22 Nov '12

Commit e0c469e58b93f852a72265919703cb6abd3779f8 that fixes the detection of image chain wasn't complete. Iteration through the backing image chain has to stop at the last existing image if some of the images are missing otherwise the backing chain that is cached contains entries with paths being set to NULL resulting to: error: Unable to allow access for disk path (null): Bad address Fortunately stat() is kind enough not to crash when it's presented with a NULL argument. --- src/util/storage_file.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 2249212..e7cbea7 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -729,6 +729,8 @@ virStorageFileGetMetadataFromBuf(int format, if (meta->backingStore == NULL) { /* the backing file is (currently) unavailable, treat this * file as standalone */ + VIR_FREE(meta->backingStoreRaw); + meta->backingStoreIsFile = false; backingFormat = VIR_STORAGE_FILE_NONE; } } -- 1.8.0

2 5

[libvirt] [PATCH v2] Correct include-password option and rewrite domdisplay
by Martin Kletzander 22 Nov '12

22 Nov '12

The 'virsh domdisplay' command is able to display the password configured for spice, but it was missing for vnc type graphics. Also, there were some inconsistencies that are cleaned now. --- tools/virsh-domain.c | 74 +++++++++++++++++++++++++++++----------------------- 1 file changed, 42 insertions(+), 32 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cc47383..cc5c830 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7038,8 +7038,11 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) /* Attempt to grab our display info */ for (iter = 0; scheme[iter] != NULL; iter++) { /* Create our XPATH lookup for the current display's port */ - virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" - "/@port)", scheme[iter]); + virAsprintf(&xpath, + "string(/domain/devices/graphics[@type='%s']" + "/@port)", + scheme[iter]); + if (!xpath) { virReportOOMError(); goto cleanup; @@ -7056,8 +7059,10 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) continue; /* Create our XPATH lookup for the current display's address */ - virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" - "/@listen)", scheme[iter]); + virAsprintf(&xpath, + "string(/domain/devices/graphics[@type='%s']" + "/@listen)", + scheme[iter]); if (!xpath) { virReportOOMError(); goto cleanup; @@ -7069,14 +7074,36 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) listen_addr = virXPathString(xpath, ctxt); VIR_FREE(xpath); + /* We can query this info for all the graphics types since we'll + * get nothing for the unsupported ones (just rdp for now) */ + if (vshCommandOptBool(cmd, "include-password")) { + /* Create our XPATH lookup for the password */ + virAsprintf(&xpath, + "string(/domain/devices/graphics" + "[@type='%s']/@passwd)", + scheme[iter]); + + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the password */ + passwd = virXPathString(xpath, ctxt); + VIR_FREE(xpath); + } + /* Per scheme data mangling */ if (STREQ(scheme[iter], "vnc")) { - /* VNC protocol handlers take their port number as 'port' - 5900 */ + /* VNC protocol handlers take their port number as + * 'port' - 5900 */ port -= 5900; } else if (STREQ(scheme[iter], "spice")) { /* Create our XPATH lookup for the SPICE TLS Port */ - virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" - "/@tlsPort)", scheme[iter]); + virAsprintf(&xpath, + "string(/domain/devices/graphics[@type='%s']" + "/@tlsPort)", + scheme[iter]); if (!xpath) { virReportOOMError(); goto cleanup; @@ -7087,25 +7114,17 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) VIR_FREE(xpath); if (tmp) tls_port = 0; - - if (vshCommandOptBool(cmd, "include-password")) { - /* Create our XPATH lookup for the SPICE password */ - virAsprintf(&xpath, "string(/domain/devices/graphics" - "[@type='%s']/@passwd)", scheme[iter]); - if (!xpath) { - virReportOOMError(); - goto cleanup; - } - - /* Attempt to get the SPICE password */ - passwd = virXPathString(xpath, ctxt); - VIR_FREE(xpath); - } } /* Build up the full URI, starting with the scheme */ virBufferAsprintf(&buf, "%s://", scheme[iter]); + /* There is no user, so just append password if there's any */ + if (passwd) { + virBufferAsprintf(&buf, ":%s@", passwd); + VIR_FREE(passwd); + } + /* Then host name or IP */ if (!listen_addr || STREQ((const char *)listen_addr, "0.0.0.0")) virBufferAddLit(&buf, "localhost"); @@ -7115,20 +7134,11 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) VIR_FREE(listen_addr); /* Add the port */ - if (STREQ(scheme[iter], "spice")) - virBufferAsprintf(&buf, "?port=%d", port); - else - virBufferAsprintf(&buf, ":%d", port); + virBufferAsprintf(&buf, ":%d", port); /* TLS Port */ if (tls_port) - virBufferAsprintf(&buf, "&tls-port=%d", tls_port); - - /* Password */ - if (passwd) { - virBufferAsprintf(&buf, "&password=%s", passwd); - VIR_FREE(passwd); - } + virBufferAsprintf(&buf, "?tls-port=%d", tls_port); /* Ensure we can print our URI */ if (virBufferError(&buf)) { -- 1.8.0

2 2

[libvirt] [PATCH] virsh: Report error when taking a snapshot with empty --memspec argument
by Peter Krempa 22 Nov '12

22 Nov '12

When the value of memspec was empty taking of a snapshot failed without reporting an error. --- This is only a quick fix. I think we should improve vshCommandOptStr to detect this for us and report an appropriate error, but this change will require a lot of changes not relevant to this problem. --- tools/virsh-snapshot.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 398730c..057ae2d 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -358,7 +358,12 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) if (desc) virBufferEscapeString(&buf, " <description>%s</description>\n", desc); - if (vshCommandOptString(cmd, "memspec", &memspec) < 0 || + if (vshCommandOptString(cmd, "memspec", &memspec) < 0) { + vshError(ctl, _("memspec argument must not be empty")); + goto cleanup; + } + + if (memspec && vshParseSnapshotMemspec(ctl, &buf, memspec) < 0) { virBufferFreeAndReset(&buf); goto cleanup; -- 1.8.0

2 2

[libvirt] [PATCH] Add device operations for lxc v1
by Heiher 22 Nov '12

22 Nov '12

Hello, Operations: 1. lxcDomainAttachDevice 2. lxcDomainAttachDeviceFlags 3. lxcDomainDetachDevice 4. lxcDomainDetachDeviceFlags 5. lxcDomainUpdateDeviceFlags In live mode, set devices.allow/deny to instance's cgroup and insert disk config into current domain. In config mode, just insert disk config into current domain. -- Best regards! Heiher https://heiher.info

2 1

[libvirt] [libvirt-glib v2] gobject: Also delete storage volume from its pool's list
by Zeeshan Ali (Khattak) 22 Nov '12

22 Nov '12

From: "Zeeshan Ali (Khattak)" <zeeshanak(a)gnome.org> Without this patch, storage pool still lists the volume even after it is deleted. Related Boxes bug: https://bugzilla.gnome.org/show_bug.cgi?id=688724 --- .../libvirt-gobject-storage-pool-private.h | 33 ++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-storage-pool.c | 20 +++++++++++++ libvirt-gobject/libvirt-gobject-storage-vol.c | 3 ++ 3 files changed, 56 insertions(+) create mode 100644 libvirt-gobject/libvirt-gobject-storage-pool-private.h diff --git a/libvirt-gobject/libvirt-gobject-storage-pool-private.h b/libvirt-gobject/libvirt-gobject-storage-pool-private.h new file mode 100644 index 0000000..5492e5b --- /dev/null +++ b/libvirt-gobject/libvirt-gobject-storage-pool-private.h @@ -0,0 +1,33 @@ +/* + * libvirt-gobject-storage-pool-private.h: libvirt gobject integration + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Zeeshan Ali (Khattak) <zeeshanak(a)gnome.org> + */ + +#ifndef __LIBVIRT_GOBJECT_STORAGE_POOL_PRIVATE_H__ +#define __LIBVIRT_GOBJECT_STORAGE_POOL_PRIVATE_H__ + +G_BEGIN_DECLS + +G_GNUC_INTERNAL void gvir_storage_pool_delete_vol(GVirStoragePool *pool, + GVirStorageVol *volume); + +G_END_DECLS + +#endif /* __LIBVIRT_GOBJECT_STORAGE_POOL_PRIVATE_H__ */ diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 8f579a1..107efa6 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -29,6 +29,7 @@ #include "libvirt-glib/libvirt-glib.h" #include "libvirt-gobject/libvirt-gobject.h" #include "libvirt-gobject-compat.h" +#include "libvirt-gobject-storage-pool-private.h" #define GVIR_STORAGE_POOL_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_STORAGE_POOL, GVirStoragePoolPrivate)) @@ -1129,3 +1130,22 @@ gboolean gvir_storage_pool_delete_finish(GVirStoragePool *pool, return TRUE; } + +G_GNUC_INTERNAL void gvir_storage_pool_delete_vol(GVirStoragePool *pool, + GVirStorageVol *volume) +{ + GVirStoragePoolPrivate *priv; + + g_return_if_fail(GVIR_IS_STORAGE_POOL(pool)); + g_return_if_fail(GVIR_IS_STORAGE_VOL(volume)); + + priv = pool->priv; + g_mutex_lock(priv->lock); + if (priv->volumes != NULL) { + const gchar *name = gvir_storage_vol_get_name(volume); + g_hash_table_remove(priv->volumes, name); + } else { + g_warn_if_reached(); + } + g_mutex_unlock(priv->lock); +} diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c b/libvirt-gobject/libvirt-gobject-storage-vol.c index c7ebb45..3a08450 100644 --- a/libvirt-gobject/libvirt-gobject-storage-vol.c +++ b/libvirt-gobject/libvirt-gobject-storage-vol.c @@ -29,6 +29,7 @@ #include "libvirt-glib/libvirt-glib.h" #include "libvirt-gobject/libvirt-gobject.h" #include "libvirt-gobject-compat.h" +#include "libvirt-gobject-storage-pool-private.h" #define GVIR_STORAGE_VOL_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_STORAGE_VOL, GVirStorageVolPrivate)) @@ -309,6 +310,8 @@ gboolean gvir_storage_vol_delete(GVirStorageVol *vol, g_return_val_if_fail(GVIR_IS_STORAGE_VOL(vol), FALSE); g_return_val_if_fail(err == NULL || *err == NULL, FALSE); + gvir_storage_pool_delete_vol(vol->priv->pool, vol); + if (virStorageVolDelete(vol->priv->handle, flags) < 0) { gvir_set_error_literal(err, GVIR_STORAGE_VOL_ERROR, -- 1.8.0

2 1

[libvirt] [PATCH v4 0/3] Qemu/Gluster support in Libvirt
by Harsh Prateek Bora 22 Nov '12

22 Nov '12

Changelog: v4: - Addressed review comment by Jiri on v3 updated series. v3 updated: - Fix other network backends (nbd, rbd, sheepdog) to initialize new members (transport, socket) of _virDomainDiskHostDef appropriately so that garbage values doesnt break the argv2xml tests. v3: - RNG schema updated as required for unix transport [Paolo] - introduced another new attribute 'socket' for unix transport [Paolo] - Uses virURIFormat and virURIParse for URI parsing. [danpb] - updated documentation as required. [Jirka] v2: - Addressed review comments by Jiri - Updated patcheset as per new URI spec Ref: http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg05199.html v1: - Initial prototype Harsh Prateek Bora (3): Add Gluster protocol as supported network disk backend qemu: Add support for gluster protocol based network storage backend. tests: Add tests for gluster protocol based network disks support docs/formatdomain.html.in | 24 ++- docs/schemas/domaincommon.rng | 33 +++- src/conf/domain_conf.c | 77 ++++++-- src/conf/domain_conf.h | 12 ++ src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 198 ++++++++++++++++++++- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-disk-drive-network-gluster.args | 1 + .../qemuxml2argv-disk-drive-network-gluster.xml | 34 ++++ tests/qemuxml2argvtest.c | 2 + 10 files changed, 355 insertions(+), 29 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml -- 1.7.11.7

1 3

[libvirt] [PATCH v1 1/2] change qemu_driver's qemuImgBinary field
by liguang 22 Nov '12

22 Nov '12

qemuImgBinary is a poor name, change to qemuImgTool, and init it at qemu driver start-up phase, before this change, if we need qemu-img or kvm-img we have to call qemuFindQemuImgBinary every time, seems a little boring, from then on, we can use qemuImgTool directly, e.g. use virCommandRun(driver->qemuImgTool, ...) instead of img_tool = qemuFindQemuImgBinary(driver); qemuFindQemuImgBinary(img_tool, ...); Signed-off-by: liguang <lig.fnst(a)cn.fujitsu.com> --- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_domain.c | 22 ++-------------------- src/qemu/qemu_domain.h | 2 -- src/qemu/qemu_driver.c | 12 ++++++++++-- 4 files changed, 13 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 2c7f70c..47f349c 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -84,7 +84,7 @@ struct qemud_driver { char *cacheDir; char *saveDir; char *snapshotDir; - char *qemuImgBinary; + char *qemuImgTool; unsigned int vncAutoUnixSocket : 1; unsigned int vncTLS : 1; unsigned int vncTLSx509verify : 1; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e0d6951..97e4fa1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1588,22 +1588,6 @@ cleanup: return ret; } -/* Locate an appropriate 'qemu-img' binary. */ -const char * -qemuFindQemuImgBinary(struct qemud_driver *driver) -{ - if (!driver->qemuImgBinary) { - driver->qemuImgBinary = virFindFileInPath("kvm-img"); - if (!driver->qemuImgBinary) - driver->qemuImgBinary = virFindFileInPath("qemu-img"); - if (!driver->qemuImgBinary) - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("unable to find kvm-img or qemu-img")); - } - - return driver->qemuImgBinary; -} - int qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, virDomainSnapshotObjPtr snapshot, @@ -1659,11 +1643,9 @@ qemuDomainSnapshotForEachQcow2Raw(struct qemud_driver *driver, int i; bool skipped = false; - qemuimgarg[0] = qemuFindQemuImgBinary(driver); - if (qemuimgarg[0] == NULL) { - /* qemuFindQemuImgBinary set the error */ + qemuimgarg[0] = driver->qemuImgTool; + if (qemuimgarg[0] == NULL) return -1; - } qemuimgarg[2] = op; qemuimgarg[3] = name; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index a2acc0a..548cd20 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -300,8 +300,6 @@ int qemuDomainAppendLog(struct qemud_driver *driver, int logFD, const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(4, 5); -const char *qemuFindQemuImgBinary(struct qemud_driver *driver); - int qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, virDomainSnapshotObjPtr snapshot, char *snapshotDir); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 436e853..e87e7b4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -643,6 +643,14 @@ qemudStartup(int privileged) { if (!qemu_driver->domainEventState) goto error; + /* find kvm-img or qemu-img */ + qemu_driver->qemuImgTool = virFindFileInPath("kvm-img"); + if (!qemu_driver->qemuImgTool) + qemu_driver->qemuImgTool = virFindFileInPath("qemu-img"); + if (!qemu_driver->qemuImgTool) + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find kvm-img or qemu-img")); + /* read the host sysinfo */ if (privileged) qemu_driver->hostsysinfo = virSysinfoRead(); @@ -1029,7 +1037,7 @@ qemudShutdown(void) { VIR_FREE(qemu_driver->cacheDir); VIR_FREE(qemu_driver->saveDir); VIR_FREE(qemu_driver->snapshotDir); - VIR_FREE(qemu_driver->qemuImgBinary); + VIR_FREE(qemu_driver->qemuImgTool); VIR_FREE(qemu_driver->autoDumpPath); VIR_FREE(qemu_driver->vncTLSx509certdir); VIR_FREE(qemu_driver->vncListen); @@ -10706,7 +10714,7 @@ qemuDomainSnapshotCreateInactiveExternal(struct qemud_driver *driver, int ret = -1; - if (!(qemuImgPath = qemuFindQemuImgBinary(driver))) + if (!(qemuImgPath = driver->qemuImgTool)) return -1; if (!(created = virBitmapNew(snap->def->ndisks))) { -- 1.7.1

4 8

[libvirt] [PATCH] storage: Improve virStorageBackendFileSystemStop
by Osier Yang 22 Nov '12

22 Nov '12

It's actually not used for DIR pool. So removing the checking. --- src/storage/storage_backend_fs.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 3322677..10daee3 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -933,7 +933,7 @@ no_memory: * @conn connection to report errors against * @pool storage pool to start * - * Stops a directory or FS based storage pool. + * Stops a FS based storage pool. * * - If it is a FS based pool, unmounts the unlying source device on the pool * - Releases all cached data about volumes @@ -943,8 +943,7 @@ static int virStorageBackendFileSystemStop(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { - if (pool->def->type != VIR_STORAGE_POOL_DIR && - virStorageBackendFileSystemUnmount(pool) < 0) + if (virStorageBackendFileSystemUnmount(pool) < 0) return -1; return 0; -- 1.7.7.6

2 2

[libvirt] [PATCH] storage: Fix bug of fs pool destroying
by Osier Yang 22 Nov '12

22 Nov '12

Regression introduced by commit 258e06c85b7, "ret" could be set to 1 or 0 by virStorageBackendFileSystemIsMounted before goto cleanup. This could mislead the callers (up to the public API virStoragePoolDestroy) to return success even the underlying umount command fails. --- src/storage/storage_backend_fs.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 10daee3..cbcbe34 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -449,6 +449,7 @@ static int virStorageBackendFileSystemUnmount(virStoragePoolObjPtr pool) { virCommandPtr cmd = NULL; int ret = -1; + int rc; if (pool->def->type == VIR_STORAGE_POOL_NETFS) { if (pool->def->source.nhost != 1) { @@ -475,8 +476,8 @@ virStorageBackendFileSystemUnmount(virStoragePoolObjPtr pool) { } /* Short-circuit if already unmounted */ - if ((ret = virStorageBackendFileSystemIsMounted(pool)) != 1) { - if (ret < 0) + if ((rc = virStorageBackendFileSystemIsMounted(pool)) != 1) { + if (rc < 0) return -1; else return 0; -- 1.7.7.6

2 2

[libvirt] [PATCH] run bootstrap if .gnulib is not present
by Hu Tao 22 Nov '12

22 Nov '12

If .gnulib is deleted unexpectedly, autogen.sh will fail with message: fatal: ambiguous argument '.gnulib': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git <command> [<revision>...] -- [<file>...]' which is actually given by git diff .gnulib, which doesn't exist. In the case to run bootstrap to create .gnulib. --- autogen.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/autogen.sh b/autogen.sh index d183397..07c58a1 100755 --- a/autogen.sh +++ b/autogen.sh @@ -62,7 +62,9 @@ bootstrap_hash() # Only run bootstrap from a git checkout, never from a tarball. if test -d .git; then curr_status=.git-module-status - t=$(bootstrap_hash; git diff .gnulib) + if test -d .gnulib; then + t=$(bootstrap_hash; git diff .gnulib) + fi case $t:${CLEAN_SUBMODULE+set} in *:set) ;; *-dirty*) -- 1.7.11.7

2 3

[libvirt] [PATCHv4 0/4] Add initial support for reverting snapshots
by Peter Krempa 22 Nov '12

22 Nov '12

I'm reposting another version of the series. This version fixes small mistakes in patches 1-3 and reposts patch 4 as it got missing due to a sending error. Peter Krempa (4): qemu: Split out guts of qemuDomainSaveImageStartVM() to allow reuse snapshot: Add flag VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED snapshot: Add flag to allow hypervisor restart when reverting snapshots snapshot: qemu: Implement reverting of external snapshots include/libvirt/libvirt.h.in | 3 + src/libvirt.c | 36 ++-- src/qemu/qemu_driver.c | 389 ++++++++++++++++++++++++++++++++++++++++--- tools/virsh-snapshot.c | 7 + tools/virsh.pod | 19 ++- 5 files changed, 410 insertions(+), 44 deletions(-) -- 1.8.0

3 14

[libvirt] libvirt RBD attach regression?
by Scott Sullivan 22 Nov '12

22 Nov '12

Hello, I have been testing libvirt v1.0.0 for deployment within my organization, and in the process discovered what appears to be a bug that breaks virsh attach-device, when attaching an RBD volume to an instance. First, here is the error presented, with v1.0.0 (this worked in v0.10.2): [root@host ~]# virsh attach-device W5APQ8 G84VV1.xml error: Failed to attach device from G84VV1.xml error: cannot open file 'dc3-1-test/G84VV1': No such file or directory [root@host ~]# W5APQ8 is my running QEMU/KVM instance, and G84VV1.xml contains the following: <disk type='network' device='disk'> <driver name='qemu' type='raw'/> <auth username="removed"> <secret type="ceph" uuid="removed"/> </auth> <source protocol='rbd' name='dc3-1-test/G84VV1'> <host name="X.X.X.X" port="6789"/> <host name="X.X.X.X" port="6789"/> <host name="X.X.X.X" port="6789"/> </source> <target dev='vdc' bus='virtio'/> <serial>hpbs-G84VV1</serial> </disk> Using git bisect, I narrowed the problem down to this as the first commit to break this setup: 4d34c92947e8cf9e9bedfa227ada1d2dba92d68a is the first bad commit commit 4d34c92947e8cf9e9bedfa227ada1d2dba92d68a Author: Eric Blake <eblake(a)redhat.com> Date: Tue Oct 9 16:08:14 2012 -0600 These all look closely related as well to the problem commit: http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=67aea3fb780346b4aa5aea… http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=38c4a9cc40476bd2e598a1… http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=4d34c92947e8cf9e9bedfa… If I build libvirt from sources before these commits and then run the exact virsh-attach command shown above, the attachment works and I do not get any errors. I wanted to see if anyone had any insights on this, or perhaps a commit to correct this issue. If I can be of any further assistance with this, let me know. Thanks, Scott Sullivan

2 3

[libvirt] [libvirt-glib] gobject: Also delete storage volume from its pool's list
by Zeeshan Ali (Khattak) 22 Nov '12

22 Nov '12

From: "Zeeshan Ali (Khattak)" <zeeshanak(a)gnome.org> Without this patch, storage pool still lists the volume even after it is deleted. Related Boxes bug: https://bugzilla.gnome.org/show_bug.cgi?id=688724 --- libvirt-gobject/libvirt-gobject-storage-pool.c | 23 +++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-storage-vol.c | 4 ++++ 2 files changed, 27 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 8f579a1..df7e77c 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -29,6 +29,7 @@ #include "libvirt-glib/libvirt-glib.h" #include "libvirt-gobject/libvirt-gobject.h" #include "libvirt-gobject-compat.h" +#include "libvirt-gobject-storage-pool-private.h" #define GVIR_STORAGE_POOL_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_STORAGE_POOL, GVirStoragePoolPrivate)) @@ -1129,3 +1130,25 @@ gboolean gvir_storage_pool_delete_finish(GVirStoragePool *pool, return TRUE; } + +G_GNUC_INTERNAL gboolean gvir_storage_pool_delete_vol(GVirStoragePool *pool, + GVirStorageVol *volume) +{ + GVirStoragePoolPrivate *priv; + gboolean ret = FALSE; + + g_return_val_if_fail(GVIR_IS_STORAGE_POOL(pool), FALSE); + g_return_val_if_fail(GVIR_IS_STORAGE_VOL(volume), FALSE); + + priv = pool->priv; + g_mutex_lock(priv->lock); + if (priv->volumes != NULL) { + const gchar *name = gvir_storage_vol_get_name(volume); + ret = g_hash_table_remove(priv->volumes, name); + } else { + g_warn_if_reached(); + } + g_mutex_unlock(priv->lock); + + return ret; +} diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c b/libvirt-gobject/libvirt-gobject-storage-vol.c index c7ebb45..4352d30 100644 --- a/libvirt-gobject/libvirt-gobject-storage-vol.c +++ b/libvirt-gobject/libvirt-gobject-storage-vol.c @@ -29,6 +29,7 @@ #include "libvirt-glib/libvirt-glib.h" #include "libvirt-gobject/libvirt-gobject.h" #include "libvirt-gobject-compat.h" +#include "libvirt-gobject-storage-pool-private.h" #define GVIR_STORAGE_VOL_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_STORAGE_VOL, GVirStorageVolPrivate)) @@ -309,6 +310,9 @@ gboolean gvir_storage_vol_delete(GVirStorageVol *vol, g_return_val_if_fail(GVIR_IS_STORAGE_VOL(vol), FALSE); g_return_val_if_fail(err == NULL || *err == NULL, FALSE); + if (!gvir_storage_pool_delete_vol(vol->priv->pool, vol)) + return FALSE; + if (virStorageVolDelete(vol->priv->handle, flags) < 0) { gvir_set_error_literal(err, GVIR_STORAGE_VOL_ERROR, -- 1.8.0

3 4

[libvirt] RFC: Migration with NPIV
by Osier Yang 21 Nov '12

21 Nov '12

Hi, This proposal is trying to figure out a solution for migration of domain which uses LUN behind vHBA as disk device (QEMU emulated disk only at this stage). And other related NPIV improvements which are not related with migration. I'm not luck to get a environment to test if the thoughts are workable, but I'd like see if guys have good idea/suggestions earlier. 1) Persistent vHBA support This is the useful stuff missed for long time. Assuming that one created a vHBA, did masking/zoning, everything works as expected. However, after a system rebooting, everything is just lost. If the user wants to get things back, he has to find out the preivous WWNN & WWPN, and create the vHBA again. On the other hand, Persistent vHBA support is actually required for domain which uses LUN behind a vHBA. Othewise the domain could fail to start after a system rebooting. To support the persistent vHBA, new APIs like virNodeDeviceDefineXML, virNodeDeviceUndefine is required. Also it's useful to introduce "autostart" for vHBA, so that the vHBA could be started automatically after system rebooting. Proposed APIs: virNodeDevicePtr virNodeDeviceDefineXML(virConnectPtr conn, const char *xml, unsigned int flags); int virNodeDeviceUndefine(virConnectPtr conn, virNodeDevicePtr dev, unsigned int flags); int virNodeDeviceSetAutostart(virNodeDevicePtr dev, int autostart, unsigned int flags); int virNodeDeviceGetAutostart(virNodeDevicePtr dev, int *autostart, unsigned int flags); 2) Associate vHBA with domain XML There are two ways to attach a LUN to a domain: as an QEMU emulated device; or passthrough. Since passthrough a LUN is not supported in libvirt yet, let's focus on the emulated LUN at this stage. New attributes "wwnn" and "wwpn" are introduced to indicate the LUN behind the vHBA. E.g. <disk type='block' device='disk'> <driver name='qemu' type='raw'/> <source wwnn="2001001b32a9da4e" wwpn="2101001b32a90004"/> <target dev='vda' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </disk> Before the domain starting, we have to check if there is LUN assigned to the vHBA, error out if not. Using the stable path of LUN also works, e.g. <source dev="/dev/disk/by-path/pci-0000\:00\:07.0-scsi-0\:0\:0\:0"/> But the disadvantage is the user have to figure out the stable path himself; And we have to do checking of every stable path to see if it's behind a vHBA in migration "Begin" stage. Or an new XML tag for element "source" to indicate that it's behind a vHBA? such as: <source dev="disk-by-path" model="vport"/> 3) Migration with vHBA One possible solution for migration with vHBA is to use one pair of WWNN & WWPN on source host, one is using for domain, one is reserved for migration purpose. It requires the storage admin maps the same LUN to the two vHBAs when doing the masking and zoning. One of the two vHBA is called "Primary vHBA", another is called "secondary vHBA". To maitain the relationship between these two vHBAs, we have to introduce new XMLs to vHBA. E.g. In XML of primary vHBA: <secondary wwpn="2101001b32a90004"/> In XML of secondary vHBA: <primary wwpn="2101001b32a90002"/> Primary vHBA is going to be guaranteed not used by any domain which is driven by libvirt (we do some checking eariler before the domain starting). And it's also guaranteed that the LUN can't be used by other domain with sVirt or Sanlock. So it's safe to have two vHBAs on source host too. To prevent one using the LUN by creating vHBA using the same WWNN & WWPN on another host, we must create the secondary vHBA on source host, even it's not being used. Both primary and secondary vHBA must be defined and marked as "autostart" so that the domain could be started after system rebooting. When do migration, we have to bake a bigger cookie with secondary vHBA's info (basically it's WWNN and WWPN) in migration "Begin" stage, and eat that in migration "Prepare" stage on target host. In "Begin" stage, the XMLs represents the secondary vHBA is constructed. And the secondary vHBA is destoyed on source host, not undefined though. In "Prepare" stage, a new vHBA is created (define and start) on target host with the same WWNN & WWPN as secondary vHBA on source host. The LUN then should be visible to target host automatically? and thus migration can be performed. After migration is finished on target host, the primary vHBA on source host is destroyed, not undefined. If migration fails, the new vHBA created on target host will be destroyed and undefined. And both primary and secondary vHBA on source host will be started, so that the domain could be resumed. Finally if migration succeeds, primary vHBA on source host will be transtered to target host as secondary vHBA (defined). And both primary and secondary vHBA on source host will be undefined. 4) Enrich HBA's XML It's hard to known the vHBAs created from a HBA with current implementation. One have to dump XML of each (v)HBAs and find out the clue with element "parent" of vHBAs. It's good to introduce new element for HBA like "vports", so that one can easily known what (how many) vHBAs are created from the HBA? And also it's good to have the maximum vports the HBA supports. Except these, other useful information should be exposed too, such as the vendor name, the HBA state, PCI address, etc. The new XMLs should be like: <vports num='2' max='64'> <vport name="scsi_host40" wwpn="2101001b32a90004"/> <vport name="scsi_host40" wwpn="2101001b32a90005"/> </vports> <online/> <vendor>QLogic</vendor> <address type="pci" domain="0" bus="0" slot="5" function="0"/> "online", "vendor", "address" make sense to vHBA too. 5) Improve the way to lookup LUN's stable path Currently, to lookup the LUN's stable path with WWNN & WWPN, it needs to iterate over the sysfs each time, maintaining the stable path in vHBA's XML doesn't make sense, as the LUN assigned to the vHBA could be changed as the storage admin's mood. I'm wondering if there is a way to notify the change asynchronously, if there is, then maintaining the stable path internally make sense. 6) Miscellaneous This is only about QEMU emulated device, passthroughed scsi_host with vHBA is still not covered, we have to support the vHBA passthough first. The good thing is the solution should be similiar. Regards, Osier

5 11

[libvirt] [libvirt-glib] Return an error when virConnectGetVersion returns 0
by Christophe Fergeau 21 Nov '12

21 Nov '12

virConnectGetVersion can return 0 'if the version can't be extracted by lack of capabilities'. I've also seen it return 0 when libvirtd 1.0 is running but virsh 0.10.2 is used to get the version. gvir_connection_get_version is documented as returning 0 on error. However, currently it only sets the corresponding GError when virConnectGetVersion returns -1. Makes sure the GError is also set when 0 is returned. The old behaviour was causing a crash in conn-test as this assumes that the GError will be set when gvir_connection_get_version returns 0. --- libvirt-gobject/libvirt-gobject-connection.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index 3157a66..91cc535 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -1108,6 +1108,12 @@ gvir_connection_get_version(GVirConnection *conn, if (virConnectGetVersion(priv->conn, &ret) < 0) { gvir_set_error_literal(err, GVIR_CONNECTION_ERROR, 0, "Unable to get hypervisor version"); + ret = 0; + } else if (ret == 0) { + /* From virConnectGetVersion doc: "if the version can't be + * extracted by lack of capacities returns 0 and @hvVer is 0" */ + g_set_error_literal(err, GVIR_CONNECTION_ERROR, 0, + "Unknown hypervisor version"); } cleanup: -- 1.8.0

1 0