- Devel - libvirt List Archives

[libvirt] UML driver struct refers to public API functions
by Matthias Bolte 02 Sep '09

02 Sep '09

Hi, The commit "Generic shared impls of all NUMA apis" (b0b968efd56f6c66bfa23eebbecd491ea993f99b) changed the UML driver struct to use the shared NUMA API. But now the UML driver struct refers to the public API functions: virNodeGetCellsFreeMemory virNodeGetFreeMemory instead of the shared NUMA API functions nodeGetCellsFreeMemory nodeGetFreeMemory This results in an infinite recursion, if someone's going to call virNodeGetCellsFreeMemory with an UML connection. Matthias

3 3

[libvirt] Re: OpenVZ : The restriction of domain name should be addressed
by Yuji NISHIDA 02 Sep '09

02 Sep '09

>> 2009/7/24 Daniel P. Berrange <berrange(a)redhat.com> >>> We should make use of this --name parameter then - I guess it didn't >>> exist when we first wrote the driver. It is useful to users to have >>> separate ID vs Name parameters - and in fact the current reusing of >>> 'ID' for the name, causes a little confusion in virsh's lookup >>> routines >>> because it can't tell whether the parameter its given is a name or >>> an >>> ID, since they are identical. >> >> There is still a question of how to specify both a name and CTID in >> XML >> description. >> By default, CTID can be obtained as openvzGimmeFirstUnusedCTID(), but >> actually >> I think that there exists a number of persons interested in giving >> CTIDs >> manually. > > Well, can <domain id=''> be used for CTID remaining <name> for > alphabetical domain name? I worte a small patch and tried with following XML setting. ### Patch ### --- a/src/openvz_driver.c +++ b/src/openvz_driver.c @@ -130,9 +130,6 @@ ADD_ARG_LIT(VZCTL); ADD_ARG_LIT("--quiet"); ADD_ARG_LIT("create"); - ADD_ARG_LIT(vmdef->id); - - ADD_ARG_LIT("--name"); ADD_ARG_LIT(vmdef->name); ### XML ### <domain id='100'> <name>abc</name> I found the type of id was identified as number( obj->type == XPATH_NUMBER ) in virXPathLongBase, but it is clearly string before converted. I think correct path is to go in the first if context( obj->type == XPATH_STRING ) and run strtol. Then I tried with following XML. ### XML ### <domain id=100> <name>abc</name> I got following error. AttValue: " or ' expected I'm not sure from which function should return this result. Maybe this is the first step to go forward. I am doubting openvz*LookupBy* functions be also modified, right? ----- Yuji Nishida nishidy(a)nict.go.jp

2 1

[libvirt] PATCH: Don't blindly reorder disk drives
by Daniel P. Berrange 02 Sep '09

02 Sep '09

Calling qsort() on the disks array causes disk to be unneccessarily re-ordered, potentially breaking the ability to boot if the boot disk gets moved later in the list. The new algorithm will insert a new disk as far to the end of the list as possible, while being ordered correctly wrt other disks on the same bus. * src/domain_conf.c, src/domain_conf.h: Remove disk sorting routines. Add API to insert a disk into existing list at the optimal position, without resorting disks * src/libvirt_private.syms: Export virDomainDiskInsert * src/xend_internal.c, src/xm_internal.c: Remove calls to qsort, use virDomainDiskInsert instead. * src/qemu_driver.c: Remove calls to qsort, use virDoaminDiskInsert instead. Fix reordering bugs when hotunplugging disks and networks. Fix memory leak in disk/net unplug --- src/domain_conf.c | 68 ++++++++++++++++++++++++++++++++++----------- src/domain_conf.h | 7 +++-- src/libvirt_private.syms | 3 +- src/qemu_driver.c | 30 +++++++++++--------- src/xend_internal.c | 2 - src/xm_internal.c | 7 +---- 6 files changed, 74 insertions(+), 43 deletions(-) Daniel diff --git a/src/domain_conf.c b/src/domain_conf.c index bad53f7..29a02c1 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -627,19 +627,8 @@ void virDomainRemoveInactive(virDomainObjListPtr doms, } } -#endif /* ! PROXY */ - - -int virDomainDiskCompare(virDomainDiskDefPtr a, - virDomainDiskDefPtr b) { - if (a->bus == b->bus) - return virDiskNameToIndex(a->dst) - virDiskNameToIndex(b->dst); - else - return a->bus - b->bus; -} -#ifndef PROXY /* Parse the XML definition for a disk * @param node XML nodeset to parse for disk definition */ @@ -2329,14 +2318,61 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virConnectPtr conn, } #endif -int virDomainDiskQSort(const void *a, const void *b) + +int virDomainDiskInsert(virDomainDefPtr def, + virDomainDiskDefPtr disk) { - const virDomainDiskDefPtr *da = a; - const virDomainDiskDefPtr *db = b; - return virDomainDiskCompare(*da, *db); + if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) + return -1; + + virDomainDiskInsertPreAlloced(def, disk); + + return 0; } +void virDomainDiskInsertPreAlloced(virDomainDefPtr def, + virDomainDiskDefPtr disk) +{ + int i; + /* Tenatively plan to insert disk at the end. */ + int insertAt = -1; + + /* Then work backwards looking for disks on + * the same bus. If we find a disk with a drive + * index greater than the new one, insert at + * that position + */ + for (i = (def->ndisks - 1) ; i >= 0 ; i--) { + /* If bus matches and current disk is after + * new disk, then new disk should go here */ + if (def->disks[i]->bus == disk->bus && + (virDiskNameToIndex(def->disks[i]->dst) > + virDiskNameToIndex(disk->dst))) { + insertAt = i; + } else if (def->disks[i]->bus == disk->bus && + insertAt == -1) { + /* Last disk with match bus is before the + * new disk, then put new disk just after + */ + insertAt = i + 1; + } + } + + /* No disks with this bus yet, so put at end of list */ + if (insertAt == -1) + insertAt = def->ndisks; + + if (insertAt < def->ndisks) + memmove(def->disks + insertAt + 1, + def->disks + insertAt, + (sizeof(def->disks[0]) * (def->ndisks-insertAt))); + + def->disks[insertAt] = disk; + def->ndisks++; +} + + #ifndef PROXY static char *virDomainDefDefaultEmulator(virConnectPtr conn, virDomainDefPtr def, @@ -2643,8 +2679,6 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn, def->disks[def->ndisks++] = disk; } - qsort(def->disks, def->ndisks, sizeof(*def->disks), - virDomainDiskQSort); VIR_FREE(nodes); /* analysis of the filesystems */ diff --git a/src/domain_conf.h b/src/domain_conf.h index 44302be..c710986 100644 --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -683,9 +683,10 @@ char *virDomainCpuSetFormat(virConnectPtr conn, char *cpuset, int maxcpu); -int virDomainDiskQSort(const void *a, const void *b); -int virDomainDiskCompare(virDomainDiskDefPtr a, - virDomainDiskDefPtr b); +int virDomainDiskInsert(virDomainDefPtr def, + virDomainDiskDefPtr disk); +void virDomainDiskInsertPreAlloced(virDomainDefPtr def, + virDomainDiskDefPtr disk); int virDomainSaveXML(virConnectPtr conn, const char *configDir, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 23fa01b..32961ca 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -80,7 +80,8 @@ virDomainDeviceTypeToString; virDomainDiskBusTypeToString; virDomainDiskDefFree; virDomainDiskDeviceTypeToString; -virDomainDiskQSort; +virDomainDiskInsert; +virDomainDiskInsertPreAlloced; virDomainFindByID; virDomainFindByName; virDomainFindByUUID; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 4b1aeea..3ff9f9c 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -5030,9 +5030,7 @@ try_command: dev->data.disk->pci_addr.bus = bus; dev->data.disk->pci_addr.slot = slot; - vm->def->disks[vm->def->ndisks++] = dev->data.disk; - qsort(vm->def->disks, vm->def->ndisks, sizeof(*vm->def->disks), - virDomainDiskQSort); + virDomainDiskInsertPreAlloced(vm->def, dev->data.disk); return 0; } @@ -5090,9 +5088,7 @@ static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn, return -1; } - vm->def->disks[vm->def->ndisks++] = dev->data.disk; - qsort(vm->def->disks, vm->def->ndisks, sizeof(*vm->def->disks), - virDomainDiskQSort); + virDomainDiskInsertPreAlloced(vm->def, dev->data.disk); VIR_FREE(reply); VIR_FREE(cmd); @@ -5626,17 +5622,19 @@ try_command: } if (vm->def->ndisks > 1) { - vm->def->disks[i] = vm->def->disks[--vm->def->ndisks]; + memmove(vm->def->disks + i, + vm->def->disks + i + 1, + sizeof(*vm->def->disks) * + (vm->def->ndisks - (i + 1))); + vm->def->ndisks--; if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks) < 0) { - virReportOOMError(conn); - goto cleanup; + /* ignore, harmless */ } - qsort(vm->def->disks, vm->def->ndisks, sizeof(*vm->def->disks), - virDomainDiskQSort); } else { VIR_FREE(vm->def->disks[0]); vm->def->ndisks = 0; } + virDomainDiskDefFree(detach); ret = 0; cleanup: @@ -5725,15 +5723,19 @@ qemudDomainDetachNetDevice(virConnectPtr conn, DEBUG("%s: host_net_remove reply: %s", vm->def->name, reply); if (vm->def->nnets > 1) { - vm->def->nets[i] = vm->def->nets[--vm->def->nnets]; + memmove(vm->def->nets + i, + vm->def->nets + i + 1, + sizeof(*vm->def->nets) * + (vm->def->nnets - (i + 1))); + vm->def->nnets--; if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets) < 0) { - virReportOOMError(conn); - goto cleanup; + /* ignore, harmless */ } } else { VIR_FREE(vm->def->nets[0]); vm->def->nnets = 0; } + virDomainNetDefFree(detach); ret = 0; cleanup: diff --git a/src/xend_internal.c b/src/xend_internal.c index 7bcee7d..99847b0 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -2540,8 +2540,6 @@ xenDaemonParseSxpr(virConnectPtr conn, } } } - qsort(def->disks, def->ndisks, sizeof(*def->disks), - virDomainDiskQSort); /* in case of HVM we have USB device emulation */ if (hvm && diff --git a/src/xm_internal.c b/src/xm_internal.c index 71b852e..dd44ce5 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -990,8 +990,6 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { disk = NULL; } } - qsort(def->disks, def->ndisks, sizeof(*def->disks), - virDomainDiskQSort); list = virConfGetValue(conf, "vif"); if (list && list->type == VIR_CONF_LIST) { @@ -2839,14 +2837,11 @@ xenXMDomainAttachDevice(virDomainPtr domain, const char *xml) { switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: { - if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) { + if (virDomainDiskInsert(def, dev->data.disk) < 0) { virReportOOMError(domain->conn); goto cleanup; } - def->disks[def->ndisks++] = dev->data.disk; dev->data.disk = NULL; - qsort(def->disks, def->ndisks, sizeof(*def->disks), - virDomainDiskQSort); } break; -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] [PATCH] storage_backend_fs: avoid NULL dereference on opendir failure
by Jim Meyering 02 Sep '09

02 Sep '09

I've just begun using clang's static analyzer, http://clang-analyzer.llvm.org/ It has uncovered a few problems in libvirt. Here are the first few fixes. I'll send more details later today. >From b6bb9d82effa56733fbee9013e66fed384d9ff63 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 2 Sep 2009 09:42:32 +0200 Subject: [PATCH 1/4] storage_backend_fs: avoid NULL dereference on opendir failure * src/storage_backend_fs.c (virStorageBackendFileSystemRefresh): Don't call closedir on a NULL pointer. --- src/storage_backend_fs.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c index 65b656d..8241504 100644 --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -983,7 +983,8 @@ no_memory: /* fallthrough */ cleanup: - closedir(dir); + if (dir) + closedir(dir); virStorageVolDefFree(vol); virStoragePoolObjClearVols(pool); return -1; -- 1.6.4.2.395.ge3d52 >From eaae148291680a72d19aa9d5320f90b98f123746 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 2 Sep 2009 09:58:28 +0200 Subject: [PATCH 2/4] storage_conf.c: avoid overflow upon use of "z" or "Z" (zebi) suffix * src/storage_conf.c (virStorageSize): Don't try to compute 1024^7, since it's too large for a 64-bit type. --- src/storage_conf.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-) diff --git a/src/storage_conf.c b/src/storage_conf.c index c446069..110f0ad 100644 --- a/src/storage_conf.c +++ b/src/storage_conf.c @@ -919,12 +919,6 @@ virStorageSize(virConnectPtr conn, 1024ull; break; - case 'z': - case 'Z': - mult = 1024ull * 1024ull * 1024ull * 1024ull * 1024ull * - 1024ull * 1024ull; - break; - default: virStorageReportError(conn, VIR_ERR_XML_ERROR, _("unknown size units '%s'"), unit); -- 1.6.4.2.395.ge3d52 >From 7f453c68bc709d542e4c40a388c92c7969ad0a3a Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 2 Sep 2009 09:58:50 +0200 Subject: [PATCH 3/4] lxc: avoid NULL dereference when we find no mount point * src/lxc_container.c (lxcContainerUnmountOldFS): Don't pass a NULL pointer to qsort. --- src/lxc_container.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lxc_container.c b/src/lxc_container.c index 950dd50..2073864 100644 --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -546,8 +546,9 @@ static int lxcContainerUnmountOldFS(void) } endmntent(procmnt); - qsort(mounts, nmounts, sizeof(mounts[0]), - lxcContainerChildMountSort); + if (mounts) + qsort(mounts, nmounts, sizeof(mounts[0]), + lxcContainerChildMountSort); for (i = 0 ; i < nmounts ; i++) { VIR_DEBUG("Umount %s", mounts[i]); -- 1.6.4.2.395.ge3d52 >From 4e97befca175af427ed3b75f59e67cd620ee3ce2 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 2 Sep 2009 10:02:49 +0200 Subject: [PATCH 4/4] lxc: don't unlink(NULL) in main * src/lxc_controller.c (main): Unlink sockpath only if it's non-NULL. --- src/lxc_controller.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/lxc_controller.c b/src/lxc_controller.c index 8d11238..914c10a 100644 --- a/src/lxc_controller.c +++ b/src/lxc_controller.c @@ -803,7 +803,8 @@ cleanup: if (def) virFileDeletePid(LXC_STATE_DIR, def->name); lxcControllerCleanupInterfaces(nveths, veths); - unlink(sockpath); + if (sockpath): + unlink(sockpath); VIR_FREE(sockpath); return rc; -- 1.6.4.2.395.ge3d52

3 3

[libvirt] Add support for (qcow*) volume encryption (v4)
by Miloslav Trmač 01 Sep '09

01 Sep '09

Hello, the following patches add full support for qcow/qcow2 volume encryption, assuming a client that supports it. (Main changes since the previous version: * Add "flags" argument to virSecretDefineXML(), virSecretGetXMLDesc(), virSecretSetValue(), virSecretGetValue() * Various clean-ups and bug fixes in the local secret storage driver * Make the Python API more consistent with other objects See the specific patch change logs for more details; patches without change logs are unchanged.) New XML tags are defined to represent encryption parameters (currently format and passphrase, more can be added in the future), e.g. <encryption format='qcow'> <secret type='passphrase' uuid='724d95f2-0ed2-6ff9-84d0-0f3d1618428d'/> </encryption> The <encryption> tag can be added to a <volume> node passed to virStorageVolCreateXML() to create an encrypted volume, or to a <disk> node inside a <domain> to specify what encryption parameters to use for a domain. uuid above refers to a separately-managed secret, which was created using virSecretDefineXML() and set using virSecretSetValue(). Other properties of the secret can be managed using an XML representation. Detailed documentation of the formats and features is inside the patches.

3 42

Re: [libvirt] [PATCH 15/20] Provide missing passphrase when creating a volume.
by Miloslav Trmac 01 Sep '09

01 Sep '09

----- "Daniel P. Berrange" <berrange(a)redhat.com> wrote: > On Thu, Aug 20, 2009 at 08:18:13PM +0200, Miloslav Trma?? wrote: > > If the <encryption format='qcow'> element does not specify a secret > > during volume creation, generate a suitable secret and add it to the > > <encryption> tag. The caller can view the updated <encryption> tag > > using virStorageVolGetXMLDesc(). > > > > Similarly, when <encryption format='default'/> is specified while > > creating a qcow or qcow2-formatted volume, change the format to "qcow" > > and generate a secret as described above. > > + virBufferAddLit(&buf, "<secret ephemeral='no' private='no'>"); > > + /* <uuid/> is chosen by the secret driver */ > > + virBufferEscapeString(&buf, > > + "<description>qcow passphrase for %s</description>", > > + vol->target.path); > > + virBufferEscapeString(&buf, "<volume>%s</volume>", vol->target.path); > > + virBufferAddLit(&buf, "</secret>"); > > + if (virBufferError(&buf)) { > > + virReportOOMError(conn); > > + goto cleanup; > > + } > > + xml = virBufferContentAndReset(&buf); > > This is the first place where we should be just calling into an > internal secret_conf.h API for formatting XML from a struct, > rather than duplicating the XML formatting. OK. > > + /* A qcow passphrase is up to 16 bytes, with any data following a NUL > > + ignored. Prohibit control and non-ASCII characters to avoid possible > > + unpleasant surprises with the qemu monitor input mechanism. */ > > + fd = open("/dev/urandom", O_RDONLY); > > + if (fd < 0) { > > + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", > > + _("Cannot open /dev/urandom")); > > + goto cleanup; > > + } > > + i = 0; > > + while (i < sizeof (value)) { > > + ssize_t r; > > + > > + while ((r = read(fd, value + i, 1)) == -1 && errno == EINTR) > > + ; > > + if (r <= 0) { > > + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", > > + _("Cannot read from /dev/urandom")); > > + goto cleanup; > > + } > > + if (value[i] >= 0x20 && value[i] <= 0x7E) > > + i++; /* Got an acceptable character */ > > + } > > + close(fd); > > I reckon this snippet of code could usefully be put into the util.h file > as virFileGenerateRandomkey(), or alternatively perhaps secret_conf.h, > as virSecretGenerateRandomKey(unsigned char *buf, size_t buflen); This code, with its limitation to ASCII characters, is qcow/qemu-specific. Mirek

2 3

Re: [libvirt] [PATCH 11/20] Add volume encryption information handling.
by Miloslav Trmac 01 Sep '09

01 Sep '09

----- "Maximilian Wilhelm" <max(a)rfc2324.org> wrote: > Anno domini 2009 Miloslav Trmač scripsit: > > > diff --git a/src/storage_encryption.c b/src/storage_encryption.c > > new file mode 100644 > > index 0000000..ccb29ed > > --- /dev/null > > +++ b/src/storage_encryption.c > > @@ -0,0 +1,241 @@ > > [...] > > +#include "base64.h" > [...] > > Is there any special need for this include? Not in this file, I forgot to clean it up. > If so, where does this include file come from? gnulib. > > My buildbot refused to compile this code as it did not have any > base64.h header file. As removing this include solved the build > problem for me, I would suggest the following patch if noone objects. That change looks correct. Mirek

1 0

[libvirt] Create a Domain from windows client
by anuj rampal 01 Sep '09

01 Sep '09

Hi all, I'm trying to call the libvirt API from my Windows machine. When i try to create to a Domain using "virDomainDefineXML" function I have to pass an XML file containing the Description of the Domain. Here is the part of the XML file that im using: <domain type='qemu' id='1'> .. .. <devices> <emulator>/usr/bin/qemu</emulator> <disk type='file' device='disk'> <source file='/var/lib/libvirt/images/WinXp.img'/> <target dev='hda' bus='ide'/> </disk> <disk type='file' device='cdrom'> <source file='/home/anuj/ISOs/WinXpSp3/Windows_XP_SP3.iso'/> <target dev='hdc' bus='ide'/> <readonly/> </disk> .. .. </domain> <source file='/var/lib/libvirt/images/WinXp.img'/> This is the path which contains the img file for the Domain. I have created this file using this command: qemu-img create -f raw /var/lib/libvirt/images/WinXp.img 10G *Now: my question is:* Is there a way by which i can create this img file using some libvirt call directly and then use this for my domain creation. Or modify the XML file itself so that virDomainDefineXML call creates the img file at the path and the size that i specify. Could someone also give me some example for the same. Regards Anuj

2 1

[libvirt] libvirt and "kvm -cpu=qemu32" problem
by Scaldov M.V. 01 Sep '09

01 Sep '09

Problem is that the images of windows 2003 server, windows 2000 and some other do not start and go to BSOD due to processor mismatch if they were created under either qemu or vmware, or from bare-metal installed OS. By default libvirt starts kvm with "-cpu=qemu32" option instead of, for example, "-cpu=pentium3". There is patch for libvirt-0.6.5 to avoid this problem. Now libvirt takes into account <cpumodel> node from .xml domain config, which overrides libvirt's default -cpu option for kvm. I think, this patch is vital, because of it helps to avoid a waste of time with reinstalling and reconfiguring windows server images. <domain type='kvm'> <name>w2k3s</name> <uuid>e2725d10-7ada-4f71-eea7-fa6cc1a541ee</uuid> <memory>262144</memory> <currentMemory>262144</currentMemory> <vcpu>1</vcpu> <cpumodel>pentium3</cpumodel> <------ <os> <type arch='i686'>hvm</type> <boot dev='hd'/> </os> Sorry for not patching GIT-version, GIT is hard to compile in a small time.

2 3

[libvirt] [PATCH] support lzop save compression for qemu
by Charles Duffy 31 Aug '09

31 Aug '09

Per prior discussion -- this was, indeed, trivial. I'm a little disappointed to be breaking the ordering characteristics of the enum (as it had been ordered by increasing time requirements and decreasing output size), but breaking any save files with the old constants in the headers would of course be worse.

3 5