- Devel - libvirt List Archives

[libvirt] VMware ESX: Don't warn if a known query parameter should be ignored
by Matthias Bolte 02 Sep '09

02 Sep '09

esxUtil_ParseQuery() warns if a known query parameter should be ignored due to the corresponding char/int pointer being NULL, instead of silently ignoring it. The attached patch changes the if/else structure to fix this. Matthias

3 2

[libvirt] VMware ESX: Allow ethernet address type 'vpx'
by Matthias Bolte 02 Sep '09

02 Sep '09

The VMX entry ethernet0.addressType may be set to 'vpx' beside 'static' and 'generated'. 'vpx' indicates that the MAC address was generated by a vCenter. The attached patch adds 'vpx' to the valid values for ethernet0.addressType. Matthias

4 6

[libvirt] [PATCH] Support for getting/setting number of cpus in VirtualBox
by Pritesh Kothari 02 Sep '09

02 Sep '09

Hi All, I have just added support for getting/setting number of cpus in VirtualBox. The patch for the same is include below. Regards, Pritesh

3 2

Re: [libvirt] [PATCH 2/2] RPM spec file updated with glusterfs dependency
by Gerrit Slomma 02 Sep '09

02 Sep '09

> On Tue, Jul 07, 2009 at 05:47:56AM -0700, Harshavardhana wrote: > > Add new dependency for glusterfs rpm. > [...] > > +# For glusterfs > > +Requires: glusterfs-client >= 2.0.2 > > %endif > > why 2.0.2 ? is taht a hard requirement ? In Fedora/Rawhide we have > only 2.0.1 at the moment, > > Daniel > > -- > Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ > daniel veillard com | Rpmfind RPM search engine http://rpmfind.net/ > http://veillard.com/ | virtualization library http://libvirt.org/ Tested libvirt-0.7.0 today. And this requirement really breaks installation of builds on RHEL/CentOS/ScientificLinux. The newest available built for glusterfs is those systems is glusterfs-client-1.3.8 from freshrpms. Even --without-storage-fs does not help a bit, must be pulled in implicitly? rpm -pq --requires libvirt-0.7.0-1.x86_64.rpm (...) glusterfs-client >= 2.0.1 (...) Is this patched in upcoming 0.7.1 or how could i go around this issue?

4 3

[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

[libvirt] [PATCH] OpenVZ: accept NULL as type for GetMaxVCPUs.
by Chris Lalancette 31 Aug '09

31 Aug '09

All of the other drivers that support the getMaxVcpus callback also accept a NULL value for type. Make openvz also accept a NULL value. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/openvz_driver.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/openvz_driver.c b/src/openvz_driver.c index 6b7c49d..43d54f9 100644 --- a/src/openvz_driver.c +++ b/src/openvz_driver.c @@ -1000,9 +1000,10 @@ cleanup: return ret; } -static int openvzGetMaxVCPUs(virConnectPtr conn, const char *type) { - if (STRCASEEQ(type, "openvz")) - return 1028; //OpenVZ has no limitation +static int openvzGetMaxVCPUs(virConnectPtr conn, const char *type) +{ + if (type == NULL || STRCASEEQ(type, "openvz")) + return 1028; /* OpenVZ has no limitation */ openvzError(conn, VIR_ERR_INVALID_ARG, _("unknown type '%s'"), type); -- 1.6.0.6

2 2

[libvirt] [PATCH] Remove use of strncpy in qemudExtractMonitorPath.
by Chris Lalancette 31 Aug '09

31 Aug '09

qemudExtractMonitorPath() was doing a VIR_ALLOC_N followed by a strncpy. However, this isn't necessary; we can do the same thing using strndup, which is much safer. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/qemu_driver.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 772f2f9..a40d386 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -1029,12 +1029,12 @@ qemudExtractMonitorPath(virConnectPtr conn, */ while (*tmp) { if (c_isspace(*tmp)) { - if (VIR_ALLOC_N(*path, (tmp-dev)+1) < 0) { + *path = strndup(dev, tmp-dev); + if (*path == NULL) { virReportOOMError(conn); return -1; } - strncpy(*path, dev, (tmp-dev)); - (*path)[(tmp-dev)] = '\0'; + /* ... now further update offset till we get EOL */ *offset = tmp - haystack; return 0; -- 1.6.0.6

2 2

[libvirt] [PATCH]: Fix up virNodeGetCellsFreeMemory
by Chris Lalancette 31 Aug '09

31 Aug '09

The documentation for virNodeGetCellsFreeMemory claims the values returned are in kilobytes, but that's actually wrong; the value returned is actually in bytes. Fix up the documentation to be correct. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- docs/devhelp/libvirt-libvirt.html | 2 +- docs/html/libvirt-libvirt.html | 2 +- docs/libvirt-api.xml | 2 +- docs/libvirt-refs.xml | 1 - src/libvirt.c | 2 +- 5 files changed, 4 insertions(+), 5 deletions(-)

2 2

[libvirt] [PATCH] Introduce virStrncpy.
by Chris Lalancette 31 Aug '09

31 Aug '09

Add the virStrncpy function, which takes a dst string, source string, the number of bytes to copy and the number of bytes available in the dest string. If the source string is too large to fit into the destination string, including the \0 byte, then no data is copied and the function returns NULL. Otherwise, this function copies n bytes from source into dst, including the \0, and returns a pointer to the dst string. This function is intended to replace all unsafe uses of strncpy in the code base, since strncpy does *not* guarantee that the buffer terminates with a \0. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- HACKING | 29 +++++++++ cfg.mk | 6 ++ proxy/libvirt_proxy.c | 6 ++- qemud/Makefile.am | 3 +- qemud/qemud.c | 6 +- qemud/remote.c | 8 ++- src/bridge.c | 89 ++++++++------------------- src/datatypes.c | 9 ++- src/esx/esx_driver.c | 9 ++- src/esx/esx_vmx.c | 7 ++- src/lxc_controller.c | 6 ++- src/lxc_driver.c | 12 +++- src/nodeinfo.c | 4 +- src/opennebula/one_client.c | 11 ++-- src/openvz_conf.c | 33 +++++++--- src/phyp/phyp_driver.c | 2 +- src/proxy_internal.c | 5 +- src/qemu_conf.c | 16 +++--- src/qemu_driver.c | 12 +++- src/remote_internal.c | 19 ++++-- src/test.c | 24 +++++-- src/uml_driver.c | 17 ++++-- src/util.c | 36 +++++++++-- src/util.h | 4 + src/xen_internal.c | 22 +++++-- src/xend_internal.c | 65 ++++++++++++------- src/xm_internal.c | 145 ++++++++++++++++++++++++++++--------------- 27 files changed, 392 insertions(+), 213 deletions(-) diff --git a/HACKING b/HACKING index da28e98..7a38f05 100644 --- a/HACKING +++ b/HACKING @@ -231,6 +231,35 @@ one of the following semantically named macros +String copying +============== + +Do not use the strncpy function. According to the man page, it does +*not* guarantee a NULL-terminated buffer, which makes it extremely dangerous +to use. Instead, use one of the functionally equivalent functions: + + - virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) + The first three arguments have the same meaning as for strncpy; namely the + destination, source, and number of bytes to copy, respectively. The last + argument is the number of bytes available in the destination string; if a + copy of the source string (including a \0) will not fit into the + destination, no bytes are copied and the routine returns NULL. + Otherwise, n bytes from the source are copied into the destination and a + trailing \0 is appended. + + - virStrcpy(char *dest, const char *src, size_t destbytes) + Use this variant if you know you want to copy the entire src string + into dest. This is equivalent to + virStrncpy(dest, src, strlen(src), destbytes) + + - virStrcpyStatic(char *dest, const char *src) + Use this variant if you know you want to copy the entire src string + into dest *and* you know that your destination string is a static string + (i.e. that sizeof(dest) returns something meaningful). This is + equivalent to virStrncpy(dest, src, strlen(src), sizeof(dest)). + + + Variable length string buffer ============================= diff --git a/cfg.mk b/cfg.mk index 64bd26e..2d2dd7e 100644 --- a/cfg.mk +++ b/cfg.mk @@ -91,6 +91,12 @@ sc_prohibit_asprintf: msg='use virAsprintf, not a'sprintf \ $(_prohibit_regexp) +sc_prohibit_strncpy: + @grep -nE 'strncpy *\(' \ + $$($(VC_LIST) | grep -v 'src/util.c') && \ + { echo '$(ME): use virStrncpy, not strncpy' \ + 1>&2; exit 1; } || : + sc_prohibit_VIR_ERR_NO_MEMORY: @re='\<V''IR_ERR_NO_MEMORY\>' \ msg='use virReportOOMError, not V'IR_ERR_NO_MEMORY \ diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c index e008a7f..f058b38 100644 --- a/proxy/libvirt_proxy.c +++ b/proxy/libvirt_proxy.c @@ -170,7 +170,11 @@ proxyListenUnixSocket(const char *path) { memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; addr.sun_path[0] = '\0'; - strncpy(&addr.sun_path[1], path, (sizeof(addr) - 4) - 2); + if (virStrcpy(&addr.sun_path[1], path, sizeof(addr.sun_path) - 1) == NULL) { + fprintf(stderr, "Path %s too long to fit into destination\n", path); + close(fd); + return -1; + } /* * now bind the socket to that address and listen on it diff --git a/qemud/Makefile.am b/qemud/Makefile.am index 3d143da..67f1b4a 100644 --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -108,9 +108,10 @@ libvirtd_LDADD = \ $(SASL_LIBS) \ $(POLKIT_LIBS) +libvirtd_LDADD += ../src/libvirt_util.la + if WITH_DRIVER_MODULES libvirtd_LDADD += ../src/libvirt_driver.la - libvirtd_LDADD += ../src/libvirt_util.la else if WITH_QEMU libvirtd_LDADD += ../src/libvirt_driver_qemu.la diff --git a/qemud/qemud.c b/qemud/qemud.c index df275e6..fd6e0af 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -506,11 +506,13 @@ static int qemudListenUnix(struct qemud_server *server, memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, path, sizeof(addr.sun_path)-1); + if (virStrcpyStatic(addr.sun_path, path) == NULL) { + VIR_ERROR(_("Path %s too long for unix socket"), path); + goto cleanup; + } if (addr.sun_path[0] == '@') addr.sun_path[0] = '\0'; - oldgrp = getgid(); oldmask = umask(readonly ? ~unix_sock_ro_mask : ~unix_sock_rw_mask); if (server->privileged) diff --git a/qemud/remote.c b/qemud/remote.c index b29d5d8..990c15f 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -554,9 +554,11 @@ remoteDispatchDomainSetSchedulerParameters (struct qemud_server *server ATTRIBUT /* Deserialise parameters. */ for (i = 0; i < nparams; ++i) { - strncpy (params[i].field, args->params.params_val[i].field, - VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[i].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[i].field, args->params.params_val[i].field) == NULL) { + remoteDispatchFormatError(rerr, _("Field %s too big for destination"), + args->params.params_val[i].field); + return -1; + } params[i].type = args->params.params_val[i].value.type; switch (params[i].type) { case VIR_DOMAIN_SCHED_FIELD_INT: diff --git a/src/bridge.c b/src/bridge.c index 414d87b..f9a7668 100644 --- a/src/bridge.c +++ b/src/bridge.c @@ -149,23 +149,19 @@ brHasBridge(brControl *ctl, const char *name) { struct ifreq ifr; - int len; if (!ctl || !name) { errno = EINVAL; return -1; } - if ((len = strlen(name)) >= BR_IFNAME_MAXLEN) { + memset(&ifr, 0, sizeof(struct ifreq)); + + if (virStrcpyStatic(ifr.ifr_name, name) == NULL) { errno = EINVAL; return -1; } - memset(&ifr, 0, sizeof(struct ifreq)); - - strncpy(ifr.ifr_name, name, len); - ifr.ifr_name[len] = '\0'; - if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr)) return -1; @@ -216,18 +212,14 @@ brAddDelInterface(brControl *ctl, const char *iface) { struct ifreq ifr; - int len; if (!ctl || !ctl->fd || !bridge || !iface) return EINVAL; - if ((len = strlen(bridge)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, bridge, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, bridge) == NULL) + return EINVAL; if (!(ifr.ifr_ifindex = if_nametoindex(iface))) return ENODEV; @@ -305,23 +297,19 @@ brDeleteInterface(brControl *ctl ATTRIBUTE_UNUSED, static int ifGetMtu(brControl *ctl, const char *ifname) { struct ifreq ifr; - int len; if (!ctl || !ifname) { errno = EINVAL; return -1; } - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) { + memset(&ifr, 0, sizeof(struct ifreq)); + + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) { errno = EINVAL; return -1; } - memset(&ifr, 0, sizeof(struct ifreq)); - - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; - if (ioctl(ctl->fd, SIOCGIFMTU, &ifr)) return -1; @@ -343,18 +331,14 @@ static int ifGetMtu(brControl *ctl, const char *ifname) static int ifSetMtu(brControl *ctl, const char *ifname, int mtu) { struct ifreq ifr; - int len; if (!ctl || !ifname) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; ifr.ifr_mtu = mtu; return ioctl(ctl->fd, SIOCSIFMTU, &ifr) == 0 ? 0 : errno; @@ -466,7 +450,7 @@ brAddTap(brControl *ctl, int vnet_hdr, int *tapfd) { - int fd, len; + int fd; struct ifreq ifr; if (!ctl || !ctl->fd || !bridge || !ifname) @@ -486,17 +470,14 @@ brAddTap(brControl *ctl, (void) vnet_hdr; #endif - strncpy(ifr.ifr_name, *ifname, IFNAMSIZ-1); - - if (ioctl(fd, TUNSETIFF, &ifr) < 0) - goto error; - - len = strlen(ifr.ifr_name); - if (len >= BR_IFNAME_MAXLEN - 1) { + if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) { errno = EINVAL; goto error; } + if (ioctl(fd, TUNSETIFF, &ifr) < 0) + goto error; + /* We need to set the interface MTU before adding it * to the bridge, because the bridge will have its * MTU adjusted automatically when we add the new interface. @@ -527,7 +508,6 @@ int brDeleteTap(brControl *ctl, const char *ifname) { struct ifreq try; - int len; int fd; if (!ctl || !ctl->fd || !ifname) @@ -539,15 +519,11 @@ int brDeleteTap(brControl *ctl, memset(&try, 0, sizeof(struct ifreq)); try.ifr_flags = IFF_TAP|IFF_NO_PI; - len = strlen(ifname); - if (len >= BR_IFNAME_MAXLEN - 1) { + if (virStrcpyStatic(try.ifr_name, ifname) == NULL) { errno = EINVAL; goto error; } - strncpy(try.ifr_name, ifname, len); - try.ifr_name[len] = '\0'; - if (ioctl(fd, TUNSETIFF, &try) == 0) { if ((errno = ioctl(fd, TUNSETPERSIST, 0))) goto error; @@ -576,19 +552,15 @@ brSetInterfaceUp(brControl *ctl, int up) { struct ifreq ifr; - int len; int flags; if (!ctl || !ifname) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr) < 0) return errno; @@ -621,18 +593,14 @@ brGetInterfaceUp(brControl *ctl, int *up) { struct ifreq ifr; - int len; if (!ctl || !ifname || !up) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr) < 0) return errno; @@ -654,18 +622,15 @@ brSetInetAddr(brControl *ctl, } s; struct ifreq ifr; struct in_addr inaddr; - int len, ret; + int ret; if (!ctl || !ctl->fd || !ifname || !addr) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; if ((ret = inet_pton(AF_INET, addr, &inaddr)) < 0) return errno; @@ -692,18 +657,14 @@ brGetInetAddr(brControl *ctl, { struct ifreq ifr; struct in_addr *inaddr; - int len; if (!ctl || !ctl->fd || !ifname || !addr) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; if (ioctl(ctl->fd, cmd, &ifr) < 0) return errno; diff --git a/src/datatypes.c b/src/datatypes.c index d03a679..4a8bffa 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -25,6 +25,7 @@ #include "virterror_internal.h" #include "logging.h" #include "memory.h" +#include "util.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -899,8 +900,12 @@ virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, const c virReportOOMError(conn); goto error; } - strncpy(ret->key, key, sizeof(ret->key)-1); - ret->key[sizeof(ret->key)-1] = '\0'; + if (virStrcpyStatic(ret->key, key) == NULL) { + virMutexUnlock(&conn->lock); + virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, + _("Volume key %s too large for destination"), key); + goto error; + } ret->magic = VIR_STORAGE_VOL_MAGIC; ret->conn = conn; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 0225e9a..24c4422 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -694,9 +694,12 @@ esxNodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo) ++ptr; } - strncpy (nodeinfo->model, dynamicProperty->val->string, - sizeof (nodeinfo->model) - 1); - nodeinfo->model[sizeof (nodeinfo->model) - 1] = '\0'; + if (virStrcpyStatic(nodeinfo->model, dynamicProperty->val->string) == NULL) { + ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "Model %s too long for destination", + dynamicProperty->val->string); + goto failure; + } } else { VIR_WARN("Unexpected '%s' property", dynamicProperty->name); } diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c index 54c2594..2a58fd8 100644 --- a/src/esx/esx_vmx.c +++ b/src/esx/esx_vmx.c @@ -883,8 +883,11 @@ esxVMX_IndexToDiskName(virConnectPtr conn, int idx, const char *prefix) return NULL; } - strncpy(buffer, prefix, sizeof (buffer) - 1); - buffer[sizeof (buffer) - 1] = '\0'; + if (virStrcpyStatic(buffer, prefix) == NULL) { + ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "Prefix %s too big for destination", prefix); + return NULL; + } if (idx < 26) { buffer[length] = 'a' + idx; diff --git a/src/lxc_controller.c b/src/lxc_controller.c index 8d11238..f7729ba 100644 --- a/src/lxc_controller.c +++ b/src/lxc_controller.c @@ -178,7 +178,11 @@ static int lxcMonitorServer(const char *sockpath) unlink(sockpath); memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, sockpath, sizeof(addr.sun_path)); + if (virStrcpyStatic(addr.sun_path, sockpath) == NULL) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Socket path %s too long for destination"), sockpath); + goto error; + } if (bind(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { virReportSystemError(NULL, errno, diff --git a/src/lxc_driver.c b/src/lxc_driver.c index bd0cf0e..fd49789 100644 --- a/src/lxc_driver.c +++ b/src/lxc_driver.c @@ -719,7 +719,11 @@ static int lxcMonitorClient(virConnectPtr conn, memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, sockpath, sizeof(addr.sun_path)); + if (virStrcpyStatic(addr.sun_path, sockpath) == NULL) { + lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, + _("Socket path %s too big for destination"), sockpath); + goto error; + } if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { virReportSystemError(conn, errno, "%s", @@ -1739,7 +1743,11 @@ static int lxcGetSchedulerParameters(virDomainPtr domain, if (virCgroupGetCpuShares(group, &val) != 0) goto cleanup; params[0].value.ul = val; - strncpy(params[0].field, "cpu_shares", sizeof(params[0].field)); + if (virStrcpyStatic(params[0].field, "cpu_shares") == NULL) { + lxcError(NULL, domain, VIR_ERR_INTERNAL_ERROR, + "%s", _("Field cpu_shares too big for destination")); + goto cleanup; + } params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG; ret = 0; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 0a211a7..7d26b2b 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -141,8 +141,8 @@ int nodeGetInfo(virConnectPtr conn, uname(&info); - strncpy(nodeinfo->model, info.machine, sizeof(nodeinfo->model)-1); - nodeinfo->model[sizeof(nodeinfo->model)-1] = '\0'; + if (virStrcpyStatic(nodeinfo->model, info.machine) == NULL) + return -1; #else /* !HAVE_UNAME */ diff --git a/src/opennebula/one_client.c b/src/opennebula/one_client.c index 21f303a..39bb1ea 100644 --- a/src/opennebula/one_client.c +++ b/src/opennebula/one_client.c @@ -24,6 +24,8 @@ #include <fcntl.h> #include <unistd.h> #include "one_client.h" +#include "datatypes.h" +#include "util.h" oneClient one_client; @@ -171,7 +173,7 @@ int c_oneFinalize(int vmid) return c_oneAction(vmid, (char *)"finalize"); } -int c_oneVmInfo(int vmid, char* ret_info,int length) +int c_oneVmInfo(int vmid, char* ret_info, int length) { xmlrpc_value *resultP; int return_code; @@ -186,10 +188,9 @@ int c_oneVmInfo(int vmid, char* ret_info,int length) if( return_code ) { - strncpy(ret_info, return_string, length-1); - ret_info[length-1] = '\0'; - - retval = 0; + if (virStrncpy(ret_info, return_string, length-1, length) != NULL) + /* Only set the return value to 0 if we succeeded */ + retval = 0; } xmlrpc_DECREF(resultP); diff --git a/src/openvz_conf.c b/src/openvz_conf.c index a172fe3..08951df 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -261,8 +261,11 @@ openvzReadNetworkConf(virConnectPtr conn, if (VIR_ALLOC_N(net->ifname, len+1) < 0) goto no_memory; - strncpy(net->ifname, p, len); - net->ifname[len] = '\0'; + if (virStrncpy(net->ifname, p, len, len+1) == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Network ifname %s too long for destination"), p); + goto error; + } } else if (STRPREFIX(p, "bridge=")) { p += 7; len = next - p; @@ -275,8 +278,11 @@ openvzReadNetworkConf(virConnectPtr conn, if (VIR_ALLOC_N(net->data.bridge.brname, len+1) < 0) goto no_memory; - strncpy(net->data.bridge.brname, p, len); - net->data.bridge.brname[len] = '\0'; + if (virStrncpy(net->data.bridge.brname, p, len, len+1) == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Bridge name %s too long for destination"), p); + goto error; + } } else if (STRPREFIX(p, "mac=")) { p += 4; len = next - p; @@ -285,8 +291,11 @@ openvzReadNetworkConf(virConnectPtr conn, "%s", _("Wrong length MAC address")); goto error; } - strncpy(cpy_temp, p, len); - cpy_temp[len] = '\0'; + if (virStrncpy(cpy_temp, p, len, sizeof(cpy_temp)) == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("MAC address %s too long for destination"), p); + goto error; + } if (virParseMacAddr(cpy_temp, net->mac) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Wrong MAC address")); @@ -629,8 +638,10 @@ openvzReadConfigParam(const char * conf_file ,const char * param, char *value, i if (sf[0] == '=' && sf[1] != '\0' ) { sf ++; if ((token = strtok_r(sf,"\"\t\n", &saveptr)) != NULL) { - strncpy(value, token, maxlen) ; - value[maxlen-1] = '\0'; + if (virStrcpy(value, token, maxlen) == NULL) { + ret = -1; + break; + } found = 1; } } @@ -812,6 +823,7 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) char uuidbuf[1024]; char iden[1024]; int fd, ret; + int retval = 0; if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0) return -1; @@ -832,13 +844,14 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) sscanf(line, "%s %s\n", iden, uuidbuf); if(STREQ(iden, "#UUID:")) { - strncpy(uuidstr, uuidbuf, len); + if (virStrcpy(uuidstr, uuidbuf, len) == NULL) + retval = -1; break; } } close(fd); - return 0; + return retval; } /* Do actual checking for UUID presence in conf file, diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 3100144..22c4fee 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1387,7 +1387,7 @@ escape_specialcharacters(char *src, char *dst, size_t dstlen) } temp_buffer[j] = '\0'; - if (strncpy(dst, temp_buffer, dstlen) == NULL) + if (virStrcpy(dst, temp_buffer, dstlen) == NULL) return -1; return 0; diff --git a/src/proxy_internal.c b/src/proxy_internal.c index 5b92ad8..8609201 100644 --- a/src/proxy_internal.c +++ b/src/proxy_internal.c @@ -197,7 +197,10 @@ retry: memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; addr.sun_path[0] = '\0'; - strncpy(&addr.sun_path[1], path, (sizeof(addr) - 4) - 2); + if (virStrcpy(&addr.sun_path[1], path, sizeof(addr.sun_path) - 1) == NULL) { + close(fd); + return -1; + } /* * now bind the socket to that address and listen on it diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 22f5edd..f47ada2 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1302,18 +1302,18 @@ static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, { switch (dev->type) { case VIR_DOMAIN_CHR_TYPE_NULL: - strncpy(buf, "null", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "null", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_VC: - strncpy(buf, "vc", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "vc", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_PTY: - strncpy(buf, "pty", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "pty", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_DEV: @@ -1335,8 +1335,8 @@ static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, break; case VIR_DOMAIN_CHR_TYPE_STDIO: - strncpy(buf, "stdio", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "stdio", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_UDP: diff --git a/src/qemu_driver.c b/src/qemu_driver.c index a40d386..f99d4c0 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -921,7 +921,11 @@ qemudOpenMonitorUnix(virConnectPtr conn, memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, monitor, sizeof(addr.sun_path)); + if (virStrcpyStatic(addr.sun_path, monitor) == NULL) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Monitor path %s too big for destination"), monitor); + goto error; + } do { ret = connect(monfd, (struct sockaddr *) &addr, sizeof(addr)); @@ -6241,8 +6245,12 @@ static int qemuGetSchedulerParameters(virDomainPtr dom, goto cleanup; } params[0].value.ul = val; - strncpy(params[0].field, "cpu_shares", sizeof(params[0].field)); params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG; + if (virStrcpyStatic(params[0].field, "cpu_shares") == NULL) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("Field cpu_shares too long for destination")); + goto cleanup; + } ret = 0; diff --git a/src/remote_internal.c b/src/remote_internal.c index 26425c2..e288188 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -652,7 +652,11 @@ doRemoteOpen (virConnectPtr conn, memset (&addr, 0, sizeof addr); addr.sun_family = AF_UNIX; - strncpy (addr.sun_path, sockname, UNIX_PATH_MAX (addr)); + if (virStrcpyStatic(addr.sun_path, sockname) == NULL) { + errorf(conn, VIR_ERR_INTERNAL_ERROR, + _("Socket %s too big for destination"), sockname); + goto failed; + } if (addr.sun_path[0] == '@') addr.sun_path[0] = '\0'; @@ -1567,8 +1571,8 @@ remoteNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info) (xdrproc_t) xdr_remote_node_get_info_ret, (char *) &ret) == -1) goto done; - strncpy (info->model, ret.model, 32); - info->model[31] = '\0'; + if (virStrcpyStatic(info->model, ret.model) == NULL) + goto done; info->memory = ret.memory; info->cpus = ret.cpus; info->mhz = ret.mhz; @@ -2999,9 +3003,12 @@ remoteDomainGetSchedulerParameters (virDomainPtr domain, /* Deserialise the result. */ for (i = 0; i < *nparams; ++i) { - strncpy (params[i].field, ret.params.params_val[i].field, - VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[i].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[i].field, ret.params.params_val[i].field) == NULL) { + errorf(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("Parameter %s too big for destination"), + ret.params.params_val[i].field); + goto cleanup; + } params[i].type = ret.params.params_val[i].value.type; switch (params[i].type) { case VIR_DOMAIN_SCHED_FIELD_INT: diff --git a/src/test.c b/src/test.c index 305f2c9..9bb8e85 100644 --- a/src/test.c +++ b/src/test.c @@ -415,10 +415,13 @@ static char *testBuildFilename(const char *relativeTo, offset = strrchr(relativeTo, '/'); if ((baseLen = (offset-relativeTo+1))) { char *absFile; - if (VIR_ALLOC_N(absFile, baseLen + strlen(filename) + 1) < 0) + int totalLen = baseLen + strlen(filename) + 1; + if (VIR_ALLOC_N(absFile, totalLen) < 0) return NULL; - strncpy(absFile, relativeTo, baseLen); - absFile[baseLen] = '\0'; + if (virStrncpy(absFile, relativeTo, baseLen, totalLen) == NULL) { + VIR_FREE(absFile); + return NULL; + } strcat(absFile, filename); return absFile; } else { @@ -571,8 +574,11 @@ static int testOpenFromFile(virConnectPtr conn, privconn->nextDomID = 1; privconn->numCells = 0; - strncpy(privconn->path, file, PATH_MAX-1); - privconn->path[PATH_MAX-1] = '\0'; + if (virStrcpyStatic(privconn->path, file) == NULL) { + testError(NULL, VIR_ERR_INTERNAL_ERROR, + _("Path %s too big for destination"), file); + goto error; + } memmove(&privconn->nodeInfo, &defaultNodeInfo, sizeof(defaultNodeInfo)); nodeInfo = &privconn->nodeInfo; @@ -628,8 +634,12 @@ static int testOpenFromFile(virConnectPtr conn, str = virXPathString(conn, "string(/node/cpu/model[1])", ctxt); if (str != NULL) { - strncpy(nodeInfo->model, str, sizeof(nodeInfo->model)-1); - nodeInfo->model[sizeof(nodeInfo->model)-1] = '\0'; + if (virStrcpyStatic(nodeInfo->model, str) == NULL) { + testError(NULL, VIR_ERR_INTERNAL_ERROR, + _("Model %s too big for destination"), str); + VIR_FREE(str); + goto error; + } VIR_FREE(str); } diff --git a/src/uml_driver.c b/src/uml_driver.c index a2b9495..ecc0c75 100644 --- a/src/uml_driver.c +++ b/src/uml_driver.c @@ -562,6 +562,7 @@ static int umlMonitorAddress(virConnectPtr conn, virDomainObjPtr vm, struct sockaddr_un *addr) { char *sockname; + int retval = 0; if (virAsprintf(&sockname, "%s/%s/mconsole", driver->monitorDir, vm->def->name) < 0) { @@ -571,10 +572,13 @@ static int umlMonitorAddress(virConnectPtr conn, memset(addr, 0, sizeof *addr); addr->sun_family = AF_UNIX; - strncpy(addr->sun_path, sockname, sizeof(addr->sun_path)-1); - NUL_TERMINATE(addr->sun_path); + if (virStrcpyStatic(addr->sun_path, sockname) == NULL) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Unix path %s too long for destination"), sockname); + retval = -1; + } VIR_FREE(sockname); - return 0; + return retval; } static int umlOpenMonitor(virConnectPtr conn, @@ -668,8 +672,11 @@ static int umlMonitorCommand(virConnectPtr conn, cmd, req.length); return -1; } - strncpy(req.data, cmd, req.length); - req.data[req.length] = '\0'; + if (virStrcpyStatic(req.data, cmd) == NULL) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Command %s too long for destination"), cmd); + return -1; + } if (sendto(vm->monitor, &req, sizeof req, 0, (struct sockaddr *)&addr, sizeof addr) != (sizeof req)) { diff --git a/src/util.c b/src/util.c index 0d4f3fa..c021953 100644 --- a/src/util.c +++ b/src/util.c @@ -1121,9 +1121,8 @@ char *virFindFileInPath(const char *file) char fullpath[PATH_MAX]; /* copy PATH env so we can tweak it */ - strncpy(pathenv, getenv("PATH"), PATH_MAX); - pathenv[PATH_MAX - 1] = '\0'; - + if (virStrcpyStatic(pathenv, getenv("PATH")) == NULL) + return NULL; /* for each path segment, append the file to search for and test for * it. return it if found. @@ -1155,8 +1154,8 @@ int virFileMakePath(const char *path) if (stat(path, &st) >= 0) return 0; - strncpy(parent, path, PATH_MAX); - parent[PATH_MAX - 1] = '\0'; + if (virStrcpyStatic(parent, path) == NULL) + return EINVAL; if (!(p = strrchr(parent, '/'))) return EINVAL; @@ -1573,6 +1572,33 @@ virAsprintf(char **strp, const char *fmt, ...) return ret; } +/** + * virStrncpy + * + * A safe version of strncpy. The last parameter is the number of bytes + * available in the destination string, *not* the number of bytes you want + * to copy. If the destination is not large enough to hold all n of the + * src string bytes plus a \0, NULL is returned and no data is copied. + * If the destination is large enough to hold the n bytes plus \0, then the + * string is copied and a pointer to the destination string is returned. + */ +char * +virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) +{ + char *ret; + + if (n > (destbytes - 1)) + return NULL; + + ret = strncpy(dest, src, n); + /* strncpy NULL terminates iff the last character is \0. Therefore + * force the last byte to be \0 + */ + dest[n] = '\0'; + + return ret; +} + /* Compare two MAC addresses, ignoring differences in case, * as well as leading zeros. */ diff --git a/src/util.h b/src/util.h index b3e628a..895e07d 100644 --- a/src/util.h +++ b/src/util.h @@ -164,6 +164,10 @@ void virSkipSpaces(const char **str); int virParseNumber(const char **str); int virAsprintf(char **strp, const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(2, 3); +char *virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) + ATTRIBUTE_RETURN_CHECK; +#define virStrcpy(dest, src, destbytes) virStrncpy(dest, src, strlen(src), destbytes) +#define virStrcpyStatic(dest, src) virStrncpy(dest, src, strlen(src), sizeof(dest)) #define VIR_MAC_BUFLEN 6 #define VIR_MAC_PREFIX_BUFLEN 3 diff --git a/src/xen_internal.c b/src/xen_internal.c index ae78f84..1f4cf55 100644 --- a/src/xen_internal.c +++ b/src/xen_internal.c @@ -1208,13 +1208,19 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain, if (ret < 0) return(-1); - strncpy (params[0].field, str_weight, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[0].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[0].field, str_weight) == NULL) { + virXenError (domain->conn, VIR_ERR_INTERNAL_ERROR, + "Weight %s too big for destination", str_weight); + return -1; + } params[0].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[0].value.ui = op_dom.u.getschedinfo.u.credit.weight; - strncpy (params[1].field, str_cap, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[1].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[1].field, str_cap) == NULL) { + virXenError (domain->conn, VIR_ERR_INTERNAL_ERROR, + "Cap %s too big for destination", str_cap); + return -1; + } params[1].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[1].value.ui = op_dom.u.getschedinfo.u.credit.cap; @@ -2431,9 +2437,11 @@ xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, while (fgets (line, sizeof line, cpuinfo)) { if (regexec (&flags_hvm_rec, line, sizeof(subs)/sizeof(regmatch_t), subs, 0) == 0 && subs[0].rm_so != -1) { - strncpy (hvm_type, - &line[subs[1].rm_so], subs[1].rm_eo-subs[1].rm_so+1); - hvm_type[subs[1].rm_eo-subs[1].rm_so] = '\0'; + if (virStrncpy(hvm_type, + &line[subs[1].rm_so], + subs[1].rm_eo-subs[1].rm_so, + sizeof(hvm_type)) == NULL) + return NULL; } else if (regexec (&flags_pae_rec, line, 0, NULL, 0) == 0) host_pae = 1; } diff --git a/src/xend_internal.c b/src/xend_internal.c index 7bcee7d..d39fd4d 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -763,7 +763,8 @@ xenDaemonOpen_unix(virConnectPtr conn, const char *path) addr = (struct sockaddr_un *)&priv->addr; addr->sun_family = AF_UNIX; memset(addr->sun_path, 0, sizeof(addr->sun_path)); - strncpy(addr->sun_path, path, sizeof(addr->sun_path)); + if (virStrcpyStatic(addr->sun_path, path) == NULL) + return -1; return (0); } @@ -1433,7 +1434,7 @@ xenDaemonParseSxprChar(virConnectPtr conn, const char *value, const char *tty) { - char prefix[10]; + const char *prefix; char *tmp; virDomainChrDefPtr def; @@ -1442,18 +1443,17 @@ xenDaemonParseSxprChar(virConnectPtr conn, return NULL; } - strncpy(prefix, value, sizeof(prefix)-1); - NUL_TERMINATE(prefix); + prefix = value; if (value[0] == '/') { def->type = VIR_DOMAIN_CHR_TYPE_DEV; } else { - if ((tmp = strchr(prefix, ':')) != NULL) { + if ((tmp = strchr(value, ':')) != NULL) { *tmp = '\0'; - value += (tmp - prefix) + 1; + value = tmp + 1; } - if (STREQ(prefix, "telnet")) { + if (STRPREFIX(prefix, "telnet")) { def->type = VIR_DOMAIN_CHR_TYPE_TCP; def->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET; } else { @@ -1647,8 +1647,13 @@ xenDaemonParseSxprDisks(virConnectPtr conn, if (VIR_ALLOC_N(disk->driverName, (offset-src)+1) < 0) goto no_memory; - strncpy(disk->driverName, src, (offset-src)); - disk->driverName[offset-src] = '\0'; + if (virStrncpy(disk->driverName, src, offset-src, + (offset-src)+1) == NULL) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver name %s too big for destination"), + src); + goto error; + } src = offset + 1; @@ -1662,8 +1667,13 @@ xenDaemonParseSxprDisks(virConnectPtr conn, if (VIR_ALLOC_N(disk->driverType, (offset-src)+1)< 0) goto no_memory; - strncpy(disk->driverType, src, (offset-src)); - disk->driverType[offset-src] = '\0'; + if (virStrncpy(disk->driverType, src, offset-src, + (offset-src)+1) == NULL) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver type %s too big for destination"), + src); + goto error; + } src = offset + 1; /* Its possible to use blktap driver for block devs @@ -1891,13 +1901,12 @@ xenDaemonParseSxprSound(virConnectPtr conn, len = (offset2 - offset); else len = strlen(offset); - if (len > (sizeof(model)-1)) { + if (virStrncpy(model, offset, len, sizeof(model)) == NULL) { virXendError(conn, VIR_ERR_INTERNAL_ERROR, - _("unexpected sound model %s"), offset); + _("Sound model %s too big for destination"), + offset); goto error; } - strncpy(model, offset, len); - model[len] = '\0'; if (VIR_ALLOC(sound) < 0) goto no_memory; @@ -4786,13 +4795,20 @@ xenDaemonGetSchedulerParameters(virDomainPtr domain, goto error; } - strncpy (params[0].field, str_weight, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[0].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[0].field, str_weight) == NULL) { + virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("Weight %s too big for destination"), + str_weight); + goto error; + } params[0].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[0].value.ui = sexpr_int(root, "domain/cpu_weight"); - strncpy (params[1].field, str_cap, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[1].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[1].field, str_cap) == NULL) { + virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("Cap %s too big for destination"), str_cap); + goto error; + } params[1].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[1].value.ui = sexpr_int(root, "domain/cpu_cap"); *nparams = XEN_SCHED_CRED_NPARAM; @@ -5842,6 +5858,7 @@ virDomainXMLDevID(virDomainPtr domain, { xenUnifiedPrivatePtr priv = domain->conn->privateData; char *xref; + char *tmp; if (dev->type == VIR_DOMAIN_DEVICE_DISK) { if (dev->data.disk->driverName && @@ -5859,9 +5876,10 @@ virDomainXMLDevID(virDomainPtr domain, if (xref == NULL) return -1; - strncpy(ref, xref, ref_len); + tmp = virStrcpy(ref, xref, ref_len); free(xref); - ref[ref_len - 1] = '\0'; + if (tmp == NULL) + return -1; } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { char mac[30]; virDomainNetDefPtr def = dev->data.net; @@ -5878,9 +5896,10 @@ virDomainXMLDevID(virDomainPtr domain, if (xref == NULL) return -1; - strncpy(ref, xref, ref_len); + tmp = virStrcpy(ref, xref, ref_len); free(xref); - ref[ref_len - 1] = '\0'; + if (tmp == NULL) + return -1; } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { diff --git a/src/xm_internal.c b/src/xm_internal.c index 71b852e..83eebee 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -872,8 +872,13 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { } else { if (VIR_ALLOC_N(disk->src, (offset - head) + 1) < 0) goto no_memory; - strncpy(disk->src, head, (offset - head)); - disk->src[(offset-head)] = '\0'; + if (virStrncpy(disk->src, head, offset - head, + (offset - head) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Source file %s too big for destination"), + head); + goto cleanup; + } } head = offset + 1; @@ -886,8 +891,12 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { goto skipdisk; if (VIR_ALLOC_N(disk->dst, (offset - head) + 1) < 0) goto no_memory; - strncpy(disk->dst, head, (offset - head)); - disk->dst[(offset-head)] = '\0'; + if (virStrncpy(disk->dst, head, offset - head, + (offset - head) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Dest file %s too big for destination"), head); + goto cleanup; + } head = offset + 1; @@ -897,8 +906,14 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { if ((tmp = strchr(disk->src, ':')) != NULL) { if (VIR_ALLOC_N(disk->driverName, (tmp - disk->src) + 1) < 0) goto no_memory; - strncpy(disk->driverName, disk->src, (tmp - disk->src)); - disk->driverName[tmp - disk->src] = '\0'; + if (virStrncpy(disk->driverName, disk->src, + (tmp - disk->src), + (tmp - disk->src) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver name %s too big for destination"), + disk->src); + goto cleanup; + } /* Strip the prefix we found off the source file name */ memmove(disk->src, disk->src+(tmp-disk->src)+1, @@ -912,8 +927,14 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { goto skipdisk; if (VIR_ALLOC_N(disk->driverType, (tmp - disk->src) + 1) < 0) goto no_memory; - strncpy(disk->driverType, disk->src, (tmp - disk->src)); - disk->driverType[tmp - disk->src] = '\0'; + if (virStrncpy(disk->driverType, disk->src, + (tmp - disk->src), + (tmp - disk->src) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver type %s too big for destination"), + disk->src); + goto cleanup; + } /* Strip the prefix we found off the source file name */ memmove(disk->src, disk->src+(tmp-disk->src)+1, @@ -1026,42 +1047,52 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { data++; if (STRPREFIX(key, "mac=")) { - int len = nextkey ? (nextkey - data) : 17; - if (len > 17) - len = 17; - strncpy(mac, data, len); - mac[len] = '\0'; + int len = nextkey ? (nextkey - data) : sizeof(mac) - 1; + if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("MAC address %s too big for destination"), + data); + goto skipnic; + } } else if (STRPREFIX(key, "bridge=")) { - int len = nextkey ? (nextkey - data) : sizeof(bridge)-1; + int len = nextkey ? (nextkey - data) : sizeof(bridge) - 1; type = 1; - if (len > (sizeof(bridge)-1)) - len = sizeof(bridge)-1; - strncpy(bridge, data, len); - bridge[len] = '\0'; + if (virStrncpy(bridge, data, len, sizeof(bridge)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Bridge %s too big for destination"), + data); + goto skipnic; + } } else if (STRPREFIX(key, "script=")) { - int len = nextkey ? (nextkey - data) : PATH_MAX-1; - if (len > (PATH_MAX-1)) - len = PATH_MAX-1; - strncpy(script, data, len); - script[len] = '\0'; + int len = nextkey ? (nextkey - data) : sizeof(script) - 1; + if (virStrncpy(script, data, len, sizeof(script)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Script %s too big for destination"), + data); + goto skipnic; + } } else if (STRPREFIX(key, "model=")) { - int len = nextkey ? (nextkey - data) : sizeof(model)-1; - if (len > (sizeof(model)-1)) - len = sizeof(model)-1; - strncpy(model, data, len); - model[len] = '\0'; + int len = nextkey ? (nextkey - data) : sizeof(model) - 1; + if (virStrncpy(model, data, len, sizeof(model)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Model %s too big for destination"), data); + goto skipnic; + } } else if (STRPREFIX(key, "vifname=")) { - int len = nextkey ? (nextkey - data) : sizeof(vifname)-1; - if (len > (sizeof(vifname)-1)) - len = sizeof(vifname)-1; - strncpy(vifname, data, len); - vifname[len] = '\0'; + int len = nextkey ? (nextkey - data) : sizeof(vifname) - 1; + if (virStrncpy(vifname, data, len, sizeof(vifname)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Vifname %s too big for destination"), + data); + goto skipnic; + } } else if (STRPREFIX(key, "ip=")) { - int len = nextkey ? (nextkey - data) : 15; - if (len > 15) - len = 15; - strncpy(ip, data, len); - ip[len] = '\0'; + int len = nextkey ? (nextkey - data) : sizeof(ip) - 1; + if (virStrncpy(ip, data, len, sizeof(ip)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("IP %s too big for destination"), data); + goto skipnic; + } } while (nextkey && (nextkey[0] == ',' || @@ -1165,32 +1196,41 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { if (!(nextkey = strchr(key, ':'))) goto skippci; - if ((nextkey - key) > (sizeof(domain)-1)) + if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Domain %s too big for destination"), key); goto skippci; - - strncpy(domain, key, sizeof(domain)); - domain[sizeof(domain)-1] = '\0'; + } key = nextkey + 1; if (!(nextkey = strchr(key, ':'))) goto skippci; - strncpy(bus, key, sizeof(bus)); - bus[sizeof(bus)-1] = '\0'; + if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Bus %s too big for destination"), key); + goto skippci; + } key = nextkey + 1; if (!(nextkey = strchr(key, '.'))) goto skippci; - strncpy(slot, key, sizeof(slot)); - slot[sizeof(slot)-1] = '\0'; + if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Slot %s too big for destination"), key); + goto skippci; + } key = nextkey + 1; if (strlen(key) != 1) goto skippci; - strncpy(func, key, sizeof(func)); - func[sizeof(func)-1] = '\0'; + if (virStrncpy(func, key, 1, sizeof(func)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Function %s too big for destination"), key); + goto skippci; + } if (virStrToLong_i(domain, NULL, 16, &domainID) < 0) goto skippci; @@ -1301,8 +1341,13 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { list->list->str) { char vfb[MAX_VFB]; char *key = vfb; - strncpy(vfb, list->list->str, MAX_VFB-1); - vfb[MAX_VFB-1] = '\0'; + + if (virStrcpyStatic(vfb, list->list->str) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("VFB %s too big for destination"), + list->list->str); + goto cleanup; + } if (VIR_ALLOC(graphics) < 0) goto no_memory; -- 1.6.0.6

3 3

[libvirt] [PATCH] Compressed save image format for Qemu.
by Chris Lalancette 28 Aug '09

28 Aug '09

Implement a compressed save image format for qemu. While ideally we would have the choice between compressed/non-compressed available to the libvirt API, unfortunately there is no "flags" parameter to the virDomainSave() API. Therefore, implement this as a qemu.conf option. Both gzip and bzip2 are implemented, and it should be very easy to implement additional compression methods. One open question is if/how we should detect the gzip and bzip2 binaries. One way to do it is to do compile-time setting of the paths (via configure.in), but that doesn't seem like a great thing to do. Another solution (my preferred solution) is not to detect at all; when we go to run the commands that need them, if they aren't available, or aren't available in one of the standard paths, then we'll fail. Maybe somebody else has another option or opinion, though. In the future, we'll have a more robust (managed) save/restore API, at which time we can expose this functionality properly in the API. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/qemu.conf | 10 +++++++ src/qemu_conf.c | 11 ++++++++ src/qemu_conf.h | 2 + src/qemu_driver.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 92 insertions(+), 4 deletions(-) diff --git a/src/qemu.conf b/src/qemu.conf index 653f487..86dcc9d 100644 --- a/src/qemu.conf +++ b/src/qemu.conf @@ -129,3 +129,13 @@ # "/dev/ptmx", "/dev/kvm", "/dev/kqemu", # "/dev/rtc", "/dev/hpet", "/dev/net/tun", #] + +# The default format for Qemu/KVM guest save images is raw; that is, the +# 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" or "bzip2" for the +# save_image_format. Note that this means you slow down the process +# of saving a domain in order to save disk space. +# +# save_image_format = "raw" diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 7ca5a15..ed87e13 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -280,6 +280,17 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, driver->cgroupDeviceACL[i] = NULL; } + p = virConfGetValue (conf, "save_image_format"); + CHECK_TYPE ("save_image_format", VIR_CONF_STRING); + if (p && p->str) { + VIR_FREE(driver->saveImageFormat); + if (!(driver->saveImageFormat = strdup(p->str))) { + virReportOOMError(NULL); + virConfFree(conf); + return -1; + } + } + virConfFree (conf); return 0; } diff --git a/src/qemu_conf.h b/src/qemu_conf.h index 8f4ef6a..e34baab 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -111,6 +111,8 @@ struct qemud_driver { char *securityDriverName; virSecurityDriverPtr securityDriver; + + char *saveImageFormat; }; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 3c92635..f642fea 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3437,11 +3437,26 @@ static int qemudDomainSave(virDomainPtr dom, struct qemud_save_header header; int ret = -1; virDomainEventPtr event = NULL; + int internalret; memset(&header, 0, sizeof(header)); 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 { + 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); @@ -3514,11 +3529,27 @@ static int qemudDomainSave(virDomainPtr dom, virReportOOMError(dom->conn); goto cleanup; } - if (virAsprintf(&command, "migrate \"exec:" - "dd of='%s' oflag=append conv=notrunc 2>/dev/null" - "\"", safe_path) == -1) { + + if (header.compressed == QEMUD_SAVE_FORMAT_RAW) + internalret = virAsprintf(&command, "migrate \"exec:" + "dd of='%s' oflag=append conv=notrunc 2>/dev/null" + "\"", safe_path); + else if (header.compressed == QEMUD_SAVE_FORMAT_GZIP) + internalret = virAsprintf(&command, "migrate \"exec:" + "gzip -c | dd of='%s' oflag=append conv=notrunc 2>/dev/null" + "\"", safe_path); + else if (header.compressed == QEMUD_SAVE_FORMAT_BZIP2) + internalret = virAsprintf(&command, "migrate \"exec:" + "bzip2 -c | dd of='%s' oflag=append conv=notrunc 2>/dev/null" + "\"", safe_path); + else { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, + _("Invalid compress format %d"), + header.compressed); + goto cleanup; + } + if (internalret < 0) { virReportOOMError(dom->conn); - command = NULL; goto cleanup; } @@ -4039,6 +4070,9 @@ static int qemudDomainRestore(virConnectPtr conn, char *xml = NULL; struct qemud_save_header header; virDomainEventPtr event = NULL; + int intermediatefd = -1; + pid_t intermediate_pid = -1; + int childstat; qemuDriverLock(driver); /* Verify the header and read the XML */ @@ -4128,8 +4162,39 @@ static int qemudDomainRestore(virConnectPtr conn, } def = NULL; + if (header.version == 2) { + const char *intermediate_argv[3] = { NULL, "-dc", NULL }; + if (header.compressed == QEMUD_SAVE_FORMAT_GZIP) + intermediate_argv[0] = "gzip"; + else if (header.compressed == QEMUD_SAVE_FORMAT_BZIP2) + intermediate_argv[0] = "bzip2"; + else if (header.compressed != QEMUD_SAVE_FORMAT_RAW) { + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("Unknown compressed save format %d"), + header.compressed); + goto cleanup; + } + if (intermediate_argv[0] != NULL) { + intermediatefd = fd; + fd = -1; + if (virExec(conn, intermediate_argv, NULL, NULL, + &intermediate_pid, intermediatefd, &fd, NULL, 0) < 0) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Failed to start decompression binary %s"), + intermediate_argv[0]); + goto cleanup; + } + } + } /* Set the migration source and start it up. */ ret = qemudStartVMDaemon(conn, driver, vm, "stdio", fd); + if (intermediate_pid != -1) { + /* Wait for intermediate process to exit */ + while (waitpid(intermediate_pid, &childstat, 0) == -1 && + errno == EINTR); + } + if (intermediatefd != -1) + close(intermediatefd); close(fd); fd = -1; if (ret < 0) { -- 1.6.0.6

3 3

[libvirt] perl bindings Sys Virt compilation problems
by Daniel de Araujo 28 Aug '09

28 Aug '09

Hi, I'm trying to compile the libvirt perl bindings (Sys-Virt-0.2.1) on x86_64 RHEL 5.3 and I'm running into the following issue: /usr/bin/perl "-Iblib/arch" "-Iblib/lib" perl-Sys-Virt.spec.PL<http://perl-sys-virt.spec.pl/>perl-Sys-Virt.spec Can't load 'blib/arch/auto/Sys/Virt/Virt.so' for module Sys::Virt: libvirt.so.0: cannot open shared object file: No such file or directory at /usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi/DynaLoader.pm line 230. at perl-Sys-Virt.spec.PL <http://perl-sys-virt.spec.pl/> line 9 make: *** [perl-Sys-Virt.spec] Error 9 I've specified the PKG_CONFIG_PATH as /usr/local/lib/pkgconfig; libvirt shared objects are in /usr/local/lib I'm using libvirt 0.6.5. Any help would be appreciated. -- -Dan

2 1

[libvirt] how to change the boot order of a qemu-kvm guest os
by Gopalakrishnan Subramanian 28 Aug '09

28 Aug '09

2 2

[libvirt] Schedule for next release 0.7.1
by Daniel Veillard 27 Aug '09

27 Aug '09

Hello, following a bit too much backlog on libxml2 for the last years and an happy familly event, I have been a bit sidetracked in the last weeks and will be in the near future. I know there is a tons of pending patches, updates for drives and cleanups waiting for a release. I will try to make sure everything is reviewed and pushed by end of next week, which would allow for a 0.7.1 release by Sept 11, I hope this won't generate too much disruption while the initial schedule was at the end of the month ! 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] [PATCH] virsh vol-key segfault fix
by Pritesh Kothari 27 Aug '09

27 Aug '09

Hi, There is a bug in virsh command vol-key, it segfaults cause the fourth option to vshCommandOptVolBy() is null instead of "pool" on virsh.c:5209. The patch to fix this is attached. Regards, Pritesh

2 1

[libvirt] ANNOUNCE: perl bindings Sys-Virt release 0.2.1
by Daniel P. Berrange 25 Aug '09

25 Aug '09

FYI I have just uploaded a new release 0.2.1 of the libvirt perl bindings, Sys-Virt, to CPAN. http://search.cpan.org/dist/Sys-Virt/ Direct download link http://search.cpan.org/CPAN/authors/id/D/DA/DANBERR/Sys-Virt-0.2.1.tar.gz Changes since 0.2.0: - Add all new APIs upto libvirt 0.6.4 (network interfaces, storage volume cloning and node device create/delete) - Fix off-by-1 in UUID stringification - Set default values for optional params. - Use SvOK check for potentially undefined string params - Export all the virError related constants - Fix some memory leaks in C layer - Remove bogus module imports from test suite There will shortly be another release that adds support for 0.7.0 newly added APIs. Regards, Daniel -- |: 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 :|

1 0

[libvirt] new API to get list of *all* interfaces matching a MAC
by Laine Stump 25 Aug '09

25 Aug '09

(This probably seems like overanalysis of a simple problem. That's what I'm best at ;-) Due to oversight, the function virInterfaceLookupByMACString() can only return a single interface, but it's possible that a host may have more than one interface with the same MAC. Since the API had already been released by the time we realized this, the existing function will remain and a new one added that can return a list of interfaces. This new API will need to deal with the fact that the list length is effectively unbounded. I can see three ways of dealing with this, and want to learn which is preferred by others before spending time on the implementation. 1) The array containing the list is allocated by libvirt, freed by caller. int virInterfaceLookupAllByMACString(virConnectPtr conn, const char *mac, virInterfacePtr **matches); "matches" will point to an array of virInterfacePtr, and that array's length will be the return value. This array will be allocated by libvirt, but must be freed by the application when it's finished. Usage (ignoring errors, of course :-): virInterfacePtr *matches; int matchCt; matchCt = virInterfaceLookupAllByMACString(conn, "00:01:02:03:04:05", &matches); for (i = 0; i < matchCt; i++) { /* do something with an interface */ virInterfaceFree(matches[i]); } free(matches); 2) The array containing the list is allocated by the application, and its length sent to libvirt. Libvirt then returns the actual number of matches, which may be greater than the max length sent by the application, but only copies as many as the application asked for. If the application sends 0 as maxMatches and/or a NULL pointer instead of a pointer to an array, libvirt will return the actual length, but not the list itself. For example, if there are 3 interfaces matching, and maxMatches is set to 2, the function will still return 3, but only set two items in the matches array. int virInterfaceLookupAllByMACString(virConnectPtr conn, const char *mac, virInterfacePtr **matches, int maxMatches); This puts all the functionality needed in a single API function, but it will generally need to be called twice - once to learn the length, and the second (after allocating an array of appropriate size) to retrieve the list. virInterfacePtr *matches; int matchCtA, matchCtB; matchCtA = virInterfaceLookupAllByMACString(conn, "00:01:02:03:04:05", NULL, 0); matches = malloc (matchCt * sizeof(virInterfacePtr)); matchCtB = virInterfaceLookupAllByMACString(conn, "00:01:02:03:04:05", &matches, matchCt); for (i = 0; i < min(matchCtA, matchCtB); i++) { /* do something with an interface */ virInterfaceFree(matches[i]); } free(matches); 3) Again, the array containing the list is allocated by the application, but it learns the proper length to allocate by calling a different API function. The lookup function itself returns the number of interfaces delivered in the array, rather than the number of matches it *wanted* to put in the array. int virInterfaceCountAllByMACString(virConnectPtr conn, const char *mac); int virInterfaceLookupAllByMACString(virConnectPtr conn, const char *mac, virInterfacePtr **matches, int maxMatches); Usage: virInterfacePtr *matches; int matchCt; matchCt = virInterfaceCountAllByMACString(conn, "00:01:02:03:04:05"); matches = malloc (matchCt * sizeof(virInterfacePtr)); matchCt = virInterfaceLookupAllByMACString(conn, "00:01:02:03:04:05", &matches, matchCt); for (i = 0; i < matchCt; i++) { /* do something with an interface */ virInterfaceFree(matches[i]); } free(matches); ******* (3) is closest to behavior of other libvirt APIs, but requires more functions in the API, and could lead to situations where newly added interfaces aren't in the list (if a new interface is added between calling the Count function and the Lookup function) . (2) has a more compact API (one which matches the netcf API exactly), but has the same problems with potential bad counts. (1) seems like the cleanest API, but I don't know what others' opinions are on having libvirt allocate the array (since that's not how its done for other similar things, eg virConnectListInterface, virConnectListNetworks, etc), or maybe there's some limitation of the RPC I haven't considered that makes it unfeasible. Which should I implement?

3 5

[libvirt] error: failed to connect to hypervisor (openvz)
by AL.Gabteni 25 Aug '09

25 Aug '09

Hi im trying to use libvirt with openvz since 2 days but without luck ,i allways get error: failed to connect to hypervisor (openvz) ; the test connection is working , any help will be appreciated . Regards

2 1

[libvirt] Small patch for ruby-libvirt to add api wrapping for DomainAttachDevice and DomainDetachDevice
by Jacob Leaver 25 Aug '09

25 Aug '09

--- ext/libvirt/_libvirt.c.orig 2009-01-19 18:24:22.000000000 -0800 +++ ext/libvirt/_libvirt.c 2009-01-19 18:24:39.000000000 -0800 @@ -728,6 +728,21 @@ gen_call_void(virDomainSave, conn(s), domain_get(s), StringValueCStr(to)); } +/* + * Call +virDomainAttachDevice+[http://www.libvirt.org/html/libvirt-libvirt.html#virDomainAttachDevice] + */ +VALUE libvirt_dom_attach_device_xml(VALUE s, VALUE xml) { + virDomainPtr dom = domain_get(s); + return virDomainAttachDevice(dom, StringValueCStr(xml)); +} + +/* + * Call +virDomainDetachDevice+[http://www.libvirt.org/html/libvirt-libvirt.html#virDomainDetachDevice] + */ +VALUE libvirt_dom_detach_device_xml(VALUE s, VALUE xml) { + virDomainPtr dom = domain_get(s); + return virDomainDetachDevice(dom, StringValueCStr(xml)); +} /* * Call +virDomainCoreDump+[http://www.libvirt.org/html/libvirt-libvirt.html#virDomainCoreDump] @@ -1921,7 +1936,8 @@ rb_define_method(c_domain, "autostart", libvirt_dom_autostart, 0); rb_define_method(c_domain, "autostart=", libvirt_dom_autostart_set, 1); rb_define_method(c_domain, "free", libvirt_dom_free, 0); - + rb_define_method(c_domain, "attach_device", libvirt_dom_attach_device_xml, 1); + rb_define_method(c_domain, "detach_device", libvirt_dom_detach_device_xml, 1); /* * Class Libvirt::Domain::Info */

2 1

[libvirt] OS Eject & Removable Media
by Jacob Leaver 25 Aug '09

25 Aug '09

Greetings all, First, I'd like to thank everyone that's working on the libvirt project, it's certainly making my life a better place. Second, (there's always a second, eh?) I am using libvirtd 0.6.4 and KVM qemu-kvm-0.10.5, and when a guest OS ejects media, libvirtd doesn't seem to pick up the change. Looking through the API, I don't see a provision to probe for a list of block devices, so although qemu and kvm know about media changes, I don't think there's a way for libvirtd to know about them. I've scanned the list archives, and haven't really seen any discussion of this, but I'd be more than happy to be directed somewhere. Also, I have a couple of minor and hackish patches for the ruby bindings for attaching and removing devices, if someone's interested. Sincerely, Jacob Leaver

2 2

[libvirt] network not autostarting
by Mariano Absatz 24 Aug '09

24 Aug '09

Hi, I'm using libvirt-bin 0.6.1 from the standard ubuntu repositories under ubuntu 9.04 (jaunty). I have, beside the default network, another one called "isolated" using the following description file: <network> <name>isolated</name> <uuid>51cffbcc-88f5-4edc-a81c-1765c1045691</uuid> <bridge name='virbr%d' stp='on' forwardDelay='0' /> <ip address='10.3.14.1' netmask='255.255.255.0'> <dhcp> <range start='10.3.14.128' end='10.3.14.254' /> </dhcp> </ip> </network> I have configured it with "net-autostart" and the symlink in the /etc/libvirt/qemu/networks/autostart/ autostart directory is just fine... however, it isn't starting automatically anymore (and it used to start up before)... Any hints on what to look up and where? TIA. -- Mariano Absatz - "El Baby" el.baby(a)gmail.com www.clueless.com.ar -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- _ | / \ | \ / ASCII RIBBON CAMPAIGN | X AGAINST HTML MAIL | / \ www.asciiribbon.org | | | -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- * TagZilla 0.066 * http://tagzilla.mozdev.org

2 1

[libvirt] Communicating with libvirt from windows
by anuj rampal 24 Aug '09

24 Aug '09

Hi, I have KVM running on one of my machine and I can controle the geues OS using the virt-manager and Virsh tools. Now what I want to do is talk to libvirt from my windows machine. I tried to compile libvirt for windows but couldnt get any success with that. Is there a way by which i can talk to libvirt without this client..?? As in can i talk to libvirt over TCP sockets using xml (something like that)...?? Thanks a lot in advance. Regards Anuj

4 3

[libvirt] [PATCH] Ignores EOPNOTSUPP when attempting to access an image on an NFS share.
by Darryl L. Pierce 21 Aug '09

21 Aug '09

rhbz#517157 Signed-off-by: Darryl L. Pierce <dpierce(a)redhat.com> --- src/security_selinux.c | 23 ++++++++++++++++------- 1 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/security_selinux.c b/src/security_selinux.c index 0db9f49..f1e3f17 100644 --- a/src/security_selinux.c +++ b/src/security_selinux.c @@ -323,6 +323,8 @@ SELinuxSetFilecon(virConnectPtr conn, const char *path, char *tcon) VIR_INFO("Setting SELinux context on '%s' to '%s'", path, tcon); if (setfilecon(path, tcon) < 0) { + int setfilecon_errno = errno; + if (getfilecon(path, &econ) >= 0) { if (STREQ(tcon, econ)) { freecon(econ); @@ -331,14 +333,21 @@ SELinuxSetFilecon(virConnectPtr conn, const char *path, char *tcon) } freecon(econ); } - virSecurityReportError(conn, VIR_ERR_ERROR, - _("%s: unable to set security context " - "'\%s\' on %s: %s."), __func__, - tcon, - path, - virStrerror(errno, ebuf, sizeof ebuf)); - if (security_getenforce() == 1) + + /* if the error complaint is related to an image hosted on + * an nfs mount, then ignore it. + * rhbz 517157 + */ + if (setfilecon_errno != EOPNOTSUPP) { + virSecurityReportError(conn, VIR_ERR_ERROR, + _("%s: unable to set security context " + "'\%s\' on %s: %s."), __func__, + tcon, + path, + virStrerror(errno, ebuf, sizeof ebuf)); + if (security_getenforce() == 1) return -1; + } } return 0; } -- 1.6.2.5

3 2

[libvirt] [PATCH] spec file: add URL to Source tag
by Mark McLoughlin 21 Aug '09

21 Aug '09

See https://fedoraproject.org/wiki/Packaging:SourceURL --- libvirt.spec.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index dad97ec..00c394c 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -81,7 +81,7 @@ Version: @VERSION@ Release: 1%{?dist}%{?extra_release} License: LGPLv2+ Group: Development/Libraries -Source: libvirt-%{version}.tar.gz +Source: http://libvirt.org/sources/libvirt-%{version}.tar.gz BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root URL: http://libvirt.org/ BuildRequires: python-devel -- 1.6.2.5

1 0

[libvirt] error: failed to connect to the hypervisor
by Dominik Klein 21 Aug '09

21 Aug '09

Hi I am running kvm-78 with libvirt 0.6.1 and see the following error from time to time when my linux-ha cluster tries to run "virsh domstate" to figure out a virtual machine's state: error: failed to connect to the hypervisor In syslog, I see: libvirtd: 23:39:11.357: error : Failed to set close-on-exec file descriptor flag Due to this error, the cluster assumes the machine is not running okay and reboots it in order to fix things. This happened twice in 2 months with 10 machines being queried like that each minute (read: not too well reproducable) What is this and how could I work around it or is this fixed in newer versions and so worth an upgrade? Regards Dominik

2 3

[libvirt] Migration fail
by Christian Hennig 20 Aug '09

20 Aug '09

hi, i am trying to migrate a VM from my local system "pc38" to a remote on "pc36": virsh # list --all Id Name State ---------------------------------- - scalaris1 shut off virsh # migrate scalaris1 qemu+ssh://csr-user@pc36/system error: monitor socket did not show up.: No such file or directory What is the monitor socket? Both system are ubuntu server 9.04 : christian@pc38:~$ virsh --version Connecting to uri: qemu:///system 0.6.1 christian@pc38:~$ Thx for help! Christian

2 1

Re: [libvirt] libvirt with libssh2?
by Emre Erenoglu 20 Aug '09

20 Aug '09

> > ---------- Forwarded message ---------- > From: Daniel Veillard <veillard(a)redhat.com> > To: Emre Erenoglu <erenoglu(a)gmail.com> > Date: Thu, 20 Aug 2009 11:14:53 +0200 > Subject: Re: [libvirt] libvirt with libssh2? > On Thu, Aug 20, 2009 at 10:50:37AM +0200, Emre Erenoglu wrote: > > Hi, > > I'm the package maintainer of libvirt for the Pardus distribution. While > > trying to update the package to the latest 0.7.0, I recognized that > > configure script does not find libssh. > > > > I checked our system and found out that we are using libssh2 instead of > > libssh. > > > > >From this website (http://www.libssh2.org/libssh2-vs-libssh.html) > libssh2 > > developers summarize their differences. > > > > My questions: > > 1) Is there a specific reason why we are using libssh? Is there a patch > to > > make libvirt use libssh2 instead? > > 2) What functionality do we loose if we don't have libssh in the system? > > (depending on this, I'll also package libssh or not) > > I guess you activated --with-phyp which currently requires libssh, > but this will be fixed in the future, the result is just that that > driver gets desactivated for now if you don't have libssh > > see http://www.mail-archive.com/libvir-list@redhat.com/msg15086.html > Thanks Daniel. I'm a bit foreign to this "Power Hypervisor" thing, but if I'm not wrong from the information that I got from this site: http://publib.boulder.ibm.com/infocenter/systems/scope/hw/index.jsp?topic=/… than it's nothing that we support in Pardus at the moment. In fact, we only support KVM in our libvirt at the moment, possibly virtualbox in the future. In this case, I'm disabling this pyhp as well as the libssh dependency. Thanks a lot for the info and let me know if my understanding is wrong :) -- Emre

1 0

[libvirt] Power Hypervisor: Fix potential segfault and memleak in phypOpen
by Matthias Bolte 20 Aug '09

20 Aug '09

Hi, I came across this line in the phypOpen function: char string[strlen(conn->uri->path)]; Here the path part of the given URI is used without checking it for NULL, this can cause a segfault as strlen expects a string != NULL. Beside that uuid_db and connection_data leak in case of an error. In this line conn->uri->path = string; the original path of the URI leaks. The patch adds a VIR_FREE call before setting the new path. The attached patch is compile-tested but I don't have a Power Hypervisor installation at hand to test it for real. Matthias

4 7

[libvirt] [PATCH] Introduce virStrcpy.
by Chris Lalancette 20 Aug '09

20 Aug '09

Add the virStrcpy function, which takes a dst string, source string, and number of bytes available in the dest string. If the source string is too large to fit into the destination string, including the \0 byte, then no data is copied and the function returns NULL. Otherwise, this function copies source into dst, including the \0, and returns a pointer to the dst string. This function is intended to replace all unsafe uses of strncpy in the code base, since strncpy does *not* guarantee that the buffer terminates with a \0. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- cfg.mk | 5 ++ proxy/libvirt_proxy.c | 7 ++- qemud/Makefile.am | 3 +- qemud/qemud.c | 7 +- qemud/remote.c | 9 ++- src/bridge.c | 89 ++++++++-------------------- src/datatypes.c | 8 ++- src/esx/esx_driver.c | 10 ++- src/esx/esx_vmx.c | 7 ++- src/lxc_controller.c | 7 ++- src/lxc_driver.c | 13 ++++- src/nodeinfo.c | 5 +- src/opennebula/one_client.c | 5 +- src/openvz_conf.c | 33 +++++++--- src/phyp/phyp_driver.c | 2 +- src/proxy_internal.c | 5 +- src/qemu_conf.c | 16 +++--- src/qemu_driver.c | 13 +++- src/remote_internal.c | 20 +++++-- src/test.c | 15 ++++- src/uml_driver.c | 17 ++++-- src/util.c | 31 ++++++++-- src/util.h | 1 + src/xen_internal.c | 22 +++++-- src/xend_internal.c | 62 ++++++++++++++------ src/xm_internal.c | 134 +++++++++++++++++++++++++++--------------- 26 files changed, 343 insertions(+), 203 deletions(-) diff --git a/cfg.mk b/cfg.mk index 64bd26e..ad96bc6 100644 --- a/cfg.mk +++ b/cfg.mk @@ -91,6 +91,11 @@ sc_prohibit_asprintf: msg='use virAsprintf, not a'sprintf \ $(_prohibit_regexp) +sc_prohibit_strncpy: + @re='strncpy *\(' \ + msg='use virStrcpy, not strncpy' \ + $(_prohibit_regexp) + sc_prohibit_VIR_ERR_NO_MEMORY: @re='\<V''IR_ERR_NO_MEMORY\>' \ msg='use virReportOOMError, not V'IR_ERR_NO_MEMORY \ diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c index e008a7f..42084c7 100644 --- a/proxy/libvirt_proxy.c +++ b/proxy/libvirt_proxy.c @@ -167,10 +167,13 @@ proxyListenUnixSocket(const char *path) { * Abstract socket do not hit the filesystem, way more secure and * guaranteed to be atomic */ - memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; addr.sun_path[0] = '\0'; - strncpy(&addr.sun_path[1], path, (sizeof(addr) - 4) - 2); + if (virStrcpy(&addr.sun_path[1], path, sizeof(addr.sun_path) - 1) == NULL) { + fprintf(stderr, "Path %s too long to fit into destination\n", path); + close(fd); + return -1; + } /* * now bind the socket to that address and listen on it diff --git a/qemud/Makefile.am b/qemud/Makefile.am index 5f1376e..3220ded 100644 --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -108,9 +108,10 @@ libvirtd_LDADD = \ $(SASL_LIBS) \ $(POLKIT_LIBS) +libvirtd_LDADD += ../src/libvirt_util.la + if WITH_DRIVER_MODULES libvirtd_LDADD += ../src/libvirt_driver.la - libvirtd_LDADD += ../src/libvirt_util.la else if WITH_QEMU libvirtd_LDADD += ../src/libvirt_driver_qemu.la diff --git a/qemud/qemud.c b/qemud/qemud.c index b2bd59d..17fc172 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -509,13 +509,14 @@ static int qemudListenUnix(struct qemud_server *server, virSetNonBlock(sock->fd) < 0) goto cleanup; - memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, path, sizeof(addr.sun_path)-1); + if (virStrcpy(addr.sun_path, path, sizeof(addr.sun_path)) == NULL) { + VIR_ERROR(_("Path %s too long for unix socket"), path); + goto cleanup; + } if (addr.sun_path[0] == '@') addr.sun_path[0] = '\0'; - oldgrp = getgid(); oldmask = umask(readonly ? ~unix_sock_ro_mask : ~unix_sock_rw_mask); if (server->privileged) diff --git a/qemud/remote.c b/qemud/remote.c index 2225d06..92cc925 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -570,9 +570,12 @@ remoteDispatchDomainSetSchedulerParameters (struct qemud_server *server ATTRIBUT /* Deserialise parameters. */ for (i = 0; i < nparams; ++i) { - strncpy (params[i].field, args->params.params_val[i].field, - VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[i].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpy(params[i].field, args->params.params_val[i].field, + sizeof(params[i].field)) == NULL) { + remoteDispatchFormatError(rerr, _("Field %s too big for destination"), + args->params.params_val[i].field); + return -1; + } params[i].type = args->params.params_val[i].value.type; switch (params[i].type) { case VIR_DOMAIN_SCHED_FIELD_INT: diff --git a/src/bridge.c b/src/bridge.c index 414d87b..378c055 100644 --- a/src/bridge.c +++ b/src/bridge.c @@ -149,23 +149,19 @@ brHasBridge(brControl *ctl, const char *name) { struct ifreq ifr; - int len; if (!ctl || !name) { errno = EINVAL; return -1; } - if ((len = strlen(name)) >= BR_IFNAME_MAXLEN) { + memset(&ifr, 0, sizeof(struct ifreq)); + + if (virStrcpy(ifr.ifr_name, name, sizeof(ifr.ifr_name)) == NULL) { errno = EINVAL; return -1; } - memset(&ifr, 0, sizeof(struct ifreq)); - - strncpy(ifr.ifr_name, name, len); - ifr.ifr_name[len] = '\0'; - if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr)) return -1; @@ -216,18 +212,14 @@ brAddDelInterface(brControl *ctl, const char *iface) { struct ifreq ifr; - int len; if (!ctl || !ctl->fd || !bridge || !iface) return EINVAL; - if ((len = strlen(bridge)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, bridge, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpy(ifr.ifr_name, bridge, sizeof(ifr.ifr_name)) == NULL) + return EINVAL; if (!(ifr.ifr_ifindex = if_nametoindex(iface))) return ENODEV; @@ -305,23 +297,19 @@ brDeleteInterface(brControl *ctl ATTRIBUTE_UNUSED, static int ifGetMtu(brControl *ctl, const char *ifname) { struct ifreq ifr; - int len; if (!ctl || !ifname) { errno = EINVAL; return -1; } - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) { + memset(&ifr, 0, sizeof(struct ifreq)); + + if (virStrcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)) == NULL) { errno = EINVAL; return -1; } - memset(&ifr, 0, sizeof(struct ifreq)); - - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; - if (ioctl(ctl->fd, SIOCGIFMTU, &ifr)) return -1; @@ -343,18 +331,14 @@ static int ifGetMtu(brControl *ctl, const char *ifname) static int ifSetMtu(brControl *ctl, const char *ifname, int mtu) { struct ifreq ifr; - int len; if (!ctl || !ifname) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)) == NULL) + return EINVAL; ifr.ifr_mtu = mtu; return ioctl(ctl->fd, SIOCSIFMTU, &ifr) == 0 ? 0 : errno; @@ -466,7 +450,7 @@ brAddTap(brControl *ctl, int vnet_hdr, int *tapfd) { - int fd, len; + int fd; struct ifreq ifr; if (!ctl || !ctl->fd || !bridge || !ifname) @@ -486,17 +470,14 @@ brAddTap(brControl *ctl, (void) vnet_hdr; #endif - strncpy(ifr.ifr_name, *ifname, IFNAMSIZ-1); - - if (ioctl(fd, TUNSETIFF, &ifr) < 0) - goto error; - - len = strlen(ifr.ifr_name); - if (len >= BR_IFNAME_MAXLEN - 1) { + if (virStrcpy(ifr.ifr_name, *ifname, sizeof(ifr.ifr_name)) == NULL) { errno = EINVAL; goto error; } + if (ioctl(fd, TUNSETIFF, &ifr) < 0) + goto error; + /* We need to set the interface MTU before adding it * to the bridge, because the bridge will have its * MTU adjusted automatically when we add the new interface. @@ -527,7 +508,6 @@ int brDeleteTap(brControl *ctl, const char *ifname) { struct ifreq try; - int len; int fd; if (!ctl || !ctl->fd || !ifname) @@ -539,15 +519,11 @@ int brDeleteTap(brControl *ctl, memset(&try, 0, sizeof(struct ifreq)); try.ifr_flags = IFF_TAP|IFF_NO_PI; - len = strlen(ifname); - if (len >= BR_IFNAME_MAXLEN - 1) { + if (virStrcpy(try.ifr_name, ifname, sizeof(try.ifr_name)) == NULL) { errno = EINVAL; goto error; } - strncpy(try.ifr_name, ifname, len); - try.ifr_name[len] = '\0'; - if (ioctl(fd, TUNSETIFF, &try) == 0) { if ((errno = ioctl(fd, TUNSETPERSIST, 0))) goto error; @@ -576,19 +552,15 @@ brSetInterfaceUp(brControl *ctl, int up) { struct ifreq ifr; - int len; int flags; if (!ctl || !ifname) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)) == NULL) + return EINVAL; if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr) < 0) return errno; @@ -621,18 +593,14 @@ brGetInterfaceUp(brControl *ctl, int *up) { struct ifreq ifr; - int len; if (!ctl || !ifname || !up) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)) == NULL) + return EINVAL; if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr) < 0) return errno; @@ -654,18 +622,15 @@ brSetInetAddr(brControl *ctl, } s; struct ifreq ifr; struct in_addr inaddr; - int len, ret; + int ret; if (!ctl || !ctl->fd || !ifname || !addr) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)) == NULL) + return EINVAL; if ((ret = inet_pton(AF_INET, addr, &inaddr)) < 0) return errno; @@ -692,18 +657,14 @@ brGetInetAddr(brControl *ctl, { struct ifreq ifr; struct in_addr *inaddr; - int len; if (!ctl || !ctl->fd || !ifname || !addr) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)) == NULL) + return EINVAL; if (ioctl(ctl->fd, cmd, &ifr) < 0) return errno; diff --git a/src/datatypes.c b/src/datatypes.c index ac61682..b10452e 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -25,6 +25,7 @@ #include "virterror_internal.h" #include "logging.h" #include "memory.h" +#include "util.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -876,8 +877,11 @@ virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, const c virReportOOMError(conn); goto error; } - strncpy(ret->key, key, sizeof(ret->key)-1); - ret->key[sizeof(ret->key)-1] = '\0'; + if (virStrcpy(ret->key, key, sizeof(ret->key)) == NULL) { + virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, + _("Volume key %s too large for destination"), key); + goto error; + } ret->magic = VIR_STORAGE_VOL_MAGIC; ret->conn = conn; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index dba6305..c5dbda2 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -696,9 +696,13 @@ esxNodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo) ++ptr; } - strncpy (nodeinfo->model, dynamicProperty->val->string, - sizeof (nodeinfo->model) - 1); - nodeinfo->model[sizeof (nodeinfo->model) - 1] = '\0'; + if (virStrcpy(nodeinfo->model, dynamicProperty->val->string, + sizeof(nodeinfo->model)) == NULL) { + ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "Model %s too long for destination", + dynamicProperty->val->string); + goto failure; + } } else { VIR_WARN("Unexpected '%s' property", dynamicProperty->name); } diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c index 91a86e2..b138d13 100644 --- a/src/esx/esx_vmx.c +++ b/src/esx/esx_vmx.c @@ -885,8 +885,11 @@ esxVMX_IndexToDiskName(virConnectPtr conn, int idx, const char *prefix) return NULL; } - strncpy(buffer, prefix, sizeof (buffer) - 1); - buffer[sizeof (buffer) - 1] = '\0'; + if (virStrcpy(buffer, prefix, sizeof(buffer)) == NULL) { + ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "Prefix %s too big for destination", prefix); + return NULL; + } if (idx < 26) { buffer[length] = 'a' + idx; diff --git a/src/lxc_controller.c b/src/lxc_controller.c index 8d11238..6bc9976 100644 --- a/src/lxc_controller.c +++ b/src/lxc_controller.c @@ -176,9 +176,12 @@ static int lxcMonitorServer(const char *sockpath) } unlink(sockpath); - memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, sockpath, sizeof(addr.sun_path)); + if (virStrcpy(addr.sun_path, sockpath, sizeof(addr.sun_path)) == NULL) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Socket path %s too long for destination"), sockpath); + goto error; + } if (bind(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { virReportSystemError(NULL, errno, diff --git a/src/lxc_driver.c b/src/lxc_driver.c index c6cf935..b42f77f 100644 --- a/src/lxc_driver.c +++ b/src/lxc_driver.c @@ -719,7 +719,11 @@ static int lxcMonitorClient(virConnectPtr conn, memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, sockpath, sizeof(addr.sun_path)); + if (virStrcpy(addr.sun_path, sockpath, sizeof(addr.sun_path)) == NULL) { + lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, + _("Socket path %s too big for destination"), sockpath); + goto error; + } if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { virReportSystemError(conn, errno, "%s", @@ -1737,7 +1741,12 @@ static int lxcGetSchedulerParameters(virDomainPtr domain, if (virCgroupGetCpuShares(group, &val) != 0) goto cleanup; params[0].value.ul = val; - strncpy(params[0].field, "cpu_shares", sizeof(params[0].field)); + if (virStrcpy(params[0].field, "cpu_shares", + sizeof(params[0].field)) == NULL) { + lxcError(NULL, domain, VIR_ERR_INTERNAL_ERROR, + "%s", _("Field cpu_shares too big for destination")); + goto cleanup; + } params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG; ret = 0; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 0a211a7..b620ced 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -141,8 +141,9 @@ int nodeGetInfo(virConnectPtr conn, uname(&info); - strncpy(nodeinfo->model, info.machine, sizeof(nodeinfo->model)-1); - nodeinfo->model[sizeof(nodeinfo->model)-1] = '\0'; + if (virStrcpy(nodeinfo->model, info.machine, + sizeof(nodeinfo->model)) == NULL) + return -1; #else /* !HAVE_UNAME */ diff --git a/src/opennebula/one_client.c b/src/opennebula/one_client.c index 21f303a..9d3a5c6 100644 --- a/src/opennebula/one_client.c +++ b/src/opennebula/one_client.c @@ -24,6 +24,8 @@ #include <fcntl.h> #include <unistd.h> #include "one_client.h" +#include "datatypes.h" +#include "util.h" oneClient one_client; @@ -186,8 +188,7 @@ int c_oneVmInfo(int vmid, char* ret_info,int length) if( return_code ) { - strncpy(ret_info, return_string, length-1); - ret_info[length-1] = '\0'; + virStrcpy(ret_info, return_string, length); retval = 0; } diff --git a/src/openvz_conf.c b/src/openvz_conf.c index a172fe3..27f0264 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -261,8 +261,11 @@ openvzReadNetworkConf(virConnectPtr conn, if (VIR_ALLOC_N(net->ifname, len+1) < 0) goto no_memory; - strncpy(net->ifname, p, len); - net->ifname[len] = '\0'; + if (virStrcpy(net->ifname, p, len+1) == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Network ifname %s too long for destination"), p); + goto error; + } } else if (STRPREFIX(p, "bridge=")) { p += 7; len = next - p; @@ -275,8 +278,11 @@ openvzReadNetworkConf(virConnectPtr conn, if (VIR_ALLOC_N(net->data.bridge.brname, len+1) < 0) goto no_memory; - strncpy(net->data.bridge.brname, p, len); - net->data.bridge.brname[len] = '\0'; + if (virStrcpy(net->data.bridge.brname, p, len+1) == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Bridge name %s too long for destination"), p); + goto error; + } } else if (STRPREFIX(p, "mac=")) { p += 4; len = next - p; @@ -285,8 +291,11 @@ openvzReadNetworkConf(virConnectPtr conn, "%s", _("Wrong length MAC address")); goto error; } - strncpy(cpy_temp, p, len); - cpy_temp[len] = '\0'; + if (virStrcpy(cpy_temp, p, sizeof(cpy_temp)) == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("MAC address %s too long for destination"), p); + goto error; + } if (virParseMacAddr(cpy_temp, net->mac) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Wrong MAC address")); @@ -629,8 +638,10 @@ openvzReadConfigParam(const char * conf_file ,const char * param, char *value, i if (sf[0] == '=' && sf[1] != '\0' ) { sf ++; if ((token = strtok_r(sf,"\"\t\n", &saveptr)) != NULL) { - strncpy(value, token, maxlen) ; - value[maxlen-1] = '\0'; + if (virStrcpy(value, token, maxlen) == NULL) { + ret = -1; + break; + } found = 1; } } @@ -812,6 +823,7 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) char uuidbuf[1024]; char iden[1024]; int fd, ret; + int retval = 0; if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0) return -1; @@ -832,13 +844,14 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) sscanf(line, "%s %s\n", iden, uuidbuf); if(STREQ(iden, "#UUID:")) { - strncpy(uuidstr, uuidbuf, len); + if (virStrcpy(uuidstr, uuidbuf, len) == NULL) + retval = -1; break; } } close(fd); - return 0; + return retval; } /* Do actual checking for UUID presence in conf file, diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index f457cf4..508767e 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1369,7 +1369,7 @@ escape_specialcharacters(char *src, char *dst, size_t dstlen) } temp_buffer[j] = '\0'; - if (strncpy(dst, temp_buffer, dstlen) == NULL) + if (virStrcpy(dst, temp_buffer, dstlen) == NULL) return -1; return 0; diff --git a/src/proxy_internal.c b/src/proxy_internal.c index 5b92ad8..8609201 100644 --- a/src/proxy_internal.c +++ b/src/proxy_internal.c @@ -197,7 +197,10 @@ retry: memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; addr.sun_path[0] = '\0'; - strncpy(&addr.sun_path[1], path, (sizeof(addr) - 4) - 2); + if (virStrcpy(&addr.sun_path[1], path, sizeof(addr.sun_path) - 1) == NULL) { + close(fd); + return -1; + } /* * now bind the socket to that address and listen on it diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 6b0b404..4690c69 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1282,18 +1282,18 @@ static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, { switch (dev->type) { case VIR_DOMAIN_CHR_TYPE_NULL: - strncpy(buf, "null", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "null", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_VC: - strncpy(buf, "vc", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "vc", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_PTY: - strncpy(buf, "pty", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "pty", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_DEV: @@ -1315,8 +1315,8 @@ static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, break; case VIR_DOMAIN_CHR_TYPE_STDIO: - strncpy(buf, "stdio", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "stdio", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_UDP: diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 37fdec2..bcacd41 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -909,7 +909,11 @@ qemudOpenMonitorUnix(virConnectPtr conn, memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, monitor, sizeof(addr.sun_path)); + if (virStrcpy(addr.sun_path, monitor, sizeof(addr.sun_path)) == NULL) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Monitor path %s too big for destination"), monitor); + goto error; + } do { ret = connect(monfd, (struct sockaddr *) &addr, sizeof(addr)); @@ -5802,8 +5806,11 @@ static int qemuGetSchedulerParameters(virDomainPtr dom, goto cleanup; } params[0].value.ul = val; - strncpy(params[0].field, "cpu_shares", sizeof(params[0].field)); - params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG; + if (virStrcpy(params[0].field, "cpu_shares", sizeof(params[0].field)) == NULL) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("Field cpu_shares too long for destination")); + goto cleanup; + } ret = 0; diff --git a/src/remote_internal.c b/src/remote_internal.c index ad0cbf0..bc69c70 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -670,7 +670,11 @@ doRemoteOpen (virConnectPtr conn, memset (&addr, 0, sizeof addr); addr.sun_family = AF_UNIX; - strncpy (addr.sun_path, sockname, UNIX_PATH_MAX (addr)); + if (virStrcpy(addr.sun_path, sockname, sizeof(addr.sun_path)) == NULL) { + errorf(conn, VIR_ERR_INTERNAL_ERROR, + _("Socket %s too big for destination"), sockname); + goto failed; + } if (addr.sun_path[0] == '@') addr.sun_path[0] = '\0'; @@ -1585,8 +1589,8 @@ remoteNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info) (xdrproc_t) xdr_remote_node_get_info_ret, (char *) &ret) == -1) goto done; - strncpy (info->model, ret.model, 32); - info->model[31] = '\0'; + if (virStrcpy(info->model, ret.model, sizeof(info->model)) == NULL) + goto done; info->memory = ret.memory; info->cpus = ret.cpus; info->mhz = ret.mhz; @@ -3017,9 +3021,13 @@ remoteDomainGetSchedulerParameters (virDomainPtr domain, /* Deserialise the result. */ for (i = 0; i < *nparams; ++i) { - strncpy (params[i].field, ret.params.params_val[i].field, - VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[i].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpy(params[i].field, ret.params.params_val[i].field, + sizeof(params[i].field)) == NULL) { + errorf(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("Parameter %s too big for destination"), + ret.params.params_val[i].field); + goto cleanup; + } params[i].type = ret.params.params_val[i].value.type; switch (params[i].type) { case VIR_DOMAIN_SCHED_FIELD_INT: diff --git a/src/test.c b/src/test.c index ab6cfdf..e265d38 100644 --- a/src/test.c +++ b/src/test.c @@ -570,8 +570,11 @@ static int testOpenFromFile(virConnectPtr conn, privconn->nextDomID = 1; privconn->numCells = 0; - strncpy(privconn->path, file, PATH_MAX-1); - privconn->path[PATH_MAX-1] = '\0'; + if (virStrcpy(privconn->path, file, sizeof(privconn->path)) == NULL) { + testError(NULL, VIR_ERR_INTERNAL_ERROR, + _("Path %s too big for destination"), file); + goto error; + } memmove(&privconn->nodeInfo, &defaultNodeInfo, sizeof(defaultNodeInfo)); nodeInfo = &privconn->nodeInfo; @@ -627,8 +630,12 @@ static int testOpenFromFile(virConnectPtr conn, str = virXPathString(conn, "string(/node/cpu/model[1])", ctxt); if (str != NULL) { - strncpy(nodeInfo->model, str, sizeof(nodeInfo->model)-1); - nodeInfo->model[sizeof(nodeInfo->model)-1] = '\0'; + if (virStrcpy(nodeInfo->model, str, sizeof(nodeInfo->model)) == NULL) { + testError(NULL, VIR_ERR_INTERNAL_ERROR, + _("Model %s too big for destination"), str); + VIR_FREE(str); + goto error; + } VIR_FREE(str); } diff --git a/src/uml_driver.c b/src/uml_driver.c index 53e7e10..815a4b3 100644 --- a/src/uml_driver.c +++ b/src/uml_driver.c @@ -562,6 +562,7 @@ static int umlMonitorAddress(virConnectPtr conn, virDomainObjPtr vm, struct sockaddr_un *addr) { char *sockname; + int retval = 0; if (virAsprintf(&sockname, "%s/%s/mconsole", driver->monitorDir, vm->def->name) < 0) { @@ -571,10 +572,13 @@ static int umlMonitorAddress(virConnectPtr conn, memset(addr, 0, sizeof *addr); addr->sun_family = AF_UNIX; - strncpy(addr->sun_path, sockname, sizeof(addr->sun_path)-1); - NUL_TERMINATE(addr->sun_path); + if (virStrcpy(addr->sun_path, sockname, sizeof(addr->sun_path)) == NULL) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Unix path %s too long for destination"), sockname); + retval = -1; + } VIR_FREE(sockname); - return 0; + return retval; } static int umlOpenMonitor(virConnectPtr conn, @@ -668,8 +672,11 @@ static int umlMonitorCommand(virConnectPtr conn, cmd, req.length); return -1; } - strncpy(req.data, cmd, req.length); - req.data[req.length] = '\0'; + if (virStrcpy(req.data, cmd, sizeof(req.data)) == NULL) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Command %s too long for destination"), cmd); + return -1; + } if (sendto(vm->monitor, &req, sizeof req, 0, (struct sockaddr *)&addr, sizeof addr) != (sizeof req)) { diff --git a/src/util.c b/src/util.c index 0d4f3fa..0d252e6 100644 --- a/src/util.c +++ b/src/util.c @@ -1121,9 +1121,8 @@ char *virFindFileInPath(const char *file) char fullpath[PATH_MAX]; /* copy PATH env so we can tweak it */ - strncpy(pathenv, getenv("PATH"), PATH_MAX); - pathenv[PATH_MAX - 1] = '\0'; - + if (virStrcpy(pathenv, getenv("PATH"), sizeof(pathenv)) == NULL) + return NULL; /* for each path segment, append the file to search for and test for * it. return it if found. @@ -1155,8 +1154,8 @@ int virFileMakePath(const char *path) if (stat(path, &st) >= 0) return 0; - strncpy(parent, path, PATH_MAX); - parent[PATH_MAX - 1] = '\0'; + if (virStrcpy(parent, path, sizeof(parent)) == NULL) + return EINVAL; if (!(p = strrchr(parent, '/'))) return EINVAL; @@ -1573,6 +1572,28 @@ virAsprintf(char **strp, const char *fmt, ...) return ret; } +/** + * virStrcpy + * + * A safe version of strcpy. The last parameter is the number of bytes + * available in the destination string, *not* the number of bytes you want + * to copy. If the destination is not large enough to hold all of the + * src string, NULL is returned and no data is copied. If the destination + * is large enough to hold all of the src, then the string is copied and + * a pointer to the destination string is returned. + */ +char * +virStrcpy(char *dest, const char *src, size_t destbytes) +{ + int len; + + len = strlen(src) + 1; + if (len > destbytes) + return NULL; + + return memcpy(dest, src, len); +} + /* Compare two MAC addresses, ignoring differences in case, * as well as leading zeros. */ diff --git a/src/util.h b/src/util.h index b3e628a..ffbf2b3 100644 --- a/src/util.h +++ b/src/util.h @@ -164,6 +164,7 @@ void virSkipSpaces(const char **str); int virParseNumber(const char **str); int virAsprintf(char **strp, const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(2, 3); +char *virStrcpy(char *dest, const char *src, size_t destbytes); #define VIR_MAC_BUFLEN 6 #define VIR_MAC_PREFIX_BUFLEN 3 diff --git a/src/xen_internal.c b/src/xen_internal.c index ae78f84..fcc4977 100644 --- a/src/xen_internal.c +++ b/src/xen_internal.c @@ -1208,13 +1208,21 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain, if (ret < 0) return(-1); - strncpy (params[0].field, str_weight, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[0].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpy(params[0].field, str_weight, + sizeof(params[0].field)) == NULL) { + virXenError (domain->conn, VIR_ERR_INTERNAL_ERROR, + "Weight %s too big for destination", str_weight); + return -1; + } params[0].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[0].value.ui = op_dom.u.getschedinfo.u.credit.weight; - strncpy (params[1].field, str_cap, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[1].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpy(params[1].field, str_cap, + sizeof(params[1].field)) == NULL) { + virXenError (domain->conn, VIR_ERR_INTERNAL_ERROR, + "Cap %s too big for destination", str_cap); + return -1; + } params[1].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[1].value.ui = op_dom.u.getschedinfo.u.credit.cap; @@ -2431,9 +2439,9 @@ xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, while (fgets (line, sizeof line, cpuinfo)) { if (regexec (&flags_hvm_rec, line, sizeof(subs)/sizeof(regmatch_t), subs, 0) == 0 && subs[0].rm_so != -1) { - strncpy (hvm_type, - &line[subs[1].rm_so], subs[1].rm_eo-subs[1].rm_so+1); - hvm_type[subs[1].rm_eo-subs[1].rm_so] = '\0'; + if (virStrcpy(hvm_type, &line[subs[1].rm_so], + sizeof(hvm_type)) == NULL) + return NULL; } else if (regexec (&flags_pae_rec, line, 0, NULL, 0) == 0) host_pae = 1; } diff --git a/src/xend_internal.c b/src/xend_internal.c index 7bcee7d..a7167ae 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -763,7 +763,8 @@ xenDaemonOpen_unix(virConnectPtr conn, const char *path) addr = (struct sockaddr_un *)&priv->addr; addr->sun_family = AF_UNIX; memset(addr->sun_path, 0, sizeof(addr->sun_path)); - strncpy(addr->sun_path, path, sizeof(addr->sun_path)); + if (virStrcpy(addr->sun_path, path, sizeof(addr->sun_path)) == NULL) + return -1; return (0); } @@ -1442,8 +1443,11 @@ xenDaemonParseSxprChar(virConnectPtr conn, return NULL; } - strncpy(prefix, value, sizeof(prefix)-1); - NUL_TERMINATE(prefix); + if (virStrcpy(prefix, value, sizeof(prefix)) == NULL) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("Value %s too big for prefix"), value); + goto error; + } if (value[0] == '/') { def->type = VIR_DOMAIN_CHR_TYPE_DEV; @@ -1647,8 +1651,12 @@ xenDaemonParseSxprDisks(virConnectPtr conn, if (VIR_ALLOC_N(disk->driverName, (offset-src)+1) < 0) goto no_memory; - strncpy(disk->driverName, src, (offset-src)); - disk->driverName[offset-src] = '\0'; + if (virStrcpy(disk->driverName, src, (offset-src)+1) == NULL) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver name %s too big for destination"), + src); + goto error; + } src = offset + 1; @@ -1662,8 +1670,12 @@ xenDaemonParseSxprDisks(virConnectPtr conn, if (VIR_ALLOC_N(disk->driverType, (offset-src)+1)< 0) goto no_memory; - strncpy(disk->driverType, src, (offset-src)); - disk->driverType[offset-src] = '\0'; + if (virStrcpy(disk->driverType, src, (offset-src)+1) == NULL) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver type %s too big for destination"), + src); + goto error; + } src = offset + 1; /* Its possible to use blktap driver for block devs @@ -1896,8 +1908,12 @@ xenDaemonParseSxprSound(virConnectPtr conn, _("unexpected sound model %s"), offset); goto error; } - strncpy(model, offset, len); - model[len] = '\0'; + if (virStrcpy(model, offset, sizeof(model)) == NULL) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("Sound model %s too big for destination"), + offset); + goto error; + } if (VIR_ALLOC(sound) < 0) goto no_memory; @@ -4786,13 +4802,22 @@ xenDaemonGetSchedulerParameters(virDomainPtr domain, goto error; } - strncpy (params[0].field, str_weight, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[0].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpy(params[0].field, str_weight, + sizeof(params[0].field)) == NULL) { + virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("Weight %s too big for destination"), + str_weight); + goto error; + } params[0].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[0].value.ui = sexpr_int(root, "domain/cpu_weight"); - strncpy (params[1].field, str_cap, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[1].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpy(params[1].field, str_cap, + sizeof(params[1].field)) == NULL) { + virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("Cap %s too big for destination"), str_cap); + goto error; + } params[1].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[1].value.ui = sexpr_int(root, "domain/cpu_cap"); *nparams = XEN_SCHED_CRED_NPARAM; @@ -5842,6 +5867,7 @@ virDomainXMLDevID(virDomainPtr domain, { xenUnifiedPrivatePtr priv = domain->conn->privateData; char *xref; + char *tmp; if (dev->type == VIR_DOMAIN_DEVICE_DISK) { if (dev->data.disk->driverName && @@ -5859,9 +5885,10 @@ virDomainXMLDevID(virDomainPtr domain, if (xref == NULL) return -1; - strncpy(ref, xref, ref_len); + tmp = virStrcpy(ref, xref, ref_len); free(xref); - ref[ref_len - 1] = '\0'; + if (tmp == NULL) + return -1; } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { char mac[30]; virDomainNetDefPtr def = dev->data.net; @@ -5878,9 +5905,10 @@ virDomainXMLDevID(virDomainPtr domain, if (xref == NULL) return -1; - strncpy(ref, xref, ref_len); + tmp = virStrcpy(ref, xref, ref_len); free(xref); - ref[ref_len - 1] = '\0'; + if (tmp == NULL) + return -1; } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { diff --git a/src/xm_internal.c b/src/xm_internal.c index 71b852e..00d9e9a 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -872,8 +872,12 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { } else { if (VIR_ALLOC_N(disk->src, (offset - head) + 1) < 0) goto no_memory; - strncpy(disk->src, head, (offset - head)); - disk->src[(offset-head)] = '\0'; + if (virStrcpy(disk->src, head, (offset - head) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Source file %s too big for destination"), + head); + goto cleanup; + } } head = offset + 1; @@ -886,8 +890,11 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { goto skipdisk; if (VIR_ALLOC_N(disk->dst, (offset - head) + 1) < 0) goto no_memory; - strncpy(disk->dst, head, (offset - head)); - disk->dst[(offset-head)] = '\0'; + if (virStrcpy(disk->dst, head, (offset - head) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Dest file %s too big for destination"), head); + goto cleanup; + } head = offset + 1; @@ -897,8 +904,13 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { if ((tmp = strchr(disk->src, ':')) != NULL) { if (VIR_ALLOC_N(disk->driverName, (tmp - disk->src) + 1) < 0) goto no_memory; - strncpy(disk->driverName, disk->src, (tmp - disk->src)); - disk->driverName[tmp - disk->src] = '\0'; + if (virStrcpy(disk->driverName, disk->src, + (tmp - disk->src) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver name %s too big for destination"), + disk->src); + goto cleanup; + } /* Strip the prefix we found off the source file name */ memmove(disk->src, disk->src+(tmp-disk->src)+1, @@ -912,8 +924,13 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { goto skipdisk; if (VIR_ALLOC_N(disk->driverType, (tmp - disk->src) + 1) < 0) goto no_memory; - strncpy(disk->driverType, disk->src, (tmp - disk->src)); - disk->driverType[tmp - disk->src] = '\0'; + if (virStrcpy(disk->driverType, disk->src, + (tmp - disk->src) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver type %s too big for destination"), + disk->src); + goto cleanup; + } /* Strip the prefix we found off the source file name */ memmove(disk->src, disk->src+(tmp-disk->src)+1, @@ -1026,42 +1043,46 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { data++; if (STRPREFIX(key, "mac=")) { - int len = nextkey ? (nextkey - data) : 17; - if (len > 17) - len = 17; - strncpy(mac, data, len); - mac[len] = '\0'; + if (virStrcpy(mac, data, sizeof(mac)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("MAC address %s too big for destination"), + data); + goto cleanup; + } } else if (STRPREFIX(key, "bridge=")) { - int len = nextkey ? (nextkey - data) : sizeof(bridge)-1; + if (virStrcpy(bridge, data, sizeof(bridge)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Bridge %s too big for destination"), + data); + goto cleanup; + } type = 1; - if (len > (sizeof(bridge)-1)) - len = sizeof(bridge)-1; - strncpy(bridge, data, len); - bridge[len] = '\0'; } else if (STRPREFIX(key, "script=")) { - int len = nextkey ? (nextkey - data) : PATH_MAX-1; - if (len > (PATH_MAX-1)) - len = PATH_MAX-1; - strncpy(script, data, len); - script[len] = '\0'; + if (virStrcpy(script, data, sizeof(script)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Script %s too big for destination"), + data); + goto cleanup; + } } else if (STRPREFIX(key, "model=")) { - int len = nextkey ? (nextkey - data) : sizeof(model)-1; - if (len > (sizeof(model)-1)) - len = sizeof(model)-1; - strncpy(model, data, len); - model[len] = '\0'; + if (virStrcpy(model, data, sizeof(model)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Model %s too big for destination"), data); + goto cleanup; + } } else if (STRPREFIX(key, "vifname=")) { - int len = nextkey ? (nextkey - data) : sizeof(vifname)-1; - if (len > (sizeof(vifname)-1)) - len = sizeof(vifname)-1; - strncpy(vifname, data, len); - vifname[len] = '\0'; + if (virStrcpy(vifname, data, sizeof(vifname)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Vifname %s too big for destination"), + data); + goto cleanup; + } } else if (STRPREFIX(key, "ip=")) { - int len = nextkey ? (nextkey - data) : 15; - if (len > 15) - len = 15; - strncpy(ip, data, len); - ip[len] = '\0'; + if (virStrcpy(ip, data, sizeof(ip)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("IP %s too big for destination"), data); + goto cleanup; + } } while (nextkey && (nextkey[0] == ',' || @@ -1168,29 +1189,41 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { if ((nextkey - key) > (sizeof(domain)-1)) goto skippci; - strncpy(domain, key, sizeof(domain)); - domain[sizeof(domain)-1] = '\0'; + if (virStrcpy(domain, key, sizeof(domain)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Domain %s too big for destination"), key); + goto cleanup; + } key = nextkey + 1; if (!(nextkey = strchr(key, ':'))) goto skippci; - strncpy(bus, key, sizeof(bus)); - bus[sizeof(bus)-1] = '\0'; + if (virStrcpy(bus, key, sizeof(bus)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Bus %s too big for destination"), key); + goto cleanup; + } key = nextkey + 1; if (!(nextkey = strchr(key, '.'))) goto skippci; - strncpy(slot, key, sizeof(slot)); - slot[sizeof(slot)-1] = '\0'; + if (virStrcpy(slot, key, sizeof(slot)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Slot %s too big for destination"), key); + goto cleanup; + } key = nextkey + 1; if (strlen(key) != 1) goto skippci; - strncpy(func, key, sizeof(func)); - func[sizeof(func)-1] = '\0'; + if (virStrcpy(func, key, sizeof(func)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Function %s too big for destination"), key); + goto cleanup; + } if (virStrToLong_i(domain, NULL, 16, &domainID) < 0) goto skippci; @@ -1301,8 +1334,13 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { list->list->str) { char vfb[MAX_VFB]; char *key = vfb; - strncpy(vfb, list->list->str, MAX_VFB-1); - vfb[MAX_VFB-1] = '\0'; + + if (virStrcpy(vfb, list->list->str, sizeof(vfb)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("VFB %s too big for destination"), + list->list->str); + goto cleanup; + } if (VIR_ALLOC(graphics) < 0) goto no_memory; -- 1.6.0.6

2 2

[libvirt] libvirt with libssh2?
by Emre Erenoglu 20 Aug '09

20 Aug '09

Hi, I'm the package maintainer of libvirt for the Pardus distribution. While trying to update the package to the latest 0.7.0, I recognized that configure script does not find libssh. I checked our system and found out that we are using libssh2 instead of libssh. >From this website (http://www.libssh2.org/libssh2-vs-libssh.html) libssh2 developers summarize their differences. My questions: 1) Is there a specific reason why we are using libssh? Is there a patch to make libvirt use libssh2 instead? 2) What functionality do we loose if we don't have libssh in the system? (depending on this, I'll also package libssh or not) Thanks a lot for any comments, Best regards, Emre Erenoglu

2 1

[libvirt] [PATCH] Small fixes for qemu save compression.
by Chris Lalancette 20 Aug '09

20 Aug '09

Fix up a small memory leak pointed out by DanB; I was forgetting to release memory allocated to driver->saveImageFormat. Also add the "save_image_format" and "security" entries to the augeas lens. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- qemud/libvirtd_qemu.aug | 2 ++ qemud/test_libvirtd_qemu.aug | 10 +++++++++- src/qemu_driver.c | 1 + 3 files changed, 12 insertions(+), 1 deletions(-) diff --git a/qemud/libvirtd_qemu.aug b/qemud/libvirtd_qemu.aug index 8b8aab2..2175e14 100644 --- a/qemud/libvirtd_qemu.aug +++ b/qemud/libvirtd_qemu.aug @@ -29,10 +29,12 @@ module Libvirtd_qemu = | str_entry "vnc_password" | bool_entry "vnc_sasl" | str_entry "vnc_sasl_dir" + | str_entry "security_driver" | str_entry "user" | str_entry "group" | str_array_entry "cgroup_controllers" | str_array_entry "cgroup_device_acl" + | str_entry "save_image_format" (* Each enty in the config is one of the following three ... *) let entry = vnc_entry diff --git a/qemud/test_libvirtd_qemu.aug b/qemud/test_libvirtd_qemu.aug index 274c89d..72f2227 100644 --- a/qemud/test_libvirtd_qemu.aug +++ b/qemud/test_libvirtd_qemu.aug @@ -80,6 +80,8 @@ vnc_sasl = 1 # vnc_sasl_dir = \"/some/directory/sasl2\" +security_driver = \"selinux\" + user = \"root\" group = \"root\" @@ -87,6 +89,8 @@ group = \"root\" cgroup_controllers = [ \"cpu\", \"devices\" ] cgroup_device_acl = [ \"/dev/null\", \"/dev/full\", \"/dev/zero\" ] + +save_image_format = \"gzip\" " test Libvirtd_qemu.lns get conf = @@ -170,7 +174,9 @@ cgroup_device_acl = [ \"/dev/null\", \"/dev/full\", \"/dev/zero\" ] { "#comment" = "" } { "vnc_sasl_dir" = "/some/directory/sasl2" } { "#empty" } -{ "user"= "root" } +{ "security_driver" = "selinux" } +{ "#empty" } +{ "user" = "root" } { "#empty" } { "group" = "root" } { "#empty" } @@ -184,3 +190,5 @@ cgroup_device_acl = [ \"/dev/null\", \"/dev/full\", \"/dev/zero\" ] { "2" = "/dev/full" } { "3" = "/dev/zero" } } +{ "#empty" } +{ "save_image_format" = "gzip" } diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 86a33f8..ffe9c16 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -666,6 +666,7 @@ qemudShutdown(void) VIR_FREE(qemu_driver->vncListen); VIR_FREE(qemu_driver->vncPassword); VIR_FREE(qemu_driver->vncSASLdir); + VIR_FREE(qemu_driver->saveImageFormat); /* Free domain callback list */ virDomainEventCallbackListFree(qemu_driver->domainEventCallbacks); -- 1.6.0.6

2 2

[libvirt] [PATCH 0/1] Multipath pool support
by David Allan 19 Aug '09

19 Aug '09

The following patch implements multipath pool support. It's very basic functionality, consisting of creating a pool that contains all the multipath devices on the host. That will cover the common case of users who just want to discover all the available multipath devices and assign them to guests. It doesn't currently allow configuration of multipathing, so for now setting the multipath configuration will have to continue to be done as part of the host system build. Example XML to create the pool is: <pool type="mpath"> <name>mpath</name> <target> <path>/dev/mapper</path> </target> </pool> The target element is ignored, as it is by the disk pool, but the config code rejects the XML if it does not exist. That behavior should obviously be cleaned up, but I think that should be done in a separate patch, as it's really a bug in the config code, not related to the addition of the new pool type. Dave

4 8

[libvirt] FYI: Updated QEMU driver docs on security model
by Daniel P. Berrange 19 Aug '09

19 Aug '09

FYI, I just pushed the following patch to the repo which adds documentation to the website for all the security model related aspects of libvirt's QEMU driver. It should appear here shortly http://libvirt.org/drvqemu.html Regards, Daniel diff --git a/docs/drvqemu.html.in b/docs/drvqemu.html.in index e4c3d03..348eaaf 100644 --- a/docs/drvqemu.html.in +++ b/docs/drvqemu.html.in @@ -54,6 +54,292 @@ qemu+ssh://root@example.com/system (remote access, SSH tunnelled) </pre> + <h2><a name="security">Driver security architecture</a></h2> + + <p> + There are multiple layers to security in the QEMU driver, allowing for + flexibility in the use of QEMU based virtual machines. + </p> + + <h3><a name="securitydriver">Driver instances</a></h3> + + <p> + As explained above there are two ways to access the QEMU driver + in libvirt. The "qemu:///session" family of URIs connect to a + libvirtd instance running as the same user/group ID as the client + application. Thus the QEMU instances spawned from this driver will + share the same privileges as the client application. The intended + use case for this driver is desktop virtualization, with virtual + machines storing their disk imags in the user's home directory and + being managed from the local desktop login session. + </p> + + <p> + The "qemu:///system" family of URIs connect to a + libvirtd instance running as the privileged system account 'root'. + Thus the QEMU instances spawned from this driver may have much + higher privileges than the client application managing them. + The intended use case for this driver is server virtualization, + where the virtual machines may need to be connected to host + resources (block, PCI, USB, network devices) whose access requires + elevated privileges. + </p> + + <h3><a name="securitydac">POSIX DAC users/groups</a></h3> + + <p> + In the "session" instance, the POSIX DAC model restricts QEMU virtual + machines (and libvirtd in general) to only have access to resources + with the same user/group ID as the client application. There is no + finer level of configuration possible for the "session" instances. + </p> + + <p> + In the "system" instance, libvirt releases from 0.7.0 onwards allow + control over the user/group that the QEMU virtual machines are run + as. A build of libvirt with no configuration parameters set will + still run QEMU processes as root:root. It is possible to change + this default by using the --with-qemu-user=$USERNAME and + --with-qemu-group=$GROUPNAME arguments to 'configure' during + build. It is strongly recommended that vendors build with both + of these arguments set to 'qemu'. Regardless of this build time + default, administrators can set a per-host default setting in + the <code>/etc/libvirt/qemu.conf</code> configuration file via + the <code>user=$USERNAME</code> and <code>group=$GROUPNAME</code> + parameters. When a non-root user or group is configured, the + libvirt QEMU driver will change uid/gid to match immediately + before executing the QEMU binary for a virtual machine. + </p> + + <p> + If QEMU virtual machines from the "system" instance are being + run as non-root, there will be greater restrictions on what + host resources the QEMU process will be able to access. The + libvirtd daemon will attempt to manage permissions on resources + to minise the likelihood of unintentionale security denials, + but the administrator / application developer must be aware of + some of the consequences / restrictions. + </p> + + <ul> + <li> + <p> + The directories <code>/var/run/libvirt/qemu/</code>, + <code>/var/lib/libvirt/qemu/</code> and + <code>/var/cache/libvirt/qemu/</code> must all have their + ownership set to match the user / group ID that QEMU + guests will be run as. If the vendor has set a non-root + user/group for the QEMU driver at build time, the + permissions should be set automatically at install time. + If a host administrator customizes user/group in + <code>/etc/libvirt/qemu.conf</code>, they will need to + manually set the ownership on these directories. + </p> + </li> + <li> + <p> + When attaching PCI and USB devices to a QEMU guest, + QEMU will need to access files in <code>/dev/bus/usb</code> + and <code>/sys/bus/devices</code>. The libvirtd daemon + will automatically set the ownership on specific devices + that are assigned to a guest at start time. There should + not be any need for administrator changes in this respect. + </p> + </li> + <li> + <p> + Any files/devices used as guest disk images must be + accessible to the user/group ID that QEMU guests are + configured to run as. The libvirtd daemon will automatically + set the ownership of the file/device path to the correct + user/group ID. Applications / administrators must be aware + though that the parent directory permissions may still + deny access. The directories containing disk images + must either have their ownership set to match the user/group + configured for QEMU, or their UNIX file permissions must + have the 'execute/search' bit enabled for 'others'. + </p> + <p> + The simplest option is the latter one, of just enabling + the 'execute/search' bit. For any directory to be used + for storing disk images, this can be achived by running + the following command on the directory itself, and any + parent directories + </p> +<pre> + chmod o+x /path/to/directory +</pre> + <p> + In particular note that if using the "system" instance + and attempting to store disk images in a user home + directory, the default permissions on $HOME are typically + too restrictive to allow access. + </p> + </li> + </ul> + + <h3><a name="securitycap">Linux DAC capabilities</a></h3> + + <p> + The libvirt QEMU driver has a build time option allowing it to use + the <a href="http://people.redhat.com/sgrubb/libcap-ng/index.html">libcap-ng</a> + library to manage process capabilities. If this build option is + enabled, then the QEMU driver will use this to ensure that all + process capabilities are dropped before executing a QEMU virtual + machine. Process capabilities are what gives the 'root' account + its high power, in particular the CAP_DAC_OVERRIDE capability + is what allows a process running as 'root' to access files owned + by any user. + </p> + + <p> + If the QEMU driver is configured to run virtual machines as non-root, + then they will already loose all their process capabilities at time + of startup. The Linux capability feature is thus aimed primarily at + the scenario where the QEMU processes are running as root. In this + case, before launching a QEMU virtual machine, libvirtd will use + libcap-ng APIs to drop all process capabilities. It is important + for administrators to note that this implies the QEMU process will + <strong>only</strong> be able to access files owned by root, and + not files owned by any other user. + </p> + + <p> + Thus, if a vendor / distributor has configured their libvirt package + to run as 'qemu' by default, a number of changes will be required + before an administrator can change a host to run guests as root. + In particular it will be neccessary to change ownership on the + directories <code>/var/run/libvirt/qemu/</code>, + <code>/var/lib/libvirt/qemu/</code> and + <code>/var/cache/libvirt/qemu/</code> back to root, in addition + to changing the <code>/etc/libvirt/qemu.conf</code> settings. + </p> + + <h3><a name="securityselinux">SELinux MAC basic confinement</a></h3> + + <p> + The basic SELinux protection for QEMU virtual machines is intended to + protect the host OS from a compromised virtual machine process. There + is no protection between guests. + </p> + + <p> + In the basic model, all QEMU virtual machines run under the confined + domain <code>root:system_r:qemu_t</code>. It is required that any + disk image assigned to a QEMU virtual machine is labelled with + <code>system_u:object_r:virt_image_t</code>. In a default deployment, + package vendors/distributor will typically ensure that the directory + <code>/var/lib/libvirt/images</code> has this label, such that any + disk images created in this directory will automatically inherit the + correct labelling. If attempting to use disk images in another + location, the user/administrator must ensure the directory has be + given this requisite label. Likewise physical block devices must + be labelled <code>system_u:object_r:virt_image_t</code>. + </p> + <p> + Not all filesystems allow for labelling of individual files. In + particular NFS, VFat and NTFS have no support for labelling. In + these cases administrators must use the 'context' option when + mounting the filesystem to set the default label to + <code>system_u:object_r:virt_image_t</code>. In the case of + NFS, there is an alternative option, of enabling the <code>virt_use_nfs</code> + SELinux boolean. + </p> + + <h3><a name="securitysvirt">SELinux MAC sVirt confinement</a></h3> + + <p> + The SELinux sVirt protection for QEMU virtual machines builds to the + basic level of protection, to also allow individual guests to be + protected from each other. + </p> + + <p> + In the sVirt model, each QEMU virtual machine runs under its own + confined domain, which is based on <code>system_u:system_r:svirt_t:s0</code> + with a unique category appended, eg, <code>system_u:system_r:svirt_t:s0:c34,c44</code>. + The rules are setup such that a domain can only access files which are + labelled with the matching category level, eg + <code>system_u:object_r:svirt_image_t:s0:c34,c44</code>. This prevents one + QEMU process accessing any file resources that are prevent to another QEMU + process. + </p> + + <p> + There are two ways of assigning labels to virtual machines under sVirt. + In the default setup, if sVirt is enabled, guests will get an automatically + assigned unique label each time they are booted. The libvirtd daemon will + also automatically relabel exclusive access disk images to match this + label. Disks that are marked as <shared> will get a generic + label <code>system_u:system_r:svirt_image_t:s0</code> allowing all guests + read/write access them, while disks marked as <readonly> will + get a generic label <code>system_u:system_r:svirt_content_t:s0</code> + which allows all guests read-only access. + </p> + + <p> + With statically assigned labels, the application should include the + desired guest and file labels in the XML at time of creating the + guest with libvirt. In this scenario the application is responsible + for ensuring the disk images & similar resources are suitably + labelled to match, libvirtd will not attempt any relabelling. + </p> + + <p> + If the sVirt security model is active, then the node capabilties + XML will include its details. If a virtual machine is currently + protected by the security model, then the guest XML will include + its assigned labels. If enabled at compile time, the sVirt security + model will always be activated if SELinux is available on the host + OS. To disable sVirt, and revert to the basic level of SELinux + protection (host protection only), the <code>/etc/libvirt/qemu.conf</code> + file can be used to change the setting to <code>security_driver="none"</code> + </p> + + + <h3><a name="securityacl">Cgroups device ACLs</a></h3> + + <p> + Recent Linux kernels have a capability known as "cgroups" which is used + for resource management. It is implemented via a number of "controllers", + each controller covering a specific task/functional area. One of the + available controllers is the "devices" controller, which is able to + setup whitelists of block/character devices that a cgroup should be + allowed to access. If the "devices" controller is mounted on a host, + then libvirt will automatically create a dedicated cgroup for each + QEMU virtual machine and setup the device whitelist so that the QEMU + process can only access shared devices, and explicitly disks images + backed by block devices. + </p> + + <p> + The list of shared devices a guest is allowed access to is + </p> + + <pre> + /dev/null, /dev/full, /dev/zero, + /dev/random, /dev/urandom, + /dev/ptmx, /dev/kvm, /dev/kqemu, + /dev/rtc, /dev/hpet, /dev/net/tun + </pre> + + <p> + In the event of unanticipated needs arising, this can be customized + via the <code>/etc/libvirt/qemu.conf</code> file. + To mount the cgroups device controller, the following command + should be run as root, prior to starting libvirtd + </p> + + <pre> + mkdir /dev/cgroup + mount -t cgroup none /dev/cgroup -o devices + </pre> + + <p> + libvirt will then place each virtual machine in a cgroup at + <code>/dev/cgroup/libvirt/qemu/$VMNAME/</code> + </p> + <h2><a name="imex">Import and export of libvirt domain XML configs</a></h2> <p>The QEMU driver currently supports a single native -- |: 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 :|

3 3

[libvirt] Add support for (qcow*) volume encryption (v3)
by Miloslav Trmač 19 Aug '09

19 Aug '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: * Significantly change the internal API, basing it on virSecretPtr instead of (virConnectPtr, secret_id); details in patch 1 * Make virsh commands more similar to the existing commands * Remove <encryption format='unencrypted'/> * Rename "secret_id" to "uuid" in the XML and API * Use "unsigned char *" for secret value 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.

4 33

Re: [libvirt] [PATCH 07/20] Secret manipulation step 7: Local driver
by Miloslav Trmac 19 Aug '09

19 Aug '09

----- "Daniel Veillard" <veillard(a)redhat.com> wrote: > On Sun, Aug 16, 2009 at 10:48:00PM +0200, Miloslav Trmač wrote: > > This implementation stores the secrets in an unencrypted text file, > > for simplicity in implementation and debugging. > > > > (Symmetric encryption, e.g. using gpgme, will not be difficult to add. > > Because the TLS private key used by libvirtd is stored unencrypted, > > encrypting the secrets file does not currently provide much additional > > security.) > > What if we change our mind in some time, would there be any obstacle > to dynamically detecting things are not encrypted and switching to > a crypted file transparently? Two simple ways: 1) use a different file name, e,g. secrets.gpg 2) check the start of the file, currently the file always starts with "id "; I can add a proper magic number if necessary. > > +static int > > +writeBase64Data(virConnectPtr conn, int fd, const char *field, > > + const void *data, size_t size) > > +{ > > + int ret = -1; > > + char *base64 = NULL; > > + > > + if (writeString(conn, fd, field) < 0 || writeString(conn, fd, " > ") < 0) > > + goto cleanup; > > + > > + base64_encode_alloc(data, size, &base64); > > where is base64_encode_alloc coming from ? gnulib - in a later patch I'm afraid, I'll reorder it. > > +static int > > +saveSecrets(virConnectPtr conn, virSecretDriverStatePtr driver) > > +{ > > + const virSecretEntry *secret; > > + char *tmp_path = NULL; > > + int fd = -1, ret = -1; > > + > > + if (virAsprintf(&tmp_path, "%sXXXXXX", driver->filename) < 0) > { > > + virReportOOMError(conn); > > + goto cleanup; > > + } > > + fd = mkstemp (tmp_path); > > Hum even if unencrypted, we should make sure of the mode of the file > beforehand... Good point. > isn't there a safer equivalent (possibly made postable by gnulib ?) Grepping of gnulib does not reveal any. > > + if (fd == -1) { > > + virReportSystemError (conn, errno, _("mkstemp(\"%s\") failed"), > > + tmp_path); > > + goto cleanup; > > + } > > + > > + for (secret = driver->secrets; secret != NULL; > > + secret = secret->next) { > > + if (!secret->ephemeral && writeSecret(conn, fd, secret) < 0) > > + goto cleanup; > > + } > > + close(fd); > > + fd = -1; > > + if (rename(tmp_path, driver->filename) < 0) { > > + virReportSystemError (conn, errno, _("rename(%s, %s) failed"), tmp_path, > > + driver->filename); > > + goto cleanup; > > + } > > Hum, the whole set smells fishy, we are creating a temp file, without > mode checked, then moving that file somewhere else. I would ratehr have > internal APIsdeling with a dump to memory and then a single open, safe > and directly to the driver->filename. "Somewhere else" is in the same directory. The mkstemp() + rename() is used to make sure the secrets file is replaced atomically, without losing any data if a save is interrupted in the middle. > > +static int > > +parseBase64String(virConnectPtr conn, const char *base64, char > **string) > > +{ > > + char *tmp; > > + size_t size; > > + > > + if (!base64_decode_alloc(base64, strlen(base64), &tmp, &size)) > { > > where is base64_decode_alloc coming from ? gnulib > > + if (stat(driver->filename, &st) < 0) { > > + if (errno == ENOENT) > > + return 0; > > + virReportSystemError (conn, errno, _("cannot stat '%s'"), > > + driver->filename); > > + goto cleanup; > > + } > > + if ((size_t)st.st_size != st.st_size || (size_t)(st.st_size + 1) == 0) { > > + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", > > + _("secrets file does not fit in memory")); > > + goto cleanup; > > + } > > + > > + fd = open(driver->filename, O_RDONLY); > > + if (fd == -1) { > > + virReportSystemError (conn, errno, _("cannot open '%s'"), > > + driver->filename); > > + goto cleanup; > > + } > > stat()/open() introduces a small race, I think open() and > then fdstat() is a bit cleaner, not a big deal though Technically there is still a race with fstat() - but fstat is a bit better. > > + if (VIR_ALLOC_N(contents, st.st_size + 1) < 0) { > > + virReportOOMError(conn); > > + goto cleanup; > > + } > > + if (saferead(fd, contents, st.st_size) != st.st_size) { > > + virReportSystemError (conn, errno, _("cannot read '%s'"), > > + driver->filename); > > + goto cleanup; > > + } > > + close(fd); > > + fd = -1; > > contents[st.st_size] = 0; > needed here. VIR_ALLOC_N automatically zeroes the memory. I'll fix all other issues you have mentioned. Thanks for the review. Mirek

1 0

Re: [libvirt] [PATCH 05/20] Secret manipulation step 5: RPC client
by Miloslav Trmac 19 Aug '09

19 Aug '09

----- "Daniel Veillard" <veillard(a)redhat.com> wrote: > On Sun, Aug 16, 2009 at 10:47:58PM +0200, Miloslav Trmač wrote: > > +static virDrvOpenStatus > > +remoteSecretOpen (virConnectPtr conn, > > + virConnectAuthPtr auth, > > + int flags) > > +{ > > + if (inside_daemon) > > + return VIR_DRV_OPEN_DECLINED; > > + > > + if (conn && > > + conn->driver && > > + STREQ (conn->driver->name, "remote")) { > > + struct private_data *priv; > > + > > + /* If we're here, the remote driver is already > > + * in use due to a) a QEMU uri, or b) a remote > > + * URI. So we can re-use existing connection > > + */ > > Hum, is that comment right ? We are certainly using a remote URI in > any case there, isn't it ? The same comment is in in all of the other "subdriver" remote...Open () routines. It's not quite clear what "remote" means where, as far as I can see "remote" means an "explicitly remote" URL, e.g. anything+tls://hostname/..., and "QEMU" means "qemu://hostname/..." (rejected by qemu_driver). Mirek

2 1

Re: [libvirt] [PATCH 01/20] Secret manipulation step 1: Public API
by Miloslav Trmac 19 Aug '09

19 Aug '09

----- "Daniel Veillard" <veillard(a)redhat.com> wrote: > On Sun, Aug 16, 2009 at 10:47:54PM +0200, Miloslav Trmač wrote: > > Rather than add explicit accessors for attributes of secrets, and > > hard-code the "secrets are related to storage volumes" association > in > > the API, the API uses XML to manipulate the association as well as > > other attributes, similarly to other areas of libvirt. > > > > The user can set attributes of the secret using XML, e.g. > > <secret ephemeral='no' private='yes'> > > <uuid>b8eecf55-798e-4db7-b2dd-025b0cf08a36</uuid> > > <volume>/var/lib/libvirt/images/mail.img</volume> > > <description>LUKS passphrase for our mail server</description> > > </secret> > > If <uuid/> is not specified, it is chosen automatically. > > Should secret always be tied to volumes. The API is generic enough > that we should make sure we can use this later to get priviledged > access to other resources, though right now I don't have a good > example in mind. The secret is not "tied" to a volume - the volume is an optional attribute. A libvirt user can ignore the volume field completely, and manage the secrets separately - but other clients, e.g. virt-manager, can use the volume attribute to locate a secret for a given volume without storing any data outside of libvirt. > > - s/secret_id/uuid/g > > Not sure I catch that, An earlier patch was calling the ID of a secret object "secret_id" everywhere, leading to constructs like <secret secret_id=.../> In an earlier patch review I was asked to change that. > > - use "unsigned char *" for secret value > > fine, assuming 0 terninated. though this may not work with everything > but since we need to import/export to XML, unless uuencoding the value > we won't be able to embed something which is not part of XML character > range (#x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | > [#x10000-#x10FFFF]) unicode code points. No, the value is an arbitrary set of bytes that is never passed through XML. Originally it was (void *value, size_t value_size), the change to (unsigned char *value, size_t value_size) was requested in an earlier review. > > diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in > > new file mode 100644 > > index 0000000..7471bf7 > > --- /dev/null > > +++ b/docs/formatsecret.html.in > > @@ -0,0 +1,52 @@ > > +<html> > > + <body> > > + <h1>Secret XML format</h1> > > + > > + <ul id="toc"></ul> > > it's good to have a document from the start :-) but this lacks a bit > of the intended use for the API, i.e. where this may be required to > use it. Those parts are documented in later patches that add references to secrets to the <volume> and <domain> XML formats. > > +virSecretPtr virSecretDefineXML (virConnectPtr conn, > > + const char *xml); > > Let's add an "unsigned int flags" to virSecretDefineXML() especially > as we don't know yet the scope of future usages. > > > +char * virSecretGetXMLDesc (virSecretPtr secret); > > And also add an "unsigned int flags" to virSecretGetXMLDesc > Will do. > > +int virSecretSetValue (virSecretPtr secret, > > + const unsigned char *value, > > + size_t value_size); > > +unsigned char * virSecretGetValue (virSecretPtr secret, > > + size_t *value_size); > > Ah, so we aren't relying on 0 termination, but in that case the value > need to be uuencoded in the XML, and that should be made clear in the > API description. Actually not having the secret completely in the clear > in the XML dump sounds like a good idea. The values are never transferred in the XML, only using the virSecret[GS]etValue functions. (Storing secrets in XML was a major objection against the first version of my patch e.g. because XML tends to be revealed in various debugging dumps.) Mirek

3 3

[libvirt] OpenVZ : The restriction of domain name should be addressed
by Yuji NISHIDA 19 Aug '09

19 Aug '09

Hi, "The current libvirt OpenVZ driver has a restriction that the domain names must match the OpenVZ container VEID" I really want to have domain name as character, not integer. It could help me very much if any patch against this problem would be released. Is there anyone tackling this problem now?( or going to? ) Otherwise, any hint would help me. Regards. ----- Yuji Nishida nishidy(a)nict.go.jp

5 5

[libvirt] Integrating MAC address based filtering into libvirt
by Gerhard Stenzel 19 Aug '09

19 Aug '09

Hello, I am currently investigating the possibility to implement MAC address based filtering in libvirt and was wondering if there is any related effort going on and what people in general would think about that. Here is a description of my current prototype implementation: I have a small setup of two guests and a network: Guest1: root@stenzel-desktop:/etc/libvirt/qemu# cat build1.xml <domain type='kvm'> <name>build1</name> ... <interface type='network'> <mac address='D0:0F:D0:0F:02:01'/> <source network='mynet'/> <model type='virtio' /> </interface> ... </domain> Guest2: root@stenzel-desktop:/etc/libvirt/qemu# cat build2.xml <domain type='kvm'> <name>build2</name> ... <interface type='network'> <mac address='D0:0F:D0:0F:02:02'/> <source network='mynet'/> <model type='virtio' /> </interface> ... </domain> and the network to which I added a new XML element "filter" with attribute "mac", which switches on the MAC address filtering: root@stenzel-desktop:/etc/libvirt/qemu# cat networks/mynet.xml <network> <name>mynet</name> <uuid>920debe0-c3ef-4395-8241-ee82d4b49c2d</uuid> <bridge name="br%d" stp="off"/> <filter mac="on"/> </network> the "filter" element is evaluated at startup of libvirtd and a generic ebtables rules is generated (all frames are dropped): stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo ebtables -L Bridge table: filter ... Bridge chain: FORWARD, entries: 0, policy: DROP ... When starting up guest1, ... stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo virsh start build1 Domain build1 started ... an ebtables rule is generated to allow its mac address on the its interface: stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo ebtables -L ... Bridge chain: FORWARD, entries: 1, policy: DROP -s d0:f:d0:f:2:1 -i vnet0 -j ACCEPT ... the same happens when starting up the second guest: stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo virsh start build2 Domain build2 started stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo ebtables -L ... Bridge chain: FORWARD, entries: 2, policy: DROP -s d0:f:d0:f:2:2 -i vnet1 -j ACCEPT -s d0:f:d0:f:2:1 -i vnet0 -j ACCEPT ... so the two guests are allowed to communicate. After destroying the two guests, the corresponding ebtables rules are removed: stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo virsh destroy build2 Domain build2 destroyed stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo ebtables -L ... Bridge chain: FORWARD, entries: 1, policy: DROP -s d0:f:d0:f:2:1 -i vnet0 -j ACCEPT ... stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo virsh destroy build1 Domain build1 destroyed stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo ebtables -L ... Bridge chain: FORWARD, entries: 0, policy: DROP ... The current prototype implementation is based on the existing iptables wrapper in libvirt. I basically cloned the iptables wrapper to an ebtables wrapper and did some ebtables specific adjustments. There are currenlty four occasions when the ebtables wrapper is called: - when creating the network - when adding a guest to the network - when removing a guest from the network - when destroying the network (currently not implemented) These calls can be augmented to also do for example tagged vlan and protocol filtering. Configuring the filter rules via virsh is also an option. Comments are appreciated. -- Best regards, Gerhard Stenzel, ----------------------------------------------------------------------------------------------------------------------------------- IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Erich Baier Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

2 2

[libvirt] PATCH: Support setting a disk drive serial (number)
by Daniel P. Berrange 18 Aug '09

18 Aug '09

All hard disks (IDE, SCSI, USB, VirtIO) have a concept of a serial number which is intended to uniquely identify them. If you mean this off, QEMU just makes up a serial on the fly. THis is not guarenteed to be stable across guest reboots with hardware changes. It is thus desirable to be able to specify this in libvirt XML. To that end, this patch allows for a <serial> element inside a disk <disk type='file' device='disk'> <source file='/media/lacie-500-disk-2/virtual-machines/scratch5.img'/> <target dev='vda' bus='virtio'/> <serial>dan123virtio</serial> </disk> the contents is free-form. It implemenmts this for the QEMU driver for any disk using -drive style args. Unfortunately this excludes USB disk, even though internally QEMU can set a serial for these, the -usbdevice syntax does not allow it Daniel * docs/schemas/domain.rng: Add <serial> element to disks * src/domain_conf.h, src/domain_conf.c: XML parsing and formatting for disk serial numbers * src/qemu_conf.c: Set serial number when launching guests * tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args, tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml: Add serial number to XML test --- docs/schemas/domain.rng | 10 ++++++++++ src/domain_conf.c | 11 +++++++++++ src/domain_conf.h | 1 + src/qemu_conf.c | 2 ++ .../qemuxml2argv-disk-drive-shared.args | 2 +- .../qemuxml2argv-disk-drive-shared.xml | 1 + 6 files changed, 26 insertions(+), 1 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index f857301..16f35e4 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -336,6 +336,11 @@ <empty/> </element> </optional> + <optional> + <element name="serial"> + <ref name="diskSerial"/> + </element> + </optional> </define> <!-- A disk description can be either of type file or block @@ -1135,6 +1140,11 @@ <param name="pattern">[A-Za-z0-9_\.\+\-&:/]+</param> </data> </define> + <define name="diskSerial"> + <data type="string"> + <param name="pattern">[A-Za-z0-9_\.\+\-]+</param> + </data> + </define> <define name="genericName"> <data type="string"> <param name="pattern">[a-zA-Z0-9_\+\-]+</param> diff --git a/src/domain_conf.c b/src/domain_conf.c index 1d2cc7c..654f753 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -284,6 +284,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) if (!def) return; + VIR_FREE(def->serial); VIR_FREE(def->src); VIR_FREE(def->dst); VIR_FREE(def->driverName); @@ -661,6 +662,7 @@ virDomainDiskDefParseXML(virConnectPtr conn, char *bus = NULL; char *cachetag = NULL; char *devaddr = NULL; + char *serial = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(conn); @@ -718,6 +720,9 @@ virDomainDiskDefParseXML(virConnectPtr conn, } else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) && xmlStrEqual(cur->name, BAD_CAST "state")) { devaddr = virXMLPropString(cur, "devaddr"); + } else if ((serial == NULL) && + (xmlStrEqual(cur->name, BAD_CAST "serial"))) { + serial = (char *)xmlNodeGetContent(cur); } } cur = cur->next; @@ -836,6 +841,8 @@ virDomainDiskDefParseXML(virConnectPtr conn, driverName = NULL; def->driverType = driverType; driverType = NULL; + def->serial = serial; + serial = NULL; cleanup: VIR_FREE(bus); @@ -847,6 +854,7 @@ cleanup: VIR_FREE(driverName); VIR_FREE(cachetag); VIR_FREE(devaddr); + VIR_FREE(serial); return def; @@ -3519,6 +3527,9 @@ virDomainDiskDefFormat(virConnectPtr conn, virBufferAddLit(buf, " <readonly/>\n"); if (def->shared) virBufferAddLit(buf, " <shareable/>\n"); + if (def->serial) + virBufferEscapeString(buf, " <serial>%s</serial>\n", + def->serial); if (flags & VIR_DOMAIN_XML_INTERNAL_STATUS) { virBufferAddLit(buf, " <state"); diff --git a/src/domain_conf.h b/src/domain_conf.h index 44302be..7bad5e7 100644 --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -109,6 +109,7 @@ struct _virDomainDiskDef { char *dst; char *driverName; char *driverType; + char *serial; int cachemode; unsigned int readonly : 1; unsigned int shared : 1; diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 082f107..95d41e6 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1748,6 +1748,8 @@ int qemudBuildCommandLine(virConnectPtr conn, if (disk->driverType && qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_FORMAT) virBufferVSprintf(&opt, ",format=%s", disk->driverType); + if (disk->serial) + virBufferVSprintf(&opt, ",serial=%s", disk->serial); if (disk->cachemode) { const char *mode = diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args index 611cd33..07cbe0e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.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 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,index=0,format=qcow2,cache=off -drive file=/dev/HostVG/QEMUGuest2,if=ide,media=cdrom,index=2,format=raw -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 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,index=0,format=qcow2,serial=XYZXYZXYZYXXYZYZYXYZY,cache=off -drive file=/dev/HostVG/QEMUGuest2,if=ide,media=cdrom,index=2,format=raw -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml index b386315..c4e4734 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml @@ -19,6 +19,7 @@ <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> <shareable/> + <serial>XYZXYZXYZYXXYZYZYXYZY</serial> </disk> <disk type='block' device='cdrom'> <driver name='qemu' type='raw'/> -- 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 :|

4 7

[libvirt] [PATCH] Don't expose 'vnet%d' to the user
by Mark McLoughlin 18 Aug '09

18 Aug '09

https://bugzilla.redhat.com/517371 Matt Booth points out that if you use a non-existent bridge name when start a guest you get a weird error message: Failed to add tap interface 'vnet%d' to bridge 'virbr0' and dev='vnet%d' appears in the dumpxml output. Fix that by not including 'vnet%d' in the error message and freeing the 'vnet%d' string if adding the tap device to the bridge fails. * src/qemu_conf.c, src/uml_conf.c: fix qemudNetworkIfaceConnect() and umlConnectTapDevice() to not expose 'vnet%d' to the user --- src/qemu_conf.c | 25 +++++++++++++++++-------- src/uml_conf.c | 28 +++++++++++++++++++--------- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 082f107..bcdab11 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1038,6 +1038,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, int err; int tapfd = -1; int vnet_hdr = 0; + int template_ifname = 0; if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { virNetworkPtr network = virNetworkLookupByName(conn, @@ -1059,6 +1060,14 @@ qemudNetworkIfaceConnect(virConnectPtr conn, return -1; } + char ebuf[1024]; + if (!driver->brctl && (err = brInit(&driver->brctl))) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("cannot initialize bridge support: %s"), + virStrerror(err, ebuf, sizeof ebuf)); + return -1; + } + if (!net->ifname || STRPREFIX(net->ifname, "vnet") || strchr(net->ifname, '%')) { @@ -1067,14 +1076,8 @@ qemudNetworkIfaceConnect(virConnectPtr conn, virReportOOMError(conn); return -1; } - } - - char ebuf[1024]; - if (!driver->brctl && (err = brInit(&driver->brctl))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("cannot initialize bridge support: %s"), - virStrerror(err, ebuf, sizeof ebuf)); - return -1; + /* avoid exposing vnet%d in dumpxml or error outputs */ + template_ifname = 1; } if (qemuCmdFlags & QEMUD_CMD_FLAG_VNET_HDR && @@ -1088,12 +1091,18 @@ qemudNetworkIfaceConnect(virConnectPtr conn, qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("Failed to add tap interface to bridge. " "%s is not a bridge device"), brname); + } else if (template_ifname) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Failed to add tap interface to bridge '%s' : %s"), + brname, virStrerror(err, ebuf, sizeof ebuf)); } else { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("Failed to add tap interface '%s' " "to bridge '%s' : %s"), net->ifname, brname, virStrerror(err, ebuf, sizeof ebuf)); } + if (template_ifname) + VIR_FREE(net->ifname); return -1; } diff --git a/src/uml_conf.c b/src/uml_conf.c index 4f756d4..a4c434f 100644 --- a/src/uml_conf.c +++ b/src/uml_conf.c @@ -104,17 +104,10 @@ umlConnectTapDevice(virConnectPtr conn, virDomainNetDefPtr net, const char *bridge) { + brControl *brctl = NULL; int tapfd = -1; + int template_ifname = 0; int err; - brControl *brctl = NULL; - - if (!net->ifname || - STRPREFIX(net->ifname, "vnet") || - strchr(net->ifname, '%')) { - VIR_FREE(net->ifname); - if (!(net->ifname = strdup("vnet%d"))) - goto no_memory; - } if ((err = brInit(&brctl))) { char ebuf[1024]; @@ -124,6 +117,16 @@ umlConnectTapDevice(virConnectPtr conn, goto error; } + if (!net->ifname || + STRPREFIX(net->ifname, "vnet") || + strchr(net->ifname, '%')) { + VIR_FREE(net->ifname); + if (!(net->ifname = strdup("vnet%d"))) + goto no_memory; + /* avoid exposing vnet%d in dumpxml or error outputs */ + template_ifname = 1; + } + if ((err = brAddTap(brctl, bridge, &net->ifname, BR_TAP_PERSIST, &tapfd))) { if (errno == ENOTSUP) { @@ -131,6 +134,11 @@ umlConnectTapDevice(virConnectPtr conn, umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("Failed to add tap interface to bridge. " "%s is not a bridge device"), bridge); + } else if (template_ifname) { + char ebuf[1024]; + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Failed to add tap interface to bridge '%s' : %s"), + bridge, virStrerror(err, ebuf, sizeof ebuf)); } else { char ebuf[1024]; umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, @@ -138,6 +146,8 @@ umlConnectTapDevice(virConnectPtr conn, "to bridge '%s' : %s"), net->ifname, bridge, virStrerror(err, ebuf, sizeof ebuf)); } + if (template_ifname) + VIR_FREE(net->ifname); goto error; } close(tapfd); -- 1.6.2.5

3 5

[libvirt] [PATCH 0/8] Various KVM PCI device assignment improvements
by Mark McLoughlin 18 Aug '09

18 Aug '09

Hi, Here's a fairly mixed set of KVM PCI device assignment improvements: - Add hotplug support - Allow PM reset on multi-function devices - Allow Secondary Bus Reset even if it causes other devices to be reset, so long as those other devices are in use by the same VM or are unused - Re-attach devices when the guest shuts down - Properly detect versions of QEMU without -pcidevice support Cheers, Mark.

4 26