[libvirt] [PATCH Java] Remove non-thread-safe error reporting
virConnCopyLastError is not thread-safe, don't use it. Reported by Ravi Pawar. --- src/main/java/org/libvirt/Connect.java |2 +- src/main/java/org/libvirt/ErrorHandler.java | 17 - 2 files changed, 1 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/libvirt/Connect.java b/src/main/java/org/libvirt/Connect.java index fb8ea89..7761c1f 100644 --- a/src/main/java/org/libvirt/Connect.java +++ b/src/main/java/org/libvirt/Connect.java @@ -1319,7 +1319,7 @@ public class Connect { * @throws LibvirtException */ protected void processError() throws LibvirtException { -ErrorHandler.processError(libvirt, VCP); +ErrorHandler.processError(libvirt); } /** diff --git a/src/main/java/org/libvirt/ErrorHandler.java b/src/main/java/org/libvirt/ErrorHandler.java index 7b723bb..e30291b 100644 --- a/src/main/java/org/libvirt/ErrorHandler.java +++ b/src/main/java/org/libvirt/ErrorHandler.java @@ -28,21 +28,4 @@ public class ErrorHandler { throw new LibvirtException(error); } } - -/** - * Look for the latest error from libvirt tied to a connection - * - * @param libvirt - *the active connection - * @throws LibvirtException - */ -public static void processError(Libvirt libvirt, ConnectionPointer conn) throws LibvirtException { -virError vError = new virError(); -int errorCode = libvirt.virConnCopyLastError(conn, vError); -if (errorCode 0) { -Error error = new Error(vError); -libvirt.virConnResetLastError(conn); -throw new LibvirtException(error); -} -} } -- 1.7.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH Java] Remove non-thread-safe error reporting
On Fri, Sep 03, 2010 at 01:33:26PM +0200, Matthias Bolte wrote: virConnCopyLastError is not thread-safe, don't use it. Reported by Ravi Pawar. --- src/main/java/org/libvirt/Connect.java |2 +- src/main/java/org/libvirt/ErrorHandler.java | 17 - 2 files changed, 1 insertions(+), 18 deletions(-) ACK Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH Java] Remove non-thread-safe error reporting
2010/9/3 Daniel P. Berrange berra...@redhat.com: On Fri, Sep 03, 2010 at 01:33:26PM +0200, Matthias Bolte wrote: virConnCopyLastError is not thread-safe, don't use it. Reported by Ravi Pawar. --- src/main/java/org/libvirt/Connect.java | 2 +- src/main/java/org/libvirt/ErrorHandler.java | 17 - 2 files changed, 1 insertions(+), 18 deletions(-) ACK Thanks, pushed. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Default to qemu:///system if accessible
If no uri is passed to one of the virConnectOpen-ish calls, libvirt attempts to autoprobe a sensible URI. Part of the current logic checks for getuid() 0, and if that check is succesful, a libvirtd daemon is spawned. This patch replaces this check with a call to access(LIBVIRTD_PRIV_UNIX_SOCKET, W_OK) so that users with access to the qemu:///system UNIX socket connect to that one by default instead of spawning a new libvirtd daemon. Signed-off-by: Soren Hansen so...@linux2go.dk --- src/remote/remote_driver.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a945710..17e6ead 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1086,7 +1086,7 @@ remoteOpen (virConnectPtr conn, if (!conn-uri) { DEBUG0(Auto-probe remote URI); #ifndef __sun -if (getuid() 0) { +if (access(LIBVIRTD_PRIV_UNIX_SOCKET, W_OK)) { DEBUG0(Auto-spawn user daemon instance); rflags |= VIR_DRV_OPEN_REMOTE_USER; if (!autostart || -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Default to qemu:///system if accessible
On Fri, Sep 03, 2010 at 03:28:58PM +0200, Soren Hansen wrote: If no uri is passed to one of the virConnectOpen-ish calls, libvirt attempts to autoprobe a sensible URI. Part of the current logic checks for getuid() 0, and if that check is succesful, a libvirtd daemon is spawned. This patch replaces this check with a call to access(LIBVIRTD_PRIV_UNIX_SOCKET, W_OK) so that users with access to the qemu:///system UNIX socket connect to that one by default instead of spawning a new libvirtd daemon. NACK, I don't think we should be changing this. If the user is unprivileged, it should always default to the unprivileged libvirtd, regardless of whether they are also authorized to connect to the privileged libvirtd (via socket permissions or policykit, or kerberos). If the unprivileged user still wants the privileged libvirtd, they should given an explicit URI. Regards, Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: avoid unused variable warning
* src/vbox/vbox_tmpl.c (vboxAttachDrives): Capture return value. --- Pushing under the build-breaker rule. Gcc didn't catch this under '-g -O0', but with -Werror and -O2, it dies with a warning that rc is used uninitialized. I suspect that clang would have also reported this, although I haven't tested that suspicion. This was a pre-existing bug; my refactoring exposed it (before the refactoring, rc contained stale contents, unrelated to the actual message being printed; after the refactoring, rc was indeed used without initialization). src/vbox/vbox_tmpl.c | 34 ++ 1 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 3e8ff23..b5cd2e4 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -3788,40 +3788,42 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) VBOX_UTF16_FREE(mediumFileUtf16); continue; } if (!medium) { PRUnichar *mediumEmpty = NULL; VBOX_UTF8_TO_UTF16(, mediumEmpty); if (def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_DISK) { -data-vboxObj-vtbl-OpenHardDisk(data-vboxObj, - mediumFileUtf16, - AccessMode_ReadWrite, - false, - mediumEmpty, - false, - mediumEmpty, - medium); +rc = data-vboxObj-vtbl-OpenHardDisk(data-vboxObj, + mediumFileUtf16, + AccessMode_ReadWrite, + false, + mediumEmpty, + false, + mediumEmpty, + medium); } else if (def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_CDROM) { -data-vboxObj-vtbl-OpenDVDImage(data-vboxObj, - mediumFileUtf16, - mediumEmpty, - medium); +rc = data-vboxObj-vtbl-OpenDVDImage(data-vboxObj, + mediumFileUtf16, + mediumEmpty, + medium); } else if (def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { -data-vboxObj-vtbl-OpenFloppyImage(data-vboxObj, - mediumFileUtf16, - mediumEmpty, - medium); +rc = data-vboxObj-vtbl-OpenFloppyImage(data-vboxObj, + mediumFileUtf16, + mediumEmpty, + medium); +} else { +rc = 0; } VBOX_UTF16_FREE(mediumEmpty); } if (!medium) { vboxError(VIR_ERR_INTERNAL_ERROR, _(Failed to attach the following disk/dvd/floppy to the machine: %s, rc=%08x), def-disks[i]-src, (unsigned)rc); -- 1.7.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] libvirt-guests and ./configure --without-libvirtd
On mingw, libvirt is typically configured --without-libvirtd, because the mingw build is designed to target other hosts, but cannot natively run as a hypervisor. Additionally, mingw doesn't really have a notion of /etc/sysconf, so installing /etc/sysconfig/libvirt-guests doesn't work too well. Should tools/Makefile.am make the build and installation of libvirt-guests dependent on the already-existing WITH_LIBVIRTD Makefile conditional, or would it ever make sense to build libvirt-guests but not libvirtd? I guess the answer to that boils down to whether it ever makes sense to use libvirt-guests on a system that is not running libvirtd, in which case it would only be able to control remote URIs. But the whole point of the script is to cleanly save state for local VMs, so I don't see using libvirt-guests as a management tool for remote URIs. Unless anyone speaks up with another opinion, I'll prepare a patch that makes libvirt-guests depend on WITH_LIBVIRTD. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt-guests and ./configure --without-libvirtd
Unless anyone speaks up with another opinion, I'll prepare a patch that makes libvirt-guests depend on WITH_LIBVIRTD. The init script is part of libvirt-client package because it doesn't require libvirtd. On hosts with VirtualBox hypervisor, libvirtd is not needed and libvirt-guests script can still handle guests on host shutdown/startup. The installation of the init script and corresponding sysconfig file depends on LIBVIRT_INIT_SCRIPT_RED_HAT, which feels like it doesn't make sense to enable it on mingw. Isn't it sufficient? Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt-guests and ./configure --without-libvirtd
On 09/03/2010 12:34 PM, Jiri Denemark wrote: Unless anyone speaks up with another opinion, I'll prepare a patch that makes libvirt-guests depend on WITH_LIBVIRTD. The init script is part of libvirt-client package because it doesn't require libvirtd. On hosts with VirtualBox hypervisor, libvirtd is not needed and libvirt-guests script can still handle guests on host shutdown/startup. The installation of the init script and corresponding sysconfig file depends on LIBVIRT_INIT_SCRIPT_RED_HAT, which feels like it doesn't make sense to enable it on mingw. Isn't it sufficient? So the real bug, then, is that when cross-compiling from fedora-mingw, that LIBVIRT_INIT_SCRIPT_RED_HAT is being defined even though it shouldn't be. Thanks; I think I can figure out something to fix that, then. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] OpenVZ: add ethernet interface type support
On 08/18/2010 09:05 AM, Jean-Baptiste Rouault wrote: Hi all, This patch adds support for ethernet interface type to OpenVZ domains as stated in this previous message: http://www.redhat.com/archives/libvir- list/2010-July/msg00658.html Regards, Jean-Baptiste Seems reasonable to me; sorry for the delayed review. ACK and pushed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] mingw: match recent changes in spec file
These changes allow './autobuild.sh' to complete again, when a full mingw cross-compilation is available on Fedora. * libvirt.spec.in (%file): List new installed files. * configure.ac (with_init_script): Assume default of none when cross-compiling. --- configure.ac|6 +++--- mingw32-libvirt.spec.in |5 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index d42d9e6..6c0fc69 100644 --- a/configure.ac +++ b/configure.ac @@ -276,10 +276,10 @@ AC_ARG_WITH([init-script], [AC_HELP_STRING([--with-init-script=@:@redhat|auto|none@:@], [Style of init script to install @:@default=auto@:@])]) if test x$with_init_script = x || test x$with_init_script = xauto; then -if test -f /etc/redhat-release ; then -with_init_script=redhat -else +if test $cross_compiling = yes || test ! -f /etc/redhat-release; then with_init_script=none +else +with_init_script=redhat fi fi AM_CONDITIONAL([LIBVIRT_INIT_SCRIPT_RED_HAT], test x$with_init_script = xredhat) diff --git a/mingw32-libvirt.spec.in b/mingw32-libvirt.spec.in index 6b2b5c6..4bbbc3b 100644 --- a/mingw32-libvirt.spec.in +++ b/mingw32-libvirt.spec.in @@ -71,6 +71,7 @@ rm -rf $RPM_BUILD_ROOT%{_mingw32_datadir}/doc/* rm -rf $RPM_BUILD_ROOT%{_mingw32_datadir}/gtk-doc/* rm $RPM_BUILD_ROOT%{_mingw32_libdir}/libvirt.a +rm $RPM_BUILD_ROOT%{_mingw32_libdir}/libvirt-qemu.a %clean @@ -83,10 +84,13 @@ rm -rf $RPM_BUILD_ROOT %{_mingw32_bindir}/virsh.exe %{_mingw32_bindir}/virt-xml-validate %{_mingw32_bindir}/virt-pki-validate +%{_mingw32_bindir}/libvirt-qemu-0.dll %{_mingw32_libdir}/libvirt.dll.a %{_mingw32_libdir}/libvirt.la %{_mingw32_libdir}/pkgconfig/libvirt.pc +%{_mingw32_libdir}/libvirt-qemu.dll.a +%{_mingw32_libdir}/libvirt-qemu.la %dir %{_mingw32_datadir}/libvirt/ %dir %{_mingw32_datadir}/libvirt/schemas/ @@ -109,6 +113,7 @@ rm -rf $RPM_BUILD_ROOT %dir %{_mingw32_includedir}/libvirt %{_mingw32_includedir}/libvirt/libvirt.h %{_mingw32_includedir}/libvirt/virterror.h +%{_mingw32_includedir}/libvirt/libvirt-qemu.h %{_mingw32_mandir}/man1/virsh.1* %{_mingw32_mandir}/man1/virt-xml-validate.1* -- 1.7.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Default to qemu:///system if accessible
On 03-09-2010 16:08, Daniel P. Berrange wrote: If no uri is passed to one of the virConnectOpen-ish calls, libvirt attempts to autoprobe a sensible URI. Part of the current logic checks for getuid() 0, and if that check is succesful, a libvirtd daemon is spawned. This patch replaces this check with a call to access(LIBVIRTD_PRIV_UNIX_SOCKET, W_OK) so that users with access to the qemu:///system UNIX socket connect to that one by default instead of spawning a new libvirtd daemon. NACK, I don't think we should be changing this. If the user is unprivileged, it should always default to the unprivileged libvirtd, regardless of whether they are also authorized to connect to the privileged libvirtd (via socket permissions or policykit, or kerberos). If the unprivileged user still wants the privileged libvirtd, they should given an explicit URI. Hm... I didn't think this was going to be controversial :) I firmly believe that if a user has access to the privileged libvirt daemon, that's the one he'll want to interact with in all but extraordinary cases. I consider it very analogous to how virt-manager doesn't even offer usermode networking if you're connected to qemu:///system, since if you aren't stuck with usermode networking, there's no reason to use it (other than to prove this statement wrong). Ubuntu has carried patches for this for virsh, virt-manager, and virt-viewer for a while now (at least a year and a half). I'm not sure if they've been submitted here (or to et-mgmt-tools) before (and if not, why not), but that's the lay of the land, and I've had nothing but good feedback on it. Now I just wanted to fix it properly (directly in libvirt) and get it upstream. It's certainly saved me a lot of typing. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Default to qemu:///system if accessible
On 09/03/2010 02:38 PM, Soren Hansen wrote: NACK, I don't think we should be changing this. If the user is unprivileged, it should always default to the unprivileged libvirtd, regardless of whether they are also authorized to connect to the privileged libvirtd (via socket permissions or policykit, or kerberos). If the unprivileged user still wants the privileged libvirtd, they should given an explicit URI. Hm... I didn't think this was going to be controversial :) Maybe a less-controversial patch would be changing configure.ac to add a configure option for the default URI string for non-privileged users? Right now, the default is hard-coded to qemu:///session, but by letting it be a configure choice, then it would be up to the end user (or distro) whether to risk the default of qemu:///system as well as exposing the socket as writable. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Default to qemu:///system if accessible
On 03-09-2010 15:59, Daniel Veillard wrote: diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a945710..17e6ead 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1086,7 +1086,7 @@ remoteOpen (virConnectPtr conn, if (!conn-uri) { DEBUG0(Auto-probe remote URI); #ifndef __sun -if (getuid() 0) { +if (access(LIBVIRTD_PRIV_UNIX_SOCKET, W_OK)) { DEBUG0(Auto-spawn user daemon instance); rflags |= VIR_DRV_OPEN_REMOTE_USER; if (!autostart || Hum, that sounds like a change of semantic. Depends. I think of the (getuid() 0) as a check for whether you can access the privileged UNIX socket that simply doesn't take into account that users other than root may have been given this privilege by way of membership of the group owning the socket. Before as non root you would span a local daemon, now you use the system one. I'm not saying it's a bad thing in most cases but it's a serious change, and we try to avoid those in general. As I suggested in another e-mail a short while ago in this thread, I'm making this change to maintain the status quo in Ubuntu. We already do this in virsh and virt-viewer and something very, very similar in virt-manager. Someone recently dropped the virt-viewer patch by mistake, and I decided to make the change directly in libvirt to not change how libvirt has behaved for us in the past couple of years, so I can certainly relate to your desire to not change the behaviour. Personally, at least, I find this behaviour much more pleasant. On all my servers, I just grant whoever needs to be able to deal with the VM's on it access to the socket (by adding them to the libvirtd group), and that's it. I don't need to instruct them to set per-application environment variables or whatever. Generally, I use libvirt on two classes of machine: Workstations/laptops (where I'm functionally the only user), or on servers whose job is to run virtual machines and nothing else. In the former case, obviously I want to use the systemwide libvirtd to get access to all the fancy networking things. I have absolutely no motivation to use qemu:///session on there anymore. In the latter case, only people who are supposed to manage the virtual machines will have access to the server, and IMO they have no business running stuff under qemu:///session on that box. The only situation where I actually need qemu:///session is if I for some reason want to run a VM on a machine that isn't mine (neither in terms of ownership, nor responsibility). So far, that's never happened. It's cool that it's possible(!), but I've never needed it. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Add .vmdk storage volume creation
2010/8/30 Eric Blake ebl...@redhat.com: On 08/28/2010 01:53 PM, Matthias Bolte wrote: + /* Validate config */ + tmp1 = strchr(def-name, '/'); + tmp2 = strrchr(def-name, '/'); + + if (tmp1 == NULL || tmp1 == def-name || + tmp2 == NULL || tmp2[1] == '\0') { tmp2 cannot be NULL if tmp1 is not NULL, since if the string contains a / in a forward search, it will also contain one in a reverse search. Also, checking that tmp1 != def-name can be done with a single-byte probe. What about using a single temporary for a faster test: tmp = strrchr(def-name, '/'); if (tmp == NULL || *def-name == '/' || tmp[1] == '\0') { Yes, that's fine too. I'll fold that in. + /* + * FIXME: The adapter type is a required parameter, but there is no + * way to let the user specifiy it in the volume XML config. Therefore, s/specifiy/specify/ + * default to 'busLogic' here. + */ Sounds like a good followup patch, but doesn't impact whether this one is okay as-is. + virtualDiskSpec-adapterType = (char *)busLogic; + + virtualDiskSpec-capacityKb-value = def-capacity / 1024; /* Scale from byte to kilobyte */ Do you need to round up? That is, should def-capacity of 1 result in 0 or 1024 bytes? I truncate here on purpose. The rest of the ESX driver does it this way, also the rest of the libvirt codebase truncates in situations like this. Therefore, I'll leave this as it is. I think those points are minor enough to not need a v2, so: ACK with those points addressed. Thanks, pushed. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Use the VirtualDisk UUID as storage volume key
2010/8/30 Eric Blake ebl...@redhat.com: On 08/29/2010 05:00 PM, Matthias Bolte wrote: VirtualDisks are .vmdk file based. Other files in a datastore like .iso or .flp files don't have a UUID attached, fall back to the path as key for them. --- src/esx/esx_storage_driver.c | 190 src/esx/esx_util.c | 19 src/esx/esx_util.h | 2 + src/esx/esx_vi.c | 53 +++ src/esx/esx_vi.h | 4 + src/esx/esx_vi_generator.input | 7 ++ 6 files changed, 256 insertions(+), 19 deletions(-) ACK. Thanks, pushed. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] esx: Fall back to path as key when QueryVirtualDiskUuid isn't available
QueryVirtualDiskUuid is only available on an ESX(i) server. vCenter returns an NotImplemented fault and a GSX server is missing the VirtualDiskManager completely. Therefore only use QueryVirtualDiskUuid with an ESX(i) server and fall back to path as storage volume key for vCenter and GSX server. --- src/esx/esx_storage_driver.c | 36 +++-- src/esx/esx_vi.c | 50 +++-- src/esx/esx_vi.h |1 + 3 files changed, 62 insertions(+), 25 deletions(-) diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index 76f8769..329020b 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -783,6 +783,13 @@ esxStorageVolumeLookupByKey(virConnectPtr conn, const char *key) return esxStorageVolumeLookupByPath(conn, key); } +if (!priv-primary-hasQueryVirtualDiskUuid) { +ESX_ERROR(VIR_ERR_INTERNAL_ERROR, %s, + _(QueryVirtualDiskUuid not avialable, cannot lookup storage +volume by UUID)); +return NULL; +} + if (esxVI_EnsureSession(priv-primary) 0) { return NULL; } @@ -916,7 +923,7 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; char *uuid_string = NULL; -char key[VIR_UUID_STRING_BUFLEN] = ; +char *key = NULL; virCheckFlags(0, NULL); @@ -1068,14 +1075,26 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, goto cleanup; } -if (esxVI_QueryVirtualDiskUuid(priv-primary, datastorePath, - priv-primary-datacenter-_reference, - uuid_string) 0) { -goto cleanup; -} +if (priv-primary-hasQueryVirtualDiskUuid) { +if (VIR_ALLOC_N(key, VIR_UUID_STRING_BUFLEN) 0) { +virReportOOMError(); +goto cleanup; +} -if (esxUtil_ReformatUuid(uuid_string, key) 0) { -goto cleanup; +if (esxVI_QueryVirtualDiskUuid(priv-primary, datastorePath, + priv-primary-datacenter-_reference, + uuid_string) 0) { +goto cleanup; +} + +if (esxUtil_ReformatUuid(uuid_string, key) 0) { +goto cleanup; +} +} else { +/* Fall back to the path as key */ +if (esxVI_String_DeepCopyValue(key, datastorePath) 0) { +goto cleanup; +} } } else { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, @@ -1103,6 +1122,7 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, esxVI_FileBackedVirtualDiskSpec_Free(virtualDiskSpec); esxVI_ManagedObjectReference_Free(task); VIR_FREE(uuid_string); +VIR_FREE(key); return volume; } diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 00e15f0..78cfdfd 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -453,6 +453,17 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, return -1; } +if (ctx-productVersion esxVI_ProductVersion_ESX) { +/* + * FIXME: Actually this should be detected by really calling + * QueryVirtualDiskUuid and checking if a NotImplemented fault is + * returned. But currently we don't deserialized the details of a + * possbile fault and therefore we don't know if the fault was a + * NotImplemented fault or not. + */ +ctx-hasQueryVirtualDiskUuid = true; +} + if (esxVI_Login(ctx, username, password, NULL, ctx-session) 0 || esxVI_BuildSelectSetCollection(ctx) 0) { return -1; @@ -3267,28 +3278,33 @@ esxVI_LookupStorageVolumeKeyByDatastorePath(esxVI_Context *ctx, return -1; } -if (esxVI_LookupFileInfoByDatastorePath(ctx, datastorePath, false, fileInfo, -esxVI_Occurrence_RequiredItem) 0) { -goto cleanup; -} - -if (esxVI_VmDiskFileInfo_DynamicCast(fileInfo) != NULL) { -/* VirtualDisks have a UUID, use it as key */ -if (esxVI_QueryVirtualDiskUuid(ctx, datastorePath, - ctx-datacenter-_reference, - uuid_string) 0) { +if (ctx-hasQueryVirtualDiskUuid) { +if (esxVI_LookupFileInfoByDatastorePath + (ctx, datastorePath, false, fileInfo, + esxVI_Occurrence_RequiredItem) 0) { goto cleanup; } -if (VIR_ALLOC_N(*key, VIR_UUID_STRING_BUFLEN) 0) { -virReportOOMError(); -goto cleanup; -} +if (esxVI_VmDiskFileInfo_DynamicCast(fileInfo) != NULL) { +/* VirtualDisks have