- Devel - libvirt List Archives

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

07 Sep '09

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

5 18

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

05 Sep '09

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

4 7

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

04 Sep '09

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

2 3

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

04 Sep '09

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

2 3

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

04 Sep '09

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

2 3

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

04 Sep '09

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

2 3

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

04 Sep '09

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

2 1

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

04 Sep '09

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

2 1

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

04 Sep '09

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

2 2

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

04 Sep '09

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

2 3