- Devel - libvirt List Archives

[libvirt] [RFC PATCH] Add proxy FS (9p) support to libvirt
by M. Mohan Kumar 18 Jan '12

18 Jan '12

From: "M. Mohan Kumar" <mohan(a)in.ibm.com> This patch *) adds new sub element <virtfs-proxy-helper> under <device> element. This sub-element is used to specify the virtfs-proxy-helper binary (part of QEMU) to be executed when proxy FS driver is used. *) invokes proxy_helper binary specified by the <virtfs-proxy-helper> sub-element with passing one of the created socket pair descriptor and adds "-sock_fd" paramter to qemu. Is it okay to add the sub-element "virtfs-proxy-helper" under "devices" element? Proxy helper binary is common for all 9p proxy FS devices, so it can not be placed under "filesystems" element. About Proxy FS driver ====================== Proxy FS driver is added to qemu 9p server to overcome two issues with pass-through security model (needs root privilege) security model. 1) TOCTTOU vulnerability: Following symbolic links in the server could provide access to files beyond 9p export path. 2) Running QEMU with root privilege could be a security issue. Proxy FS uses chroot + socket combination for securing the vulnerability known with following symbolic links. Intention of adding a new filesystem type is to allow qemu to run in non-root mode, but doing privileged operations using socket IO. Proxy helper(a stand alone binary part of qemu) is invoked with root privileges. Proxy helper chroots into 9p export path and creates a socket pair or a named socket based on the command line parameter. Qemu and proxy helper communicate using this socket. QEMU proxy fs driver sends filesystem request to proxy helper and receives the response from it. Proxy helper is designed such a way that it needs only few capabilities related to filesystem operations (such as CAP_DAC_OVERRIDE, CAP_FOWNER, etc) and all other capabilities are dropped (CAP_SYS_CHROOT, etc) Proxy patches http://permalink.gmane.org/gmane.comp.emulators.qemu/128735 Signed-off-by: M. Mohan Kumar <mohan(a)in.ibm.com> --- docs/formatdomain.html.in | 3 +- src/conf/domain_conf.c | 7 ++++- src/conf/domain_conf.h | 4 +- src/qemu/qemu_capabilities.c | 3 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 64 ++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_command.h | 3 +- tests/qemuhelptest.c | 3 +- 8 files changed, 79 insertions(+), 9 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 18b7e22..e398779 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1336,7 +1336,8 @@ This mode also has an optional sub-element <code>driver</code>, with an attribute <code>type='path'</code> - or <code>type='handle'</code> <span class="since">(since + or <code>type='handle'</code> + or <code>type='proxy'</code> <span class="since">(since 0.9.7)</span>. </dd> <dt><code>type='template'</code></dt> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0190a81..694c166 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -250,7 +250,8 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, VIR_ENUM_IMPL(virDomainFSDriverType, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, "default", "path", - "handle") + "handle", + "proxy") VIR_ENUM_IMPL(virDomainFSAccessMode, VIR_DOMAIN_FS_ACCESSMODE_LAST, "passthrough", @@ -1465,6 +1466,7 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def->name); VIR_FREE(def->cpumask); VIR_FREE(def->emulator); + VIR_FREE(def->virtfs_proxy_helper); VIR_FREE(def->description); virBlkioDeviceWeightArrayClear(def->blkio.devices, @@ -7490,6 +7492,9 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto error; } + def->virtfs_proxy_helper = + virXPathString("string(./devices/virtfs-proxy-helper[1])", ctxt); + /* analysis of the disk devices */ if ((n = virXPathNodeSet("./devices/disk", ctxt, &nodes)) < 0) { goto error; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 03aa5b6..a7d248a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -450,7 +450,6 @@ struct _virDomainControllerDef { virDomainDeviceInfo info; }; - /* Two types of disk backends */ enum virDomainFSType { VIR_DOMAIN_FS_TYPE_MOUNT, /* Better named 'bind' */ @@ -466,6 +465,7 @@ enum virDomainFSDriverType { VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT = 0, VIR_DOMAIN_FS_DRIVER_TYPE_PATH, VIR_DOMAIN_FS_DRIVER_TYPE_HANDLE, + VIR_DOMAIN_FS_DRIVER_TYPE_PROXY, VIR_DOMAIN_FS_DRIVER_TYPE_LAST }; @@ -491,7 +491,6 @@ struct _virDomainFSDef { virDomainDeviceInfo info; }; - /* 5 different types of networking config */ enum virDomainNetType { VIR_DOMAIN_NET_TYPE_USER, @@ -1503,6 +1502,7 @@ struct _virDomainDef { void *namespaceData; virDomainXMLNamespace ns; + char *virtfs_proxy_helper; }; enum virDomainTaintFlags { diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 43c7578..9c01a3b 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -144,6 +144,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "ich9-ahci", "no-acpi", "fsdev-readonly", + "fsdev-proxy", ); struct qemu_feature_flags { @@ -1083,6 +1084,8 @@ qemuCapsComputeCmdFlags(const char *help, qemuCapsSet(flags, QEMU_CAPS_FSDEV); if (strstr(fsdev, "readonly")) qemuCapsSet(flags, QEMU_CAPS_FSDEV_READONLY); + if (strstr(fsdev, "sock_fd")) + qemuCapsSet(flags, QEMU_CAPS_FSDEV_PROXY); } if (strstr(help, "-smbios type")) qemuCapsSet(flags, QEMU_CAPS_SMBIOS_TYPE); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c759baf..e129e35 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -117,6 +117,7 @@ enum qemuCapsFlags { QEMU_CAPS_ICH9_AHCI = 77, /* -device ich9-ahci */ QEMU_CAPS_NO_ACPI = 78, /* -no-acpi */ QEMU_CAPS_FSDEV_READONLY =79, /* -fsdev readonly supported */ + QEMU_CAPS_FSDEV_PROXY = 80, /* -fsdev proxy supported */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f2e9cfa..9ac5a4c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -106,7 +106,8 @@ VIR_ENUM_DECL(qemuDomainFSDriver) VIR_ENUM_IMPL(qemuDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, "local", "local", - "handle"); + "handle", + "proxy"); static void @@ -2080,9 +2081,32 @@ error: return NULL; } +/* + * Invokes the Proxy Helper with one of the socketpair as its parameter + * + */ +static int qemuInvokeProxyHelper(const char *binary, int sock, const char *path) +{ + int ret_val, status; + virCommandPtr cmd; + + cmd = virCommandNewArgList(binary, NULL); + virCommandAddArg(cmd, "-f"); + virCommandAddArgFormat(cmd, "%d", sock); + virCommandAddArg(cmd, "-p"); + virCommandAddArgFormat(cmd, "%s", path); + virCommandTransferFD(cmd, sock); + virCommandDaemonize(cmd); + ret_val = virCommandRun(cmd, &status); + if (ret_val < 0) + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("%s can't execute"), binary); + virCommandFree(cmd); + return ret_val; +} char *qemuBuildFSStr(virDomainFSDefPtr fs, - virBitmapPtr qemuCaps ATTRIBUTE_UNUSED) + virBitmapPtr qemuCaps ATTRIBUTE_UNUSED, int qemuSocket) { virBuffer opt = VIR_BUFFER_INITIALIZER; const char *driver = qemuDomainFSDriverTypeToString(fs->fsdriver); @@ -2108,6 +2132,10 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, virBufferAddLit(&opt, ",security_model=none"); } virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); + + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PROXY) + virBufferAsprintf(&opt, ",sock_fd=%d", qemuSocket); + virBufferAsprintf(&opt, ",path=%s", fs->src); if (fs->readonly) { @@ -4426,10 +4454,40 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuCapsGet(qemuCaps, QEMU_CAPS_FSDEV)) { for (i = 0 ; i < def->nfss ; i++) { char *optstr; + int sockets[2] = {-1, -1}; virDomainFSDefPtr fs = def->fss[i]; + /* + * If its a proxy FS, we need to create a socket pair + * and invoke proxy_helper + */ + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PROXY) { + if (qemuCapsGet(qemuCaps, QEMU_CAPS_FSDEV_PROXY) < 0) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("proxy helper not supported")); + goto error; + } + if (!def->virtfs_proxy_helper) { + qemuReportError(VIR_ERR_XML_ERROR, "%s", + _("proxy helper not specified")); + goto error; + } + /* create a socket pair */ + if (socketpair(PF_UNIX, SOCK_STREAM, 0, sockets) < 0) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("socketpair failed")); + goto error; + } + virCommandTransferFD(cmd, sockets[1]); + if (qemuInvokeProxyHelper(def->virtfs_proxy_helper, sockets[0], + fs->src) < 0) { + VIR_FORCE_CLOSE(sockets[0]); + VIR_FORCE_CLOSE(sockets[1]); + goto error; + } + } virCommandAddArg(cmd, "-fsdev"); - if (!(optstr = qemuBuildFSStr(fs, qemuCaps))) + if (!(optstr = qemuBuildFSStr(fs, qemuCaps, sockets[1]))) goto error; virCommandAddArg(cmd, optstr); VIR_FREE(optstr); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index de61cf3..8e4f335 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -87,7 +87,8 @@ char *qemuBuildDriveStr(virConnectPtr conn, bool bootable, virBitmapPtr qemuCaps); char *qemuBuildFSStr(virDomainFSDefPtr fs, - virBitmapPtr qemuCaps); + virBitmapPtr qemuCaps, + int qemuSocket); /* Current, best practice */ char * qemuBuildDriveDevStr(virDomainDiskDefPtr disk, diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 60155e7..455a7e1 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -648,7 +648,8 @@ mymain(void) QEMU_CAPS_PCI_ROMBAR, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_NO_ACPI, - QEMU_CAPS_FSDEV_READONLY); + QEMU_CAPS_FSDEV_READONLY, + QEMU_CAPS_FSDEV_PROXY); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.7.6

2 3

[libvirt] [PATCH v2] qemu: Introduce inactive PCI device list
by Osier Yang 18 Jan '12

18 Jan '12

From: root <root(a)hp-dl360g7-01.lab.eng.brq.redhat.com> pciTrySecondaryBusReset checks if there is active device on the same bus, however, qemu driver doesn't maintain an effective list for the inactive devices, and it passes meaningless argment for parameter "inactiveDevs". e.g. (qemuPrepareHostdevPCIDevices) if (!(pcidevs = qemuGetPciHostDeviceList(hostdevs, nhostdevs))) return -1; ...skipped... if (pciResetDevice(dev, driver->activePciHostdevs, pcidevs) < 0) goto reattachdevs; NB, the "pcidevs" used above are extracted from domain def, and thus one won't be able to attach a device of which bus has other device even detached from host (nodedev-detach). To see more details of the problem: RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=773667 This patch is to resolve the problem by introduce an inactive PCI device list (just like qemu_driver->activePciHostdevs), and the whole logic is: * Add the device to inactive list when do nodedev-dettach * Remove the device from inactive list when do nodedev-reattach * Remove the device from inactive list when do attach-device (for not managed device) * Add the device to inactive list after detach-device, only if the device is not managed With above, we have sufficient inactive PCI device list, and thus we can use it for pciResetDevice. e.g.(qemuPrepareHostdevPCIDevices) if (pciResetDevice(dev, driver->activePciHostdevs, driver->inactivePciHostdevs) < 0) goto reattachdevs; v1 ~ v2 Fixed a potential crash. --- Got a testing box from Miroslav and tested the patch, it works well. --- src/qemu/qemu_conf.h | 5 +++++ src/qemu/qemu_driver.c | 19 +++++++++++++++---- src/qemu/qemu_hostdev.c | 32 +++++++++++++++++++++++--------- src/util/pci.c | 28 ++++++++++++++++++++++++---- src/util/pci.h | 8 ++++++-- src/xen/xen_driver.c | 4 ++-- 7 files changed, 77 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index d8d7915..3f1b668 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -128,6 +128,11 @@ struct qemud_driver { pciDeviceList *activePciHostdevs; usbDeviceList *activeUsbHostdevs; + /* The device which is not used by both host + * and any guest. + */ + pciDeviceList *inactivePciHostdevs; + virBitmapPtr reservedVNCPorts; virSysinfoDefPtr hostsysinfo; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 712f1fc..47e2380 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -588,6 +588,9 @@ qemudStartup(int privileged) { if ((qemu_driver->activeUsbHostdevs = usbDeviceListNew()) == NULL) goto error; + if ((qemu_driver->inactivePciHostdevs = pciDeviceListNew()) == NULL) + goto error; + if (privileged) { if (chown(qemu_driver->libDir, qemu_driver->user, qemu_driver->group) < 0) { virReportSystemError(errno, @@ -778,6 +781,7 @@ qemudShutdown(void) { qemuDriverLock(qemu_driver); pciDeviceListFree(qemu_driver->activePciHostdevs); + pciDeviceListFree(qemu_driver->inactivePciHostdevs); usbDeviceListFree(qemu_driver->activeUsbHostdevs); virCapabilitiesFree(qemu_driver->caps); @@ -9211,6 +9215,7 @@ qemudNodeDeviceDettach (virNodeDevicePtr dev) pciDevice *pci; unsigned domain, bus, slot, function; int ret = -1; + bool in_inactive_list = false; if (qemudNodeDeviceGetPciInfo(dev, &domain, &bus, &slot, &function) < 0) return -1; @@ -9220,13 +9225,17 @@ qemudNodeDeviceDettach (virNodeDevicePtr dev) return -1; qemuDriverLock(driver); - if (pciDettachDevice(pci, driver->activePciHostdevs) < 0) + in_inactive_list = pciDeviceListFind(driver->inactivePciHostdevs, pci); + + if (pciDettachDevice(pci, driver->activePciHostdevs, + driver->inactivePciHostdevs) < 0) goto out; ret = 0; out: qemuDriverUnlock(driver); - pciFreeDevice(pci); + if (in_inactive_list) + pciFreeDevice(pci); return ret; } @@ -9248,7 +9257,8 @@ qemudNodeDeviceReAttach (virNodeDevicePtr dev) pciDeviceReAttachInit(pci); qemuDriverLock(driver); - if (pciReAttachDevice(pci, driver->activePciHostdevs) < 0) + if (pciReAttachDevice(pci, driver->activePciHostdevs, + driver->inactivePciHostdevs) < 0) goto out; ret = 0; @@ -9275,7 +9285,8 @@ qemudNodeDeviceReset (virNodeDevicePtr dev) qemuDriverLock(driver); - if (pciResetDevice(pci, driver->activePciHostdevs, NULL) < 0) + if (pciResetDevice(pci, driver->activePciHostdevs, + driver->inactivePciHostdevs) < 0) goto out; ret = 0; diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index c7adb1d..6141cfe 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -212,7 +212,7 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, for (i = 0; i < pciDeviceListCount(pcidevs); i++) { pciDevice *dev = pciDeviceListGet(pcidevs, i); if (pciDeviceGetManaged(dev) && - pciDettachDevice(dev, driver->activePciHostdevs) < 0) + pciDettachDevice(dev, driver->activePciHostdevs, NULL) < 0) goto reattachdevs; } @@ -220,7 +220,8 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, * can safely reset them */ for (i = 0; i < pciDeviceListCount(pcidevs); i++) { pciDevice *dev = pciDeviceListGet(pcidevs, i); - if (pciResetDevice(dev, driver->activePciHostdevs, pcidevs) < 0) + if (pciResetDevice(dev, driver->activePciHostdevs, + driver->inactivePciHostdevs) < 0) goto reattachdevs; } @@ -233,7 +234,13 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, } } - /* Loop 5: Now set the used_by_domain of the device in + /* Loop 5: Now remove the devices from inactive list. */ + for (i = 0; i < pciDeviceListCount(pcidevs); i++) { + pciDevice *dev = pciDeviceListGet(pcidevs, i); + pciDeviceListDel(driver->inactivePciHostdevs, dev); + } + + /* Loop 6: Now set the used_by_domain of the device in * driver->activePciHostdevs as domain name. */ for (i = 0; i < pciDeviceListCount(pcidevs); i++) { @@ -245,7 +252,7 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, pciDeviceSetUsedBy(activeDev, name); } - /* Loop 6: Now set the original states for hostdev def */ + /* Loop 7: Now set the original states for hostdev def */ for (i = 0; i < nhostdevs; i++) { pciDevice *dev; pciDevice *pcidev; @@ -277,7 +284,7 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, pciFreeDevice(dev); } - /* Loop 7: Now steal all the devices from pcidevs */ + /* Loop 8: Now steal all the devices from pcidevs */ while (pciDeviceListCount(pcidevs) > 0) { pciDevice *dev = pciDeviceListGet(pcidevs, 0); pciDeviceListSteal(pcidevs, dev); @@ -298,7 +305,7 @@ inactivedevs: reattachdevs: for (i = 0; i < pciDeviceListCount(pcidevs); i++) { pciDevice *dev = pciDeviceListGet(pcidevs, i); - pciReAttachDevice(dev, driver->activePciHostdevs); + pciReAttachDevice(dev, driver->activePciHostdevs, NULL); } cleanup: @@ -445,8 +452,13 @@ void qemuReattachPciDevice(pciDevice *dev, struct qemud_driver *driver) { int retries = 100; - if (!pciDeviceGetManaged(dev)) + /* If the device is not managed and was attached to guest + * successfully, it must had be inactive. + */ + if (!pciDeviceGetManaged(dev)) { + pciDeviceListAdd(driver->inactivePciHostdevs, dev); return; + } while (pciWaitForDeviceCleanup(dev, "kvm_assigned_device") && retries) { @@ -454,7 +466,8 @@ void qemuReattachPciDevice(pciDevice *dev, struct qemud_driver *driver) retries--; } - if (pciReAttachDevice(dev, driver->activePciHostdevs) < 0) { + if (pciReAttachDevice(dev, driver->activePciHostdevs, + driver->inactivePciHostdevs) < 0) { virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to re-attach PCI device: %s"), err ? err->message : _("unknown error")); @@ -503,7 +516,8 @@ void qemuDomainReAttachHostdevDevices(struct qemud_driver *driver, for (i = 0; i < pciDeviceListCount(pcidevs); i++) { pciDevice *dev = pciDeviceListGet(pcidevs, i); - if (pciResetDevice(dev, driver->activePciHostdevs, pcidevs) < 0) { + if (pciResetDevice(dev, driver->activePciHostdevs, + driver->inactivePciHostdevs) < 0) { virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to reset PCI device: %s"), err ? err->message : _("unknown error")); iff --git a/src/util/pci.c b/src/util/pci.c index 17cde81..633dcd8 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1117,7 +1117,9 @@ cleanup: } int -pciDettachDevice(pciDevice *dev, pciDeviceList *activeDevs) +pciDettachDevice(pciDevice *dev, + pciDeviceList *activeDevs, + pciDeviceList *inactiveDevs) { const char *driver = pciFindStubDriver(); if (!driver) { @@ -1132,11 +1134,22 @@ pciDettachDevice(pciDevice *dev, pciDeviceList *activeDevs) return -1; } - return pciBindDeviceToStub(dev, driver); + if (pciBindDeviceToStub(dev, driver) < 0) + return -1; + + /* Add the dev into list inactiveDevs */ + if (inactiveDevs && !pciDeviceListFind(inactiveDevs, dev)) { + if (pciDeviceListAdd(inactiveDevs, dev) < 0) + return -1; + } + + return 0; } int -pciReAttachDevice(pciDevice *dev, pciDeviceList *activeDevs) +pciReAttachDevice(pciDevice *dev, + pciDeviceList *activeDevs, + pciDeviceList *inactiveDevs) { const char *driver = pciFindStubDriver(); if (!driver) { @@ -1151,7 +1164,14 @@ pciReAttachDevice(pciDevice *dev, pciDeviceList *activeDevs) return -1; } - return pciUnbindDeviceFromStub(dev, driver); + if (pciUnbindDeviceFromStub(dev, driver) < 0) + return -1; + + /* Steal the dev from list inactiveDevs */ + if (inactiveDevs) + pciDeviceListSteal(inactiveDevs, dev); + + return 0; } /* Certain hypervisors (like qemu/kvm) map the PCI bar(s) on diff --git a/src/util/pci.h b/src/util/pci.h index 5493e0a..25b5b66 100644 --- a/src/util/pci.h +++ b/src/util/pci.h @@ -40,8 +40,12 @@ pciDevice *pciGetDevice (unsigned domain, unsigned function); void pciFreeDevice (pciDevice *dev); const char *pciDeviceGetName (pciDevice *dev); -int pciDettachDevice (pciDevice *dev, pciDeviceList *activeDevs); -int pciReAttachDevice (pciDevice *dev, pciDeviceList *activeDevs); +int pciDettachDevice (pciDevice *dev, + pciDeviceList *activeDevs, + pciDeviceList *inactiveDevs); +int pciReAttachDevice (pciDevice *dev, + pciDeviceList *activeDevs, + pciDeviceList *inactiveDevs); int pciResetDevice (pciDevice *dev, pciDeviceList *activeDevs, pciDeviceList *inactiveDevs); diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 20671c0..520ec03 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1985,7 +1985,7 @@ xenUnifiedNodeDeviceDettach (virNodeDevicePtr dev) if (!pci) return -1; - if (pciDettachDevice(pci, NULL) < 0) + if (pciDettachDevice(pci, NULL, NULL) < 0) goto out; ret = 0; @@ -2075,7 +2075,7 @@ xenUnifiedNodeDeviceReAttach (virNodeDevicePtr dev) goto out; } - if (pciReAttachDevice(pci, NULL) < 0) + if (pciReAttachDevice(pci, NULL, NULL) < 0) goto out; ret = 0; -- 1.7.1

2 3

[libvirt] [PATCH] remote: avoid memory leak on successful path
by ajia＠redhat.com 18 Jan '12

18 Jan '12

From: Alex Jia <ajia(a)redhat.com> when everything is okay, 'rv' will be 0, however, 'cleanup' lable only free allocated memory with 'rv < 0', so memory leak on successful path. The patch uses new virTypedParameterArrayClear function to free memory instead. * src/remote/remote_driver.c: fix memory leak on remoteDeserializeTypedParameters. * Detected by valgrind: ==31957== 1 bytes in 1 blocks are definitely lost in loss record 1 of 2,097 ==31957== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==31957== by 0x39CF07F6E1: strdup (in /lib64/libc-2.12.so) ==31957== by 0xD84CDA1: remoteDeserializeTypedParameters.clone.0 (remote_driver.c:1404) ==31957== by 0xD84D11B: remoteDomainGetNumaParameters (remote_driver.c:1566) ==31957== by 0xD81EBFA: virDomainGetNumaParameters (libvirt.c:3887) ==31957== by 0xD4F8E25: libvirt_virDomainGetNumaParameters (libvirt-override.c:1127) ==31957== by 0x39E1ADE7F3: PyEval_EvalFrameEx (ceval.c:3794) ==31957== by 0x39E1ADF99E: PyEval_EvalFrameEx (ceval.c:3880) ==31957== by 0x39E1AE0466: PyEval_EvalCodeEx (ceval.c:3044) ==31957== by 0x39E1AE0541: PyEval_EvalCode (ceval.c:545) ==31957== by 0x39E1AFB88B: run_mod (pythonrun.c:1351) ==31957== by 0x39E1AFB95F: PyRun_FileExFlags (pythonrun.c:1337) Signed-off-by: Alex Jia <ajia(a)redhat.com> --- src/remote/remote_driver.c | 7 +------ 1 files changed, 1 insertions(+), 6 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e28840b..cfbcd0f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1417,12 +1417,7 @@ remoteDeserializeTypedParameters(remote_typed_param *ret_params_val, rv = 0; cleanup: - if (rv < 0) { - int j; - for (j = 0; j < i; j++) - if (params[j].type == VIR_TYPED_PARAM_STRING) - VIR_FREE(params[j].value.s); - } + virTypedParameterArrayClear(params, *nparams); return rv; } -- 1.7.1

3 2

[libvirt] [PATCH] qemu: Prohibit reattaching node device if it is in use
by Osier Yang 18 Jan '12

18 Jan '12

It doesn't make sense to reattach a device to host while it's still in use, e.g, by a domain. --- src/qemu/qemu_driver.c | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 47e2380..c920bfd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9244,6 +9244,7 @@ qemudNodeDeviceReAttach (virNodeDevicePtr dev) { struct qemud_driver *driver = dev->conn->privateData; pciDevice *pci; + pciDevice *other; unsigned domain, bus, slot, function; int ret = -1; @@ -9254,6 +9255,20 @@ qemudNodeDeviceReAttach (virNodeDevicePtr dev) if (!pci) return -1; + other = pciDeviceListFind(driver->activePciHostdevs, pci); + if (other) { + const char *other_name = pciDeviceGetUsedBy(other); + + if (other_name) + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("PCI device %s is still in use by domain %s"), + pciDeviceGetName(pci), other_name); + else + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("PCI device %s is still in use"), + pciDeviceGetName(pci)); + } + pciDeviceReAttachInit(pci); qemuDriverLock(driver); -- 1.7.7.3

2 1

[libvirt] [PATCH] Fix startup of LXC containers with filesystems containing symlinks
by Daniel P. Berrange 17 Jan '12

17 Jan '12

From: "Daniel P. Berrange" <berrange(a)redhat.com> Given an LXC guest with a root filesystem path of /export/lxc/roots/helloworld/root During startup, we will pivot the root filesystem to end up at /.oldroot/export/lxc/roots/helloworld/root We then try to open /.oldroot/export/lxc/roots/helloworld/root/dev/pts Now consider if '/export/lxc' is an absolute symlink pointing to '/media/lxc'. The kernel will try to open /media/lxc/roots/helloworld/root/dev/pts whereas it should be trying to open /.oldroot//media/lxc/roots/helloworld/root/dev/pts To deal with the fact that the root filesystem can be moved, we need to resolve symlinks in *any* part of the filesystem source path. * src/libvirt_private.syms, src/util/util.c, src/util/util.h: Add virFileResolveAllLinks to resolve all symlinks in a path * src/lxc/lxc_container.c: Resolve all symlinks in filesystem paths during startup --- src/libvirt_private.syms | 1 + src/lxc/lxc_container.c | 23 +++++++++++++++++++++++ src/util/util.c | 43 ++++++++++++++++++++++++++++++++----------- src/util/util.h | 2 ++ 4 files changed, 58 insertions(+), 11 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index faab0e2..f344e73 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1100,6 +1100,7 @@ virFileOpenTty; virFileReadAll; virFileReadLimFD; virFileResolveLink; +virFileResolveAllLinks; virFileSanitizePath; virFileStripSuffix; virFileUnlock; diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 1e7803a..4e4388b 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1111,11 +1111,34 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef) return 0; } + +static int lxcContainerResolveSymlinks(virDomainDefPtr vmDef) +{ + char *newroot; + size_t i; + + for (i = 0 ; i < vmDef->nfss ; i++) { + virDomainFSDefPtr fs = vmDef->fss[i]; + if (virFileResolveAllLinks(fs->src, &newroot) < 0) + return -1; + + VIR_DEBUG("Resolved '%s' to %s", fs->src, newroot); + + VIR_FREE(fs->src); + fs->src = newroot; + } + + return 0; +} + static int lxcContainerSetupMounts(virDomainDefPtr vmDef, virDomainFSDefPtr root, char **ttyPaths, size_t nttyPaths) { + if (lxcContainerResolveSymlinks(vmDef) < 0) + return -1; + if (root) return lxcContainerSetupPivotRoot(vmDef, root, ttyPaths, nttyPaths); else diff --git a/src/util/util.c b/src/util/util.c index 6f46d53..8663c4d 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -536,16 +536,10 @@ int virFileLinkPointsTo(const char *checkLink, -/* - * Attempt to resolve a symbolic link, returning an - * absolute path where only the last component is guaranteed - * not to be a symlink. - * - * Return 0 if path was not a symbolic, or the link was - * resolved. Return -1 with errno set upon error - */ -int virFileResolveLink(const char *linkpath, - char **resultpath) +static int +virFileResolveLinkHelper(const char *linkpath, + bool intermediatePaths, + char **resultpath) { struct stat st; @@ -554,7 +548,7 @@ int virFileResolveLink(const char *linkpath, /* We don't need the full canonicalization of intermediate * directories, if linkpath is absolute and the basename is * already a non-symlink. */ - if (IS_ABSOLUTE_FILE_NAME(linkpath)) { + if (IS_ABSOLUTE_FILE_NAME(linkpath) && !intermediatePaths) { if (lstat(linkpath, &st) < 0) return -1; @@ -570,6 +564,33 @@ int virFileResolveLink(const char *linkpath, return *resultpath == NULL ? -1 : 0; } +/* + * Attempt to resolve a symbolic link, returning an + * absolute path where only the last component is guaranteed + * not to be a symlink. + * + * Return 0 if path was not a symbolic, or the link was + * resolved. Return -1 with errno set upon error + */ +int virFileResolveLink(const char *linkpath, + char **resultpath) +{ + return virFileResolveLinkHelper(linkpath, false, resultpath); +} + +/* + * Attempt to resolve a symbolic link, returning an + * absolute path where every component is guaranteed + * not to be a symlink. + * + * Return 0 if path was not a symbolic, or the link was + * resolved. Return -1 with errno set upon error + */ +int virFileResolveAllLinks(const char *linkpath, + char **resultpath) +{ + return virFileResolveLinkHelper(linkpath, true, resultpath); +} /* * Check whether the given file is a link. diff --git a/src/util/util.h b/src/util/util.h index c9c785b..977ab6c 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -77,6 +77,8 @@ int virFileLinkPointsTo(const char *checkLink, int virFileResolveLink(const char *linkpath, char **resultpath) ATTRIBUTE_RETURN_CHECK; +int virFileResolveAllLinks(const char *linkpath, + char **resultpath) ATTRIBUTE_RETURN_CHECK; int virFileIsLink(const char *linkpath) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; -- 1.7.7.5

2 1

[libvirt] [PATCH] Permission change for systemtap examples.
by Martin Kletzander 17 Jan '12

17 Jan '12

Execute bit on *.stp files in examples/systemtap/ caused dependency when building RPM packages. Disabling execute permission should help the auto dependency resolver to see that systemtap is not needed. --- 0 files changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 examples/systemtap/events.stp mode change 100755 => 100644 examples/systemtap/rpc-monitor.stp diff --git a/examples/systemtap/events.stp b/examples/systemtap/events.stp old mode 100755 new mode 100644 diff --git a/examples/systemtap/rpc-monitor.stp b/examples/systemtap/rpc-monitor.stp old mode 100755 new mode 100644 -- 1.7.3.4

3 2

[libvirt] [PATCH] qemu: Don't break domain with 0:0:2.0 assigned to anything but VGA
by Jiri Denemark 17 Jan '12

17 Jan '12

In the past we didn't reserve 0:0:2.0 PCI address if there was no video device assigned to a domain, which made it impossible to add a video device later on. So we fixed it (commit v0.9.0-37-g7b2cac1) by always reserving that address. However, that breaks existing domains without video devices that already have another device assigned to the problematic address. This patch reserves address 0:0:2.0 only in case it was not explicitly assigned to another device, which means libvirt will try to keep this address free and will not automatically assign it new devices. But existing domains for which older libvirt already assigned the address to a non-video device will keep working as they used to work before 0.9.1. Moreover, users who want to create a domain without a video device and use its address for another device may do so by explicitly configuring the PCI address in domain XML. --- src/qemu/qemu_command.c | 21 +++++++++++++++------ 1 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b45f65f..eccbe01 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1206,7 +1206,6 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) int i; bool reservedIDE = false; bool reservedUSB = false; - bool reservedVGA = false; int function; /* Host bridge */ @@ -1287,20 +1286,30 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) } /* If TYPE==PCI, then qemuCollectPCIAddress() function * has already reserved the address, so we must skip */ - reservedVGA = true; } else { def->videos[0]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; def->videos[0]->info.addr.pci.domain = 0; def->videos[0]->info.addr.pci.bus = 0; def->videos[0]->info.addr.pci.slot = 2; def->videos[0]->info.addr.pci.function = 0; + if (qemuDomainPCIAddressReserveSlot(addrs, 2) < 0) + goto error; + } + } else { + virDomainDeviceInfo dev; + memset(&dev, 0, sizeof(dev)); + dev.addr.pci.slot = 2; + + if (qemuDomainPCIAddressCheckSlot(addrs, &dev) < 0) { + VIR_DEBUG("PCI address 0:0:2.0 in use, future addition of a video" + " device will not be possible without manual" + " intervention"); + virResetLastError(); + } else if (qemuDomainPCIAddressReserveSlot(addrs, 2) < 0) { + goto error; } } - if (!reservedVGA - && qemuDomainPCIAddressReserveSlot(addrs, 2) < 0) - goto error; - for (i = 0; i < def->nfss ; i++) { if (def->fss[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) continue; -- 1.7.8.3

2 2

[libvirt] [PATCH] qemuxml2argvtest: Add /usr/bin to PATH
by Michal Privoznik 17 Jan '12

17 Jan '12

On some systems, notably Fedora 16, dirname is located under /usr/bin making our qemuxml2argvtest fail as we default to /bin; --- tests/qemuxml2argvtest.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d1834f5..96f538d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -338,7 +338,7 @@ mymain(void) /* Unset or set all envvars here that are copied in qemudBuildCommandLine * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected * values for these envvars */ - setenv("PATH", "/bin", 1); + setenv("PATH", "/bin:/usr/bin", 1); setenv("USER", "test", 1); setenv("LOGNAME", "test", 1); setenv("HOME", "/home/test", 1); -- 1.7.3.4

2 1

[libvirt] [PATCH] Fixed dumxml of <iotune> parameters
by Martin Kletzander 17 Jan '12

17 Jan '12

The output of dumpxml for <iotune> settings was misformatted, this patch just adds missing newlines. --- src/conf/domain_conf.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d474551..f84e76d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10079,12 +10079,12 @@ virDomainDiskDefFormat(virBufferPtr buf, } if (def->blkdeviotune.read_iops_sec) { - virBufferAsprintf(buf, " <read_iops_sec>%llu</read_iops_sec>", + virBufferAsprintf(buf, " <read_iops_sec>%llu</read_iops_sec>\n", def->blkdeviotune.read_iops_sec); } if (def->blkdeviotune.write_iops_sec) { - virBufferAsprintf(buf, " <write_iops_sec>%llu</write_iops_sec>", + virBufferAsprintf(buf, " <write_iops_sec>%llu</write_iops_sec>\n", def->blkdeviotune.write_iops_sec); } -- 1.7.3.4

2 1

[libvirt] [PATCH] Remove dmidecode dependancy outside PC arches
by Daniel Veillard 17 Jan '12

17 Jan '12

The new dependancy is only available on ix86, x86_64 and ia64 diff --git a/libvirt.spec.in b/libvirt.spec.in index d06c067..a5bd173 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -346,8 +346,10 @@ Requires: device-mapper %if %{with_cgconfig} Requires: libcgroup %endif +%ifarch i386 i586 i686 x86_64 ia64 # For virConnectGetSysinfo Requires: dmidecode +%endif # For service management %if %{with_systemd} Requires(post): systemd-units -- 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/

2 2