October 2014 - Devel - libvirt List Archives

[libvirt] [PATCH 01/21] Added missing virBufferFreeAndReset on the query buffer to free some memory
by Yves Vinter 27 Oct '14

27 Oct '14

From: yvinter <yves.vinter(a)bull.net> --- src/hyperv/hyperv_driver.c | 13 +++++++++++++ src/hyperv/hyperv_wmi.c | 15 +++++++++++---- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index aed9307..372ff39 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -201,6 +201,7 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags VIR_FREE(username); VIR_FREE(password); hypervFreeObject(priv, (hypervObject *)computerSystem); + virBufferFreeAndReset(&query); return result; } @@ -254,6 +255,7 @@ hypervConnectGetHostname(virConnectPtr conn) cleanup: hypervFreeObject(priv, (hypervObject *)computerSystem); + virBufferFreeAndReset(&query); return hostname; } @@ -352,6 +354,7 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) cleanup: hypervFreeObject(priv, (hypervObject *)computerSystem); hypervFreeObject(priv, (hypervObject *)processorList); + virBufferFreeAndReset(&query); return result; } @@ -396,6 +399,7 @@ hypervConnectListDomains(virConnectPtr conn, int *ids, int maxids) cleanup: hypervFreeObject(priv, (hypervObject *)computerSystemList); + virBufferFreeAndReset(&query); return success ? count : -1; } @@ -432,6 +436,7 @@ hypervConnectNumOfDomains(virConnectPtr conn) cleanup: hypervFreeObject(priv, (hypervObject *)computerSystemList); + virBufferFreeAndReset(&query); return success ? count : -1; } @@ -464,6 +469,7 @@ hypervDomainLookupByID(virConnectPtr conn, int id) cleanup: hypervFreeObject(priv, (hypervObject *)computerSystem); + virBufferFreeAndReset(&query); return domain; } @@ -500,6 +506,7 @@ hypervDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) cleanup: hypervFreeObject(priv, (hypervObject *)computerSystem); + virBufferFreeAndReset(&query); return domain; } @@ -533,6 +540,7 @@ hypervDomainLookupByName(virConnectPtr conn, const char *name) cleanup: hypervFreeObject(priv, (hypervObject *)computerSystem); + virBufferFreeAndReset(&query); return domain; } @@ -748,6 +756,7 @@ hypervDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) hypervFreeObject(priv, (hypervObject *)virtualSystemSettingData); hypervFreeObject(priv, (hypervObject *)processorSettingData); hypervFreeObject(priv, (hypervObject *)memorySettingData); + virBufferFreeAndReset(&query); return result; } @@ -915,6 +924,7 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) hypervFreeObject(priv, (hypervObject *)virtualSystemSettingData); hypervFreeObject(priv, (hypervObject *)processorSettingData); hypervFreeObject(priv, (hypervObject *)memorySettingData); + virBufferFreeAndReset(&query); return xml; } @@ -971,6 +981,7 @@ hypervConnectListDefinedDomains(virConnectPtr conn, char **const names, int maxn } hypervFreeObject(priv, (hypervObject *)computerSystemList); + virBufferFreeAndReset(&query); return count; } @@ -1007,6 +1018,7 @@ hypervConnectNumOfDefinedDomains(virConnectPtr conn) cleanup: hypervFreeObject(priv, (hypervObject *)computerSystemList); + virBufferFreeAndReset(&query); return success ? count : -1; } @@ -1346,6 +1358,7 @@ hypervConnectListAllDomains(virConnectPtr conn, } hypervFreeObject(priv, (hypervObject *)computerSystemList); + virBufferFreeAndReset(&query); return ret; } diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index acb705c..d93abd9 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -526,6 +526,7 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, VIR_FREE(returnValue); VIR_FREE(instanceID); hypervFreeObject(priv, (hypervObject *)concreteJob); + virBufferFreeAndReset(&query); return result; } @@ -646,13 +647,14 @@ int hypervMsvmComputerSystemFromDomain(virDomainPtr domain, Msvm_ComputerSystem **computerSystem) { + int result = -1; hypervPrivate *priv = domain->conn->privateData; char uuid_string[VIR_UUID_STRING_BUFLEN]; virBuffer query = VIR_BUFFER_INITIALIZER; if (computerSystem == NULL || *computerSystem != NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; + goto cleanup; } virUUIDFormat(domain->uuid, uuid_string); @@ -663,16 +665,21 @@ hypervMsvmComputerSystemFromDomain(virDomainPtr domain, virBufferAsprintf(&query, "and Name = \"%s\"", uuid_string); if (hypervGetMsvmComputerSystemList(priv, &query, computerSystem) < 0) { - return -1; + goto cleanup; } if (*computerSystem == NULL) { virReportError(VIR_ERR_NO_DOMAIN, _("No domain with UUID %s"), uuid_string); - return -1; + goto cleanup; } - return 0; + result = 0; + + cleanup: + virBufferFreeAndReset(&query); + + return result; } -- 1.9.1

2 34

[libvirt] [PATCHv3 0/2] Add non-FreeBSD guest support to Bhyve driver.
by Conrad Meyer 27 Oct '14

27 Oct '14

3 8

[libvirt] [RFC] Can we error out early for unknown device models?
by Martin Kletzander 27 Oct '14

27 Oct '14

Hi everyone, I had this idea that since we are probing QEMU binaries for devices using 'qom-list-types', we could store that data in the capabilities and check whether device models are supported before starting QEMU. We do that for _some newer_ devices, but this would be global. It would help out particularly with devices like the following one, for example: <interface type='network'> <source network='default'/> <model type='non-existing_device'/> </interface> where we simply construct QEMU command-line with that device model and it then properly errors out: error: internal error: process exited while connecting to monitor: qemu-system-x86_64: -device non-existing_device,...: Parameter 'driver' expects device type This would be easy to achieve with current data unless there are some models hidden from qom-list-types' output. I hope there are none. When I checked the output of 'qemu-kvm -device \?', the devices listed there are separated into categories and have buses assigned to them and that lead me to another idea. What if that data is added to qom-list-types' output and used in libvirt as well? We could then solve another annoying cases like misusing devices or plugging them into unsupported buses. Although I don't know any person who would do such a thing, even when solving the first idea, there'd still be a possibility to do a thing like: <interface type='network'> <source network='default'/> <model type='AC97'/> </interface> This idea is obviously meant for the QEMU driver, but if there's something similar in other drivers, it might be useful as well. I'd be interested in any feedback and welcome any ideas to whether it is useful, how the storing should be done or even if it's something we want to have or not. Have a nice day, Martin

3 5

[libvirt] [libvirt-python PATCH] Add a type check for time in libvirt_virDomainSetTime
by Luyao Huang 27 Oct '14

27 Oct '14

When pass a number or other things to setTime,no error output,but set time to 0. Add a type check and give a clear error messages: TypeError: time must be dict Signed-off-by: Luyao Huang <lhuang(a)redhat.com> --- libvirt-override.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libvirt-override.c b/libvirt-override.c index 9ba87eb..05552a7 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -7795,6 +7795,11 @@ libvirt_virDomainSetTime(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { return NULL; domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + if (!PyDict_Check(py_dict)) { + PyErr_Format(PyExc_TypeError, "time must be dict"); + return NULL; + } + py_dict_size = PyDict_Size(py_dict); if (py_dict_size == 2) { -- 1.8.3.1

2 2

[libvirt] [PATCHv2 0/2] Add non-FreeBSD guest support to Bhyve driver.
by Conrad Meyer 27 Oct '14

27 Oct '14

5 7

[libvirt] Coming release plans for libvirt-1.2.10
by Daniel Veillard 27 Oct '14

27 Oct '14

It's already the end of the month and I didn't pushed plans yet (forgot !) for the coming release. We have only until the end of the week to get to next month, o we should freeze this week. There isn't that many commits yet sine 1.2.9, less than 140 and a number of them are the large splitting changes for the hypervisor driver. So a slightly smaller freeze may be just fine, I'm tempted to push the rc1 for 1.2.10 freeze very early on Wednesday and targetting a release out next Monday. Hopefully that works for everyone, thanks, Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/

2 1

Re: [libvirt] Close the BlockDriverState when guest eject the media
by Weidong Huang 27 Oct '14

27 Oct '14

On 2014/10/20 19:39, Kevin Wolf wrote: > Am 20.10.2014 um 13:27 hat Weidong Huang geschrieben: >> On 2014/10/20 17:41, Kevin Wolf wrote: >> >>> Am 18.10.2014 um 12:02 hat Weidong Huang geschrieben: >>>> Hi ALL: >>>> >>>> There are two ways to eject the cdrom tray. One is by the eject's qmp commmand(eject_device). >>>> The another one is by the guest(bdrv_eject). They have different results. >>> >>> Yes, they are different things. >>> >>> If a guest opens the tray (using bdrv_eject) and then closes it again, >>> with no user interaction in between, the virtual media must still be in >>> the drive and the guest must be able to access the same image again. >>> Calling bdrv_close() in this case would be a bug. >>> >>> The goal of the monitor command "eject" on the other hand is to remove >>> the medium so that the drive is empty. That a device with a closed tray >>> has to be opened for this is only secondary. >> >> >> Thanks for your reply. >> >> There is a problem. >> >> 1. Qemu receive the "eject" command. >> 2. Runs "eject_request_cb" when an eject request is issued from the monitor, the tray >> is closed, and the medium is locked. But the drive is not closed. >> 3. Guest agree with opening tray and qemu will call bdrv_eject to complete. The drive is >> still not close. >> >> So the result of the monitor command "eject" is not to remove the medium in this situation. > > Now I understand, thanks for explaining. > > But I think libvirt can actually work correctly with what qemu offers > today. qemu returns an error if the medium cannot be removed with the > 'eject' command and it only sends an eject request to the guest. > > With this error, libvirt can know that the DEVICE_TRAY_MOVED event > doesn't mean that the medium has removed, but that it needs to issue > another 'eject' command. > > If this isn't implemented correctly in libvirt today, this needs a > libvirt fix rather than a qemu one. > hi all! How about fix it in libvirt? >>>> eject_device: close the BlockDriverState(bdrv_close(bs)) >>>> bdrv_eject: don't close the BlockDriverState, >>>> >>>> This is ambiguous. So libvirt can't handle some situations. >>>> >>>> libvirt send eject qmp command ---> qemu send eject request to guest ---> >>>> guest respond to qemu ---> qemu emit tray_open event to libvirt ---> >>>> libvirt will not send change qmp command if media source is null. So >>>> the media is not be replace to the null. >>> >>> What is the problem that libvirt has with the guest opening the tray? I >>> don't think libvirt should even care about that case. >> >> >> For example, using libvirt to change media by xml below(media source is null): >> <disk type='file' device='cdrom'> >> <driver name='qemu'/> >> <target dev='hdb' bus='ide'/> >> </disk> >> >> libivrt return ok. But media still is in the guest. >> This is confused. > > Kevin > > . >

4 3

[libvirt] [PATCHv4 0/3] Add non-FreeBSD guest support to Bhyve driver.
by Conrad Meyer 26 Oct '14

26 Oct '14

1 3

[libvirt] [PATCH] util: eliminate "use after free" in callers of virNetDevLinkDump
by Laine Stump 26 Oct '14

26 Oct '14

virNetDevLinkDump() gets a message from netlink into "resp", then calls nlmsg_parse() to fill the table "tb" with pointers into resp. It then returns tb to its caller, but not before freeing the buffer at resp. That means that all the callers of virNetDevLinkDump() are examining memory that has already been freed. This can be verified by filling the buffer at resp with garbage prior to freeing it (or, I suppose, just running libvirtd under valgrind) then performing some operation that calls virNetDevLinkDump(). The code has been like this ever since virNetDevLinkDump() was written - the original author didn't notice it, and neither did later additional users of the function. It has only been pure luck (or maybe a lack of heavy load, and/or maybe an allocation algorithm in malloc() that delays re-use of just-freed memory) that has kept this from causing errors, for example when configuring a PCI passthrough or macvtap passthrough network interface. The solution taken in this patch is the simplest - just return resp to the caller along with tb, then have the caller free it after they are finished using the data (pointers) in tb. I alternately could have made a cleaner interface by creating a new struct that put tb and resp together along with a vir*Free() function for it, but this function is only used in a couple places, and I'm not sure there will be additional new uses of virNetDevLinkDump(), so the value of adding a new type, extra APIs, etc. is dubious. --- src/util/virnetdev.c | 26 +++++++++++++++++--------- src/util/virnetdev.h | 2 +- src/util/virnetdevvportprofile.c | 17 ++++++++++++----- 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index db5623a..c22a0e6 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1353,23 +1353,25 @@ static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = { /** * virNetDevLinkDump: * - * @ifname: The name of the interface; only use if ifindex < 0 - * @ifindex: The interface index; may be < 0 if ifname is given - * @nlattr: pointer to a pointer of netlink attributes that will contain - * the results + * @ifname: The name of the interface; only use if ifindex <= 0 + * @ifindex: The interface index; may be <= 0 if ifname is given + * @data: Gets a pointer to the raw data from netlink. + MUST BE FREED BY CALLER! + * @nlattr: Pointer to a pointer of netlink attributes that will contain + * the results * @src_pid: pid used for nl_pid of the local end of the netlink message * (0 == "use getpid()") * @dst_pid: pid of destination nl_pid if the kernel * is not the target of the netlink message but it is to be * sent to another process (0 if sending to the kernel) * - * Get information about an interface given its name or index. + * Get information from netlink about an interface given its name or index. * * Returns 0 on success, -1 on fatal error. */ int virNetDevLinkDump(const char *ifname, int ifindex, - struct nlattr **tb, + void **nlData, struct nlattr **tb, uint32_t src_pid, uint32_t dst_pid) { int rc = -1; @@ -1451,7 +1453,9 @@ virNetDevLinkDump(const char *ifname, int ifindex, rc = 0; cleanup: nlmsg_free(nl_msg); - VIR_FREE(resp); + if (rc < 0) + VIR_FREE(resp); + *nlData = resp; return rc; malformed_resp: @@ -1647,15 +1651,18 @@ virNetDevGetVfConfig(const char *ifname, int vf, virMacAddrPtr mac, int *vlanid) { int rc = -1; + void *nlData = NULL; struct nlattr *tb[IFLA_MAX + 1] = {NULL, }; int ifindex = -1; - rc = virNetDevLinkDump(ifname, ifindex, tb, 0, 0); + rc = virNetDevLinkDump(ifname, ifindex, &nlData, tb, 0, 0); if (rc < 0) - return rc; + goto cleanup; rc = virNetDevParseVfConfig(tb, vf, mac, vlanid); + cleanup: + VIR_FREE(nlData); return rc; } @@ -1798,6 +1805,7 @@ virNetDevRestoreNetConfig(const char *linkdev, int vf, const char *stateDir) int virNetDevLinkDump(const char *ifname ATTRIBUTE_UNUSED, int ifindex ATTRIBUTE_UNUSED, + void **nlData ATTRIBUTE_UNUSED, struct nlattr **tb ATTRIBUTE_UNUSED, uint32_t src_pid ATTRIBUTE_UNUSED, uint32_t dst_pid ATTRIBUTE_UNUSED) diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 2a6e67d..5c463ef 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -166,7 +166,7 @@ int virNetDevGetVirtualFunctions(const char *pfname, ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; int virNetDevLinkDump(const char *ifname, int ifindex, - struct nlattr **tb, + void **nlData, struct nlattr **tb, uint32_t src_pid, uint32_t dst_pid) ATTRIBUTE_RETURN_CHECK; diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index 75679b0..6ee20d3 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -787,7 +787,8 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int int *parent_ifindex, char *parent_ifname, unsigned int *nth) { - int rc; + int rc = -1; + void *nlData = NULL; struct nlattr *tb[IFLA_MAX + 1] = { NULL, }; bool end = false; size_t i = 0; @@ -798,7 +799,8 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int return -1; while (!end && i <= nthParent) { - rc = virNetDevLinkDump(ifname, ifindex, tb, 0, 0); + VIR_FREE(nlData); + rc = virNetDevLinkDump(ifname, ifindex, &nlData, tb, 0, 0); if (rc < 0) break; @@ -807,7 +809,8 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int IFNAMSIZ)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("buffer for root interface name is too small")); - return -1; + rc = -1; + goto cleanup; } *parent_ifindex = ifindex; } @@ -824,6 +827,8 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int *nth = i - 1; + cleanup: + VIR_FREE(nlData); return rc; } @@ -845,6 +850,7 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex, int rc; int src_pid = 0; uint32_t dst_pid = 0; + void *nlData = NULL; struct nlattr *tb[IFLA_MAX + 1] = { NULL, }; int repeats = STATUS_POLL_TIMEOUT_USEC / STATUS_POLL_INTERVL_USEC; uint16_t status = 0; @@ -877,7 +883,8 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex, } while (--repeats >= 0) { - rc = virNetDevLinkDump(NULL, ifindex, tb, src_pid, dst_pid); + VIR_FREE(nlData); + rc = virNetDevLinkDump(NULL, ifindex, &nlData, tb, src_pid, dst_pid); if (rc < 0) goto cleanup; @@ -909,7 +916,7 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex, } cleanup: - + VIR_FREE(nlData); return rc; } -- 1.9.3

2 2

[libvirt] [PATCH v2] tests: Adding SELINUX_LIBS to fix viridentitytest linker bug.
by Julio Faracco 25 Oct '14

25 Oct '14

In a clean build system (Ubuntu 14.04), the viridentitytest failed to compile. Even if all the SELINUX libraries and depedencies are installed. See the error message below: [...] CC viridentitytest.o CCLD viridentitytest /usr/bin/ld: viridentitytest.o: undefined reference to symbol 'security_disable' //lib/x86_64-linux-gnu/libselinux.so.1: error adding symbols: DSO missing from command line collect2: error: ld returned 1 exit status make: *** [viridentitytest] Error 1 Simply adding the variable SELINUX_LIBS in viridentitytest rules of Makefile.am to include SELINUX libraries into viridentitytest solved that compilation issue. A conditional was added to consider when libvirt is using or not SELINUX. Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com> --- tests/Makefile.am | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 4c3d4ef..363c902 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -926,11 +926,13 @@ virstoragetest_LDADD = $(LDADDS) \ viridentitytest_SOURCES = \ viridentitytest.c testutils.h testutils.c -viridentitytest_LDADD = $(LDADDS) if WITH_SELINUX +viridentitytest_LDADD = $(LDADDS) $(SELINUX_LIBS) viridentitytest_DEPENDENCIES = libsecurityselinuxhelper.la \ ../src/libvirt.la -endif WITH_SELINUX +else ! WITH_SELINUX +viridentitytest_LDADD = $(LDADDS) +endif ! WITH_SELINUX viriscsitest_SOURCES = \ viriscsitest.c testutils.h testutils.c -- 1.9.1

2 3