January 2012 - Devel - libvirt List Archives

[libvirt] [PATCH] python: Expose virDomain{G, S}etInterfaceParameters' APIs in python binding
by ajia＠redhat.com 18 Jan '12

18 Jan '12

From: Alex Jia <ajia(a)redhat.com> An simple example to show how to use it: #!/usr/bin/env python import libvirt conn = libvirt.open(None) dom = conn.lookupByName('foo') print dom.interfaceParameters('vnet0', 0) params = {'outbound.peak': 20, 'inbound.peak': 20, 'inbound.burst': 10, 'inbound.average': 10, 'outbound.average': 30, 'outbound.burst': 30} print dom.setInterfaceParameters('vnet0', params, 0) print dom.interfaceParameters('vnet0', 0) RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=770971 https://bugzilla.redhat.com/show_bug.cgi?id=771015 Signed-off-by: Alex Jia <ajia(a)redhat.com> --- python/libvirt-override-api.xml | 15 +++ python/libvirt-override.c | 179 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 194 insertions(+), 0 deletions(-) diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 704fee9..b29c149 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -421,5 +421,20 @@ <arg name='flags' type='unsigned int' info='an OR'ed set of virDomainMemoryFlags'/> <return type='char *' info='the returned buffer or None in case of error'/> </function> + <function name='virDomainSetInterfaceParameters' file='python'> + <info>Change the bandwidth tunables for a interface device</info> + <arg name='dom' type='virDomainPtr' info='pointer to the domain'/> + <arg name='device' type='const char *' info='interface name'/> + <arg name='params' type='virTypedParameterPtr' info='Pointer to bandwidth tuning params object'/> + <arg name='flags' type='unsigned int' info='an OR'ed set of virDomainModificationImpact'/> + <return type='int' info='0 in case of success, -1 in case of failure'/> + </function> + <function name='virDomainGetInterfaceParameters' file='python'> + <info>Get the bandwidth tunables for a interface device</info> + <arg name='dom' type='virDomainPtr' info='pointer to the domain'/> + <arg name='device' type='const char *' info='interface name'/> + <arg name='flags' type='unsigned int' info='an OR'ed set of virDomainModificationImpact'/> + <return type='virTypedParameterPtr' info='the bandwidth tunables value or None in case of error'/> + </function> </symbols> </api> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index d2aad0f..ecd9355 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -5108,6 +5108,183 @@ cleanup: return py_retval; } +static PyObject * +libvirt_virDomainSetInterfaceParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + virDomainPtr domain; + PyObject *pyobj_domain, *info; + int i_retval; + int nparams = 0, i; + unsigned int flags; + const char *device; + virTypedParameterPtr params; + + if (!PyArg_ParseTuple(args, + (char *)"OzOi:virDomainSetInterfaceParameters", + &pyobj_domain, &device, &info, &flags)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetInterfaceParameters(domain, device, NULL, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) + return VIR_PY_INT_FAIL; + + if ((params = malloc(sizeof(*params)*nparams)) == NULL) + return VIR_PY_INT_FAIL; + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetInterfaceParameters(domain, device, params, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) { + free(params); + return VIR_PY_INT_FAIL; + } + + /* convert to a Python tuple of long objects */ + for (i = 0; i < nparams; i++) { + PyObject *key, *val; + key = libvirt_constcharPtrWrap(params[i].field); + val = PyDict_GetItem(info, key); + Py_DECREF(key); + + if (val == NULL) + continue; + + switch (params[i].type) { + case VIR_TYPED_PARAM_INT: + params[i].value.i = (int)PyInt_AS_LONG(val); + break; + + case VIR_TYPED_PARAM_UINT: + params[i].value.ui = (unsigned int)PyInt_AS_LONG(val); + break; + + case VIR_TYPED_PARAM_LLONG: + params[i].value.l = (long long)PyLong_AsLongLong(val); + break; + + case VIR_TYPED_PARAM_ULLONG: + params[i].value.ul = (unsigned long long)PyLong_AsLongLong(val); + break; + + case VIR_TYPED_PARAM_DOUBLE: + params[i].value.d = (double)PyFloat_AsDouble(val); + break; + + case VIR_TYPED_PARAM_BOOLEAN: + { + /* Hack - Python's definition of Py_True breaks strict + * aliasing rules, so can't directly compare :-( + */ + PyObject *hacktrue = PyBool_FromLong(1); + params[i].value.b = hacktrue == val ? 1: 0; + Py_DECREF(hacktrue); + } + break; + + default: + free(params); + return VIR_PY_INT_FAIL; + } + } + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainSetInterfaceParameters(domain, device, params, nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + if (i_retval < 0) { + free(params); + return VIR_PY_INT_FAIL; + } + + free(params); + return VIR_PY_INT_SUCCESS; +} + +static PyObject * +libvirt_virDomainGetInterfaceParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + virDomainPtr domain; + PyObject *pyobj_domain, *info; + int i_retval; + int nparams = 0, i; + unsigned int flags; + const char *device; + virTypedParameterPtr params; + + if (!PyArg_ParseTuple(args, (char *)"Ozi:virDomainGetInterfaceParameters", + &pyobj_domain, &device, &flags)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetInterfaceParameters(domain, device, NULL, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) + return VIR_PY_NONE; + + if ((params = malloc(sizeof(*params)*nparams)) == NULL) + return VIR_PY_NONE; + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetInterfaceParameters(domain, device, params, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) { + free(params); + return VIR_PY_NONE; + } + + /* convert to a Python tuple of long objects */ + if ((info = PyDict_New()) == NULL) { + free(params); + return VIR_PY_NONE; + } + for (i = 0 ; i < nparams ; i++) { + PyObject *key, *val; + + switch (params[i].type) { + case VIR_TYPED_PARAM_INT: + val = PyInt_FromLong((long)params[i].value.i); + break; + + case VIR_TYPED_PARAM_UINT: + val = PyInt_FromLong((long)params[i].value.ui); + break; + + case VIR_TYPED_PARAM_LLONG: + val = PyLong_FromLongLong((long long)params[i].value.l); + break; + + case VIR_TYPED_PARAM_ULLONG: + val = PyLong_FromLongLong((long long)params[i].value.ul); + break; + + case VIR_TYPED_PARAM_DOUBLE: + val = PyFloat_FromDouble((double)params[i].value.d); + break; + + case VIR_TYPED_PARAM_BOOLEAN: + val = PyBool_FromLong((long)params[i].value.b); + break; + + default: + free(params); + Py_DECREF(info); + return VIR_PY_NONE; + } + + key = libvirt_constcharPtrWrap(params[i].field); + PyDict_SetItem(info, key, val); + } + free(params); + return(info); +} + /************************************************************************ * * * The registration stuff * @@ -5206,6 +5383,8 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virDomainMigrateGetMaxSpeed", libvirt_virDomainMigrateGetMaxSpeed, METH_VARARGS, NULL}, {(char *) "virDomainBlockPeek", libvirt_virDomainBlockPeek, METH_VARARGS, NULL}, {(char *) "virDomainMemoryPeek", libvirt_virDomainMemoryPeek, METH_VARARGS, NULL}, + {(char *) "virDomainSetInterfaceParameters", libvirt_virDomainSetInterfaceParameters, METH_VARARGS, NULL}, + {(char *) "virDomainGetInterfaceParameters", libvirt_virDomainGetInterfaceParameters, METH_VARARGS, NULL}, {NULL, NULL, 0, NULL} }; -- 1.7.1

1 0

[libvirt] [PATCH] qemu: Do not fail if detachment succeeded but failed on resetting device
by Osier Yang 18 Jan '12

18 Jan '12

It makes one curious to see error "Failed to detach device" while the device is detached successfully, only because of it failed on resetting the device. And on the other hand, failed on resetting is not the end of world, one could resolve the problem of resetting outside of libvirt, and do reattachment then. --- src/qemu/qemu_hotplug.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4b60839..c3167ea 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2033,11 +2033,11 @@ qemuDomainDetachHostPciDevice(struct qemud_driver *driver, driver->inactivePciHostdevs) == 0) qemuReattachPciDevice(activePci, driver); else - ret = -1; + VIR_WARN("Device is detached, but failed to reset PCI device"); pciFreeDevice(pci); pciFreeDevice(activePci); } else { - ret = -1; + VIR_WARN("Device is detached, but failed to reset PCI device"); } if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && -- 1.7.7.3

3 2

[libvirt] [PATCH v4] Add new attribute wrpolicy to <driver> element
by Deepak C Shetty 17 Jan '12

17 Jan '12

This introduces new attribute wrpolicy with only supported value as immediate. This will be an optional attribute with no defaults. This helps specify whether to skip the host page cache. When wrpolicy is specified, meaning when wrpolicy=immediate a writeback is explicitly initiated for the dirty pages in the host page cache as part of the guest file write operation. Usage: <filesystem type='mount' accessmode='passthrough'> <driver type='path' wrpolicy='immediate'/> <source dir='/export/to/guest'/> <target dir='mount_tag'/> </filesystem> Currently this only works with type='mount' for the QEMU/KVM driver. Signed-off-by: Deepak C Shetty <deepakcs(a)linux.vnet.ibm.com> --- v2: - added writeout as a qemu cap - cosmetic changes in comments - moved to using VIR_ERR_CONFIG_UNSUPPORTED - corrected doc v3: - changed attr name from writeout to wrpolicy - moved attr to be part of driver xml element v4: - implemented review comments given by eric blake against v3 - added wrpolicy testcase docs/formatdomain.html.in | 9 ++++++-- docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 27 ++++++++++++++++++++++++- src/conf/domain_conf.h | 10 +++++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 12 +++++++++++ tests/qemuxml2argvdata/qemuxml2argv-fs9p.args | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml | 4 ++-- tests/qemuxml2argvtest.c | 2 +- 11 files changed, 71 insertions(+), 8 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index faf42df..875ac22 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1328,7 +1328,7 @@ <target dir='/'/> </filesystem> <filesystem type='mount' accessmode='passthrough'> - <driver type='path'/> + <driver type='path' wrpolicy='immediate'/> <source dir='/export/to/guest'/> <target dir='/import/from/host'/> <readonly/> @@ -1355,7 +1355,12 @@ sub-element <code>driver</code>, with an attribute <code>type='path'</code> or <code>type='handle'</code> (since - 0.9.7). + 0.9.7). The driver block has an optional attribute + <code>wrpolicy</code> that further controls interaction with + the host page cache; omitting the attribute gives default behavior, + while the value <code>immediate</code> means that a host writeback + is immediately triggered for all pages touched during a guest file + write operation (since 0.9.10). </dd> <dt><code>type='template'</code></dt> <dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 243ff93..f72f7d1 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1162,6 +1162,11 @@ <value>handle</value> </choice> </attribute> + <optional> + <attribute name="wrpolicy"> + <value>immediate</value> + </attribute> + </optional> <empty/> </element> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6c42f36..5fa23dc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -265,6 +265,9 @@ VIR_ENUM_IMPL(virDomainFSAccessMode, VIR_DOMAIN_FS_ACCESSMODE_LAST, "mapped", "squash") +VIR_ENUM_IMPL(virDomainFSWrpolicy, VIR_DOMAIN_FS_WRPOLICY_LAST, + "default", + "immediate") VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "user", @@ -3459,6 +3462,7 @@ virDomainFSDefParseXML(xmlNodePtr node, char *source = NULL; char *target = NULL; char *accessmode = NULL; + char *wrpolicy = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -3508,6 +3512,7 @@ virDomainFSDefParseXML(xmlNodePtr node, def->readonly = 1; } else if ((fsdriver == NULL) && (xmlStrEqual(cur->name, BAD_CAST "driver"))) { fsdriver = virXMLPropString(cur, "type"); + wrpolicy = virXMLPropString(cur, "wrpolicy"); } } cur = cur->next; @@ -3521,6 +3526,16 @@ virDomainFSDefParseXML(xmlNodePtr node, } } + if (wrpolicy) { + if ((def->wrpolicy = virDomainFSWrpolicyTypeFromString(wrpolicy)) <= 0) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown filesystem write policy '%s'"), wrpolicy); + goto error; + } + } else { + def->wrpolicy = VIR_DOMAIN_FS_WRPOLICY_DEFAULT; + } + if (source == NULL) { virDomainReportError(VIR_ERR_NO_SOURCE, target ? "%s" : NULL, target); @@ -3547,6 +3562,7 @@ cleanup: VIR_FREE(target); VIR_FREE(source); VIR_FREE(accessmode); + VIR_FREE(wrpolicy); return def; @@ -10165,6 +10181,7 @@ virDomainFSDefFormat(virBufferPtr buf, const char *type = virDomainFSTypeToString(def->type); const char *accessmode = virDomainFSAccessModeTypeToString(def->accessmode); const char *fsdriver = virDomainFSDriverTypeTypeToString(def->fsdriver); + const char *wrpolicy = virDomainFSWrpolicyTypeToString(def->wrpolicy); if (!type) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -10184,7 +10201,15 @@ virDomainFSDefFormat(virBufferPtr buf, type, accessmode); if (def->fsdriver) { - virBufferAsprintf(buf, " <driver type='%s'/>\n", fsdriver); + virBufferAsprintf(buf, " <driver type='%s'", fsdriver); + + /* Don't generate anything if wrpolicy is set to default */ + if (def->wrpolicy) { + virBufferAsprintf(buf, " wrpolicy='%s'", wrpolicy); + } + + virBufferAddLit(buf, "/>\n"); + } if (def->src) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7c7e93b..ac8e768 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -491,12 +491,21 @@ enum virDomainFSAccessMode { VIR_DOMAIN_FS_ACCESSMODE_LAST }; +/* Filesystem Write policy */ +enum virDomainFSWrpolicy { + VIR_DOMAIN_FS_WRPOLICY_DEFAULT = 0, + VIR_DOMAIN_FS_WRPOLICY_IMMEDIATE, + + VIR_DOMAIN_FS_WRPOLICY_LAST +}; + typedef struct _virDomainFSDef virDomainFSDef; typedef virDomainFSDef *virDomainFSDefPtr; struct _virDomainFSDef { int type; int fsdriver; int accessmode; + int wrpolicy; /* enum virDomainFSWrpolicy */ char *src; char *dst; unsigned int readonly : 1; @@ -1984,6 +1993,7 @@ VIR_ENUM_DECL(virDomainControllerModelUSB) VIR_ENUM_DECL(virDomainFS) VIR_ENUM_DECL(virDomainFSDriverType) VIR_ENUM_DECL(virDomainFSAccessMode) +VIR_ENUM_DECL(virDomainFSWrpolicy) VIR_ENUM_DECL(virDomainNet) VIR_ENUM_DECL(virDomainNetBackend) VIR_ENUM_DECL(virDomainNetVirtioTxMode) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cfb88f8..3e1ba0c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -313,6 +313,8 @@ virDomainDiskTypeToString; virDomainFSDefFree; virDomainFSTypeFromString; virDomainFSTypeToString; +virDomainFSWrpolicyTypeFromString; +virDomainFSWrpolicyTypeToString; virDomainFindByID; virDomainFindByName; virDomainFindByUUID; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 530bbb1..3069dbe 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -148,6 +148,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "virtio-blk-pci.scsi", "blk-sg-io", "drive-copy-on-read", + "fsdev-writeout", ); struct qemu_feature_flags { @@ -1089,6 +1090,8 @@ qemuCapsComputeCmdFlags(const char *help, qemuCapsSet(flags, QEMU_CAPS_FSDEV); if (strstr(fsdev, "readonly")) qemuCapsSet(flags, QEMU_CAPS_FSDEV_READONLY); + if (strstr(fsdev, "writeout")) + qemuCapsSet(flags, QEMU_CAPS_FSDEV_WRITEOUT); } 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 ee9d5ab..4823aae 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -121,6 +121,7 @@ enum qemuCapsFlags { QEMU_CAPS_VIRTIO_BLK_SCSI = 80, /* virtio-blk-pci.scsi */ QEMU_CAPS_VIRTIO_BLK_SG_IO = 81, /* support for SG_IO commands, reportedly added in 0.11 */ QEMU_CAPS_DRIVE_COPY_ON_READ = 82, /* -drive copy-on-read */ + QEMU_CAPS_FSDEV_WRITEOUT = 83, /* -fsdev writeout 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 98824ac..77f2896 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2129,6 +2129,7 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, { virBuffer opt = VIR_BUFFER_INITIALIZER; const char *driver = qemuDomainFSDriverTypeToString(fs->fsdriver); + const char *wrpolicy = virDomainFSWrpolicyTypeToString(fs->wrpolicy); if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -2161,6 +2162,17 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, goto error; } } + + if (fs->wrpolicy) { + if (qemuCapsGet(qemuCaps, QEMU_CAPS_FSDEV_WRITEOUT)) { + virBufferAsprintf(&opt, ",writeout=%s", wrpolicy); + } else { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("filesystem writeout not supported")); + goto error; + } + } + virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); virBufferAsprintf(&opt, ",path=%s", fs->src); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fs9p.args b/tests/qemuxml2argvdata/qemuxml2argv-fs9p.args index 8579810..f244114 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-fs9p.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-fs9p.args @@ -4,10 +4,10 @@ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda \ /dev/HostVG/QEMUGuest1 -fsdev local,security_model=passthrough,id=fsdev-fs0,\ path=/export/to/guest -device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,\ mount_tag=/import/from/host,bus=pci.0,addr=0x3 \ --fsdev local,security_model=mapped,id=fsdev-fs1,\ +-fsdev local,security_model=mapped,writeout=immediate,id=fsdev-fs1,\ path=/export/to/guest2 -device virtio-9p-pci,id=fs1,fsdev=fsdev-fs1,\ mount_tag=/import/from/host2,bus=pci.0,addr=0x4 \ --fsdev handle,id=fsdev-fs2,\ +-fsdev handle,writeout=immediate,id=fsdev-fs2,\ path=/export/to/guest3 -device virtio-9p-pci,id=fs2,fsdev=fsdev-fs2,\ mount_tag=/import/from/host3,bus=pci.0,addr=0x5 \ -usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml b/tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml index 07d7e8a..e31db48 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml @@ -25,12 +25,12 @@ <target dir='/import/from/host'/> </filesystem> <filesystem accessmode='mapped'> - <driver type='path'/> + <driver type='path' wrpolicy='immediate'/> <source dir='/export/to/guest2'/> <target dir='/import/from/host2'/> </filesystem> <filesystem> - <driver type='handle'/> + <driver type='handle' wrpolicy='immediate'/> <source dir='/export/to/guest3'/> <target dir='/import/from/host3'/> </filesystem> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 7beaa4b..8feda9e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -631,7 +631,7 @@ mymain(void) DO_TEST("sound-device", false, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_HDA_DUPLEX); DO_TEST("fs9p", false, - QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_FSDEV); + QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_FSDEV, QEMU_CAPS_FSDEV_WRITEOUT); DO_TEST("hostdev-usb-address", false, NONE); DO_TEST("hostdev-usb-address-device", false,

2 2

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

17 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> (since + or <code>type='handle'</code> + or <code>type='proxy'</code> (since 0.9.7). </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 17 Jan '12

17 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 17 Jan '12

17 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 17 Jan '12

17 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