- Devel - libvirt List Archives

[libvirt] [PATCH]: Bug when you try to start machines made will older VBox versions
by Pritesh Kothari 14 Sep '09

14 Sep '09

Hi All, Thanks to Florian, who pointed out the bug, due to which you can't start domains defined with older builds or with empty "FRONTEND/Type" tag in their xml files. Fixed it in the following patch. Regards, Pritesh

2 1

[libvirt] [PATCH 0/4] Changes to API/ABI for secrets handling
by Daniel P. Berrange 14 Sep '09

14 Sep '09

This series makes two changes to the public API and wire protocol ABI for the secrets handling support recently committed. As such this is a must fix before we do the release, since we can't break compatability once released. The first patch changes all the code to store UUIDs in raw format instead of printable format. This is important because it ensures that e3a9758f-b0c6-7a3a-ebb9-71a69c930289 is treated exactly the same as e3a9758fb0c67a3aebb971a69c930289, which is not the case with the currently committed code. It also makes the secrets and storage encryption XML parsers use the internal UUID handling APIs The second patch adds a new unique property to virSecretPtr. The "owner ID" is intended to be unique on a host wrt all secrets with matching usage type. This entails adding 2 APIs to querying the usage type, and another API to lookup a secret based on its usage. In testing the current code I frequently found myself getting into the situation where there were many secrets for the same volume which was not at all good. So if defining a secret to be used with a storage volume this ensures that you only ever have one secret associated with each storage volume path. The third patch ensures that when you query the XML for a qcow encrypted voluem, it will include the UUID of the secret. Previously if you created a volume, and then restarted libvirtd, the secret UUID would be lost. This patch is why we need the new API from the second patch virLookupSecretByUsage - without that it would be impossible to lookup the secret efficiently. THe final patch is a minor tweak to the RNG

3 13

[libvirt] [PATCH] Fix win32 platform build
by Daniel P. Berrange 14 Sep '09

14 Sep '09

* configure.in: Only define WITH_SECRETS if libvirtd is present * src/Makefile.am: Only build secrets driver if WITH_SECRETS is defined. Always add SECRET_DRIVER_SOURCES to EXTRA_DIST --- configure.in | 11 +++++++++++ src/Makefile.am | 3 +++ 2 files changed, 14 insertions(+), 0 deletions(-) diff --git a/configure.in b/configure.in index 120f7e7..fad1297 100644 --- a/configure.in +++ b/configure.in @@ -1062,6 +1062,17 @@ AM_CONDITIONAL([WITH_NETCF], [test "$with_netcf" = "yes"]) AC_SUBST([NETCF_CFLAGS]) AC_SUBST([NETCF_LIBS]) + +with_secrets=yes +if test "$with_libvirtd" = "no"; then + with_secrets=no +fi +if test "$with_secrets" = "yes" ; then + AC_DEFINE_UNQUOTED([WITH_SECRETS], 1, [whether local secrets management driver is available]) +fi +AM_CONDITIONAL([WITH_SECRETS], [test "$with_secrets" = "yes"]) + + AC_ARG_WITH([storage-fs], [ --with-storage-fs with FileSystem backend for the storage driver (on)],[],[with_storage_fs=check]) AC_ARG_WITH([storage-lvm], diff --git a/src/Makefile.am b/src/Makefile.am index 14a3a63..69f5ee7 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -461,6 +461,7 @@ endif libvirt_driver_interface_la_SOURCES = $(INTERFACE_DRIVER_SOURCES) endif +if WITH_SECRETS if WITH_DRIVER_MODULES mod_LTLIBRARIES += libvirt_driver_secret.la else @@ -471,6 +472,7 @@ if WITH_DRIVER_MODULES libvirt_driver_secret_la_LDFLAGS = -module -avoid-version endif libvirt_driver_secret_la_SOURCES = $(SECRET_DRIVER_SOURCES) +endif # Needed to keep automake quiet about conditionals libvirt_driver_storage_la_SOURCES = @@ -574,6 +576,7 @@ EXTRA_DIST += \ $(NODE_DEVICE_DRIVER_HAL_SOURCES) \ $(NODE_DEVICE_DRIVER_DEVKIT_SOURCES) \ $(SECURITY_DRIVER_SELINUX_SOURCES) \ + $(SECRET_DRIVER_SOURCES) \ $(VBOX_DRIVER_EXTRA_DIST) # -- 1.6.2.5

2 1

[libvirt] ESX [07/12]: Whitespace cleanup
by Matthias Bolte 14 Sep '09

14 Sep '09

Subject says it all.

2 1

[libvirt] ESX [05/12]: Also strip (C) and (TM) from the CPU model string
by Matthias Bolte 14 Sep '09

14 Sep '09

* src/esx/esx_driver.c: also strip (C) and (TM) from the CPU model string

2 1

[libvirt] ESX [11/12]: Reorder some function calls to avoid potential virDomainPtr leaks
by Matthias Bolte 14 Sep '09

14 Sep '09

* src/esx/esx_driver.c: avoid potential virDomainPtr leaks

2 1

[libvirt] [PATCH]: API Change patch for VBox 3.0.6
by Pritesh Kothari 14 Sep '09

14 Sep '09

Hi All, VirtualBox 3.0.6 was released few days ago and there were some minor API changes in it. I have created a patch for the same and posting it here. Regards, Pritesh

2 1

[libvirt] ESX [12/12]: Remove phantom mode
by Matthias Bolte 14 Sep '09

14 Sep '09

* src/esx/esx_driver.c: remove phantom mode

1 0

[libvirt] ESX [10/12]: Replace esxUtil_EqualSuffix() with virFileHasSuffix()
by Matthias Bolte 14 Sep '09

14 Sep '09

* src/esx/esx_util.[ch]: remove esxUtil_EqualSuffix() * src/esx/esx_driver.c, src/esx/esx_vmx.c: replace esxUtil_EqualSuffix() with virFileHasSuffix()

1 0

[libvirt] ESX [09/12]: Add esxDomainDefineXML()
by Matthias Bolte 14 Sep '09

14 Sep '09

A given domain XML gets converted to a VMX config, uploaded to the host and registered as new virtual machine. * src/esx/esx_driver.c: refactor datastore related path parsing into esxUtil_ParseDatastoreRelatedPath() * src/esx/esx_util.[ch]: add esxUtil_ParseDatastoreRelatedPath() * src/esx/esx_vi.[ch]: add esxVI_Context_UploadFile(), add datastores to the traversal in esxVI_BuildFullTraversalSpecList(), add esxVI_LookupDatastoreByName() * src/esx/esx_vi_methods.[ch]: add esxVI_RegisterVM_Task() * src/esx/esx_vi_types.c: make some error message more verbose * src/esx/esx_vmx.[ch]: add esxVMX_AbsolutePathToDatastoreRelatedPath() to convert a path into a datastore related path, add esxVMX_ParseFileName() to convert from VMX path format to domain XML path format, extend the other parsing function to be datastore aware, add esxVMX_FormatFileName() to convert from domain XML path format to VMX path format, fix VMX ethernet entry formating * tests/esxutilstest.c: add test for esxUtil_ParseDatastoreRelatedPath() * tests/vmx2xmldata/*: update domain XML files to use datastore related paths * tests/xml2vmxdata/*: update domain XML files to use datastore related paths, update VMX files to use absolute paths

1 0

[libvirt] ESX [08/12]: Add esxVI_Occurence enum to describe expected occurence of items
by Matthias Bolte 14 Sep '09

14 Sep '09

* src/esx/esx_driver.c: update the use of esxVI_LookupVirtualMachineByUuid() * src/esx/esx_vi.c: add an esxVI_Occurence parameter to esxVI_LookupVirtualMachineByUuid() and take care if esxVI_FindByUuid() can't find anything for a given uuid * src/esx/esx_vi.h: add esxVI_Occurence enum * src/esx/esx_vi_methods.c: expect null or more items to be returned from esxVI_FindByUuid()

1 0

[libvirt] ESX [06/12]: Add x86_64 detection based on the CPUID
by Matthias Bolte 14 Sep '09

14 Sep '09

* src/esx/esx_driver.c: add esxSupportsLongMode() and update esxCapsInit() * src/esx/esx_vi.[ch]: Add AnyType handling for lists * src/esx/esx_vi_types.c: bind VI type HostCpuIdInfo

1 0

[libvirt] ESX [04/12]: Add tests for the VMX to/from domain XML mapping
by Matthias Bolte 14 Sep '09

14 Sep '09

* tests/.gitignore: ignore new test binaries * tests/Makefile.am: add new tests * tests/esxutilstest.c: test esxVMX_IndexToDiskName() * tests/vmx2xmldata/*: config files for the VMX to domain XML test * tests/vmx2xmltest.c: test the VMX to domain XML mapping * tests/xml2vmxdata/*: config files for the domain XML to VMX test * tests/xml2vmxtest.c: test the domain XML to VMX mapping

1 0

[libvirt] ESX [03/12]: Add esxDomainXMLToNative()
by Matthias Bolte 14 Sep '09

14 Sep '09

Extend and cleanup the VMX to domain XML mapping. Add the domain XML to VMX mapping functions. * src/esx/esx_driver.c: add esxDomainXMLToNative() * src/esx/esx_vmx.[ch]: add esxVMX_SCSIDiskNameToControllerAndID(), esxVMX_IDEDiskNameToControllerAndID(), esxVMX_FloppyDiskNameToController(), esxVMX_GatherSCSIControllers(), add basic handling for the VMX guestOS entry to distinguish between i686 and x86_64, make SCSI virtualDev VMX entry optional as it should be, map the VMX networkName entry to the domain XML interface bridge name, add basic mapping for serial devices in pipe mode, add several esxVMX_Format*() functions

1 0

[libvirt] ESX [02/12]: Set challenge for auth callback to hostname
by Matthias Bolte 14 Sep '09

14 Sep '09

This enables the auth callback to automatically distinguish between requests for ESX host and vCenter credentials. * src/esx/esx_util.[ch]: set challenge for auth callback to hostname

1 0

[libvirt] Interface driver and ESX support
by Shahar Klein 13 Sep '09

13 Sep '09

I'm trying to get a feel of the python binding and the esx driver Trying to get the number of interfaces hit(as anticipated) a non-support wall: Traceback (most recent call last): File "test.py", line 362, in ? vmm.refresh() File "test.py", line 189, in refresh print self.conn.numOfDefinedInterfaces() File "/usr/lib/python2.4/site-packages/libvirt.py", line 1347, in numOfDefinedInterfaces if ret == -1: raise libvirtError ('virConnectNumOfDefinedInterfaces() failed', conn=self) libvirt.libvirtError: this function is not supported by the hypervisor: virConnectNumOfDefinedInterfaces I'm wondering how to fix it can I register the esx driver as the interface driver? so I can use the ESX web-services to handle all the interface issues that normally(?) netcf would handle? if yes - how is it done? I thought that configure --without-netcf would do the trick(it didn't-I think) Thanks Shahar

4 14

[libvirt] Re: [PATCH 0/2] port over extboot from kvm
by Jan Kiszka 12 Sep '09

12 Sep '09

Gerd Hoffmann wrote: > On 09/08/09 15:47, Jan Kiszka wrote: > >> Before setting this definitely useful feature in stone, I have two >> questions though: >> >> - -drive ...,boot=on is logically in conflict with -boot. Yes, -boot >> for x86 currently cannot differentiate between multiple disks, only >> between boot media types. Still, this two-stage configuration is >> rather unintuitive and looks like a patchwork. Given that we have >> full control over all components, is it really the preferred >> approach? I already thought about, e.g., -boot c2 to select the >> second disk. Not that nice, but I would rather vote for a consistent >> configuration than a scattered one. > > Disk numbers are bad. Define "second hard disk". Especially for a > system with different kinds of disks (say one scsi and one virtio). One could use the specification order, but I agree it's not very handy. > > Drives have names though which can be used to reference the disks, so we > could use that instead. -boot cmd line syntax becomes a bit tricky then > though, we somehow have to figure whenever the user gave us names or > old-style letters. Something like this ... > > -drive if=virtio,id=sys,file=/path/to/disk.img > -cdrom /path/to/install.iso > -boot order=[sys],once=d,menu=off Yes, this looks powerful and clean. One could even still define probe orders like "-boot order=[sys][backup]d". > > ... might work out nicely. I suspect the libvirt folks will hate us for > that though. Does anyone from libvirt want to comment on this? > >> - This is just an implementation detail: Do we really need to implement >> booting from virtio and scsi via an extension rom? Isn't it possible >> to merge the corresponding support into the main bios? > > Well. There are quite a few. bochs pcbios, seabios, coreboot ... Ok, but that's only an argument to have extboot as a workaround for bioses not yet supporting scsi and virtio natively, isn't it? I'm thinking long-term here, not arguing against a extboot-based short-term solution. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux

4 6

[libvirt] vnet<X>'s & interfaceStats().. KVM device numbering confusion?
by r-c＠rocketmail.com 12 Sep '09

12 Sep '09

Hi libirt-list, I am attempting to migrate from Xen->KVM and am having a little trouble understanding the new vnet<X> interface numbering scheme and how to use it to collect statistics of the interface using the interfaceStats() function. In Xen, the ID for a given domain was know, and a corresponding vif<ID>.0 interface was brought up so it was easy to grab the stats (amongst other things). Under my new KVM config I've realised that vnet<X> is brought up in a 'first free' fashion as opposed to a counter in the Xen case. This leaves me scratching my head a little bit, say I have a domain which has ended up with ID = 8, but may for instance have network 'vnet1' associated with it. I'm not sure how I can determine any association between the domain & which interface belongs to it! (e.g. a call to domain.interfaceStats() requires the correct network interface as a parameter). Surely I must be missing something here? Anyone have any advice/workaround for this situation? Ideally I would like to either be able to hardcode vnet-labels in the XML description (tried but it's ignored), or otherwise keep vnet numbering in sync with domain ID. Thanks in advance, Ravi

2 2

[libvirt] [PATCH] Save vcpuinfo in status file
by Daniel P. Berrange 11 Sep '09

11 Sep '09

* src/qemu_driver.c: Don't trust monitor for vcpu PID info on restart * src/domain_conf.c: Save and load vCPU PID info from domain status file --- src/domain_conf.c | 35 +++++++++++++++++++++++++++++++++++ src/qemu_driver.c | 5 +---- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/domain_conf.c b/src/domain_conf.c index 050cf50..5ae0775 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -3061,6 +3061,8 @@ static virDomainObjPtr virDomainObjParseXML(virConnectPtr conn, xmlNodePtr oldnode; virDomainObjPtr obj; char *monitorpath; + xmlNodePtr *nodes = NULL; + int n, i; if (!(obj = virDomainObjNew(conn))) return NULL; @@ -3133,9 +3135,32 @@ static virDomainObjPtr virDomainObjParseXML(virConnectPtr conn, break; } + n = virXPathNodeSet(conn, "./vcpus/vcpu", ctxt, &nodes); + if (n < 0) + goto error; + if (n) { + obj->nvcpupids = n; + if (VIR_REALLOC_N(obj->vcpupids, obj->nvcpupids) < 0) + goto error; + + for (i = 0 ; i < n ; i++) { + char *pidstr = virXMLPropString(nodes[i], "pid"); + if (!pidstr) + goto error; + + if (virStrToLong_i(pidstr, NULL, 10, &(obj->vcpupids[i])) < 0) { + VIR_FREE(pidstr); + goto error; + } + VIR_FREE(pidstr); + } + VIR_FREE(nodes); + } + return obj; error: + VIR_FREE(nodes); virDomainChrDefFree(obj->monitor_chr); virDomainObjFree(obj); return NULL; @@ -4422,6 +4447,16 @@ char *virDomainObjFormat(virConnectPtr conn, virBufferVSprintf(&buf, " type='%s'/>\n", virDomainChrTypeToString(obj->monitor_chr->type)); + + if (obj->nvcpupids) { + int i; + virBufferAddLit(&buf, " <vcpus>\n"); + for (i = 0 ; i < obj->nvcpupids ; i++) { + virBufferVSprintf(&buf, " <vcpu pid='%d'/>\n", obj->vcpupids[i]); + } + virBufferAddLit(&buf, " </vcpus>\n"); + } + if (!(config_xml = virDomainDefFormat(conn, obj->def, flags))) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 0e7d8d4..a65334f 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -336,10 +336,6 @@ qemuReconnectDomain(struct qemud_driver *driver, goto error; } - if (qemudDetectVcpuPIDs(NULL, obj) < 0) { - goto error; - } - if (qemuUpdateActivePciHostdevs(driver, obj->def) < 0) { goto error; } @@ -2519,6 +2515,7 @@ qemudMonitorCommandWithHandler(const virDomainObjPtr vm, qemuMonitorDiscardPendingData(vm); + VIR_DEBUG("Send '%s'", cmd); if (qemudMonitorSend(vm, cmd, scm_fd) < 0) return -1; -- 1.6.2.5

2 1

[libvirt] Postponing 0.7.1 release by a couple of days
by Daniel Veillard 11 Sep '09

11 Sep '09

We still commited quite a bit in the last few days and I think taking a couple of extra days for testing is a bit safer, so I suggest to make the release only on Tuesday. So we have today and this week end for testing, push fixes if needed on Monday and I would make the release early on tuesday, 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/

1 0

[libvirt] [PATCH] Restart libvirtd upon RPM upgrade
by Daniel P. Berrange 11 Sep '09

11 Sep '09

* libvirt.spec.in: Restart libvirtd in %post --- libvirt.spec.in | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 0157eee..8d4c8cc 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -471,6 +471,9 @@ fi %endif /sbin/chkconfig --add libvirtd +if [ "$1" -ge "1" ]; then + /sbin/service libvirtd condrestart > /dev/null 2>&1 +fi %endif %preun -- 1.6.2.5

2 1

[libvirt][qemu] unable to start guest under kvm
by Anton Protopopov 11 Sep '09

11 Sep '09

Hi, all. I have the following problem: when I creating a guest, libvirt resolves <type ... machine="pc" ...> to <type ... machine="pc-0.11" ...> as provided by http://www.mail-archive.com/libvir-list@redhat.com/msg15014.html When <type ... machine="pc-0.11" ...> or <type ... machine="pc-0.10" ...> is set, I get two following errors (which would happend is unpredictable) # virsh -c qemu:///system start $d error: Failed to start domain Desktop error: internal error unable to start guest: Supported machines are: pc Standard PC (default) isapc ISA-only PC and # virsh -c qemu:///system start $d error: Failed to start domain Desktop error: monitor socket did not show up.: Connection refused When <type ... machine="pc" ...> is set, this guest starts successfully. The domain type of this machine is "kvm". But I have # qemu --version && kvm --help | head -1 QEMU PC emulator version 0.11.50, Copyright (c) 2003-2008 Fabrice Bellard QEMU PC emulator version 0.10.0 (kvm-devel), Copyright (c) 2003-2008 Fabrice Bellard on that HN. - why does libvirt sets machine to "pc-0.11" even if kvm qemu version is 0.10? - why does an error occure if I set <type ... machine="pc-0.10 ... ">

3 34

[libvirt] Volume encryption missing pieces
by Miloslav Trmač 10 Sep '09

10 Sep '09

Major changes since the previous submission: - Use one or two separate files per secret, using XML for metadata, instead of a single file for all secrets using a custom format. - Split <secret> XML handling from the local driver into secret_conf.[ch], use it when auto-generating secrets. - Use <usage type='volume'><volume/></usage> instead of <volume>. Also available at http://fedorapeople.org/gitweb?p=mitr/public_git/libvirt.git;a=summary .

4 22

[libvirt] [PATCH] Fix use of dlopen modules
by Daniel P. Berrange 10 Sep '09

10 Sep '09

Remove the bogus dependancy between node_device.c & storage_backend.c by moving the virWaitForDevices into util.h where it can be shared safely * src/storage_backend_disk.c, src/storage_backend_logical.c, src/storage_backend_mpath.c, src/storage_backend_scsi.c: Replace virStorageBackendWaitForDevices with virFileWaitForDevices * src/storage_backend.c, src/storage_backend.h: Remove virStorageBackendWaitForDevices, virWaitForDevices * src/util.h, src/util.c: Add virFileWaitForDevices * configure.in: Move xmlrpc check further down after pkgconfig is detected * src/Makefile.am: Add missing XMLRPC_CFLAGS/LIBS to opennebula * src/libvirt_private.syms: Add many missing exports --- configure.in | 6 +++--- src/Makefile.am | 4 ++-- src/libvirt_private.syms | 14 +++++++++++++- src/node_device.c | 4 ++-- src/storage_backend.c | 31 ------------------------------- src/storage_backend.h | 2 -- src/storage_backend_disk.c | 4 ++-- src/storage_backend_logical.c | 2 +- src/storage_backend_mpath.c | 2 +- src/storage_backend_scsi.c | 4 ++-- src/util.c | 27 +++++++++++++++++++++++++++ src/util.h | 2 ++ 12 files changed, 55 insertions(+), 47 deletions(-) diff --git a/configure.in b/configure.in index 060eb50..120f7e7 100644 --- a/configure.in +++ b/configure.in @@ -463,6 +463,9 @@ if test "$with_qemu" = "yes" -o "$with_lxc" = "yes" ; then fi +dnl Need to test if pkg-config exists +PKG_PROG_PKG_CONFIG + dnl OpenNebula driver Compilation setting dnl @@ -491,9 +494,6 @@ AC_SUBST([XMLRPC_CFLAGS]) AC_SUBST([XMLRPC_LIBS]) -dnl Need to test if pkg-config exists -PKG_PROG_PKG_CONFIG - dnl ========================================================================== dnl find libxml2 library, borrowed from xmlsec dnl ========================================================================== diff --git a/src/Makefile.am b/src/Makefile.am index 628edc5..02c1f74 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -396,8 +396,8 @@ noinst_LTLIBRARIES += libvirt_driver_one.la # Stateful, so linked to daemon instead #libvirt_la_LIBADD += libvirt_driver_one.la endif -libvirt_driver_one_la_CFLAGS = $(ONE_LIBS) -libvirt_driver_one_la_LDFLAGS = $(ONE_LIBS) +libvirt_driver_one_la_CFLAGS = $(XMLRPC_CFLAGS) +libvirt_driver_one_la_LDFLAGS = $(XMLRPC_LIBS) #libvirt_driver_one_la_CFLAGS = "-DWITH_ONE" if WITH_DRIVER_MODULES libvirt_driver_one_la_LDFLAGS += -module -avoid-version diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8dd279d..06192fc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -10,6 +10,7 @@ virBufferAdd; virBufferAddChar; virBufferContentAndReset; virBufferError; +virBufferURIEncodeString; # caps.h @@ -167,7 +168,16 @@ virHashSize; virInterfaceDefFormat; virInterfaceDefParseFile; virInterfaceDefParseString; +virInterfaceDefParseNode; virInterfaceDefFree; +virInterfaceFindByName; +virInterfaceFindByMACString; +virInterfaceAssignDef; +virInterfaceRemove; +virInterfaceObjLock; +virInterfaceObjUnlock; +virInterfaceObjListFree; + # interface_driver.h interfaceRegister; @@ -214,6 +224,7 @@ virRegisterNetworkDriver; virRegisterStateDriver; virRegisterStorageDriver; virRegisterDeviceMonitor; +virRegisterSecretDriver; # logging.h @@ -410,6 +421,7 @@ virGetUserName; virGetUserID; virGetGroupID; virFileFindMountPoint; +virFileWaitForDevices; # uuid.h @@ -421,7 +433,7 @@ virUUIDParse; # virterror_internal.h virReportErrorHelper; virErrorMsg; -virRaiseError; +virRaiseErrorFull; virReportSystemErrorFull; virReportOOMErrorFull; virStrerror; diff --git a/src/node_device.c b/src/node_device.c index ded203f..69e15b6 100644 --- a/src/node_device.c +++ b/src/node_device.c @@ -35,7 +35,7 @@ #include "node_device_conf.h" #include "node_device_hal.h" #include "node_device.h" -#include "storage_backend.h" /* For virWaitForDevices */ +#include "util.h" #define VIR_FROM_THIS VIR_FROM_NODEDEV @@ -599,7 +599,7 @@ find_new_device(virConnectPtr conn, const char *wwnn, const char *wwpn) while ((now - start) < LINUX_NEW_DEVICE_WAIT_TIME) { - virWaitForDevices(conn); + virFileWaitForDevices(conn); dev = nodeDeviceLookupByWWN(conn, wwnn, wwpn); diff --git a/src/storage_backend.c b/src/storage_backend.c index 5e04f35..0a20255 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -630,31 +630,6 @@ virStorageBackendGetBuildVolFromFunction(virConnectPtr conn, return virStorageBackendCreateRaw; } -#if defined(UDEVADM) || defined(UDEVSETTLE) -void virWaitForDevices(virConnectPtr conn) -{ -#ifdef UDEVADM - const char *const settleprog[] = { UDEVADM, "settle", NULL }; -#else - const char *const settleprog[] = { UDEVSETTLE, NULL }; -#endif - int exitstatus; - - if (access(settleprog[0], X_OK) != 0) - return; - - /* - * NOTE: we ignore errors here; this is just to make sure that any device - * nodes that are being created finish before we try to scan them. - * If this fails for any reason, we still have the backup of polling for - * 5 seconds for device nodes. - */ - virRun(conn, settleprog, &exitstatus); -} -#else -void virWaitForDevices(virConnectPtr conn ATTRIBUTE_UNUSED) {} -#endif - virStorageBackendPtr virStorageBackendForType(int type) { @@ -893,12 +868,6 @@ virStorageBackendUpdateVolTargetFormatFD(virConnectPtr conn, } -void virStorageBackendWaitForDevices(virConnectPtr conn) -{ - virWaitForDevices(conn); - return; -} - /* * Given a volume path directly in /dev/XXX, iterate over the * entries in the directory pool->def->target.path and find the diff --git a/src/storage_backend.h b/src/storage_backend.h index eb5bf87..88c6161 100644 --- a/src/storage_backend.h +++ b/src/storage_backend.h @@ -95,7 +95,6 @@ int virStorageBackendUpdateVolTargetFormatFD(virConnectPtr conn, virStorageVolTargetPtr target, int fd); -void virStorageBackendWaitForDevices(virConnectPtr conn); char *virStorageBackendStablePath(virConnectPtr conn, virStoragePoolObjPtr pool, @@ -128,6 +127,5 @@ int virStorageBackendRunProgNul(virConnectPtr conn, virStorageBackendListVolNulFunc func, void *data); -void virWaitForDevices(virConnectPtr conn); #endif /* __VIR_STORAGE_BACKEND_H__ */ diff --git a/src/storage_backend_disk.c b/src/storage_backend_disk.c index 6fdb566..e82959c 100644 --- a/src/storage_backend_disk.c +++ b/src/storage_backend_disk.c @@ -325,7 +325,7 @@ virStorageBackendDiskRefreshPool(virConnectPtr conn, VIR_FREE(pool->def->source.devices[0].freeExtents); pool->def->source.devices[0].nfreeExtent = 0; - virStorageBackendWaitForDevices(conn); + virFileWaitForDevices(conn); if (virStorageBackendDiskReadGeometry(conn, pool) != 0) { return -1; @@ -583,7 +583,7 @@ virStorageBackendDiskCreateVol(virConnectPtr conn, return -1; /* wait for device node to show up */ - virStorageBackendWaitForDevices(conn); + virFileWaitForDevices(conn); /* Blow away free extent info, as we're about to re-populate it */ VIR_FREE(pool->def->source.devices[0].freeExtents); diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c index 43117e8..4389120 100644 --- a/src/storage_backend_logical.c +++ b/src/storage_backend_logical.c @@ -478,7 +478,7 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn, }; int exitstatus; - virStorageBackendWaitForDevices(conn); + virFileWaitForDevices(conn); /* Get list of all logical volumes */ if (virStorageBackendLogicalFindLVs(conn, pool, NULL) < 0) { diff --git a/src/storage_backend_mpath.c b/src/storage_backend_mpath.c index ae02aac..fcc38ba 100644 --- a/src/storage_backend_mpath.c +++ b/src/storage_backend_mpath.c @@ -332,7 +332,7 @@ virStorageBackendMpathRefreshPool(virConnectPtr conn, pool->def->allocation = pool->def->capacity = pool->def->available = 0; - virStorageBackendWaitForDevices(conn); + virFileWaitForDevices(conn); virStorageBackendGetMaps(conn, pool); diff --git a/src/storage_backend_scsi.c b/src/storage_backend_scsi.c index a09103f..c70b1ed 100644 --- a/src/storage_backend_scsi.c +++ b/src/storage_backend_scsi.c @@ -494,7 +494,7 @@ virStorageBackendSCSIFindLUs(virConnectPtr conn, VIR_DEBUG(_("Discovering LUs on host %u"), scanhost); - virStorageBackendWaitForDevices(conn); + virFileWaitForDevices(conn); if (virAsprintf(&device_path, "/sys/bus/scsi/devices") < 0) { virReportOOMError(conn); @@ -542,7 +542,7 @@ virStorageBackendSCSIGetHostNumber(virConnectPtr conn, VIR_DEBUG(_("Finding host number from '%s'"), sysfs_path); - virStorageBackendWaitForDevices(conn); + virFileWaitForDevices(conn); sysdir = opendir(sysfs_path); diff --git a/src/util.c b/src/util.c index af50028..1878e33 100644 --- a/src/util.c +++ b/src/util.c @@ -2019,3 +2019,30 @@ cleanup: return ret; } #endif + +#ifndef PROXY +#if defined(UDEVADM) || defined(UDEVSETTLE) +void virFileWaitForDevices(virConnectPtr conn) +{ +#ifdef UDEVADM + const char *const settleprog[] = { UDEVADM, "settle", NULL }; +#else + const char *const settleprog[] = { UDEVSETTLE, NULL }; +#endif + int exitstatus; + + if (access(settleprog[0], X_OK) != 0) + return; + + /* + * NOTE: we ignore errors here; this is just to make sure that any device + * nodes that are being created finish before we try to scan them. + * If this fails for any reason, we still have the backup of polling for + * 5 seconds for device nodes. + */ + virRun(conn, settleprog, &exitstatus); +} +#else +void virFileWaitForDevices(virConnectPtr conn ATTRIBUTE_UNUSED) {} +#endif +#endif diff --git a/src/util.h b/src/util.h index 896e1b4..f9715ab 100644 --- a/src/util.h +++ b/src/util.h @@ -237,4 +237,6 @@ int virRandom(int max); char *virFileFindMountPoint(const char *type); #endif +void virFileWaitForDevices(virConnectPtr conn); + #endif /* __VIR_UTIL_H__ */ -- 1.6.2.5

2 2

[libvirt] libvir: QEMU error : remoteDomainProcessEvent: unmarshalling msg
by Kenneth Nagin 10 Sep '09

10 Sep '09

I am monitoring events using the example event monitor, event-test, but remoteDomainProcessEvent is outputing an "unmarshalling msg" error when creating a VM. I've installed libvirt 0.6.5. It is configured without XEN; otherwise it is the default configuration. Any suggestions on how to overcome the problem? Kenneth Nagin

3 3

[libvirt] [PATCH 1/5] Add esx and tcp migration uri transports to capabilities schema
by Mark McLoughlin 10 Sep '09

10 Sep '09

* docs/schemas/capabilities.rng: include esx and tcp uri_transport --- docs/schemas/capability.rng | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 1e3c5f3..dbfe1c5 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -79,6 +79,8 @@ <oneOrMore> <element name='uri_transport'> <choice> + <value>esx</value> + <value>tcp</value> <value>xenmigr</value> </choice> </element> -- 1.6.2.5

2 10

[libvirt] [PATCH] Fix regression from "Avoid polling on FDs with no events"
by Chris Lalancette 10 Sep '09

10 Sep '09

After the mentioned patch was applied, I noticed that shutting down a kvm guest from inside (i.e. poweroff) caused the guest to shutdown, but not removed from the list of active guests. DanB pointed out that the problem is that the virEventAddHandle() call in the qemu driver was asking to watch for 0 events, not HANGUP | ERROR as it should. Add these events so that shutdown works again. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/qemu_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 683fe20..996bfff 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -979,7 +979,8 @@ qemudOpenMonitorCommon(virConnectPtr conn, if (ret != 0) return ret; - if ((vm->monitorWatch = virEventAddHandle(vm->monitor, 0, + if ((vm->monitorWatch = virEventAddHandle(vm->monitor, + VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR, qemudDispatchVMEvent, driver, NULL)) < 0) return -1; -- 1.6.0.6

2 2

[libvirt] [PATCH] Reintroduce lzop compression
by Charles Duffy 10 Sep '09

10 Sep '09

3 4

[libvirt] [PATCH] Close logfile fd after spawning qemu
by Ryota Ozaki 10 Sep '09

10 Sep '09

Hi, This patch closes logfile fd after spawing qemu in qemudStartVMDaemon. The fd seems to be closed in the error path, but not in the normal path. The fd is passed to virExecDaemonize though, but looks not being closed inside it. Eventually, the fd is never closed during libvirtd lifetime. Thanks, ozaki-r >From b3d3e0f24c5df5c7677e539bbc2598f2d7fbc3b8 Mon Sep 17 00:00:00 2001 From: Ryota Ozaki <ozaki.ryota(a)gmail.com> Date: Thu, 10 Sep 2009 12:53:56 +0900 Subject: [PATCH] Close logfile fd after spawning qemu --- src/qemu_driver.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 8f16e72..f2182c4 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -2187,6 +2187,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, VIR_EXEC_NONBLOCK | VIR_EXEC_CLEAR_CAPS, qemudSecurityHook, &hookData, pidfile); + close(logfile); VIR_FREE(pidfile); /* wait for qemu process to to show up */ -- 1.6.0.6

3 5

[libvirt] 2 patches: dead increment and 4 useless (always-false) disjuncts
by Jim Meyering 10 Sep '09

10 Sep '09

>From 74f57af2010f9cbe2315d625c6502a0b259e6802 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 7 Sep 2009 10:03:22 +0200 Subject: [PATCH 1/2] xm_internal.c: remove dead increment of "data" * src/xm_internal.c (xenXMDomainConfigParse): Don't increment it. --- src/xm_internal.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/xm_internal.c b/src/xm_internal.c index f206c8c..18613d4 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -1314,7 +1314,6 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { if (!(data = strchr(key, '=')) || (data[0] == '\0')) break; - data++; if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { if (STRPREFIX(key, "vncunused=")) { -- 1.6.4.2.419.gab238 >From d4db2dfaafbf827d1c8b8626a3dbdaa9f371e479 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 7 Sep 2009 10:09:20 +0200 Subject: [PATCH 2/2] xm_internal.c: remove four useless comparisons after strchr * src/xm_internal.c (xenXMDomainConfigParse): After t=strchr... don't test *t; it's known. This was *not* detected by clang, but I spotted it since once instance was in the vicinity of the dead increment of "data". --- src/xm_internal.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/xm_internal.c b/src/xm_internal.c index 18613d4..e7f6a55 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -862,7 +862,7 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { */ /* Extract the source file path*/ - if (!(offset = strchr(head, ',')) || offset[0] == '\0') + if (!(offset = strchr(head, ','))) goto skipdisk; if ((offset - head) >= (PATH_MAX-1)) goto skipdisk; @@ -882,7 +882,7 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { head = head + 6; /* Extract the dest device name */ - if (!(offset = strchr(head, ',')) || offset[0] == '\0') + if (!(offset = strchr(head, ','))) goto skipdisk; if (VIR_ALLOC_N(disk->dst, (offset - head) + 1) < 0) goto no_memory; @@ -1018,7 +1018,7 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { char *data; char *nextkey = strchr(key, ','); - if (!(data = strchr(key, '=')) || (data[0] == '\0')) + if (!(data = strchr(key, '='))) goto skipnic; data++; @@ -1312,7 +1312,7 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { nextkey++; } - if (!(data = strchr(key, '=')) || (data[0] == '\0')) + if (!(data = strchr(key, '='))) break; if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { -- 1.6.4.2.419.gab238

3 5

[libvirt] [PATCH] Fix misc thread locking bugs / bogus warnings
by Daniel P. Berrange 10 Sep '09

10 Sep '09

Fix all thread locking bugs reported by object-locking test case. NB, some of the driver locking is getting too coarse. Driver mutexes really need to be turned into RW locks instead to significantly increase concurrency. * src/lxc_driver.c: Fix useof driver when unlocked in the methods lxcDomainGetInfo, lxcSetSchedulerParameters, and lxcGetSchedulerParameters * src/opennebula/one_driver.c: Fix missing unlock in oneDomainUndefine. Fix use of driver when unlocked in oneDomainGetInfo, oneGetOSType, oneDomainShutdown * src/qemu_driver.c: Fix use of driver when unlocked in qemudDomainSavem, qemuGetSchedulerType, qemuSetSchedulerParameters and qemuGetSchedulerParameters * src/storage_driver.c: Re-work storagePoolCreate to avoid bogus lock checking warning. Re-work storageVolumeCreateXMLFrom to remove a potential NULL de-reference & avoid bogus lock check warnings * src/test.c: Remove testDomainAssignDef since it break lock chekc warnings. * tests/object-locking.ml: Add oneDriverLock, oneDriverUnlock and one_driver_t methods/types to allow lock checking on the OpenNebula drivers --- src/lxc_driver.c | 6 ++-- src/opennebula/one_driver.c | 35 +++++++++++++++--------- src/qemu_driver.c | 60 +++++++++++++++++++++++------------------- src/storage_driver.c | 35 ++++++++++++------------- src/test.c | 55 ++++++++++++++++++++------------------- tests/object-locking.ml | 7 +++- 6 files changed, 108 insertions(+), 90 deletions(-) diff --git a/src/lxc_driver.c b/src/lxc_driver.c index bd0cf0e..0ec1e92 100644 --- a/src/lxc_driver.c +++ b/src/lxc_driver.c @@ -439,7 +439,6 @@ static int lxcDomainGetInfo(virDomainPtr dom, lxcDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); - lxcDriverUnlock(driver); if (!vm) { lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN, @@ -470,6 +469,7 @@ static int lxcDomainGetInfo(virDomainPtr dom, ret = 0; cleanup: + lxcDriverUnlock(driver); if (cgroup) virCgroupFree(&cgroup); if (vm) @@ -1667,7 +1667,6 @@ static int lxcSetSchedulerParameters(virDomainPtr domain, lxcDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, domain->uuid); - lxcDriverUnlock(driver); if (vm == NULL) { lxcError(NULL, domain, VIR_ERR_INTERNAL_ERROR, @@ -1698,6 +1697,7 @@ static int lxcSetSchedulerParameters(virDomainPtr domain, ret = 0; cleanup: + lxcDriverUnlock(driver); virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); @@ -1725,7 +1725,6 @@ static int lxcGetSchedulerParameters(virDomainPtr domain, lxcDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, domain->uuid); - lxcDriverUnlock(driver); if (vm == NULL) { lxcError(NULL, domain, VIR_ERR_INTERNAL_ERROR, @@ -1745,6 +1744,7 @@ static int lxcGetSchedulerParameters(virDomainPtr domain, ret = 0; cleanup: + lxcDriverUnlock(driver); virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); diff --git a/src/opennebula/one_driver.c b/src/opennebula/one_driver.c index 135397c..9587a2d 100644 --- a/src/opennebula/one_driver.c +++ b/src/opennebula/one_driver.c @@ -310,6 +310,8 @@ static int oneDomainUndefine(virDomainPtr dom) ret=0; return_point: + if (vm) + virDomainObjUnlock(vm); oneDriverUnlock(driver); return ret; } @@ -392,9 +394,9 @@ static int oneDomainGetInfo(virDomainPtr dom, static char *oneGetOSType(virDomainPtr dom) { - one_driver_t *driver = (one_driver_t *)dom->conn->privateData; virDomainObjPtr vm = NULL; + char *ret = NULL; oneDriverLock(driver); vm =virDomainFindByUUID(&driver->domains, dom->uuid); @@ -402,11 +404,17 @@ static char *oneGetOSType(virDomainPtr dom) if (!vm) { oneError(dom->conn,dom, VIR_ERR_INVALID_DOMAIN, "%s", _("no domain with matching uuid")); - return NULL; + goto cleanup; } - virDomainObjUnlock(vm); - return strdup(vm->def->os.type); + ret = strdup(vm->def->os.type); + if (!ret) + virReportOOMError(dom->conn); + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; } static int oneDomainStart(virDomainPtr dom) @@ -499,24 +507,25 @@ static int oneDomainShutdown(virDomainPtr dom) int ret=-1; oneDriverLock(driver); - if((vm=virDomainFindByID(&driver->domains, dom->id))) { - if(!(c_oneShutdown(vm->pid) ) ) { - vm->state=VIR_DOMAIN_SHUTDOWN; - ret= 0; - goto return_point; - } + if (!(vm=virDomainFindByID(&driver->domains, dom->id))) { + oneError(dom->conn,dom, VIR_ERR_INVALID_DOMAIN, + _("no domain with id %d"), dom->id); + goto return_point; + } + + if (c_oneShutdown(vm->pid)) { oneError(dom->conn, dom, VIR_ERR_OPERATION_INVALID, _("Wrong state to perform action")); goto return_point; } - oneError(dom->conn,dom, VIR_ERR_INVALID_DOMAIN, - _("no domain with id %d"), dom->id); - goto return_point; + vm->state=VIR_DOMAIN_SHUTDOWN; + ret= 0; if (!vm->persistent) { virDomainRemoveInactive(&driver->domains, vm); vm = NULL; } + return_point: if(vm) virDomainObjUnlock(vm); diff --git a/src/qemu_driver.c b/src/qemu_driver.c index d45d33a..b4cd63d 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3575,24 +3575,6 @@ static int qemudDomainSave(virDomainPtr dom, memcpy(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic)); header.version = QEMUD_SAVE_VERSION; - if (driver->saveImageFormat == NULL) - header.compressed = QEMUD_SAVE_FORMAT_RAW; - else if (STREQ(driver->saveImageFormat, "raw")) - header.compressed = QEMUD_SAVE_FORMAT_RAW; - else if (STREQ(driver->saveImageFormat, "gzip")) - header.compressed = QEMUD_SAVE_FORMAT_GZIP; - else if (STREQ(driver->saveImageFormat, "bzip2")) - header.compressed = QEMUD_SAVE_FORMAT_BZIP2; - else if (STREQ(driver->saveImageFormat, "lzma")) - header.compressed = QEMUD_SAVE_FORMAT_LZMA; - else if (STREQ(driver->saveImageFormat, "lzop")) - header.compressed = QEMUD_SAVE_FORMAT_LZOP; - else { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, - "%s", _("Invalid save image format specified in configuration file")); - return -1; - } - qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -3632,6 +3614,24 @@ static int qemudDomainSave(virDomainPtr dom, } header.xml_len = strlen(xml) + 1; + if (driver->saveImageFormat == NULL) + header.compressed = QEMUD_SAVE_FORMAT_RAW; + else if (STREQ(driver->saveImageFormat, "raw")) + header.compressed = QEMUD_SAVE_FORMAT_RAW; + else if (STREQ(driver->saveImageFormat, "gzip")) + header.compressed = QEMUD_SAVE_FORMAT_GZIP; + else if (STREQ(driver->saveImageFormat, "bzip2")) + header.compressed = QEMUD_SAVE_FORMAT_BZIP2; + else if (STREQ(driver->saveImageFormat, "lzma")) + header.compressed = QEMUD_SAVE_FORMAT_LZMA; + else if (STREQ(driver->saveImageFormat, "lzop")) + header.compressed = QEMUD_SAVE_FORMAT_LZOP; + else { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("Invalid save image format specified in configuration file")); + goto cleanup; + } + /* Write header to file, followed by XML */ if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, @@ -4429,7 +4429,9 @@ static char *qemuDomainXMLFromNative(virConnectPtr conn, goto cleanup; } + qemuDriverLock(driver); def = qemuParseCommandLineString(conn, driver->caps, config); + qemuDriverUnlock(driver); if (!def) goto cleanup; @@ -6183,12 +6185,13 @@ static char *qemuGetSchedulerType(virDomainPtr dom, int *nparams) { struct qemud_driver *driver = dom->conn->privateData; - char *ret; + char *ret = NULL; + qemuDriverLock(driver); if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, __FUNCTION__); - return NULL; + goto cleanup; } if (nparams) @@ -6197,6 +6200,9 @@ static char *qemuGetSchedulerType(virDomainPtr dom, ret = strdup("posix"); if (!ret) virReportOOMError(dom->conn); + +cleanup: + qemuDriverUnlock(driver); return ret; } @@ -6210,15 +6216,14 @@ static int qemuSetSchedulerParameters(virDomainPtr dom, virDomainObjPtr vm = NULL; int ret = -1; + qemuDriverLock(driver); if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, __FUNCTION__); - return -1; + goto cleanup; } - qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); if (vm == NULL) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, @@ -6261,6 +6266,7 @@ cleanup: virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); + qemuDriverUnlock(driver); return ret; } @@ -6275,21 +6281,20 @@ static int qemuGetSchedulerParameters(virDomainPtr dom, int ret = -1; int rc; + qemuDriverLock(driver); if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, __FUNCTION__); - return -1; + goto cleanup; } if ((*nparams) != 1) { qemudReportError(dom->conn, domain, NULL, VIR_ERR_INVALID_ARG, "%s", _("Invalid parameter count")); - return -1; + goto cleanup; } - qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); if (vm == NULL) { qemudReportError(dom->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, @@ -6319,6 +6324,7 @@ cleanup: virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); + qemuDriverUnlock(driver); return ret; } diff --git a/src/storage_driver.c b/src/storage_driver.c index e9ecb20..14e3bc2 100644 --- a/src/storage_driver.c +++ b/src/storage_driver.c @@ -489,24 +489,27 @@ storagePoolCreate(virConnectPtr conn, def = NULL; if (backend->startPool && - backend->startPool(conn, pool) < 0) + backend->startPool(conn, pool) < 0) { + virStoragePoolObjRemove(&driver->pools, pool); + pool = NULL; goto cleanup; + } if (backend->refreshPool(conn, pool) < 0) { if (backend->stopPool) backend->stopPool(conn, pool); + virStoragePoolObjRemove(&driver->pools, pool); + pool = NULL; goto cleanup; } pool->active = 1; ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid); - virStoragePoolObjUnlock(pool); - pool = NULL; cleanup: virStoragePoolDefFree(def); if (pool) - virStoragePoolObjRemove(&driver->pools, pool); + virStoragePoolObjUnlock(pool); storageDriverUnlock(driver); return ret; } @@ -1321,27 +1324,23 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj, virStorageBackendPtr backend; virStorageVolDefPtr origvol = NULL, newvol = NULL; virStorageVolPtr ret = NULL, volobj = NULL; - int buildret, diffpool; - - diffpool = !STREQ(obj->name, vobj->pool); + int buildret; storageDriverLock(driver); pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); - if (diffpool) { + if (pool && STRNEQ(obj->name, vobj->pool)) { virStoragePoolObjUnlock(pool); origpool = virStoragePoolObjFindByName(&driver->pools, vobj->pool); virStoragePoolObjLock(pool); - } else - origpool = pool; + } storageDriverUnlock(driver); - if (!pool) { virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL, "%s", _("no storage pool with matching uuid")); goto cleanup; } - if (diffpool && !origpool) { + if (STRNEQ(obj->name, vobj->pool) && !origpool) { virStorageReportError(obj->conn, VIR_ERR_NO_STORAGE_POOL, _("no storage pool with matching name '%s'"), vobj->pool); @@ -1354,7 +1353,7 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj, goto cleanup; } - if (diffpool && !virStoragePoolObjIsActive(origpool)) { + if (origpool && !virStoragePoolObjIsActive(origpool)) { virStorageReportError(obj->conn, VIR_ERR_INTERNAL_ERROR, "%s", _("storage pool is not active")); goto cleanup; @@ -1363,7 +1362,7 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj, if ((backend = virStorageBackendForType(pool->def->type)) == NULL) goto cleanup; - origvol = virStorageVolDefFindByName(origpool, vobj->name); + origvol = virStorageVolDefFindByName(origpool ? origpool : pool, vobj->name); if (!origvol) { virStorageReportError(obj->conn, VIR_ERR_NO_STORAGE_VOL, _("no storage vol with matching name '%s'"), @@ -1429,7 +1428,7 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj, newvol->building = 1; virStoragePoolObjUnlock(pool); - if (diffpool) { + if (origpool) { origpool->asyncjobs++; virStoragePoolObjUnlock(origpool); } @@ -1438,7 +1437,7 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj, storageDriverLock(driver); virStoragePoolObjLock(pool); - if (diffpool) + if (origpool) virStoragePoolObjLock(origpool); storageDriverUnlock(driver); @@ -1447,7 +1446,7 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj, newvol = NULL; pool->asyncjobs--; - if (diffpool) { + if (origpool) { origpool->asyncjobs--; virStoragePoolObjUnlock(origpool); origpool = NULL; @@ -1469,7 +1468,7 @@ cleanup: virStorageVolDefFree(newvol); if (pool) virStoragePoolObjUnlock(pool); - if (diffpool && origpool) + if (origpool) virStoragePoolObjUnlock(origpool); return ret; } diff --git a/src/test.c b/src/test.c index 7c8f85b..0e4203e 100644 --- a/src/test.c +++ b/src/test.c @@ -261,12 +261,10 @@ testDomainGenerateIfname(virConnectPtr conn, return NULL; } -static virDomainObjPtr -testDomainAssignDef(virConnectPtr conn, - virDomainObjList *domlist, - virDomainDefPtr domdef) +static int +testDomainGenerateIfnames(virConnectPtr conn, + virDomainDefPtr domdef) { - virDomainObjPtr domobj = NULL; int i = 0; for (i = 0; i < domdef->nnets; i++) { @@ -276,18 +274,15 @@ testDomainAssignDef(virConnectPtr conn, ifname = testDomainGenerateIfname(conn, domdef); if (!ifname) - goto error; + return -1; domdef->nets[i]->ifname = ifname; } - if (!(domobj = virDomainAssignDef(conn, domlist, domdef))) - goto error; - -error: - return domobj; + return 0; } + static int testOpenDefault(virConnectPtr conn) { int u; struct timeval tv; @@ -342,10 +337,11 @@ static int testOpenDefault(virConnectPtr conn) { defaultDomainXML, VIR_DOMAIN_XML_INACTIVE))) goto error; - if (!(domobj = testDomainAssignDef(conn, &privconn->domains, domdef))) { - virDomainDefFree(domdef); + if (testDomainGenerateIfnames(conn, domdef) < 0) goto error; - } + if (!(domobj = virDomainAssignDef(conn, &privconn->domains, domdef))) + goto error; + domdef = NULL; domobj->def->id = privconn->nextDomID++; domobj->state = VIR_DOMAIN_RUNNING; domobj->persistent = 1; @@ -399,6 +395,7 @@ error: testDriverUnlock(privconn); conn->privateData = NULL; VIR_FREE(privconn); + virDomainDefFree(domdef); return VIR_DRV_OPEN_ERROR; } @@ -668,7 +665,8 @@ static int testOpenFromFile(virConnectPtr conn, goto error; } - if (!(dom = testDomainAssignDef(conn, &privconn->domains, def))) { + if (testDomainGenerateIfnames(conn, def) < 0 || + !(dom = virDomainAssignDef(conn, &privconn->domains, def))) { virDomainDefFree(def); goto error; } @@ -980,11 +978,11 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, VIR_DOMAIN_XML_INACTIVE)) == NULL) goto cleanup; - if ((dom = testDomainAssignDef(conn, &privconn->domains, - def)) == NULL) { - virDomainDefFree(def); + if (testDomainGenerateIfnames(conn, def) < 0) goto cleanup; - } + if (!(dom = virDomainAssignDef(conn, &privconn->domains, def))) + goto cleanup; + def = NULL; dom->state = VIR_DOMAIN_RUNNING; dom->def->id = privconn->nextDomID++; @@ -992,15 +990,17 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_BOOTED); - ret = virGetDomain(conn, def->name, def->uuid); + ret = virGetDomain(conn, dom->def->name, dom->def->uuid); if (ret) - ret->id = def->id; + ret->id = dom->def->id; cleanup: if (dom) virDomainObjUnlock(dom); if (event) testDomainEventQueue(privconn, event); + if (def) + virDomainDefFree(def); testDriverUnlock(privconn); return ret; } @@ -1531,13 +1531,14 @@ static int testDomainRestore(virConnectPtr conn, if (!def) goto cleanup; - if ((dom = testDomainAssignDef(conn, &privconn->domains, - def)) == NULL) + if (testDomainGenerateIfnames(conn, def) < 0) goto cleanup; + if (!(dom = virDomainAssignDef(conn, &privconn->domains, def))) + goto cleanup; + def = NULL; dom->state = VIR_DOMAIN_RUNNING; dom->def->id = privconn->nextDomID++; - def = NULL; event = virDomainEventNewFromObj(dom, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_RESTORED); @@ -1823,10 +1824,10 @@ static virDomainPtr testDomainDefineXML(virConnectPtr conn, VIR_DOMAIN_XML_INACTIVE)) == NULL) goto cleanup; - if ((dom = testDomainAssignDef(conn, &privconn->domains, - def)) == NULL) { + if (testDomainGenerateIfnames(conn, def) < 0) + goto cleanup; + if (!(dom = virDomainAssignDef(conn, &privconn->domains, def))) goto cleanup; - } def = NULL; dom->persistent = 1; diff --git a/tests/object-locking.ml b/tests/object-locking.ml index 215a2e5..0c66fc7 100644 --- a/tests/object-locking.ml +++ b/tests/object-locking.ml @@ -128,7 +128,8 @@ let driverLockMethods = [ "umlDriverLock"; "nodedevDriverLock"; "networkDriverLock"; - "storageDriverLock" + "storageDriverLock"; + "oneDriverLock" ] (* @@ -142,7 +143,8 @@ let driverUnlockMethods = [ "umlDriverUnlock"; "nodedevDriverUnlock"; "networkDriverUnlock"; - "storageDriverUnlock" + "storageDriverUnlock"; + "oneDriverUnlock" ] (* @@ -159,6 +161,7 @@ let lockableDrivers = [ "virStorageDriverStatePtr"; "network_driver"; "virDeviceMonitorState"; + "one_driver_t"; ] -- 1.6.2.5

2 2

[libvirt] [PATCH] Fix handling of Xen(ner) detection
by Daniel P. Berrange 10 Sep '09

10 Sep '09

Latest upstream QEMU can be built with Xen support, which introduces a -xen-domid argument. This was mistakenly detected as -domid due to old Xenner support. Adapt to cope with both syntax. Also only set domid if the virt type is xen, or the guest type is xen * src/qemu_conf.c, src/qemu_conf.h: Detect new -xen-domid flag in preference to -domid. * tests/qemuxml2argvdata/qemuxml2argv-bootloader.args, tests/qemuxml2argvdata/qemuxml2argv-input-xen.args: Add missing -domid param * tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.args: Remove bogus -boot param. * tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.xml: Fix domain type to be xen * tests/qemuxml2argvtest.c: Add missing QEMUD_CMD_FLAG_DOMID params --- src/qemu_conf.c | 31 +++++++++++++++---- src/qemu_conf.h | 1 + .../qemuxml2argvdata/qemuxml2argv-bootloader.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-input-xen.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.xml | 3 +- tests/qemuxml2argvtest.c | 4 +- 7 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 22f5edd..e8414c3 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -769,7 +769,9 @@ static unsigned int qemudComputeCmdFlags(const char *help, flags |= QEMUD_CMD_FLAG_NAME; if (strstr(help, "-uuid")) flags |= QEMUD_CMD_FLAG_UUID; - if (strstr(help, "-domid")) + if (strstr(help, "-xen-domid")) + flags |= QEMUD_CMD_FLAG_XEN_DOMID; + else if (strstr(help, "-domid")) flags |= QEMUD_CMD_FLAG_DOMID; if (strstr(help, "-drive")) { flags |= QEMUD_CMD_FLAG_DRIVE; @@ -1594,9 +1596,22 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT("-uuid"); ADD_ARG_LIT(uuid); } - if (qemuCmdFlags & QEMUD_CMD_FLAG_DOMID) { - ADD_ARG_LIT("-domid"); - ADD_ARG_LIT(domid); + if (def->virtType == VIR_DOMAIN_VIRT_XEN || + STREQ(def->os.type, "xen") || + STREQ(def->os.type, "linux")) { + if (qemuCmdFlags & QEMUD_CMD_FLAG_DOMID) { + ADD_ARG_LIT("-domid"); + ADD_ARG_LIT(domid); + } else if (qemuCmdFlags & QEMUD_CMD_FLAG_XEN_DOMID) { + ADD_ARG_LIT("-xen-attach"); + ADD_ARG_LIT("-xen-domid"); + ADD_ARG_LIT(domid); + } else { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("qemu emulator '%s' does not support xen"), + def->emulator); + goto error; + } } /* @@ -1649,9 +1664,11 @@ int qemudBuildCommandLine(virConnectPtr conn, break; } } - boot[def->os.nBootDevs] = '\0'; - ADD_ARG_LIT("-boot"); - ADD_ARG_LIT(boot); + if (def->os.nBootDevs) { + boot[def->os.nBootDevs] = '\0'; + ADD_ARG_LIT("-boot"); + ADD_ARG_LIT(boot); + } if (def->os.kernel) { ADD_ARG_LIT("-kernel"); diff --git a/src/qemu_conf.h b/src/qemu_conf.h index a126dac..99aac81 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -67,6 +67,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_HOST_NET_ADD = QEMUD_CMD_FLAG_0_10, /* host_net_add monitor command */ QEMUD_CMD_FLAG_PCIDEVICE = (1 << 17), /* PCI device assignment only supported by qemu-kvm */ + QEMUD_CMD_FLAG_XEN_DOMID = (1 << 18), /* -xen-domid (new style xen integration) */ }; /* Main driver state */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bootloader.args b/tests/qemuxml2argvdata/qemuxml2argv-bootloader.args index 8a15524..bcae18a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-bootloader.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-bootloader.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/xenner -S -M xenner -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -bootloader /usr/bin/pygrub -cdrom /dev/cdrom -net none -serial none -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/xenner -S -M xenner -m 214 -smp 1 -domid 6 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -bootloader /usr/bin/pygrub -cdrom /dev/cdrom -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-input-xen.args b/tests/qemuxml2argvdata/qemuxml2argv-input-xen.args index 64c0e1c..65fb073 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-input-xen.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-input-xen.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/xenner -S -M xenner -m 214 -smp 1 -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -bootloader /foo -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -vnc 127.0.0.1:3 +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/xenner -S -M xenner -m 214 -smp 1 -domid 6 -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -bootloader /foo -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -vnc 127.0.0.1:3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.args b/tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.args index c735dc7..c4df9ae 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -name QEMUGuest1 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 -domid 6 -nographic -monitor unix:/tmp/test-monitor,server,nowait -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -name QEMUGuest1 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 -domid 6 -nographic -monitor unix:/tmp/test-monitor,server,nowait -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.xml b/tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.xml index 1b37bdc..7f77e01 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.xml @@ -5,8 +5,7 @@ <currentMemory>219200</currentMemory> <vcpu>1</vcpu> <os> - <type arch='i686' machine='pc'>hvm</type> - <boot dev='hd'/> + <type arch='x86_64' machine='pc'>xen</type> </os> <features> <acpi/> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 6f25e7d..ad7a690 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -186,7 +186,7 @@ mymain(int argc, char **argv) DO_TEST("boot-cdrom", 0); DO_TEST("boot-network", 0); DO_TEST("boot-floppy", 0); - DO_TEST("bootloader", 0); + DO_TEST("bootloader", QEMUD_CMD_FLAG_DOMID); DO_TEST("clock-utc", 0); DO_TEST("clock-localtime", 0); DO_TEST("disk-cdrom", 0); @@ -236,7 +236,7 @@ mymain(int argc, char **argv) DO_TEST("graphics-sdl-fullscreen", 0); DO_TEST("input-usbmouse", 0); DO_TEST("input-usbtablet", 0); - DO_TEST("input-xen", 0); + DO_TEST("input-xen", QEMUD_CMD_FLAG_DOMID); DO_TEST("misc-acpi", 0); DO_TEST("misc-no-reboot", 0); DO_TEST("misc-uuid", QEMUD_CMD_FLAG_NAME | -- 1.6.2.5

2 2

[libvirt] [PATCH] also allow use of XZ for Qemu image compression
by Jim Meyering 09 Sep '09

09 Sep '09

While this patch stays minimal by simply adding XZ/xz to the list, I think it would be better to remove lzma, since it uses an inferior format (which lacks an integrity check), and has been effectively subsumed by xz. Let me know if you'd like that, and I'll prepare the slightly more invasive patch. >From afb7275f14e8062e6d5eebf704e1fd0f582cb407 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 8 Sep 2009 20:52:37 +0200 Subject: [PATCH] also allow use of XZ for Qemu image compression * src/qemu_driver.c (enum qemud_save_formats) [QEMUD_SAVE_FORMAT_XZ]: New member. [QEMUD_SAVE_FORMAT_LZMA]: Mark as deprecated. (qemudDomainSave, qemudDomainRestore): Handle the new member. * src/qemu.conf: Mention xz, too. --- src/qemu.conf | 2 +- src/qemu_driver.c | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/qemu.conf b/src/qemu.conf index 06babc4..342bb8a 100644 --- a/src/qemu.conf +++ b/src/qemu.conf @@ -134,7 +134,7 @@ # memory from the domain is dumped out directly to a file. If you have # guests with a large amount of memory, however, this can take up quite # a bit of space. If you would like to compress the images while they -# are being saved to disk, you can also set "gzip", "bzip2", "lzma" +# are being saved to disk, you can also set "gzip", "bzip2", "lzma", "xz", # or "lzop" for save_image_format. Note that this means you slow down # the process of saving a domain in order to save disk space. # diff --git a/src/qemu_driver.c b/src/qemu_driver.c index f64d70b..7b64712 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3622,7 +3622,8 @@ enum qemud_save_formats { QEMUD_SAVE_FORMAT_RAW, QEMUD_SAVE_FORMAT_GZIP, QEMUD_SAVE_FORMAT_BZIP2, - QEMUD_SAVE_FORMAT_LZMA, + QEMUD_SAVE_FORMAT_LZMA, /* deprecated, in favor of xz */ + QEMUD_SAVE_FORMAT_XZ, QEMUD_SAVE_FORMAT_LZOP, }; @@ -3664,6 +3665,8 @@ static int qemudDomainSave(virDomainPtr dom, header.compressed = QEMUD_SAVE_FORMAT_BZIP2; else if (STREQ(driver->saveImageFormat, "lzma")) header.compressed = QEMUD_SAVE_FORMAT_LZMA; + else if (STREQ(driver->saveImageFormat, "xz")) + header.compressed = QEMUD_SAVE_FORMAT_XZ; else if (STREQ(driver->saveImageFormat, "lzop")) header.compressed = QEMUD_SAVE_FORMAT_LZOP; else { @@ -3758,6 +3761,9 @@ static int qemudDomainSave(virDomainPtr dom, else if (header.compressed == QEMUD_SAVE_FORMAT_LZMA) internalret = virAsprintf(&command, "migrate \"exec:" "lzma -c >> '%s' 2>/dev/null\"", safe_path); + else if (header.compressed == QEMUD_SAVE_FORMAT_XZ) + internalret = virAsprintf(&command, "migrate \"exec:" + "xz -c >> '%s' 2>/dev/null\"", safe_path); else if (header.compressed == QEMUD_SAVE_FORMAT_LZOP) internalret = virAsprintf(&command, "migrate \"exec:" "lzop -c >> '%s' 2>/dev/null\"", safe_path); @@ -4383,6 +4389,8 @@ static int qemudDomainRestore(virConnectPtr conn, intermediate_argv[0] = "bzip2"; else if (header.compressed == QEMUD_SAVE_FORMAT_LZMA) intermediate_argv[0] = "lzma"; + else if (header.compressed == QEMUD_SAVE_FORMAT_XZ) + intermediate_argv[0] = "xz"; else if (header.compressed == QEMUD_SAVE_FORMAT_LZOP) intermediate_argv[0] = "lzop"; else if (header.compressed != QEMUD_SAVE_FORMAT_RAW) { -- 1.6.5.rc0.164.g5f6b0

5 19

[libvirt] [PATCH] Automagically check if libssh2 is new enough for Phyp-support to be build
by Maximilian Wilhelm 09 Sep '09

09 Sep '09

Hi! My buildbot spotted a problem with the latest changes to Phyp, as the function libssh2_session_block_directions() only is available with libssh2 version 1.0 and later. At least on Debian Lenny there is an older one which prevents libVirt from building, so I updated the configure.in script to add another piece of magic (and thereby fixed the indenation of the Phypchecks). See attached patch. Ciao Max -- "really soon now": an unspecified period of time, likly to be greater than any reasonable definition of "soon".

3 6

[libvirt] HVM startup problem"unknown root elementi for storage pool"
by Andreas Sommer 09 Sep '09

09 Sep '09

Hi, I'm having a problem with starting a HVM virtual machine on Xen-3.4 with python-libvirt. This is the XML: <?xml version="1.0" encoding="utf-8"?> <domain type="xen"> <name>b07d6538-60f5-44a9-90e7-761304968e17</name> <uuid>b07d6538-60f5-44a9-90e7-761304968e17</uuid> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <memory>131072</memory> <vcpu>1</vcpu> <os> <type>hvm</type> <boot dev="hd" /> </os> <devices> <graphics type="vnc" /> <input type="tablet" /> <interface type="bridge"> <source bridge="eth0" /> </interface> <disk type="file"> <source file="/xen-hvm-test/disk0.img" /> <target dev="sda1" /> </disk> </devices> </domain> Then I call connection.createXML(libvirtXmlDescription, 0) which leads to an exception "unknown root elementi for storage pool". I'm using the Debian package "libvirt0" 0.4.6-10. I guess that's 0.6.4 actually, please tell me if that's a wrong assumption?? The 0.6.4 source code seems to have corrected "elementi" to "element" that's why I'm wondering which version I actually have. Why does this happen? Is there anything missing in my XML description? Best regards

1 1

[libvirt] [PATCH] openvz_conf.c: remove dead store to "p"; use strchrnul
by Jim Meyering 09 Sep '09

09 Sep '09

I may separate this into two commits. Currently we can't use the part that adds the strchrnul use, since gnulib's module lists its license as LGPL, which means LGPLv3+, and that conflicts with libvirt's LGPLv2+ requirement. However, since strchrnul is derived from glibc, it should be easy to adjust that. I've asked the maintainer of that module on the bug-gnulib list. We'll see later today when the US wakes up. >From b76bc73df65c793958ce646fd80763c19a9e4dbd Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 8 Sep 2009 11:07:32 +0200 Subject: [PATCH] openvz_conf.c: remove dead store to "p"; use strchrnul * src/openvz_conf.c (openvzReadNetworkConf): Replace open-coded while loop with equivalent use of strchrnul. * bootstrap (modules): Add strchrnul. --- bootstrap | 1 + src/openvz_conf.c | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bootstrap b/bootstrap index 8b81e0e..5560792 100755 --- a/bootstrap +++ b/bootstrap @@ -89,6 +89,7 @@ send setsockopt socket stpcpy +strchrnul strndup strerror strsep diff --git a/src/openvz_conf.c b/src/openvz_conf.c index ce4fc6e..be94b9e 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -239,15 +239,14 @@ openvzReadNetworkConf(virConnectPtr conn, net->type = VIR_DOMAIN_NET_TYPE_BRIDGE; - char *p = token, *next = token; + char *p = token; char cpy_temp[32]; int len; /*parse string*/ do { - while (*next != '\0' && *next != ',') next++; + char *next = strchrnul (token, ','); if (STRPREFIX(p, "ifname=")) { - p += 7; /* skip in libvirt */ } else if (STRPREFIX(p, "host_ifname=")) { p += 12; -- 1.6.5.rc0.164.g5f6b0

2 2

[libvirt] [PATCH] two syntax-check related updates
by Jim Meyering 08 Sep '09

08 Sep '09

These changes affect only "make syntax-check". The first patch is clearing out now-useless or unnecessary exclusions. The second enables one more test and fixes the few small violations it spotted. >From e4719bd092b03ef666d196ebc4b97086a7ecae98 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 8 Sep 2009 15:08:07 +0200 Subject: [PATCH 1/2] build: syntax-check: exclude fewer tests * cfg.mk (local-checks-to-skip): Don't skip tests that pass. Remove names of renamed or removed tests. --- cfg.mk | 14 -------------- 1 files changed, 0 insertions(+), 14 deletions(-) diff --git a/cfg.mk b/cfg.mk index 64bd26e..79dcb81 100644 --- a/cfg.mk +++ b/cfg.mk @@ -26,31 +26,17 @@ url_dir_list = \ # Tests not to run as part of "make distcheck". local-checks-to-skip = \ - changelog-check \ - check-AUTHORS \ - makefile-check \ - makefile_path_separator_check \ - patch-check \ sc_GPL_version \ - sc_always_defined_macros \ - sc_cast_of_alloca_return_value \ - sc_dd_max_sym_length \ sc_error_exit_success \ sc_file_system \ sc_immutable_NEWS \ sc_makefile_path_separator_check \ - sc_obsolete_symbols \ - sc_prohibit_S_IS_definition \ sc_prohibit_atoi_atof \ - sc_prohibit_jm_in_m4 \ sc_prohibit_quote_without_use \ sc_prohibit_quotearg_without_use \ sc_prohibit_stat_st_blocks \ - sc_root_tests \ sc_space_tab \ sc_sun_os_names \ - sc_system_h_headers \ - sc_tight_scope \ sc_two_space_separator_in_usage \ sc_error_message_uppercase \ sc_program_name \ -- 1.6.5.rc0.164.g5f6b0 >From 7bf84b60be8603f226af6244a3e63335586555d6 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 8 Sep 2009 15:24:46 +0200 Subject: [PATCH 2/2] build: no longer skip the makefile PATH separator check * cfg.mk (local-checks-to-skip): Remove sc_makefile_path_separator_check. * python/tests/Makefile.am (tests): Use $(PATH_SEPARATOR), not ":" in shell search PATH definitions. --- cfg.mk | 1 - python/tests/Makefile.am | 23 +++++++++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/cfg.mk b/cfg.mk index 79dcb81..7e152f8 100644 --- a/cfg.mk +++ b/cfg.mk @@ -30,7 +30,6 @@ local-checks-to-skip = \ sc_error_exit_success \ sc_file_system \ sc_immutable_NEWS \ - sc_makefile_path_separator_check \ sc_prohibit_atoi_atof \ sc_prohibit_quote_without_use \ sc_prohibit_quotearg_without_use \ diff --git a/python/tests/Makefile.am b/python/tests/Makefile.am index 6011fef..11218a8 100644 --- a/python/tests/Makefile.am +++ b/python/tests/Makefile.am @@ -12,14 +12,21 @@ EXTRA_DIST = $(PYTESTS) if WITH_PYTHON tests: $(PYTESTS) @echo "## running Python regression tests" - -@(PYTHONPATH="..:../.libs:../src/.libs:$(srcdir)/../src:$$PYTHONPATH";\ - export PYTHONPATH; \ - LD_LIBRARY_PATH="$(top_builddir)/src/.libs:$$LD_LIBRARY_PATH" ; \ - export LD_LIBRARY_PATH; \ - for test in $(PYTESTS) ; \ - do log=`$(PYTHON) $(srcdir)/$$test` ; \ - if [ "`echo $$log | grep OK`" = "" ] ; then \ - echo "-- $$test" ; echo "$$log" ; fi ; done) + -@(p=..; \ + -@(p="$$p$(PATH_SEPARATOR)../.libs"; \ + p="$$p$(PATH_SEPARATOR)../src/.libs"; \ + p="$$p$(PATH_SEPARATOR)$(srcdir)/../src"; \ + p="$$p$(PATH_SEPARATOR)$$PYTHONPATH"; \ + PYTHONPATH=$$p; export PYTHONPATH; \ + p="$(top_builddir)/src/.libs"; \ + p="$$p$(PATH_SEPARATOR)$$LD_LIBRARY_PATH"; \ + LD_LIBRARY_PATH=$$p; export LD_LIBRARY_PATH; \ + for test in $(PYTESTS); do \ + log=`$(PYTHON) $(srcdir)/$$test`; \ + if [ "`echo $$log | grep OK`" = "" ]; then \ + echo "-- $$test"; echo "$$log"; \ + fi; \ + done) else tests: endif -- 1.6.5.rc0.164.g5f6b0

1 0

[libvirt] Free name in xenDaemonLookupByID(), if virGetDomain() fails
by Matthias Bolte 08 Sep '09

08 Sep '09

Subject says it all. Matthias

2 3

[libvirt] rebased multipath patch
by Dave Allan 08 Sep '09

08 Sep '09

Here's the multipath patch, rebased against the current head. Dave >From 52cb0ff30dd5a52c5a246f3a2022f933cd2a0eab Mon Sep 17 00:00:00 2001 From: David Allan <dallan(a)redhat.com> Date: Tue, 21 Jul 2009 12:43:01 -0400 Subject: [PATCH] Add a simple pool type for multipath devices This pool type contains volumes for the multipath devices that are present on the host. It does not (yet) support any sort of multipath configuration, so that aspect of system administration must still be done at host build time. --- configure.in | 24 +++ po/POTFILES.in | 1 + src/Makefile.am | 9 + src/storage_backend.c | 82 ++++++++++ src/storage_backend.h | 5 +- src/storage_backend_mpath.c | 352 +++++++++++++++++++++++++++++++++++++++++++ src/storage_backend_mpath.h | 31 ++++ src/storage_conf.c | 7 +- src/storage_conf.h | 1 + 9 files changed, 510 insertions(+), 2 deletions(-) create mode 100644 src/storage_backend_mpath.c create mode 100644 src/storage_backend_mpath.h diff --git a/configure.in b/configure.in index d28c44a..7c93ab7 100644 --- a/configure.in +++ b/configure.in @@ -1046,6 +1046,8 @@ AC_ARG_WITH([storage-iscsi], [ --with-storage-iscsi with iSCSI backend for the storage driver (on)],[],[with_storage_iscsi=check]) AC_ARG_WITH([storage-scsi], [ --with-storage-scsi with SCSI backend for the storage driver (on)],[],[with_storage_scsi=check]) +AC_ARG_WITH([storage-mpath], +[ --with-storage-mpath with mpath backend for the storage driver (on)],[],[with_storage_mpath=check]) AC_ARG_WITH([storage-disk], [ --with-storage-disk with GPartd Disk backend for the storage driver (on)],[],[with_storage_disk=check]) @@ -1056,6 +1058,7 @@ if test "$with_libvirtd" = "no"; then with_storage_lvm=no with_storage_iscsi=no with_storage_scsi=no + with_storage_mpath=no with_storage_disk=no fi if test "$with_storage_dir" = "yes" ; then @@ -1177,6 +1180,26 @@ if test "$with_storage_scsi" = "check"; then fi AM_CONDITIONAL([WITH_STORAGE_SCSI], [test "$with_storage_scsi" = "yes"]) +if test "$with_storage_mpath" = "check"; then + with_storage_mpath=yes + + AC_DEFINE_UNQUOTED([WITH_STORAGE_MPATH], 1, + [whether mpath backend for storage driver is enabled]) +fi +AM_CONDITIONAL([WITH_STORAGE_MPATH], [test "$with_storage_mpath" = "yes"]) + +if test "$with_storage_mpath" = "yes"; then + DEVMAPPER_REQUIRED=0.0 + DEVMAPPER_CFLAGS= + DEVMAPPER_LIBS= + PKG_CHECK_MODULES(DEVMAPPER, devmapper >= $DEVMAPPER_REQUIRED, + [], [ + AC_MSG_ERROR( + [You must install device-mapper-devel >= $DEVMAPPER_REQUIRED to compile libvirt]) + ]) +fi +AC_SUBST([DEVMAPPER_CFLAGS]) +AC_SUBST([DEVMAPPER_LIBS]) LIBPARTED_CFLAGS= LIBPARTED_LIBS= @@ -1677,6 +1700,7 @@ AC_MSG_NOTICE([ NetFS: $with_storage_fs]) AC_MSG_NOTICE([ LVM: $with_storage_lvm]) AC_MSG_NOTICE([ iSCSI: $with_storage_iscsi]) AC_MSG_NOTICE([ SCSI: $with_storage_scsi]) +AC_MSG_NOTICE([ mpath: $with_storage_mpath]) AC_MSG_NOTICE([ Disk: $with_storage_disk]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Security Drivers]) diff --git a/po/POTFILES.in b/po/POTFILES.in index 0a53dab..1586368 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -38,6 +38,7 @@ src/storage_backend_fs.c src/storage_backend_iscsi.c src/storage_backend_logical.c src/storage_backend_scsi.c +src/storage_backend_mpath.c src/storage_conf.c src/storage_driver.c src/storage_encryption_conf.c diff --git a/src/Makefile.am b/src/Makefile.am index bedeb84..cf3420b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -199,6 +199,9 @@ STORAGE_DRIVER_ISCSI_SOURCES = \ STORAGE_DRIVER_SCSI_SOURCES = \ storage_backend_scsi.h storage_backend_scsi.c +STORAGE_DRIVER_MPATH_SOURCES = \ + storage_backend_mpath.h storage_backend_mpath.c + STORAGE_DRIVER_DISK_SOURCES = \ storage_backend_disk.h storage_backend_disk.c @@ -478,6 +481,10 @@ if WITH_STORAGE_SCSI libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_SCSI_SOURCES) endif +if WITH_STORAGE_MPATH +libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_MPATH_SOURCES) +endif + if WITH_STORAGE_DISK libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_DISK_SOURCES) endif @@ -539,6 +546,7 @@ EXTRA_DIST += \ $(STORAGE_DRIVER_LVM_SOURCES) \ $(STORAGE_DRIVER_ISCSI_SOURCES) \ $(STORAGE_DRIVER_SCSI_SOURCES) \ + $(STORAGE_DRIVER_MPATH_SOURCES) \ $(STORAGE_DRIVER_DISK_SOURCES) \ $(NODE_DEVICE_DRIVER_SOURCES) \ $(NODE_DEVICE_DRIVER_HAL_SOURCES) \ @@ -607,6 +615,7 @@ libvirt_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)libvirt.syms \ $(COVERAGE_CFLAGS:-f%=-Wc,-f%) \ $(LIBXML_LIBS) $(SELINUX_LIBS) \ $(XEN_LIBS) $(DRIVER_MODULE_LIBS) \ + $(DEVMAPPER_LIBS) \ @CYGWIN_EXTRA_LDFLAGS@ @MINGW_EXTRA_LDFLAGS@ libvirt_la_CFLAGS = $(COVERAGE_CFLAGS) -DIN_LIBVIRT libvirt_la_DEPENDENCIES = $(libvirt_la_LIBADD) libvirt.syms diff --git a/src/storage_backend.c b/src/storage_backend.c index c818142..4dec06b 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -59,6 +59,9 @@ #if WITH_STORAGE_SCSI #include "storage_backend_scsi.h" #endif +#if WITH_STORAGE_MPATH +#include "storage_backend_mpath.h" +#endif #if WITH_STORAGE_DISK #include "storage_backend_disk.h" #endif @@ -89,6 +92,9 @@ static virStorageBackendPtr backends[] = { #if WITH_STORAGE_SCSI &virStorageBackendSCSI, #endif +#if WITH_STORAGE_MPATH + &virStorageBackendMpath, +#endif #if WITH_STORAGE_DISK &virStorageBackendDisk, #endif @@ -812,6 +818,82 @@ virStorageBackendUpdateVolTargetInfoFD(virConnectPtr conn, return 0; } + +struct diskType { + int part_table_type; + unsigned short offset; + unsigned short length; + unsigned long long magic; +}; + + +static struct diskType const disk_types[] = { + { VIR_STORAGE_POOL_DISK_LVM2, 0x218, 8, 0x31303020324D564CULL }, + { VIR_STORAGE_POOL_DISK_GPT, 0x200, 8, 0x5452415020494645ULL }, + { VIR_STORAGE_POOL_DISK_DVH, 0x0, 4, 0x41A9E50BULL }, + { VIR_STORAGE_POOL_DISK_MAC, 0x0, 2, 0x5245ULL }, + { VIR_STORAGE_POOL_DISK_BSD, 0x40, 4, 0x82564557ULL }, + { VIR_STORAGE_POOL_DISK_SUN, 0x1fc, 2, 0xBEDAULL }, + /* + * NOTE: pc98 is funky; the actual signature is 0x55AA (just like dos), so + * we can't use that. At the moment I'm relying on the "dummy" IPL + * bootloader data that comes from parted. Luckily, the chances of running + * into a pc98 machine running libvirt are approximately nil. + */ + /*{ 0x1fe, 2, 0xAA55UL },*/ + { VIR_STORAGE_POOL_DISK_PC98, 0x0, 8, 0x314C5049000000CBULL }, + /* + * NOTE: the order is important here; some other disk types (like GPT and + * and PC98) also have 0x55AA at this offset. For that reason, the DOS + * one must be the last one. + */ + { VIR_STORAGE_POOL_DISK_DOS, 0x1fe, 2, 0xAA55ULL }, + { -1, 0x0, 0, 0x0ULL }, +}; + + +int +virStorageBackendUpdateVolTargetFormatFD(virConnectPtr conn, + virStorageVolTargetPtr target, + int fd) +{ + int i; + off_t start; + unsigned char buffer[1024]; + ssize_t bytes; + + /* make sure to set the target format "unknown" to begin with */ + target->format = VIR_STORAGE_POOL_DISK_UNKNOWN; + + start = lseek(fd, 0, SEEK_SET); + if (start < 0) { + virReportSystemError(conn, errno, + _("cannot seek to beginning of file '%s'"), + target->path); + return -1; + } + bytes = saferead(fd, buffer, sizeof(buffer)); + if (bytes < 0) { + virReportSystemError(conn, errno, + _("cannot read beginning of file '%s'"), + target->path); + return -1; + } + + for (i = 0; disk_types[i].part_table_type != -1; i++) { + if (disk_types[i].offset + disk_types[i].length > bytes) + continue; + if (memcmp(buffer+disk_types[i].offset, &disk_types[i].magic, + disk_types[i].length) == 0) { + target->format = disk_types[i].part_table_type; + break; + } + } + + return 0; +} + + void virStorageBackendWaitForDevices(virConnectPtr conn) { virWaitForDevices(conn); diff --git a/src/storage_backend.h b/src/storage_backend.h index e80b05d..eb5bf87 100644 --- a/src/storage_backend.h +++ b/src/storage_backend.h @@ -91,7 +91,10 @@ int virStorageBackendUpdateVolTargetInfoFD(virConnectPtr conn, int fd, unsigned long long *allocation, unsigned long long *capacity); - +int +virStorageBackendUpdateVolTargetFormatFD(virConnectPtr conn, + virStorageVolTargetPtr target, + int fd); void virStorageBackendWaitForDevices(virConnectPtr conn); char *virStorageBackendStablePath(virConnectPtr conn, diff --git a/src/storage_backend_mpath.c b/src/storage_backend_mpath.c new file mode 100644 index 0000000..41dadf1 --- /dev/null +++ b/src/storage_backend_mpath.c @@ -0,0 +1,352 @@ +/* + * storage_backend_mpath.c: storage backend for multipath handling + * + * Copyright (C) 2007-2008 Red Hat, Inc. + * Copyright (C) 2007-2008 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange redhat com> + */ + +#include <config.h> + +#include <unistd.h> +#include <stdio.h> +#include <dirent.h> +#include <fcntl.h> + +#include <libdevmapper.h> + +#include "virterror_internal.h" +#include "storage_conf.h" +#include "storage_backend.h" +#include "memory.h" +#include "logging.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + +static int +virStorageBackendMpathUpdateVolTargetInfo(virConnectPtr conn, + virStorageVolTargetPtr target, + unsigned long long *allocation, + unsigned long long *capacity) +{ + int ret = 0; + int fd = -1; + + if ((fd = open(target->path, O_RDONLY)) < 0) { + virReportSystemError(conn, errno, + _("cannot open volume '%s'"), + target->path); + ret = -1; + goto out; + } + + if (virStorageBackendUpdateVolTargetInfoFD(conn, + target, + fd, + allocation, + capacity) < 0) { + + VIR_ERROR(_("Failed to update volume target info for %s"), + target->path); + + ret = -1; + goto out; + } + + if (virStorageBackendUpdateVolTargetFormatFD(conn, + target, + fd) < 0) { + VIR_ERROR(_("Failed to update volume target format for %s"), + target->path); + + ret = -1; + goto out; + } + +out: + if (fd != -1) { + close(fd); + } + return ret; +} + + +static int +virStorageBackendMpathNewVol(virConnectPtr conn, + virStoragePoolObjPtr pool, + const int devnum, + const char *dev) +{ + virStorageVolDefPtr vol; + int retval = 0; + + if (VIR_ALLOC(vol) < 0) { + virReportOOMError(conn); + retval = -1; + goto out; + } + + vol->type = VIR_STORAGE_VOL_BLOCK; + + if (virAsprintf(&(vol->name), "dm-%u", devnum) < 0) { + virReportOOMError(conn); + retval = -1; + goto free_vol; + } + + if (virAsprintf(&vol->target.path, "/dev/%s", dev) < 0) { + virReportOOMError(conn); + retval = -1; + goto free_vol; + } + + if (virStorageBackendMpathUpdateVolTargetInfo(conn, + &vol->target, + &vol->allocation, + &vol->capacity) < 0) { + + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Failed to update volume for '%s'"), + vol->target.path); + retval = -1; + goto free_vol; + } + + /* XXX should use logical unit's UUID instead */ + vol->key = strdup(vol->target.path); + if (vol->key == NULL) { + virReportOOMError(conn); + retval = -1; + goto free_vol; + } + + pool->def->capacity += vol->capacity; + pool->def->allocation += vol->allocation; + + if (VIR_REALLOC_N(pool->volumes.objs, + pool->volumes.count + 1) < 0) { + virReportOOMError(conn); + retval = -1; + goto free_vol; + } + pool->volumes.objs[pool->volumes.count++] = vol; + + goto out; + +free_vol: + virStorageVolDefFree(vol); +out: + return retval; +} + + +static int +virStorageBackendIsMultipath(const char *devname) +{ + int ret = 0; + struct dm_task *dmt = NULL; + void *next = NULL; + uint64_t start, length; + char *target_type = NULL; + char *params = NULL; + + dmt = dm_task_create(DM_DEVICE_TABLE); + if (dmt == NULL) { + ret = -1; + goto out; + } + + if (dm_task_set_name(dmt, devname) == 0) { + ret = -1; + goto out; + } + + dm_task_no_open_count(dmt); + + if (!dm_task_run(dmt)) { + ret = -1; + goto out; + } + + next = dm_get_next_target(dmt, next, &start, &length, + &target_type, &params); + + if (target_type == NULL) { + ret = -1; + goto out; + } + + if (!(strcmp(target_type, "multipath"))) { + ret = 1; + } + +out: + if (dmt != NULL) { + dm_task_destroy(dmt); + } + return ret; +} + + +static int +virStorageBackendGetMinorNumber(const char *devname, uint32_t *minor) +{ + int ret = -1; + struct dm_task *dmt; + struct dm_info info; + + if (!(dmt = dm_task_create(DM_DEVICE_INFO))) { + goto out; + } + + if (!dm_task_set_name(dmt, devname)) { + goto out; + } + + if (!dm_task_run(dmt)) { + goto out; + } + + if (!dm_task_get_info(dmt, &info)) { + goto out; + } + + *minor = info.minor; + ret = 0; + +out: + if (dmt != NULL) { + dm_task_destroy(dmt); + } + + return ret; +} + + +static int +virStorageBackendCreateVols(virConnectPtr conn, + virStoragePoolObjPtr pool, + struct dm_names *names) +{ + int retval = 0, is_mpath = 0; + char *map_device = NULL; + uint32_t minor = -1; + + do { + is_mpath = virStorageBackendIsMultipath(names->name); + + if (is_mpath < 0) { + retval = -1; + goto out; + } + + if (is_mpath == 1) { + + if (virAsprintf(&map_device, "mapper/%s", names->name) < 0) { + virReportOOMError(conn); + retval = -1; + goto out; + } + + if (virStorageBackendGetMinorNumber(names->name, &minor) < 0) { + retval = -1; + goto out; + } + + virStorageBackendMpathNewVol(conn, + pool, + minor, + map_device); + + VIR_FREE(map_device); + } + + /* Given the way libdevmapper returns its data, I don't see + * any way to avoid this series of casts. */ + names = (struct dm_names *)(((char *)names) + names->next); + + } while (names->next); + +out: + return retval; +} + + +static int +virStorageBackendGetMaps(virConnectPtr conn, + virStoragePoolObjPtr pool) +{ + int retval = 0; + struct dm_task *dmt = NULL; + struct dm_names *names = NULL; + + if (!(dmt = dm_task_create(DM_DEVICE_LIST))) { + retval = 1; + goto out; + } + + dm_task_no_open_count(dmt); + + if (!dm_task_run(dmt)) { + retval = 1; + goto out; + } + + if (!(names = dm_task_get_names(dmt))) { + retval = 1; + goto out; + } + + if (!names->dev) { + /* No devices found */ + goto out; + } + + virStorageBackendCreateVols(conn, pool, names); + +out: + if (dmt != NULL) { + dm_task_destroy (dmt); + } + return retval; +} + + +static int +virStorageBackendMpathRefreshPool(virConnectPtr conn, + virStoragePoolObjPtr pool) +{ + int retval = 0; + + VIR_ERROR(_("in %s"), __func__); + + pool->def->allocation = pool->def->capacity = pool->def->available = 0; + + virStorageBackendWaitForDevices(conn); + + virStorageBackendGetMaps(conn, pool); + + return retval; +} + + +virStorageBackend virStorageBackendMpath = { + .type = VIR_STORAGE_POOL_MPATH, + + .refreshPool = virStorageBackendMpathRefreshPool, +}; diff --git a/src/storage_backend_mpath.h b/src/storage_backend_mpath.h new file mode 100644 index 0000000..9de2b79 --- /dev/null +++ b/src/storage_backend_mpath.h @@ -0,0 +1,31 @@ +/* + * storage_backend_scsi.h: storage backend for SCSI handling + * + * Copyright (C) 2007-2008 Red Hat, Inc. + * Copyright (C) 2007-2008 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange redhat com> + */ + +#ifndef __VIR_STORAGE_BACKEND_MPATH_H__ +#define __VIR_STORAGE_BACKEND_MPATH_H__ + +#include "storage_backend.h" + +extern virStorageBackend virStorageBackendMpath; + +#endif /* __VIR_STORAGE_BACKEND_MPATH_H__ */ diff --git a/src/storage_conf.c b/src/storage_conf.c index c446069..af04080 100644 --- a/src/storage_conf.c +++ b/src/storage_conf.c @@ -53,7 +53,7 @@ VIR_ENUM_IMPL(virStoragePool, VIR_STORAGE_POOL_LAST, "dir", "fs", "netfs", "logical", "disk", "iscsi", - "scsi") + "scsi", "mpath") VIR_ENUM_IMPL(virStoragePoolFormatFileSystem, VIR_STORAGE_POOL_FS_LAST, @@ -198,6 +198,11 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { .formatToString = virStoragePoolFormatDiskTypeToString, } }, + { .poolType = VIR_STORAGE_POOL_MPATH, + .volOptions = { + .formatToString = virStoragePoolFormatDiskTypeToString, + } + }, { .poolType = VIR_STORAGE_POOL_DISK, .poolOptions = { .flags = (VIR_STORAGE_POOL_SOURCE_DEVICE), diff --git a/src/storage_conf.h b/src/storage_conf.h index bcf9b93..421d305 100644 --- a/src/storage_conf.h +++ b/src/storage_conf.h @@ -119,6 +119,7 @@ enum virStoragePoolType { VIR_STORAGE_POOL_DISK, /* Disk partitions */ VIR_STORAGE_POOL_ISCSI, /* iSCSI targets */ VIR_STORAGE_POOL_SCSI, /* SCSI HBA */ + VIR_STORAGE_POOL_MPATH, /* Multipath devices */ VIR_STORAGE_POOL_LAST, }; -- 1.6.0.6

3 3

Re: [libvirt] [PATCH 04/10] Secret manipulation step 7: Local driver
by Miloslav Trmac 08 Sep '09

08 Sep '09

----- "Daniel Veillard" <veillard(a)redhat.com> wrote: > On Mon, Sep 07, 2009 at 04:12:39PM +0200, Miloslav Trmač wrote: > > + if ((size_t)st.st_size != st.st_size) { > > shouldn't we chaeck against SECRET_MAX_XML_FILE instead ? No, this code reads the secret value, not the XML, and there's little reason to impose an arbitrary limit on the size. SECRET_MAX_XML_FILE is a left-over from an earlier version, the attached updated patch removes the definition. Mirek

1 0

Re: [libvirt] [PATCH 03/10] Add an internal <secret> XML handling API
by Miloslav Trmac 08 Sep '09

08 Sep '09

----- "Daniel Veillard" <veillard(a)redhat.com> wrote: > > + default: > > + VIR_ERROR(_("unexpected secret usage type %d"), def->usage_type); > > + break; > > Hum, since the virSecretDefPtr is allocated by our own code, it's > probably better to remove the default so that the compiler can tell us > we missed one enum case if new ones gets added. > > + type_str = virXPathString(conn, "string(./usage/@type)", ctxt); > > + if (type_str == NULL) { > > + virSecretReportError(conn, VIR_ERR_XML_ERROR, "%s", > > + _("unknown secret usage type")); > > _("missing secret usage type") would be more appropriate I guess > > + case VIR_SECRET_USAGE_TYPE_VOLUME: > > + def->usage.volume = virXPathString(conn, "string(./usage/volume)", > > + ctxt); > > + break; > > + > > + default: > > Again default: here means a mismatch between > virSecretUsageTypeTypeFromString and this function, best handled > statically IMHO. Thanks for the review, attached is an updated patch. Mirek

1 0

Re: [libvirt] [PATCH 02/10] Add VIR_SECRET_GET_VALUE_INTERNAL_CALL.
by Miloslav Trmac 08 Sep '09

08 Sep '09

----- "Daniel Veillard" <veillard(a)redhat.com> wrote: > > +/* Make sure ... INTERNAL_CALL can not be set by the caller */ > > +verify((VIR_SECRET_GET_VALUE_INTERNAL_CALL & > > + VIR_SECRET_GET_VALUE_FLAGS_MASK) == 0); > > ??? what's that ? an assert at compile time ? I don't know that construct > I would rather avoid it if it's not portable. Yes, a compile-time assert, provided by gnulib. The mechanism depends only on ISO C, and gnulib/lib/verify.h contains detailed comments about why the particular variant was chosen. It seems safe enough to me. Mirek

1 0

[libvirt] [PATCH]: Fixed minor bugs in display and added OOM checks.
by Pritesh Kothari 08 Sep '09

08 Sep '09

Hi All, There was a minor bug in selecting the graphics type. if the graphics type was desktop it was assumed that display is set for it, and thus crashed on strdup, so now checking if display is present before setting it. The second bug was while setting the 3d acceleration parameter, VIR_ALLOC() returns 0 or greater if success and not the other way round. Lastly added OOM checks in few places which were missed earlier. Regards, Pritesh

3 4

[libvirt] [PATCH] 6 dead-store patches
by Jim Meyering 07 Sep '09

07 Sep '09

>From 129dc57243df4e73daa24aac671087bcd25f51ad Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 16:49:37 +0200 Subject: [PATCH 1/6] xm_internal.c: remove dead stores of local, "type" * src/xm_internal.c (xenXMDomainConfigParse): Remove declaration and useless containing if-block, too. --- src/xm_internal.c | 9 +-------- 1 files changed, 1 insertions(+), 8 deletions(-) diff --git a/src/xm_internal.c b/src/xm_internal.c index 9627ffb..661172b 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -1,7 +1,7 @@ /* * xm_internal.h: helper routines for dealing with inactive domains * - * Copyright (C) 2006-2007 Red Hat + * Copyright (C) 2006-2007, 2009 Red Hat * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -995,7 +995,6 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { if (list && list->type == VIR_CONF_LIST) { list = list->list; while (list) { - int type = -1; char script[PATH_MAX]; char model[10]; char ip[16]; @@ -1031,7 +1030,6 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { mac[len] = '\0'; } else if (STRPREFIX(key, "bridge=")) { int len = nextkey ? (nextkey - data) : sizeof(bridge)-1; - type = 1; if (len > (sizeof(bridge)-1)) len = sizeof(bridge)-1; strncpy(bridge, data, len); @@ -1069,11 +1067,6 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { key = nextkey; } - /* XXX Forcing to pretend its a bridge */ - if (type == -1) { - type = 1; - } - if (VIR_ALLOC(net) < 0) goto cleanup; -- 1.6.4.2.419.gab238 >From b8b1ea4894ac9bf2a59472958a8bb0749526847f Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 17:22:19 +0200 Subject: [PATCH 2/6] xm_internal.c: remove two ret=... dead stores * src/xm_internal.c (xenXMDomainCreate): Remove dead stores. --- src/xm_internal.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/xm_internal.c b/src/xm_internal.c index 661172b..de3aca9 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -1850,10 +1850,10 @@ int xenXMDomainCreate(virDomainPtr domain) { goto error; domain->id = ret; - if ((ret = xend_wait_for_devices(domain->conn, domain->name)) < 0) + if (xend_wait_for_devices(domain->conn, domain->name) < 0) goto error; - if ((ret = xenDaemonDomainResume(domain)) < 0) + if (xenDaemonDomainResume(domain) < 0) goto error; xenUnifiedUnlock(priv); -- 1.6.4.2.419.gab238 >From a74d747d27bde38ece90df9a14acb648d83b9993 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 17:27:34 +0200 Subject: [PATCH 3/6] domain_conf.c: remove two dead stores * src/domain_conf.c (virDomainSaveXML): Remove use and decl of "err". (virDomainDefParseXML): Likewise. --- src/domain_conf.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/domain_conf.c b/src/domain_conf.c index 8dde5dd..050cf50 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -2520,8 +2520,7 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn, /* Extract domain uuid */ tmp = virXPathString(conn, "string(./uuid[1])", ctxt); if (!tmp) { - int err; - if ((err = virUUIDGenerate(def->uuid))) { + if (virUUIDGenerate(def->uuid)) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to generate UUID")); goto error; @@ -4456,12 +4455,11 @@ int virDomainSaveXML(virConnectPtr conn, char *configFile = NULL; int fd = -1, ret = -1; size_t towrite; - int err; if ((configFile = virDomainConfigFile(conn, configDir, def->name)) == NULL) goto cleanup; - if ((err = virFileMakePath(configDir))) { + if (virFileMakePath(configDir)) { virReportSystemError(conn, errno, _("cannot create config directory '%s'"), configDir); -- 1.6.4.2.419.gab238 >From b9c2d697d732b4c740fd9e1f87ad135c52e02c34 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 18:53:20 +0200 Subject: [PATCH 4/6] util.c: avoid dead store to "flag" * src/util.c (virExecDaemonize): Change flag |= VAR to "flag | VAR". --- src/util.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util.c b/src/util.c index 2529837..af50028 100644 --- a/src/util.c +++ b/src/util.c @@ -663,7 +663,7 @@ int virExecDaemonize(virConnectPtr conn, ret = virExecWithHook(conn, argv, envp, keepfd, retpid, infd, outfd, errfd, - flags |= VIR_EXEC_DAEMON, + flags | VIR_EXEC_DAEMON, hook, data, pidfile); /* __virExec should have set an error */ -- 1.6.4.2.419.gab238 >From 6470cb9312a13fd018fb83b79e66cb50190b1f4f Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 19:36:17 +0200 Subject: [PATCH 5/6] iptables.c: remove dead store to "s" * src/iptables.c (iptablesAddRemoveRule): Remove dead store. --- src/iptables.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/iptables.c b/src/iptables.c index 73b39d1..4562800 100644 --- a/src/iptables.c +++ b/src/iptables.c @@ -398,7 +398,7 @@ iptablesAddRemoveRule(iptRules *rules, int action, const char *arg, ...) 1; /* arg */ va_start(args, arg); - while ((s = va_arg(args, const char *))) + while (va_arg(args, const char *)) n++; va_end(args); -- 1.6.4.2.419.gab238 >From 65c10b030fb43d6678be9bd5eeeb326a024d6902 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 19:37:37 +0200 Subject: [PATCH 6/6] network_driver.c: remove dead store to "err" * src/network_driver.c (networkSetAutostart): ...and its decl. --- src/network_driver.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/src/network_driver.c b/src/network_driver.c index 84910ab..49855bf 100644 --- a/src/network_driver.c +++ b/src/network_driver.c @@ -1428,9 +1428,7 @@ static int networkSetAutostart(virNetworkPtr net, goto cleanup; if (autostart) { - int err; - - if ((err = virFileMakePath(driver->networkAutostartDir))) { + if (virFileMakePath(driver->networkAutostartDir)) { virReportSystemError(net->conn, errno, _("cannot create autostart directory '%s'"), driver->networkAutostartDir); -- 1.6.4.2.419.gab238

2 3

[libvirt] [PATCH] xm_internal.c: remove dead store and associated leak
by Jim Meyering 07 Sep '09

07 Sep '09

This one took a little thinking. I'm not 100% sure that the comment in my log is sufficient justification for removing the virGetDomain call. If we leave it, we'll have to free it and remove the XXX comment. >From 324e07f17e6a2ed6df236af955adc693670d1b16 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 19:46:59 +0200 Subject: [PATCH] xm_internal.c: remove dead store and associated leak * src/xm_internal.c (xenXMDomainDefineXML): Dead store to "olddomain" -- and comment ;-) highlighted that the virGetDomain call represented a leak. It was also useless, seeing as how preceding call to virHashLookup(priv->nameConfigMap found def->name. --- src/xm_internal.c | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-) diff --git a/src/xm_internal.c b/src/xm_internal.c index de3aca9..e7f6a55 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -2532,7 +2532,6 @@ cleanup: */ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) { virDomainPtr ret; - virDomainPtr olddomain; char filename[PATH_MAX]; const char * oldfilename; virDomainDefPtr def = NULL; @@ -2578,10 +2577,6 @@ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) { goto error; } - /* XXX wtf.com is this line for - it appears to be amemory leak */ - if (!(olddomain = virGetDomain(conn, def->name, entry->def->uuid))) - goto error; - /* Remove the name -> filename mapping */ if (virHashRemoveEntry(priv->nameConfigMap, def->name, NULL) < 0) { xenXMError(conn, VIR_ERR_INTERNAL_ERROR, -- 1.6.4.2.419.gab238

2 1

[libvirt] [PATCH] network_conf.c: remove dead store to "err"
by Jim Meyering 07 Sep '09

07 Sep '09

>From 66133e3b9d80e9fadefd810786b0ff2f5c8e6875 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 19:15:51 +0200 Subject: [PATCH] network_conf.c: remove dead store to "err" * src/network_conf.c (virNetworkDefParseXML): ...and its decl. --- src/network_conf.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/network_conf.c b/src/network_conf.c index 58a4f32..3764bb4 100644 --- a/src/network_conf.c +++ b/src/network_conf.c @@ -1,7 +1,7 @@ /* * network_conf.c: network XML handling * - * Copyright (C) 2006-2008 Red Hat, Inc. + * Copyright (C) 2006-2009 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -329,8 +329,7 @@ virNetworkDefParseXML(virConnectPtr conn, /* Extract network uuid */ tmp = virXPathString(conn, "string(./uuid[1])", ctxt); if (!tmp) { - int err; - if ((err = virUUIDGenerate(def->uuid))) { + if (virUUIDGenerate(def->uuid)) { virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to generate UUID")); goto error; -- 1.6.4.2.409.g85dc3

2 1

[libvirt] merge-commit push prohibition now in place, for master
by Jim Meyering 07 Sep '09

07 Sep '09

Just today we noticed that it was possible to push accidental merge commits to "master". While the commit hook to prevent that was in place, the config setting to enable the hook for "master" was missing. I've just done this on the server: git --git-dir /git/libvirt.git config hooks.denymerge.master true so *now*, we should be protected from ourselves.

2 1

[libvirt] [PATCH] xend_internal.c: Remove two dead stores to "ret"
by Jim Meyering 07 Sep '09

07 Sep '09

>From 99610dd77b95914623bd78f2bcac5d15a6c87024 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 19:14:09 +0200 Subject: [PATCH] xend_internal.c: Remove two dead stores to "ret" * src/xend_internal.c (xenDaemonCreateXML): Don't set "ret" after last use. --- src/xend_internal.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/xend_internal.c b/src/xend_internal.c index cf45cd6..2fa08f1 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -4028,10 +4028,10 @@ xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc, if (!(dom = virDomainLookupByName(conn, def->name))) goto error; - if ((ret = xend_wait_for_devices(conn, def->name)) < 0) + if (xend_wait_for_devices(conn, def->name) < 0) goto error; - if ((ret = xenDaemonDomainResume(dom)) < 0) + if (xenDaemonDomainResume(dom) < 0) goto error; virDomainDefFree(def); -- 1.6.4.2.409.g85dc3

2 1

[libvirt] [PATCH] openvz_driver.c: avoid dead store to "err"
by Jim Meyering 07 Sep '09

07 Sep '09

>From 559c1ba31a89fd3a6ed54631d5d900fa16140187 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 19:11:31 +0200 Subject: [PATCH] openvz_driver.c: avoid dead store to "err" * src/openvz_driver.c (openvzGenerateContainerVethName): Remove use and decl of "err". --- src/openvz_driver.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/src/openvz_driver.c b/src/openvz_driver.c index 43d54f9..a8c24ba 100644 --- a/src/openvz_driver.c +++ b/src/openvz_driver.c @@ -504,11 +504,10 @@ openvzGenerateVethName(int veid, char *dev_name_ve) static char * openvzGenerateContainerVethName(int veid) { - int ret; char temp[1024]; /* try to get line "^NETIF=..." from config */ - if ( (ret = openvzReadVPSConfigParam(veid, "NETIF", temp, sizeof(temp))) <= 0) { + if (openvzReadVPSConfigParam(veid, "NETIF", temp, sizeof(temp)) <= 0) { snprintf(temp, sizeof(temp), "eth0"); } else { char *saveptr; -- 1.6.4.2.409.g85dc3

2 1

[libvirt] 3 more dead stores
by Jim Meyering 07 Sep '09

07 Sep '09

>From 7a2202539b6445017c420644e5327ce64eaf3bac Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 17:27:34 +0200 Subject: [PATCH 1/2] domain_conf.c: remove two dead stores * src/domain_conf.c (virDomainSaveXML): Remove use and decl of "err". (virDomainDefParseXML): Likewise. --- src/domain_conf.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/domain_conf.c b/src/domain_conf.c index 8dde5dd..050cf50 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -2520,8 +2520,7 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn, /* Extract domain uuid */ tmp = virXPathString(conn, "string(./uuid[1])", ctxt); if (!tmp) { - int err; - if ((err = virUUIDGenerate(def->uuid))) { + if (virUUIDGenerate(def->uuid)) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to generate UUID")); goto error; @@ -4456,12 +4455,11 @@ int virDomainSaveXML(virConnectPtr conn, char *configFile = NULL; int fd = -1, ret = -1; size_t towrite; - int err; if ((configFile = virDomainConfigFile(conn, configDir, def->name)) == NULL) goto cleanup; - if ((err = virFileMakePath(configDir))) { + if (virFileMakePath(configDir)) { virReportSystemError(conn, errno, _("cannot create config directory '%s'"), configDir); -- 1.6.4.2.409.g85dc3 >From 89e2837b4382690e4d434c086541da1dcd3d4c58 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 18:53:20 +0200 Subject: [PATCH 2/2] util.c: avoid dead store to "flag" * src/util.c (virExecDaemonize): Change flag |= VAR to "flag | VAR". --- src/util.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util.c b/src/util.c index 2529837..af50028 100644 --- a/src/util.c +++ b/src/util.c @@ -663,7 +663,7 @@ int virExecDaemonize(virConnectPtr conn, ret = virExecWithHook(conn, argv, envp, keepfd, retpid, infd, outfd, errfd, - flags |= VIR_EXEC_DAEMON, + flags | VIR_EXEC_DAEMON, hook, data, pidfile); /* __virExec should have set an error */ -- 1.6.4.2.409.g85dc3

2 1

Re: [libvirt] [PATCH 0/6] sVirt AppArmor security driver
by Daniel Veillard 07 Sep '09

07 Sep '09

[ Sending again as my mail Sat, Sep 05, 2009 at 11:00:46AM +0200 didn't seem to have reach the list, Daniel ] On Fri, Sep 04, 2009 at 04:03:50PM -0500, Jamie Strandboge wrote: > On Fri, 04 Sep 2009, Daniel Veillard wrote: > > > On Fri, Sep 04, 2009 at 12:23:33PM -0500, Jamie Strandboge wrote: > > > This patch series implements the AppArmor security driver for sVirt. > > > This implementation was developed for the Ubuntu AppArmorLibvirtProfile > > > specification[1], but is general enough for any AppArmor deployment > > > (such as Ubuntu, *SUSE and Mandriva). > > > > > > This patch has seen quite a bit of real world testing in Ubuntu 9.10 > > > (our development release) in our 0.7.0-1ubuntu3 package. I did make a > > > few small changes after going through HACKING, but mostly I got the > > > tests going and added documentation. > > > > Could you triple check that the make syntax-check run clean with > > your patches applied, because a very quick glimpse shows up a number > > of malloc() which we don't allow for example. It may be because you > > didn't enter the files in the SCM but the checks should be done anyway > > on the new files please. > > > > I noticed that syntax-check wouldn't scan the new files unless I did a > git commit first. I did not run 'syntax-check' after adding the > documention examples, and just found that examples/apparmor/libvirt-qemu > had a trailing blank line (I was a bit surprised syntax-check checked > these files (attached is an updated docs patch)). okay > I can demonstrate that the files are being checked by removing the > '#include <config.h>' line in security_apparmor.c and virt-aa-helper.c > and syntax-check failing: [...] > I don't see that 'syntax-check' enforces not using *alloc, but I did > read in HACKING that *alloc are deprecated, though I had already written > the code before reading it. I do see other examples of *alloc in the I really though all cases of direct malloc or realloc had been eliminated, apparently it's not the case yet, and syntax-check doesn't enforce it... we need to do this. I note we also still have a few free() left around in the code base to cleanup too. > codebase, and I do properly check the return code of all calls to *alloc. > If you insist on those calls being replaced, I can do that, but I didn't > before submitting because I knew the current calls were correct and > didn't want to introduce bugs into the code. yeah, I think it's better to fix them before. In any case we have a week of bug fix only before the next release in a week, so best is to fix them now, wait for the actual reviews and then resubmit patches for inclusion. thanks, 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/

1 0

[libvirt] Some close/fclose/closedir calls are missing
by Matthias Bolte 07 Sep '09

07 Sep '09

While debugging something that looked like an file descriptor leak, we (Maximilian Wilhelm and me) found some places where close/fclose/closedir calls are missing, mostly in error handling if-branches. See attached patch. Max and Matthias

3 2

[libvirt] [PATCH 0/6] sVirt AppArmor security driver
by Jamie Strandboge 07 Sep '09

07 Sep '09

This patch series implements the AppArmor security driver for sVirt. This implementation was developed for the Ubuntu AppArmorLibvirtProfile specification[1], but is general enough for any AppArmor deployment (such as Ubuntu, *SUSE and Mandriva). This patch has seen quite a bit of real world testing in Ubuntu 9.10 (our development release) in our 0.7.0-1ubuntu3 package. I did make a few small changes after going through HACKING, but mostly I got the tests going and added documentation. DESIGN ------ When a virtual machine is started, determine if a profile is currently defined for the machine, and use it if available. If not, generate a new profile for the machine based on a template, which is by default a very restrictive profile allowing access to disk files, and anything else needed to run, such as the pid, monitor and log files. Virtual machines should have a unique profile specific to that machine. To ensure uniqueness, the profile name will be derived from the UUID of the virtual machine. These profiles should be configurable, either by adjusting the profile template for new machines, creating/modifying the VM profile directly or through the use of AppArmor abstractions. This will allow for administrators to fine-tune confinement for individual machines if desired. If enabled at compile time, the sVirt security model will be activated if AppArmor is available on the host OS and a profile for the libvirtd daemon is loaded when libvirtd is started. libvirtd should not be allowed to create arbitrary profiles or modify profiles directly, so as to not allow libvirtd to potentially (ie via a security bug in libvirtd itself) bootstrap out of AppArmor confinement. Because root privileges are needed to manipulate AppArmor profiles, qemu:///session will not be supported at this time, but the implementation must allow for a confined libvirtd with qemu:///session guests running unconfined. This can be revisited when AppArmor supports per-user profiles. Please see the specification[1] for more details. PATCHES ------- The patches are all against trunk as of yesterday. Testing was done on trunk and there seem to be no regressions over the the 0.7.0-1ubuntu3 package in Ubuntu. [PATCH 1*] patch_1a_reenable-nonfile-labels.patch: When James Morris originally submitted his sVirt patches (as seen in libvirt 0.6.1), he did not require on disk labelling for virSecurityDomainRestoreImageLabel. A later commit[2] changed this behavior to assume on disk labelling, which halts implementations for path-based MAC systems such as AppArmor and TOMOYO where vm->def->seclabel is required to obtain the label. This patch simply adds the 'virDomainObjPtr vm' argument back to *RestoreImageLabel. patch_1b_optional.patch: Due to the above change, 'make syntax-check' fails because SELinuxRestoreSecurityImageLabel() does not use the 'virDomainObjPtr vm'. patch_1b_optional.patch is a simple patch to fix this by checking if vm->def->seclabel == NULL and returns with error if it does. I realize this may not be desired in the long term, but it should be harmless enough to include. [PATCH 2] patch_2_security_c.patch: Updates src/security.c for AppArmor [PATCH 3] patch_3_security_apparmor.patch: Adds security_apparmor.c, security_apparmor.h, virt-aa-helper.c and updates po/POTFILES.in. virt-aa-helper.c is a new binary which is used exclusively by the AppArmor security driver to manipulate AppArmor. These files compile without warning and pass syntax-check. [PATCH 4] patch_4_tests.patch: Adds tests for virt-aa-helper and the security driver. secaatest.c is identical to seclabeltest.c except it initializes the 'apparmor' driver instead of 'selinux'. These tests are integrated into 'make check' and pass. [PATCH 5] patch_5_docs.patch: Updates docs/drvqemu.html.in for AppArmor and adds profile examples to examples/apparmor. [PATCH 6] patch_6_autoconf.patch: Updates Makefile.am and configure.in for AppArmor. It is based on and should operate the same as the SELinux configuration. Caveats and known issues: 1. it does not take advantage of the recent host device labelling functionality yet 2. it does not properly handle hot-plugging of devices yet 3. qemu:///session runs unconfined (see above) Thanks! Jamie (jdstrand on Freenode and OFTC) [1] https://wiki.ubuntu.com/SecurityTeam/Specifications/AppArmorLibvirtProfile [2] http://libvirt.org/git/?p=libvirt.git;a=commit;h=c86afc85ee0d1ec6d76c2d254b… -- Jamie Strandboge | http://www.canonical.com

3 10

[libvirt] [PATCH 0/6] Automatic sVirt management of host device labelling
by Daniel P. Berrange 07 Sep '09

07 Sep '09

This patch series extends the libvirt security driver API, and sVirt implementation to cover management of host device labelling. Previously users would have to set a global boolean tunable virt_use_pci/usb to allow all domains access to all host devices. With this series applied libvirt will automatically relabel only the individual PCI/USB devices which are assigned to a guest. ie it should make host device assignment 'just work' when sVirt is enforcing, and improve security It also attempts to address a problem with restoration of disk labels. The current code uses matchpathcon() to find the defalt label for a path. This works fine for locations which have a defined label in the policy (eg like /var/lib/libvirt/images), but if storing disk images in non-defualt locations, eg a external USB drive mounted under a place like /media/myusbdisk/virtual-images/, matchpathcon() returns NULL. In this scenario the disk would remain labelled with the MCS level specific to the just stopped VM. Since MCS labels are allocated on demand on each boot, this could allow a future VMs to access disks that it ought not to be able to. Dan Walsh suggested that we default to using the label defined for matchpathcon("/var/libvirt/images/00-DEFAULT") in this case, but this doesn't work for restoring USB/PCI device labels[1]. In all the case I've had this problem so far, the files' original label matched that of the directory it was contained in, so this patch just uses the containing directory's label when restoring labels. Dan didn't like this idea when I first mentioned it in IRC though, so perhaps I need to implement different logic still... ? Regards, Daniel [1] PCI device access from VMs requires labelling /sys/bus/pci/devices/$DOMAIN:$BUS:$SLOT:FUNCTION/{config, resource*, rom} while USB device access requires labelling /dev/bus/usb/$BUS/$DEVICE

5 18

[libvirt] [PATCH] Fix several memory leaks
by Ryota Ozaki 05 Sep '09

05 Sep '09

Hi, This patch fixes several memory leaks being scattered over libvirt. Thanks, ozaki-r >From 6fc282eae5192cccda208d8d4fd14c5c0676992b Mon Sep 17 00:00:00 2001 From: Ryota Ozaki <ozaki.ryota(a)gmail.com> Date: Tue, 1 Sep 2009 01:34:40 +0900 Subject: [PATCH] Fix several memory leaks --- qemud/qemud.c | 2 +- src/domain_conf.c | 1 + src/network_conf.c | 5 +++-- src/qemu_conf.c | 13 ++++++++----- src/storage_backend_fs.c | 5 +++++ 5 files changed, 18 insertions(+), 8 deletions(-) diff --git a/qemud/qemud.c b/qemud/qemud.c index df275e6..17ba44a 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -1733,7 +1733,7 @@ readmore: /* Possibly need to create another receive buffer */ if ((client->nrequests < max_client_requests && - VIR_ALLOC(client->rx) < 0)) { + !client->rx && VIR_ALLOC(client->rx) < 0)) { qemudDispatchClientFailure(client); } else { if (client->rx) diff --git a/src/domain_conf.c b/src/domain_conf.c index 1d2cc7c..4b64219 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -4361,6 +4361,7 @@ int virDomainSaveXML(virConnectPtr conn, cleanup: if (fd != -1) close(fd); + VIR_FREE(configFile); return ret; } diff --git a/src/network_conf.c b/src/network_conf.c index bb649a4..58a4f32 100644 --- a/src/network_conf.c +++ b/src/network_conf.c @@ -820,6 +820,7 @@ int virNetworkDeleteConfig(virConnectPtr conn, { char *configFile = NULL; char *autostartLink = NULL; + int ret = -1; if ((configFile = virNetworkConfigFile(conn, configDir, net->def->name)) == NULL) goto error; @@ -836,12 +837,12 @@ int virNetworkDeleteConfig(virConnectPtr conn, goto error; } - return 0; + ret = 0; error: VIR_FREE(configFile); VIR_FREE(autostartLink); - return -1; + return ret; } char *virNetworkConfigFile(virConnectPtr conn, diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 22f5edd..32d6a48 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1034,7 +1034,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, virDomainNetDefPtr net, int qemuCmdFlags) { - char *brname; + char *brname = NULL; int err; int tapfd = -1; int vnet_hdr = 0; @@ -1053,7 +1053,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, if (brname == NULL) return -1; } else if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { - brname = net->data.bridge.brname; + brname = strdup(net->data.bridge.brname); } else { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("Network type %d is not supported"), net->type); @@ -1063,7 +1063,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, if (!driver->brctl && (err = brInit(&driver->brctl))) { virReportSystemError(conn, err, "%s", _("cannot initialize bridge support")); - return -1; + goto cleanup; } if (!net->ifname || @@ -1072,7 +1072,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, VIR_FREE(net->ifname); if (!(net->ifname = strdup("vnet%d"))) { virReportOOMError(conn); - return -1; + goto cleanup; } /* avoid exposing vnet%d in dumpxml or error outputs */ template_ifname = 1; @@ -1100,9 +1100,12 @@ qemudNetworkIfaceConnect(virConnectPtr conn, } if (template_ifname) VIR_FREE(net->ifname); - return -1; + tapfd = -1; } +cleanup: + VIR_FREE(brname); + return tapfd; } diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c index ca6d329..222e591 100644 --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -856,6 +856,8 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, vol->type = VIR_STORAGE_VOL_FILE; vol->target.format = VIR_STORAGE_VOL_FILE_RAW; /* Real value is filled in during probe */ + if (vol->target.path) + VIR_FREE(vol->target.path); if (virAsprintf(&vol->target.path, "%s/%s", pool->def->target.path, vol->name) == -1) @@ -1022,6 +1024,9 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn, vol->type = VIR_STORAGE_VOL_FILE; + if (vol->target.path) + VIR_FREE(vol->target.path); + if (virAsprintf(&vol->target.path, "%s/%s", pool->def->target.path, vol->name) == -1) { -- 1.6.0.6

4 7

[libvirt] [PATCH] storage_driver.c: remove two useless calls to virStorageBackendForType
by Jim Meyering 04 Sep '09

04 Sep '09

>From dead-store warnings: >From c925b02828083a016f7b9a7ae93eb3a7bf5f4609 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 17:18:29 +0200 Subject: [PATCH] storage_driver.c: remove two useless calls to virStorageBackendForType * src/storage_driver.c (storagePoolGetInfo, storagePoolDefine): The result was never used, and the function has no side effects. --- src/storage_driver.c | 8 -------- 1 files changed, 0 insertions(+), 8 deletions(-) diff --git a/src/storage_driver.c b/src/storage_driver.c index e9ecb20..e8d43b7 100644 --- a/src/storage_driver.c +++ b/src/storage_driver.c @@ -519,15 +519,11 @@ storagePoolDefine(virConnectPtr conn, virStoragePoolDefPtr def; virStoragePoolObjPtr pool = NULL; virStoragePoolPtr ret = NULL; - virStorageBackendPtr backend; storageDriverLock(driver); if (!(def = virStoragePoolDefParseString(conn, xml))) goto cleanup; - if ((backend = virStorageBackendForType(def->type)) == NULL) - goto cleanup; - if (!(pool = virStoragePoolObjAssignDef(conn, &driver->pools, def))) goto cleanup; @@ -847,7 +843,6 @@ storagePoolGetInfo(virStoragePoolPtr obj, virStoragePoolInfoPtr info) { virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; - virStorageBackendPtr backend; int ret = -1; storageDriverLock(driver); @@ -860,9 +855,6 @@ storagePoolGetInfo(virStoragePoolPtr obj, goto cleanup; } - if ((backend = virStorageBackendForType(pool->def->type)) == NULL) - goto cleanup; - memset(info, 0, sizeof(virStoragePoolInfo)); if (pool->active) info->state = VIR_STORAGE_POOL_RUNNING; -- 1.6.4.2.409.g85dc3

2 3

[libvirt] [PATCH 6/7] VMware ESX: Add esxDomainUndefine() based on esxVI_UnregisterVM()
by Matthias Bolte 04 Sep '09

04 Sep '09

* src/esx/esx_driver.c: add esxDomainUndefine() based on esxVI_UnregisterVM() * src/esx/esx_vi_methods.[ch]: add esxVI_UnregisterVM()

2 3

[libvirt] [PATCH 5/7] VMware ESX: Add esxGetCapabilities() with basic default capabilities
by Matthias Bolte 04 Sep '09

04 Sep '09

* src/esx/esx_driver.c: add esxCapsInit() with default caps and add esxGetCapabilities()

2 3

[libvirt] [PATCH 1/7] VMware ESX: Fix VMX path parsing and URL encoding
by Matthias Bolte 04 Sep '09

04 Sep '09

* src/esx/esx_driver.c: handle spaces in VMX file path and use a virBuffer to encode spaces correctly in the resulting URL * src/esx/esx_vi.c: include the URL in the error message in case of a download error

2 3

[libvirt] [PATCH] domain_conf.c: remove dead store
by Jim Meyering 04 Sep '09

04 Sep '09

Another dead store: >From 06c5ed162cbdcfec74461e9d09c82244242cc6a2 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 17:27:34 +0200 Subject: [PATCH] domain_conf.c: remove dead store * src/domain_conf.c (virDomainSaveXML): ...and decl of "err". --- src/domain_conf.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/src/domain_conf.c b/src/domain_conf.c index 8dde5dd..4e8ad37 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -4456,12 +4456,11 @@ int virDomainSaveXML(virConnectPtr conn, char *configFile = NULL; int fd = -1, ret = -1; size_t towrite; - int err; if ((configFile = virDomainConfigFile(conn, configDir, def->name)) == NULL) goto cleanup; - if ((err = virFileMakePath(configDir))) { + if (virFileMakePath(configDir)) { virReportSystemError(conn, errno, _("cannot create config directory '%s'"), configDir); -- 1.6.4.2.409.g85dc3

2 1

[libvirt] [PATCH] xm_internal.c: remove two ret=... dead stores
by Jim Meyering 04 Sep '09

04 Sep '09

Two more dead stores: >From 92bf654b02e5c1eb3334fc13ef52f5b0679512ea Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 17:22:19 +0200 Subject: [PATCH] xm_internal.c: remove two ret=... dead stores * src/xm_internal.c (xenXMDomainCreate): Remove dead stores. --- src/xm_internal.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/xm_internal.c b/src/xm_internal.c index e675672..e948fa0 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -1851,10 +1851,10 @@ int xenXMDomainCreate(virDomainPtr domain) { goto error; domain->id = ret; - if ((ret = xend_wait_for_devices(domain->conn, domain->name)) < 0) + if (xend_wait_for_devices(domain->conn, domain->name) < 0) goto error; - if ((ret = xenDaemonDomainResume(domain)) < 0) + if (xenDaemonDomainResume(domain) < 0) goto error; xenUnifiedUnlock(priv); -- 1.6.4.2.409.g85dc3

2 1

[libvirt] [PATCH] lxc_container.c: avoid a leak on error paths
by Jim Meyering 04 Sep '09

04 Sep '09

This started with a dead-store report: File: lxc_container.c Location: line 417, column 10 Description: Although the value stored to 'rc' is used in the enclosing expression, the value is never actually read from 'rc' But there was a leak, too, upon any but the first failure. This fixes both. >From 141ba8bab2de00c911017bff6fca240c72a80633 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 16:12:35 +0200 Subject: [PATCH] lxc_container.c: avoid a leak on error paths * src/lxc_container.c (lxcContainerMountBasicFS): Don't leak upon failure. Add "cleanup:" label and change each post-allocation failure to use "goto cleanup" rather than returning immediately. --- src/lxc_container.c | 20 ++++++++++++-------- 1 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/lxc_container.c b/src/lxc_container.c index 2073864..f4381e7 100644 --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -1,6 +1,6 @@ /* * Copyright IBM Corp. 2008 - * Copyright Red Hat 2008 + * Copyright Red Hat 2008-2009 * * lxc_container.c: file description * @@ -384,12 +384,12 @@ static int lxcContainerMountBasicFS(virDomainFSDefPtr root) { "none", "/selinux", "selinuxfs" }, #endif }; - int i, rc; + int i, rc = -1; char *devpts; if (virAsprintf(&devpts, "/.oldroot%s/dev/pts", root->src) < 0) { virReportOOMError(NULL); - return -1; + return rc; } for (i = 0 ; i < ARRAY_CARDINALITY(mnts) ; i++) { @@ -397,31 +397,35 @@ static int lxcContainerMountBasicFS(virDomainFSDefPtr root) virReportSystemError(NULL, errno, _("failed to mkdir %s"), mnts[i].src); - return -1; + goto cleanup; } if (mount(mnts[i].src, mnts[i].dst, mnts[i].type, 0, NULL) < 0) { virReportSystemError(NULL, errno, _("failed to mount %s on %s"), mnts[i].type, mnts[i].type); - return -1; + goto cleanup; } } if ((rc = virFileMakePath("/dev/pts") < 0)) { virReportSystemError(NULL, rc, "%s", _("cannot create /dev/pts")); - return -1; + goto cleanup; } VIR_DEBUG("Trying to move %s to %s", devpts, "/dev/pts"); if ((rc = mount(devpts, "/dev/pts", NULL, MS_MOVE, NULL)) < 0) { virReportSystemError(NULL, errno, "%s", _("failed to mount /dev/pts in container")); - return -1; + goto cleanup; } + + rc = 0; + + cleanup: VIR_FREE(devpts); - return 0; + return rc; } static int lxcContainerPopulateDevices(void) -- 1.6.4.2.409.g85dc3

2 2

[libvirt] Dead code vs. XXX comment: remove or not?
by Jim Meyering 04 Sep '09

04 Sep '09

clang reported that this assignment to type is a dead store, since type is never used after this point. This is xm_internal.c, line 1074: /* XXX Forcing to pretend its a bridge */ if (type == -1) { type = 1; } Normally I'd just remove the whole if block, but the XXX comment makes me think someone has plans for this code. I'll wait for a second opinion.

2 3

[libvirt] address 3 more dead stores
by Jim Meyering 04 Sep '09

04 Sep '09

These changes fix two dead stores and add a comment suggesting why *not* to remove the third one. >From 41c778fdbaef9abf52bc4c3e278d3389d8198c34 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 10:55:55 +0200 Subject: [PATCH 1/3] interface_conf.c: remove a dead-store and declaration * src/interface_conf.c (virInterfaceDefParseDhcp): Remove unused "old". --- src/interface_conf.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/src/interface_conf.c b/src/interface_conf.c index a74d17e..96e0710 100644 --- a/src/interface_conf.c +++ b/src/interface_conf.c @@ -228,11 +228,9 @@ static int virInterfaceDefParseDhcp(virConnectPtr conn, virInterfaceDefPtr def, xmlNodePtr dhcp, xmlXPathContextPtr ctxt) { char *tmp; - xmlNodePtr old; int ret = 0; def->proto.dhcp = 1; - old = ctxt->node; ctxt->node = dhcp; /* Not much to do in the current version */ tmp = virXPathString(conn, "string(./@peerdns)", ctxt); -- 1.6.4.2.409.g85dc3 >From 10037ca58c8d93e427c480082704e7dd52424525 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 11:01:00 +0200 Subject: [PATCH 2/3] hash.c: remove a dead store * src/hash.c (virHashFree): Remove useless assignment to inside_table. --- src/hash.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/hash.c b/src/hash.c index 9308c0c..45c5747 100644 --- a/src/hash.c +++ b/src/hash.c @@ -231,7 +231,6 @@ virHashFree(virHashTablePtr table, virHashDeallocator f) inside_table = 0; iter = next; } - inside_table = 0; } VIR_FREE(table->table); } -- 1.6.4.2.409.g85dc3 >From 3b16e6d50d364a5307284a78a7b23d8631146663 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 11:20:23 +0200 Subject: [PATCH 3/3] qemu_conf.c: add a comment suggesting why we leave a dead-store * src/qemu_conf.c (qemuBuildHostNetStr): Do not remove the type_sep=',' dead store, since not having it would be a problem if we ever add a new attribute=VAL option. --- src/qemu_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 2c4a37d..6ffca2f 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1269,7 +1269,7 @@ qemuBuildHostNetStr(virConnectPtr conn, if (net->hostnet_name) { virBufferVSprintf(&buf, "%cname=%s", type_sep, net->hostnet_name); - type_sep = ','; + type_sep = ','; /* dead-store, but leave it, in case... */ } if (virBufferError(&buf)) { virReportOOMError(conn); -- 1.6.4.2.409.g85dc3

2 1

[libvirt] [PATCH 7/7] VMware ESX: Check the result of virUUIDParse() in esxUtil_GetConfigUUID()
by Matthias Bolte 04 Sep '09

04 Sep '09

* src/esx/esx_util.c: let esxUtil_GetConfigUUID() report an error if virUUIDParse() fails

2 1

[libvirt] [PATCH 4/7] VMware ESX: Simplify SOAP request and response handling
by Matthias Bolte 04 Sep '09

04 Sep '09

* src/esx/esx_vi.[ch]: convert esxVI_RemoteRequest_Execute() to a simpler esxVI_Context_Execute() version, remove esxVI_RemoteRequest and convert esxVI_RemoteResponse to esxVI_Response * src/esx/esx_vi_methods.c: update and simplify callers to use esxVI_Context_Execute() instead of esxVI_RemoteRequest_Execute()

2 1

[libvirt] [PATCH 3/7] VMware ESX: Use virXPathNode*() to simplify XPath handling
by Matthias Bolte 04 Sep '09

04 Sep '09

* src/esx/esx_vi.[ch]: use virXPathNode*() in esxVI_RemoteRequest_Execute() and remove esxVI_RemoteResponse_DeserializeXPathObject*() * src/esx/esx_vi_methods.c: update callers to use the new syntax of esxVI_RemoteRequest_Execute()

2 1

[libvirt] [PATCH 2/7] VMware ESX: Make esxVI_GetVirtualMachineIdentity() more robust
by Matthias Bolte 04 Sep '09

04 Sep '09

Check the config status before requesting config subelements * src/esx/esx_driver.c: add configStatus to the requested properties to check it in esxVI_GetVirtualMachineIdentity() * src/esx/esx_vi.[ch]: add esxVI_GetManagedEntityStatus() and use it in esxVI_GetVirtualMachineIdentity() * src/esx/esx_vi_types.[ch]: add VI type esxVI_ManagedEntityStatus

2 1

[libvirt] [PATCH] Fix more OOM handling bugs
by Daniel P. Berrange 04 Sep '09

04 Sep '09

* src/qemu_conf.c: Fix leak of values upon OOM * src/xend_internal.c: Fix missing check for OOM failure * tests/qemuargv2xmltest.c, tests/qemuxml2argvtest.c: Free stateDir upon exit to avoid leak --- src/qemu_conf.c | 21 ++++++++++++++++++--- src/xend_internal.c | 6 ++++++ tests/qemuargv2xmltest.c | 1 + tests/qemuxml2argvtest.c | 1 + 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 22f5edd..ad41104 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1722,6 +1722,8 @@ int qemudBuildCommandLine(virConnectPtr conn, continue; } + ADD_ARG_SPACE; + if (idx < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("unsupported disk type '%s'"), disk->dst); @@ -1773,7 +1775,10 @@ int qemudBuildCommandLine(virConnectPtr conn, optstr = virBufferContentAndReset(&opt); - ADD_ARG_LIT("-drive"); + if ((qargv[qargc++] = strdup("-drive")) == NULL) { + VIR_FREE(optstr); + goto no_memory; + } ADD_ARG(optstr); } } else { @@ -1829,6 +1834,7 @@ int qemudBuildCommandLine(virConnectPtr conn, net->vlan = i; + ADD_ARG_SPACE; if ((qemuCmdFlags & QEMUD_CMD_FLAG_NET_NAME) && qemuAssignNetNames(def, net) < 0) goto no_memory; @@ -1836,9 +1842,14 @@ int qemudBuildCommandLine(virConnectPtr conn, if (qemuBuildNicStr(conn, net, NULL, ',', net->vlan, &nic) < 0) goto error; - ADD_ARG_LIT("-net"); + if ((qargv[qargc++] = strdup("-net")) == NULL) { + VIR_FREE(nic); + goto no_memory; + } ADD_ARG(nic); + + ADD_ARG_SPACE; if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK || net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { int tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags); @@ -1862,7 +1873,10 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; } - ADD_ARG_LIT("-net"); + if ((qargv[qargc++] = strdup("-net")) == NULL) { + VIR_FREE(host); + goto no_memory; + } ADD_ARG(host); VIR_FREE(tapfd_name); @@ -2230,6 +2244,7 @@ static int qemuStringToArgvEnv(const char *args, goto no_memory; for (i = 0 ; i < envend ; i++) { progenv[i] = arglist[i]; + arglist[i] = NULL; } progenv[i] = NULL; } diff --git a/src/xend_internal.c b/src/xend_internal.c index 99847b0..20ddb89 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -5212,6 +5212,9 @@ xenDaemonFormatSxprChr(virConnectPtr conn, break; } + if (virBufferError(buf)) + return -1; + return 0; } @@ -5535,6 +5538,9 @@ xenDaemonFormatSxprSound(virConnectPtr conn, virBufferVSprintf(buf, "%s%s", i ? "," : "", str); } + if (virBufferError(buf)) + return -1; + return 0; } diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 7861520..4a92280 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -223,6 +223,7 @@ mymain(int argc, char **argv) DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat"); DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, "tcp:10.0.0.1:5000"); + free(driver.stateDir); virCapabilitiesFree(driver.caps); return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 6f25e7d..2f91288 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -270,6 +270,7 @@ mymain(int argc, char **argv) DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat"); DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, "tcp:10.0.0.1:5000"); + free(driver.stateDir); virCapabilitiesFree(driver.caps); return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); -- 1.6.2.5

2 1

[libvirt] [PATCH] Fix logging buffer overrun read
by Daniel P. Berrange 04 Sep '09

04 Sep '09

* src/logging.c: Fix buffer offset in logging read --- src/logging.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/logging.c b/src/logging.c index e46e2be..07c2b0e 100644 --- a/src/logging.c +++ b/src/logging.c @@ -240,7 +240,7 @@ static void virLogStr(const char *str, int len) { tmp = LOG_BUFFER_SIZE - virLogEnd; memcpy(&virLogBuffer[virLogEnd], str, tmp); virLogBuffer[LOG_BUFFER_SIZE] = 0; - memcpy(&virLogBuffer[0], &str[len], len - tmp); + memcpy(&virLogBuffer[0], &str[tmp], len - tmp); virLogEnd = len - tmp; } else { memcpy(&virLogBuffer[virLogEnd], str, len); -- 1.6.2.5

3 2

[libvirt] [PATCH] uml_conf.c: don't return an uninitialized pointer
by Jim Meyering 04 Sep '09

04 Sep '09

Another "real" bug: >From 6697607bf0b023ffb692576b31d652d10719b08a Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 12:05:52 +0200 Subject: [PATCH] uml_conf.c: don't return an uninitialized pointer * src/uml_conf.c (umlBuildCommandLineChr): Initialize "ret" also in the final switch cases. --- src/uml_conf.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/uml_conf.c b/src/uml_conf.c index 838fedd..2e9c25c 100644 --- a/src/uml_conf.c +++ b/src/uml_conf.c @@ -331,6 +331,7 @@ umlBuildCommandLineChr(virConnectPtr conn, default: umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("unsupported chr device type %d"), def->type); + ret = NULL; break; } -- 1.6.4.2.395.ge3d52

3 9

[libvirt] three more clang-inspired dead-store-fixing patches
by Jim Meyering 04 Sep '09

04 Sep '09

>From a2d03c987bb724283207dbeef873178c08a6c4c5 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 18:14:48 +0200 Subject: [PATCH 1/3] storage_backend_logical.c: appease clang: remove useless increment * src/storage_backend_logical.c (virStorageBackendLogicalBuildPool): Don't increment "n" when we won't use the result. --- src/storage_backend_logical.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c index bc40dd7..43117e8 100644 --- a/src/storage_backend_logical.c +++ b/src/storage_backend_logical.c @@ -437,7 +437,7 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn, goto cleanup; } - vgargv[n++] = NULL; + vgargv[n] = NULL; /* Now create the volume group itself */ if (virRun(conn, vgargv, NULL) < 0) -- 1.6.4.2.395.ge3d52 >From b72ed1ba5d5e6444f000fd9be706a51a90dd2292 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 18:23:51 +0200 Subject: [PATCH 2/3] openvz_conf.c: Remove dead store to copy_fd * src/openvz_conf.c (openvz_copyfile): Remove unused assignment. --- src/openvz_conf.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/openvz_conf.c b/src/openvz_conf.c index 41c6684..2f0471c 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -692,7 +692,6 @@ openvz_copyfile(char* from_path, char* to_path) fd = -1; if (close(copy_fd) < 0) goto error; - copy_fd = -1; return 0; -- 1.6.4.2.395.ge3d52 >From a8d2eca13635578084e3c2cc2093562d9a8fcfed Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 18:25:03 +0200 Subject: [PATCH 3/3] eventtest.c: detect write failure and avoid dead stores * tests/eventtest.c (mymain): Exit nonzero upon write failure. This also avoids several dead stores of the form ret = safewrite... --- tests/eventtest.c | 15 +++++++++------ 1 files changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/eventtest.c b/tests/eventtest.c index d25381d..ef37ff1 100644 --- a/tests/eventtest.c +++ b/tests/eventtest.c @@ -248,7 +248,6 @@ resetAll(void) static int mymain(int argc, char **argv) { - int ret = 0; char *progname; int i; pthread_t eventThread; @@ -304,7 +303,8 @@ mymain(int argc, char **argv) /* First time, is easy - just try triggering one of our * registered handles */ startJob("Simple write", &test); - ret = safewrite(handles[1].pipeFD[1], &one, 1); + if (safewrite(handles[1].pipeFD[1], &one, 1) != 1) + return EXIT_FAILURE; if (finishJob(1, -1) != EXIT_SUCCESS) return EXIT_FAILURE; @@ -314,7 +314,8 @@ mymain(int argc, char **argv) * try triggering another handle */ virEventRemoveHandleImpl(handles[0].watch); startJob("Deleted before poll", &test); - ret = safewrite(handles[1].pipeFD[1], &one, 1); + if (safewrite(handles[1].pipeFD[1], &one, 1) != 1) + return EXIT_FAILURE; if (finishJob(1, -1) != EXIT_SUCCESS) return EXIT_FAILURE; @@ -346,8 +347,9 @@ mymain(int argc, char **argv) * about */ startJob("Deleted during dispatch", &test); handles[2].delete = handles[3].watch; - ret = safewrite(handles[2].pipeFD[1], &one, 1); - ret = safewrite(handles[3].pipeFD[1], &one, 1); + if (safewrite(handles[2].pipeFD[1], &one, 1) != 1 + || safewrite(handles[3].pipeFD[1], &one, 1) != 1) + return EXIT_FAILURE; if (finishJob(2, -1) != EXIT_SUCCESS) return EXIT_FAILURE; @@ -356,7 +358,8 @@ mymain(int argc, char **argv) /* Extreme fun, lets delete ourselves during dispatch */ startJob("Deleted during dispatch", &test); handles[2].delete = handles[2].watch; - ret = safewrite(handles[2].pipeFD[1], &one, 1); + if (safewrite(handles[2].pipeFD[1], &one, 1) != 1) + return EXIT_FAILURE; if (finishJob(2, -1) != EXIT_SUCCESS) return EXIT_FAILURE; -- 1.6.4.2.395.ge3d52

2 2

[libvirt] [PATCH] Added Storage Volume support to VIrtualBox
by Pritesh Kothari 04 Sep '09

04 Sep '09

Hi All, I have added support for Storage Volumes in VirtualBox, the patch for the same is attached here. (needs the "little bit cleanup" patch previously posted) Regards, Pritesh

2 1

[libvirt] [PATCH] openvz_conf.c: don't use undefined local, "net"
by Jim Meyering 04 Sep '09

04 Sep '09

This looks like a real bug. >From 170af3320e68a0ac2cfe854fba28abe0e4040d2c Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 11:24:44 +0200 Subject: [PATCH] openvz_conf.c: don't use undefined local, "net" * src/openvz_conf.c (openvzReadNetworkConf): Upon openvzRead... failure, simply return -1, rather than "goto error;" where an uninitialized "net" could be dereferenced. --- src/openvz_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/openvz_conf.c b/src/openvz_conf.c index a172fe3..b1cb31a 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -197,7 +197,7 @@ openvzReadNetworkConf(virConnectPtr conn, openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Could not read 'IP_ADDRESS' from config for container %d"), veid); - goto error; + return -1; } else if (ret > 0) { token = strtok_r(temp, " ", &saveptr); while (token != NULL) { -- 1.6.4.2.395.ge3d52

3 4

[libvirt] [PATCH] storage_backend.c: assure clang that inputvol can't be NULL
by Jim Meyering 04 Sep '09

04 Sep '09

clang was complaining that a NULL inputvol would be dereferenced in that "could not open..." diagnostic. Since the two sole callers of this function are careful to call it only when inputvol is non-NULL, this is a good case for giving the parameter the nonnull attribute: >From 314278acb021a1f2e63494fab352bd6e9a4a38bb Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 10:22:57 +0200 Subject: [PATCH] storage_backend.c: assure clang that inputvol can't be NULL * src/storage_backend.c (virStorageBackendCopyToFD): Declare inputvol parameter to be "nonnull". Remove test for non-NULL inputvol. Both callers ensure it's non-NULL. --- src/storage_backend.c | 16 +++++++--------- 1 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/storage_backend.c b/src/storage_backend.c index c818142..8d1187e 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -1,7 +1,7 @@ /* * storage_backend.c: internal storage driver backend contract * - * Copyright (C) 2007-2008 Red Hat, Inc. + * Copyright (C) 2007-2009 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -106,7 +106,7 @@ enum { static int virStorageBackendCopyToFD(virConnectPtr conn, virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol, + virStorageVolDefPtr inputvol ATTRIBUTE_NONNULL, int fd, unsigned long long *total, int is_dest_file) @@ -119,13 +119,11 @@ virStorageBackendCopyToFD(virConnectPtr conn, char zerobuf[512]; char *buf = NULL; - if (inputvol) { - if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) { - virReportSystemError(conn, errno, - _("could not open input path '%s'"), - inputvol->target.path); - goto cleanup; - } + if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) { + virReportSystemError(conn, errno, + _("could not open input path '%s'"), + inputvol->target.path); + goto cleanup; } bzero(&zerobuf, sizeof(zerobuf)); -- 1.6.4.2.395.ge3d52

3 4

[libvirt] [PATCH] little bit cleanup
by Pritesh Kothari 04 Sep '09

04 Sep '09

Hi All, Just cleaned up some code to make the it more readable. The patch is attached here with. Directly applies to the HEAD as of today. Regards, Pritesh

3 2

[libvirt] [PATCH] Minor comment changes.
by Laine Stump 04 Sep '09

04 Sep '09

Fix some minor grammer (and one other) nits in comments that end up in generated API reference documentation. No functional/binary differences. --- src/libvirt.c | 24 ++++++++++++------------ 1 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index ca8e003..ce3d6ba 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -592,9 +592,9 @@ virRegisterNetworkDriver(virNetworkDriverPtr driver) /** * virRegisterInterfaceDriver: - * @driver: pointer to a interface driver block + * @driver: pointer to an interface driver block * - * Register a interface virtualization driver + * Register an interface virtualization driver * * Returns the driver priority or -1 in case of error. */ @@ -5365,7 +5365,7 @@ error: /** * virNetworkGetXMLDesc: * @network: a network object - * @flags: and OR'ed set of extraction flags, not used yet + * @flags: an OR'ed set of extraction flags, not used yet * * Provide an XML description of the network. The description may be reused * later to relaunch the network with virNetworkCreateXML(). @@ -5545,7 +5545,7 @@ error: /** * virInterfaceGetConnect: - * @iface: pointer to a interface + * @iface: pointer to an interface * * Provides the connection pointer associated with an interface. The * reference counter on the connection is not increased by this @@ -5819,7 +5819,7 @@ error: /** * virInterfaceGetName: - * @iface: a interface object + * @iface: an interface object * * Get the public name for that interface * @@ -5842,9 +5842,9 @@ virInterfaceGetName(virInterfacePtr iface) /** * virInterfaceGetMACString: - * @iface: a interface object + * @iface: an interface object * - * Get the MAC for a interface as string. For more information about + * Get the MAC for an interface as string. For more information about * MAC see RFC4122. * * Returns a pointer to the MAC address (in null-terminated ASCII @@ -5867,11 +5867,11 @@ virInterfaceGetMACString(virInterfacePtr iface) /** * virInterfaceGetXMLDesc: - * @iface: a interface object - * @flags: and OR'ed set of extraction flags, not used yet + * @iface: an interface object + * @flags: an OR'ed set of extraction flags, not used yet * * Provide an XML description of the interface. The description may be reused - * later to recreate the interface with virInterfaceCreateXML(). + * later to redefine the interface with virInterfaceDefineXML(). * * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error. * the caller must free() the returned value. @@ -6100,7 +6100,7 @@ error: * This method is typically useful for applications where multiple * threads are using a connection, and it is required that the * connection remain open until all threads have finished using - * it. ie, each new thread using a interface would increment + * it. ie, each new thread using an interface would increment * the reference count. * * Returns 0 in case of success, -1 in case of failure. @@ -6121,7 +6121,7 @@ virInterfaceRef(virInterfacePtr iface) /** * virInterfaceFree: - * @iface: a interface object + * @iface: an interface object * * Free the interface object. The interface itself is unaltered. * The data structure is freed and should not be used thereafter. -- 1.6.2.5

3 2

[libvirt] [PATCH] Fix a memory leak in virsh.
by Laine Stump 04 Sep '09

04 Sep '09

I found this while looking for examples of using virNodeDeviceGetXMLDesc(). AFAIK, *all* of the *GetXMLDesc() functions return a newly allocated chunk of memory that is owned by the caller, who must free it when they're done... --- src/virsh.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/src/virsh.c b/src/virsh.c index 15e0cef..910d860 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -5521,6 +5521,7 @@ cmdNodeDeviceDumpXML (vshControl *ctl, const vshCmd *cmd) { const char *name; virNodeDevicePtr device; + char *xml; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -5531,7 +5532,14 @@ cmdNodeDeviceDumpXML (vshControl *ctl, const vshCmd *cmd) return FALSE; } - vshPrint(ctl, "%s\n", virNodeDeviceGetXMLDesc(device, 0)); + xml = virNodeDeviceGetXMLDesc(device, 0); + if (!xml) { + virNodeDeviceFree(device); + return FALSE; + } + + vshPrint(ctl, "%s\n", xml); + free(xml); virNodeDeviceFree(device); return TRUE; } -- 1.6.2.5

3 2

[libvirt] [PATCH] Fix sexpr2string() to handle empty list
by Jim Fehlig 04 Sep '09

04 Sep '09

Finally have some time to submit this small patch for a bug I'm seeing with Xen 3.3.1. If s-expression returned by xend contains an empty list, sexpr2string() failed to serialize it. E.g. sexpr containing (cpus (()())) would cause sexpr2string() to fail. I spoke with danpb on IRC about this bug and he suggested adding a XML/sexpr pair that exhibits the problem to tests/xml2sexprtest.c. I can easily produce the sexpr but not the case with XML. The empty cpus lists indicate no explicit affinity and AFAIK the only way to express this in XML is absence of cpuset attribute in vcpu element. But IMO we should not produce the (cpus (()()...)) sexpr in this case. It should only be produced when user has explicitly specified affinity. Does this sound reasonable? Regards, Jim

2 1

[libvirt] [PATCH] Fix ID field in virDomainPtr after starting Xen guest
by Daniel P. Berrange 04 Sep '09

04 Sep '09

* src/xend_internal.c: Lookup guest after starting persistent config in order to refresh the domain ID field --- src/xend_internal.c | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/src/xend_internal.c b/src/xend_internal.c index 7bcee7d..459b6bc 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -4537,6 +4537,8 @@ virDomainPtr xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc) { int xenDaemonDomainCreate(virDomainPtr domain) { xenUnifiedPrivatePtr priv; + int ret; + virDomainPtr tmp; if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { virXendError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG, @@ -4549,7 +4551,17 @@ int xenDaemonDomainCreate(virDomainPtr domain) if (priv->xendConfigVersion < 3) return(-1); - return xend_op(domain->conn, domain->name, "op", "start", NULL); + ret = xend_op(domain->conn, domain->name, "op", "start", NULL); + + if (ret != -1) { + /* Need to force a refresh of this object's ID */ + tmp = virDomainLookupByName(domain->conn, domain->name); + if (tmp) { + domain->id = tmp->id; + virDomainFree(tmp); + } + } + return ret; } int xenDaemonDomainUndefine(virDomainPtr domain) -- 1.6.4

2 1

[libvirt] Save & Restore Synchronization
by Erkan Unal 04 Sep '09

04 Sep '09

Hi, I am using libvirt for qemu/kvm. I have following questions in terms of save and restore (Version 0.6.2): 1) As I checked the libvirt code, restore command is asynchronous. There is a macro called VIR_EXEC_NONBLOCK but there is no macro called VIR_EXEC_BLOCK. Is it possible to execute the restore in blocking mode so that I can measure the whole restore time? 2) I tried to measure the time spent to save the VM. I am executing save command and getting a timing. However when I execute the following operations in order I got an error right after script issues the restore operation (No delay between them): "save->restore->save" ... the error is: 20:19:02.478: error : internal error Unable to open monitor path /dev/pts/3 libvir: QEMU error : internal error Unable to open monitor path /dev/pts/3 20:19:02.478: error : internal error unable to start guest: char device redirected to /dev/pts/3 inet_listen: bind(ipv4,127.0.0.1,5912): Address already in use inet_listen: FAILED libvir: QEMU error : internal error unable to start guest: char device redirected to /dev/pts/3 inet_listen: bind(ipv4,127.0.0.1,5912): Address already in use inet_listen: FAILED 20:19:02.482: error : operation failed: failed to start VM libvir: QEMU error : operation failed: failed to start VM error: Failed to restore domain from /path/to/the/save/file error: operation failed: failed to start VM (You can find the detailed error report in the attachement when LIBVIRT_DEBUG flag is 1) I think that some of the cleanup is non-blocking which are cleanup of pty device and vncserver. However, I am not sure if destroying the qemu process is asynchronous?? P.S.: If I put one second delay between first save and restore, there is no error. Save and restore work fine. Thanks, Erkan Unal 20:26:07.506: debug : virInitialize:287 : register drivers 20:26:07.506: debug : virRegisterDriver:660 : registering Test as driver 0 20:26:07.506: debug : virRegisterNetworkDriver:560 : registering Test as network driver 0 20:26:07.506: debug : virRegisterStorageDriver:591 : registering Test as storage driver 0 20:26:07.506: debug : virRegisterDeviceMonitor:622 : registering Test as device driver 0 20:26:07.506: debug : virRegisterDriver:660 : registering Xen as driver 1 20:26:07.506: debug : virRegisterDriver:660 : registering OPENVZ as driver 2 20:26:07.506: debug : virRegisterDriver:660 : registering remote as driver 3 20:26:07.506: debug : virRegisterNetworkDriver:560 : registering remote as network driver 1 20:26:07.506: debug : virRegisterStorageDriver:591 : registering remote as storage driver 1 20:26:07.506: debug : virRegisterDeviceMonitor:622 : registering remote as device driver 1 20:26:07.506: debug : virConnectOpenAuth:1089 : name=qemu:///session, auth=0x7fa21cbae660, flags=0 20:26:07.506: debug : do_open:909 : name "qemu:///session" to URI components: scheme qemu opaque (null) authority (null) server (null) user (null) port 0 path /session 20:26:07.506: debug : do_open:919 : trying driver 0 (Test) ... 20:26:07.506: debug : do_open:925 : driver 0 Test returned DECLINED 20:26:07.506: debug : do_open:919 : trying driver 1 (Xen) ... 20:26:07.506: debug : do_open:925 : driver 1 Xen returned DECLINED 20:26:07.506: debug : do_open:919 : trying driver 2 (OPENVZ) ... 20:26:07.506: debug : do_open:925 : driver 2 OPENVZ returned DECLINED 20:26:07.506: debug : do_open:919 : trying driver 3 (remote) ... 20:26:07.506: debug : remoteOpen:974 : Auto-remote UNIX socket 20:26:07.506: debug : remoteOpen:992 : Auto-spawn user daemon instance 20:26:07.506: debug : doRemoteOpen:511 : proceeding with name = qemu:///session 20:26:07.507: debug : call:6473 : Doing call 66 (nil) 20:26:07.507: debug : call:6543 : We have the buck 66 0x7fa21bdb5010 0x7fa21bdb5010 20:26:07.507: debug : processCallRecvLen:6131 : Got length, now need 36 total (32 more) 20:26:07.507: debug : processCalls:6399 : Giving up the buck 66 0x7fa21bdb5010 (nil) 20:26:07.507: debug : call:6574 : All done with our call 66 (nil) 0x7fa21bdb5010 20:26:07.507: debug : call:6473 : Doing call 1 (nil) 20:26:07.507: debug : call:6543 : We have the buck 1 0xf86d80 0xf86d80 20:26:07.509: debug : processCallRecvLen:6131 : Got length, now need 28 total (24 more) 20:26:07.509: debug : processCalls:6399 : Giving up the buck 1 0xf86d80 (nil) 20:26:07.509: debug : call:6574 : All done with our call 1 (nil) 0xf86d80 20:26:07.509: debug : doRemoteOpen:822 : Adding Handler for remote events 20:26:07.509: debug : doRemoteOpen:829 : virEventAddHandle failed: No addHandleImpl defined. continuing without events. 20:26:07.509: debug : do_open:925 : driver 3 remote returned SUCCESS 20:26:07.509: debug : do_open:945 : network driver 0 Test returned DECLINED 20:26:07.509: debug : do_open:945 : network driver 1 remote returned SUCCESS 20:26:07.509: debug : do_open:967 : storage driver 0 Test returned DECLINED 20:26:07.509: debug : do_open:967 : storage driver 1 remote returned SUCCESS 20:26:07.510: debug : do_open:988 : node driver 0 Test returned DECLINED 20:26:07.510: debug : do_open:988 : node driver 1 remote returned SUCCESS 20:26:07.510: debug : virDomainRestore:2012 : conn=0xf82430, from=/path/to/save/file 20:26:07.510: debug : call:6473 : Doing call 54 (nil) 20:26:07.510: debug : call:6543 : We have the buck 54 0xf86d80 0xf86d80 20:26:07.625: error : internal error Unable to open monitor path /dev/pts/3 libvir: QEMU error : internal error Unable to open monitor path /dev/pts/3 20:26:07.625: error : internal error unable to start guest: char device redirected to /dev/pts/3 inet_listen: bind(ipv4,127.0.0.1,5912): Address already in use inet_listen: FAILED libvir: QEMU error : internal error unable to start guest: char device redirected to /dev/pts/3 inet_listen: bind(ipv4,127.0.0.1,5912): Address already in use inet_listen: FAILED 20:26:07.629: error : operation failed: failed to start VM libvir: QEMU error : operation failed: failed to start VM 20:26:07.629: debug : processCallRecvLen:6131 : Got length, now need 160 total (156 more) 20:26:07.629: debug : processCalls:6399 : Giving up the buck 54 0xf86d80 (nil) 20:26:07.629: debug : call:6574 : All done with our call 54 (nil) 0xf86d80 error: Failed to restore domain from /path/to/save/file error: operation failed: failed to start VM 20:26:07.629: debug : virConnectClose:1107 : conn=0xf82430 20:26:07.629: debug : call:6473 : Doing call 2 (nil) 20:26:07.629: debug : call:6543 : We have the buck 2 0xf86d80 0xf86d80 20:26:07.629: debug : processCallRecvLen:6131 : Got length, now need 28 total (24 more) 20:26:07.629: debug : processCalls:6399 : Giving up the buck 2 0xf86d80 (nil) 20:26:07.629: debug : call:6574 : All done with our call 2 (nil) 0xf86d80 20:26:07.629: debug : virUnrefConnect:210 : unref connection 0xf82430 1 20:26:07.629: debug : virReleaseConnect:171 : release connection 0xf82430

2 1

[libvirt] libvirt socket closed unexpectedly / libvirt broken pipe
by Dave 04 Sep '09

04 Sep '09

Hello, I'm trying to get eucalyptus v1.5.2 working with debian lenny, the relevant versions of libvirt software installed are from the apt-get repos, which are: # dpkg -l|grep libvirt ii libvirt-bin 0.4.6-10 the programs for the libvirt library ii libvirt0 0.4.6-10 library for interfacing with different virtualization systems The the conf file looks like so: # cat libvirtd.conf|grep -v '#' unix_sock_group = "libvirt" unix_sock_rw_perms = "0770" auth_unix_ro = "none" auth_unix_rw = "none" ----- my eucalyptus nc.log has the following error, which i believe is the issue, basically any instance i start, terminates about a minute later by itself, any guidance or help is much appreciated [Sat Aug 15 03:20:41 2009][004602][EUCAERROR ] libvirt: socket closed unexpectedly (code=39) [Sat Aug 15 03:20:43 2009][004602][EUCAERROR ] libvirt: Broken pipe (code=38) thoughts? ideas?

2 1

[libvirt] [PATCH] Allow libvirtd to RPC to external libvirtd.
by Chris Lalancette 04 Sep '09

04 Sep '09

Allow the daemon itself to make RPCs to an external libvirtd, but only if the URI is fully specified. While this isn't used at the moment, it will be for the tunnelled migration support in the future. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/remote_internal.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/remote_internal.c b/src/remote_internal.c index bc69c70..a416cf3 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -1001,7 +1001,7 @@ remoteOpen (virConnectPtr conn, int ret, rflags = 0; const char *autostart = getenv("LIBVIRT_AUTOSTART"); - if (inside_daemon) + if (inside_daemon && (!conn->uri || (conn->uri && !conn->uri->server))) return VIR_DRV_OPEN_DECLINED; if (!(priv = remoteAllocPrivateData(conn))) -- 1.6.0.6

2 3

[libvirt] On pidfile security and using -daemonize to start qemu/kvm
by Charles Duffy 03 Sep '09

03 Sep '09

I had a brief conversation with Daniel Berrange on IRC this morning regarding race conditions in libvirt's invocation of qemu domains. The preferred approach from qemu's maintainership is using -daemonize at invocation, and then waiting for the process to exit before attempting to connect to the monitor; after this occurs, qemu guarantees monitor availability, so no retry/timeout behavior is necessary. Dan pointed out an issue with this approach: To determine the PID of the qemu instance started, the -pidfile argument to qemu needs to be used with -daemonize. However, -pidfile writes the requested pidfile with qemu's permissions; if the guest were subverted, it could rewrite its pidfile and lead libvirt to kill other, arbitrary processes. Neither changing permissions with -runas rather than prior to invocation nor chown'ing the pidfile from libvirt after creation helps, as qemu keeps a file handle on it open throughout operation (to maintain a POSIX lock on the file). However, a workaround exists, and it meshes well with a security fix which needs to be implemented anyhow: At present, on initial startup or libvirtd restart, we read the PIDs associated with vCPUs from the monitor in virDetectVcpuPIDs(); doing this on libvirtd restart poses a security risk for the reason given above, so we need to have libvirtd record the PIDs on initial startup (before any untrusted code within the guest is ever run) regardless. If we also read the libvirtd pidfile at this same early time (before the guest has been started and thus had an opportunity to run untrusted code) and include its contents when writing out the vCPU PIDs, this would eliminate the issue with pidfile ownership blocking use of -daemonize. Thoughts?

2 1

[libvirt] [PATCH] xen_internal.c: remove two unused local variables
by Jim Meyering 03 Sep '09

03 Sep '09

Obviously unnecessary. Maybe someone had plans for these long ago? >From 94730ec1d99076cd18b04b950adad7c7a576071f Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 17:11:14 +0200 Subject: [PATCH] xen_internal.c: remove two unused local variables * src/xen_internal.c (xenHypervisorSetSchedulerParameters): Remove set-but-never-used local variables, cap_set and weight_set. --- src/xen_internal.c | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-) diff --git a/src/xen_internal.c b/src/xen_internal.c index ae78f84..30fd8bd 100644 --- a/src/xen_internal.c +++ b/src/xen_internal.c @@ -1299,9 +1299,6 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, TODO return(-1); case XEN_SCHEDULER_CREDIT: { - int weight_set = 0; - int cap_set = 0; - memset(&op_dom, 0, sizeof(op_dom)); op_dom.cmd = XEN_V2_OP_SCHEDULER; op_dom.domain = (domid_t) domain->id; @@ -1326,7 +1323,6 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, return(-1); } op_dom.u.getschedinfo.u.credit.weight = val; - weight_set = 1; } else if (STREQ (params[i].field, str_cap) && params[i].type == VIR_DOMAIN_SCHED_FIELD_UINT) { val = params[i].value.ui; @@ -1336,7 +1332,6 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, return(-1); } op_dom.u.getschedinfo.u.credit.cap = val; - cap_set = 1; } else { virXenErrorFunc (domain->conn, VIR_ERR_INVALID_ARG, __FUNCTION__, "Credit scheduler accepts 'cap' and 'weight' integer parameters", -- 1.6.4.2.395.ge3d52

2 1

[libvirt] [PATCH] python: let libvirt_virConnectDomainEventCallback indicate success
by Jim Meyering 03 Sep '09

03 Sep '09

First dead-store-related bug fix: >From 6024fb05dfa2dd408ad79abbac88791f5048af11 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 14:09:49 +0200 Subject: [PATCH] python: let libvirt_virConnectDomainEventCallback indicate success * python/libvir.c (libvirt_virConnectDomainEventCallback): Return 0 when successful, rather than always returning -1. clang flagged this function for its dead-store of "ret=0". Once "ret" was set to 0, it was never used, and the function would always return -1. --- python/libvir.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/python/libvir.c b/python/libvir.c index e2c196a..0c00b80 100644 --- a/python/libvir.c +++ b/python/libvir.c @@ -4,7 +4,7 @@ * entry points where an automatically generated stub is * unpractical * - * Copyright (C) 2005, 2007, 2008 Red Hat, Inc. + * Copyright (C) 2005, 2007-2009 Red Hat, Inc. * * Daniel Veillard <veillard(a)redhat.com> */ @@ -1812,7 +1812,7 @@ libvirt_virConnectDomainEventCallback(virConnectPtr conn ATTRIBUTE_UNUSED, cleanup: LIBVIRT_RELEASE_THREAD_STATE; - return -1; + return ret; } static PyObject * -- 1.6.4.2.395.ge3d52

2 1

[libvirt] [PATCH 0/3] QEMU directory/monitor socket fixes/cleanup
by Daniel P. Berrange 03 Sep '09

03 Sep '09

This patch series does three things - Moves QEMU monitor sockets in /var/lib/libvirt/qemu, since this is created file and thus should be separate from libvirtd created data to avoid risk from QEMU compromise. - Sets QEMU directory ownership to match configured qemu user/group - Fixes a minor memory leak / socket unlink

2 4

[libvirt] [PATCH 2/2]: VirtualBox: Updated vboxNetworkUndefine() and vboxNetworkDestroy()
by Pritesh Kothari 03 Sep '09

03 Sep '09

Hi All, I have made some changes to the functions vboxNetworkCreateXML(), vboxNetworkDefineXML(), vboxNetworkUndefine() and vboxNetworkDestroy() to handle multiple host only interfaces as multiple host only interfaces are supported by VirtualBox 3.0 and greater. The patch's are as below: PATCH 1/2: Merged vboxNetworkCreateXML() and vboxNetworkDefineXML() and added code to handle multiple hostonly interfaces. PATCH 2/2: Merged vboxNetworkUndefine() and vboxNetworkDestroy() and added code to handle multiple hostonly interfaces. Regards, Pritesh

3 2

[libvirt] [PATCH] esx_vi: remove unreachable code
by Jim Meyering 03 Sep '09

03 Sep '09

This removes dead code and suppresses a warning from clang. However, see below: >From 2ed548cbe948aa7ef37ccd121bd084bd05019ac8 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 16:35:47 +0200 Subject: [PATCH] esx_vi: remove unreachable code * src/esx/esx_vi.c (esxVI_Enumeration_Deserialize): Remove unreachable code. --- src/esx/esx_vi.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 0da908b..c689a7a 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -857,8 +857,6 @@ esxVI_Enumeration_Deserialize(virConnectPtr conn, failure: goto cleanup; - - result = -1; } -------------------------------------------- This function does not allow the caller to distinguish via its return value whether it has failed. It always returns 0. I see that this function's return value is used in numerous places (each esxVI_.*Deserialize function), so this should be fixed. int esxVI_Enumeration_Deserialize(virConnectPtr conn, const esxVI_Enumeration *enumeration, xmlNodePtr node, int *value) { int i; int result = 0; char *name = NULL; if (value == NULL) { ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Invalid argument"); goto failure; } *value = 0; /* undefined */ if (esxVI_String_DeserializeValue(conn, node, &name) < 0) { goto failure; } for (i = 0; enumeration->values[i].name != NULL; ++i) { if (STREQ(name, enumeration->values[i].name)) { *value = enumeration->values[i].value; goto cleanup; } } ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Unknown value '%s' for %s", name, enumeration->type); cleanup: VIR_FREE(name); return result; failure: goto cleanup; }

2 2

[libvirt] [PATCH 0/1] Support huge pages in guests
by Daniel P. Berrange 03 Sep '09

03 Sep '09

This patch is an update of John Cooper's original work from last month http://www.redhat.com/archives/libvir-list/2009-July/msg00753.html Changes since that patch - We create a directory $MOUNTPOINT/libvirt/qemu inside the hugetlbfs mount point. This is neccessary because QEMU now runs as 'qemu:qemu' in Fedora and thus cannot create files in the root. libvirtd will automatically chown() this subdir to allow QEMU guests access - Don't automatically probe for hugetlbfs mount if runing as an unprivileged libvirtd, since user won't have access to the mount unless administrator has manually created them a subdir. - Change the XML to look like <memoryBacking> <hugepages/> </memoryBacking> This is because I fear we may have more config options for memory backing store in the future, such as whether KVM can use KSM for sharing on the guest. - Add a test case for it in the XML -> ARGV convetor - Document the new XML options - Add the new XML options to the RNG schema - Fix setup & auto-detection of hugetlbfs mount if no qemu.conf file exists at all - Add config parameter to the augeas schema - Move the code which finds a mount point into util.c since its not specific to QEMU

4 5

[libvirt] PATCH 0/0: Support setting a disk drive serial (number) v2
by Daniel P. Berrange 03 Sep '09

03 Sep '09

This is a second version of the patch posted here http://www.redhat.com/archives/libvir-list/2009-August/msg00341.html In this version, I make the QEMU driver refuse to run with any serial that doesn't contain 0-9, a-Z, -, _, since there's no way to escape special chars in QEMU command line args. Hopefully we can lift this restriction in a future version of QEMU I also add a check that serial=s is actually supported by QEMU

3 3

[libvirt] [PATCH] mdns.c: remove dead initialization
by Jim Meyering 03 Sep '09

03 Sep '09

The only following uses of group look like this: (always preceded by initialization): AVAHI_DEBUG("Client collision/connecting %p", mdns->client); group = mdns->group; while (group) { if (group->handle) avahi_entry_group_reset(group->handle); group = group->next; } So the value used in the initialization at the top is never used. >From 515e4a4542ce396b8775c618cbe17a30b78299b4 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 13:45:22 +0200 Subject: [PATCH] mdns.c: remove dead initialization * qemud/mdns.c (libvirtd_mdns_client_callback): Remove dead initialization. --- qemud/mdns.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/qemud/mdns.c b/qemud/mdns.c index ae6ff08..73b7d11 100644 --- a/qemud/mdns.c +++ b/qemud/mdns.c @@ -178,7 +178,7 @@ static void libvirtd_mdns_create_services(struct libvirtd_mdns_group *group) { static void libvirtd_mdns_client_callback(AvahiClient *c, AvahiClientState state, void *userdata) { struct libvirtd_mdns *mdns = (struct libvirtd_mdns *)userdata; - struct libvirtd_mdns_group *group = mdns->group; + struct libvirtd_mdns_group *group; if (!mdns->client) mdns->client = c; -- 1.6.4.2.395.ge3d52

2 1

[libvirt] [PATCH] node_device_conf.c: remove dead initialization
by Jim Meyering 03 Sep '09

03 Sep '09

The following use of "caps" is the assignment in a for-loop, so the initialization is unnecessary. >From 7032a0b370db1a9489cf19fce0cabbb6959320ab Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 13:38:57 +0200 Subject: [PATCH] node_device_conf.c: remove dead initialization * src/node_device_conf.c (virNodeDeviceDefFormat): Remove dead initialization. --- src/node_device_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/node_device_conf.c b/src/node_device_conf.c index 56a9bb5..5b25bb7 100644 --- a/src/node_device_conf.c +++ b/src/node_device_conf.c @@ -213,7 +213,7 @@ char *virNodeDeviceDefFormat(virConnectPtr conn, const virNodeDeviceDefPtr def) { virBuffer buf = VIR_BUFFER_INITIALIZER; - virNodeDevCapsDefPtr caps = def->caps; + virNodeDevCapsDefPtr caps; char *tmp; virBufferAddLit(&buf, "<device>\n"); -- 1.6.4.2.395.ge3d52

2 1

[libvirt] [PATCH] Fix misc OOM bugs
by Daniel P. Berrange 03 Sep '09

03 Sep '09

* tests/testutils.c: Run test function twice, once to prime it for static allocations, once to count the non-static allocations. * tests/testutilsqemu.c: Initialize variable correctl * src/capabilities.c: Don't free machines variable upon failure since caller must do that * src/xm_internal.c: Add missing check for OOM in building VIF config param --- src/capabilities.c | 17 +++++++++-------- src/xm_internal.c | 3 +++ tests/testutils.c | 12 ++++++++---- tests/testutilsqemu.c | 2 +- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/capabilities.c b/src/capabilities.c index 193a9fe..289180d 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -354,10 +354,6 @@ virCapabilitiesAddGuest(virCapsPtr caps, if (loader && (guest->arch.defaultInfo.loader = strdup(loader)) == NULL) goto no_memory; - if (nmachines) { - guest->arch.defaultInfo.nmachines = nmachines; - guest->arch.defaultInfo.machines = machines; - } if (VIR_REALLOC_N(caps->guests, caps->nguests + 1) < 0) @@ -365,6 +361,11 @@ virCapabilitiesAddGuest(virCapsPtr caps, caps->guests[caps->nguests] = guest; caps->nguests++; + if (nmachines) { + guest->arch.defaultInfo.nmachines = nmachines; + guest->arch.defaultInfo.machines = machines; + } + return guest; no_memory: @@ -407,10 +408,6 @@ virCapabilitiesAddGuestDomain(virCapsGuestPtr guest, if (loader && (dom->info.loader = strdup(loader)) == NULL) goto no_memory; - if (nmachines) { - dom->info.nmachines = nmachines; - dom->info.machines = machines; - } if (VIR_REALLOC_N(guest->arch.domains, guest->arch.ndomains + 1) < 0) @@ -418,6 +415,10 @@ virCapabilitiesAddGuestDomain(virCapsGuestPtr guest, guest->arch.domains[guest->arch.ndomains] = dom; guest->arch.ndomains++; + if (nmachines) { + dom->info.nmachines = nmachines; + dom->info.machines = machines; + } return dom; diff --git a/src/xm_internal.c b/src/xm_internal.c index 71b852e..66f1e1c 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -2048,6 +2048,9 @@ static int xenXMDomainConfigFormatNet(virConnectPtr conn, virBufferVSprintf(&buf, ",vifname=%s", net->ifname); + if (virBufferError(&buf)) + goto cleanup; + if (VIR_ALLOC(val) < 0) { virReportOOMError(conn); goto cleanup; diff --git a/tests/testutils.c b/tests/testutils.c index 7a1dbdc..5072fec 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -367,10 +367,7 @@ int virtTestMain(int argc, } } - if (testOOM) - virAllocTestInit(); - - /* Run once to count allocs, and ensure it passes :-) */ + /* Run once to prime any static allocations & ensure it passes */ ret = (func)(argc, argv); if (ret != EXIT_SUCCESS) goto cleanup; @@ -385,6 +382,13 @@ int virtTestMain(int argc, testOOM++; virSetErrorFunc(NULL, virtTestErrorFuncQuiet); + virAllocTestInit(); + + /* Run again to count allocs, and ensure it passes :-) */ + ret = (func)(argc, argv); + if (ret != EXIT_SUCCESS) + goto cleanup; + approxAlloc = virAllocTestCount(); testCounter++; if (testDebug) diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 58707e1..6b6a185 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -9,7 +9,7 @@ virCapsPtr testQemuCapsInit(void) { struct utsname utsname; virCapsPtr caps; virCapsGuestPtr guest; - virCapsGuestMachinePtr *machines; + virCapsGuestMachinePtr *machines = NULL; int nmachines; static const char *const x86_machines[] = { "pc", "isapc" -- 1.6.2.5

2 2

[libvirt] [PATCH] remote_internal.c: don't dereference a NULL "conn"
by Jim Meyering 03 Sep '09

03 Sep '09

Here's the test just before the else-if in the patch below: if (conn && conn->driver && STREQ (conn->driver->name, "remote")) { So, in the else-branch, "conn" is guaranteed to be NULL. And dereferenced. This may be only a theoretical risk, but if so, the test of "conn" above should be changed to an assertion, and/or the parameter should get the nonnull attribute. >From a1b1d36d96f6b50ddf514539af85da20ca671bf5 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 2 Sep 2009 11:54:38 +0200 Subject: [PATCH] remote_internal.c: don't dereference a NULL "conn" * src/remote_internal.c (remoteDevMonOpen): Avoid NULL-dereference. --- src/remote_internal.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/remote_internal.c b/src/remote_internal.c index ea50c11..141fef9 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -5148,7 +5148,7 @@ remoteDevMonOpen(virConnectPtr conn, conn->devMonPrivateData = priv; remoteDriverUnlock(priv); return VIR_DRV_OPEN_SUCCESS; - } else if (conn->networkDriver && + } else if (conn && conn->networkDriver && STREQ (conn->networkDriver->name, "remote")) { struct private_data *priv = conn->networkPrivateData; remoteDriverLock(priv); -- 1.6.4.2.395.ge3d52

5 9

[libvirt] [PATCH] test.c: don't use undefined local, "def"
by Jim Meyering 03 Sep '09

03 Sep '09

another real bug: >From fd41bd47252654f84684830e02ecbeb3d26e7dee Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 12:12:19 +0200 Subject: [PATCH] test.c: don't use undefined local, "def" * src/test.c (testOpenVolumesForPool): Upon early virAsprintf or virXPathNodeSet failure, "goto error" would take us to virStorageVolDefFree(def), but with "def" not defined. Initialize it to NULL. --- src/test.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/test.c b/src/test.c index 7c8f85b..895cce1 100644 --- a/src/test.c +++ b/src/test.c @@ -435,7 +435,7 @@ static int testOpenVolumesForPool(virConnectPtr conn, char *vol_xpath; int i, ret, func_ret = -1; xmlNodePtr *vols = NULL; - virStorageVolDefPtr def; + virStorageVolDefPtr def = NULL; /* Find storage volumes */ if (virAsprintf(&vol_xpath, "/node/pool[%d]/volume", poolidx) < 0) { -- 1.6.4.2.395.ge3d52

2 1

[libvirt] Support for XCI (Xen Client)
by Andreas Sommer 03 Sep '09

03 Sep '09

Hi, I just wondered whether the Xen Client project (http://www.xen.org/products/xci.html) has been tested, or is supported by libvirt at all? I'm not sure if it has a different Xen API, but it definitely has different domain configuration files - that's why I ask. Best regards

2 1

[libvirt] [PATCH] Support for defining/dumping video device in VirtualBox
by Pritesh Kothari 03 Sep '09

03 Sep '09

Hi All, I have added support for defining/dumping video device in VirtualBox. The patch for the same is attached here. Also this patch depends on the earlier patch "Support for 3d Acceleration in video tag". Regards, Pritesh

3 3

[libvirt] [PATCH] Support for 3d Acceleration in video tag
by Pritesh Kothari 03 Sep '09

03 Sep '09

Hi All, I have added support for 3d Acceleration in the video tag, the patch for the same is attached here. Regards, Pritesh

3 5

[libvirt] [PATCH] Fix bugs in virDomainMigrate v2 code.
by Chris Lalancette 03 Sep '09

03 Sep '09

Paolo Bonzini points out that in my refactoring of the code for virDomainMigrate(), I added a check for the return value from virDomainMigratePerform(). The problem is that we don't want to exit if we fail, we actually want to go on and do virDomainMigrateFinish2() with a non-0 return code to clean things up. Remove the check. While reproducing this issue, I also noticed that we wouldn't always properly propagate an error message. In particular, I found that if you blocked off the migration ports (with iptables) and then tried the migration, it would actually fail but we would get no failure output from Qemu. Therefore, we would think we succeeded, and leave a huge mess behind us. Execute the monitor command "info migrate", and look for a failure string in there as well. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/libvirt.c | 2 -- src/qemu_driver.c | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index ca8e003..5c1efb6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2962,8 +2962,6 @@ virDomainMigrateVersion2 (virDomainPtr domain, */ ret = domain->conn->driver->domainMigratePerform (domain, cookie, cookielen, uri, flags, dname, bandwidth); - if (ret == -1) - goto done; /* In version 2 of the migration protocol, we pass the * status code from the sender to the destination host, diff --git a/src/qemu_driver.c b/src/qemu_driver.c index eb22940..772f2f9 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -6956,6 +6956,21 @@ qemudDomainMigratePerform (virDomainPtr dom, goto cleanup; } + /* it is also possible that the migrate didn't fail initially, but + * rather failed later on. Check the output of "info migrate" + */ + VIR_FREE(info); + if (qemudMonitorCommand(vm, "info migrate", &info) < 0) { + qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("could not get info about migration")); + goto cleanup; + } + if (strstr(info, "fail") != NULL) { + qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + _("migrate failed: %s"), info); + goto cleanup; + } + /* Clean up the source domain. */ qemudShutdownVMDaemon (dom->conn, driver, vm); paused = 0; -- 1.6.0.6

2 2