January 2016 - Devel - libvirt List Archives

[libvirt] Bug in RPC code causes failure to start LXC container using virDomainCreateXMLWithFiles
by Ben Gray 12 Jan '16

12 Jan '16

Hi, Occasionally when trying to start LXC containers with fds I get the following error: virNetMessageDupFD:562 : Unable to duplicate FD -1: Bad file descriptor I tracked it down to the code that handles EAGAIN errors from recvfd. In such cases the virNetMessageDecodeNumFDs function may be called multiple times from virNetServerClientDispatchRead and each time it overwrites the msg->fds array. In the best case (when msg->donefds == 0) this results in a memory leak, in the worse case it will leak any fd's already in msg->fds and cause subsequent failures when dup is called. A very similar problem is mention here: https://www.redhat.com/archives/libvir-list/2012-December/msg01306.html Below is my patch to fix the issue. --- a/src/rpc/virnetserverclient.c 2015-01-23 11:46:24.000000000 +0000 +++ b/src/rpc/virnetserverclient.c 2015-11-26 15:30:51.214462290 +0000 @@ -1107,36 +1107,40 @@ /* Now figure out if we need to read more data to get some * file descriptors */ - if (msg->header.type == VIR_NET_CALL_WITH_FDS && - virNetMessageDecodeNumFDs(msg) < 0) { - virNetMessageQueueServe(&client->rx); - virNetMessageFree(msg); - client->wantClose = true; - return; /* Error */ - } + if (msg->header.type == VIR_NET_CALL_WITH_FDS) { + size_t i; - /* Try getting the file descriptors (may fail if blocking) */ - for (i = msg->donefds; i < msg->nfds; i++) { - int rv; - if ((rv = virNetSocketRecvFD(client->sock, &(msg->fds[i]))) < 0) { + if (msg->nfds == 0 && + virNetMessageDecodeNumFDs(msg) < 0) { virNetMessageQueueServe(&client->rx); virNetMessageFree(msg); client->wantClose = true; - return; + return; /* Error */ } - if (rv == 0) /* Blocking */ - break; - msg->donefds++; - } - /* Need to poll() until FDs arrive */ - if (msg->donefds < msg->nfds) { - /* Because DecodeHeader/NumFDs reset bufferOffset, we - * put it back to what it was, so everything works - * again next time we run this method - */ - client->rx->bufferOffset = client->rx->bufferLength; - return; + /* Try getting the file descriptors (may fail if blocking) */ + for (i = msg->donefds; i < msg->nfds; i++) { + int rv; + if ((rv = virNetSocketRecvFD(client->sock, &(msg->fds[i]))) < 0) { + virNetMessageQueueServe(&client->rx); + virNetMessageFree(msg); + client->wantClose = true; + return; + } + if (rv == 0) /* Blocking */ + break; + msg->donefds++; + } + + /* Need to poll() until FDs arrive */ + if (msg->donefds < msg->nfds) { + /* Because DecodeHeader/NumFDs reset bufferOffset, we + * put it back to what it was, so everything works + * again next time we run this method + */ + client->rx->bufferOffset = client->rx->bufferLength; + return; + } } /* Definitely finished reading, so remove from queue */

2 1

[libvirt] DNS for IPv6 addresses?
by Yaniv Kaul 12 Jan '16

12 Jan '16

Is there a way to define DNS for IPv6 addresses? Something like: <dns forwardPlainNames="yes"> <host ip="192.168.200.4"> <hostname>lago_basic_suite_3_6_storage-iscsi</hostname> </host> </dns> Only for IPv6? I reckon I can't just use an IPv6 address in the 'IP' attribute? TIA, Y.

2 1

[libvirt] [PATCH 0/5] logging fixes
by Laine Stump 11 Jan '16

11 Jan '16

These are all related to excessive, misleading, or missing info in logs when trying to debug problems with SR-IOV network devices. Patch 2 does change the logging to eliminate an error message when no error has occurred (or prevent overwriting a prior error if a DISASSOCIATE is happening as part of the cleanup after said prior error), but there is a change to behavior in patch 2 that could have unintended bad consequences, which is why I've Cc'ed Christian at Cisco and Stefan and Shivaprasad at IBM, in hopes that they (or someone they can contact at their respective organizations) can look at the change and report back if it will cause a problem. The change in question (again, in 2/5) is that we would previously always return a status of 0 (PORT_VDP_RESPONSE_SUCCESS) from virNetDevVPortProfileGetStatus if instanceId was NULL; that is *always* the case for both ASSOCIATE and DISASSOCIATE for 802.1Qbg, and is true for all DISASSOCIATE commands for 802.1Qbh. With the change in Patch 2/5, we now will now set status to the actual IFLA_PORT_RESPONSE from the response message, which seems to be correct behavior, but could have bad side effects if there is a previously undiscovered bug at the other end of the communication. Laine Stump (5): util: report the MAC address that couldn't be set util: don't log error in virNetDevVPortProfileGetStatus if instanceId is NULL util: improve error reporting in virNetDevVPortProfileGetStatus util: reduce debug log in virPCIGetVirtualFunctions() docs: update to properly reflect meaning of fields in log filter daemon/libvirtd.conf | 14 ++++++--- docs/logging.html.in | 15 ++++++---- src/util/virnetdev.c | 23 +++++++++----- src/util/virnetdevvportprofile.c | 65 ++++++++++++++++++++++++++++++++-------- src/util/virpci.c | 37 ++++++----------------- src/util/virpci.h | 4 +-- 6 files changed, 98 insertions(+), 60 deletions(-) -- 2.5.0

4 11

[libvirt] [PATCH] qemu: use enum when setting PCI "multi" value, not 0 or 1
by Laine Stump 11 Jan '16

11 Jan '16

From: root <root(a)vhost2.laine.org> No functional change. --- This was suggested by jferlan in an earlier review. src/qemu/qemu_command.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 936a43e..3c9caea 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1,7 +1,7 @@ /* * qemu_command.c: QEMU command generation * - * Copyright (C) 2006-2015 Red Hat, Inc. + * Copyright (C) 2006-2016 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -2142,12 +2142,12 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 0x1F; tmp_addr.function = 0; - tmp_addr.multi = 1; + tmp_addr.multi = VIR_TRISTATE_SWITCH_ON; if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, false, false) < 0) goto cleanup; tmp_addr.function = 3; - tmp_addr.multi = 0; + tmp_addr.multi = VIR_TRISTATE_SWITCH_ABSENT; if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, false, false) < 0) goto cleanup; @@ -2623,7 +2623,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, addrs->lastaddr = addr; addrs->lastaddr.function = 0; - addrs->lastaddr.multi = 0; + addrs->lastaddr.multi = VIR_TRISTATE_SWITCH_ABSENT; } /* Finally we can reserve the slot+function */ if (virDomainPCIAddressReserveAddr(addrs, &addr, flags, -- 2.5.0

2 1

[libvirt] [PATCH 0/5] auto-add USB2 controller set for Q35
by Laine Stump 11 Jan '16

11 Jan '16

For just about every other machinetype, libvirt automatically adds a USB controller if there is no controller (including "type='none'") specified in the config. It doesn't do this for the Q35 machinetype, because Q35 hardware would have a USB2 controller, USB2 controllers come in sets of multiple devices, and the code that auto-adds the USB controller was really setup to just add a single controller. Expanding that to adding a set of related controllers was beyond the amount of time I had when putting in the initial Q35 support, so I left it "for later", and then forgot about it until someone reminded me in the hall at KVM Forum this summer. I find the practice of auto-adding devices that aren't required for operation of the virtual machine to be a bit odd, but this does make the Q35 machinetype more consistent with all the others, and it is still possible to force no USB controllers by specifying: <controller type='usb' model='none'/> Since the USB controllers on a real Q35 machine are on bus 0 slot 0x1D, there is also a patch here to attempt to use that address for the first set of USB controllers (and 0x1A for the 2nd set). Finally, patch 1 is a bugfix for a problem that hadn't been noticed before, because nobody had tried to connect a USB controller to a pcie-root-port (which has a single slot that is numbered 0). Laine Stump (5): qemu: don't assume slot 0 is unused/reserved. qemu: prefer 00:1D.x and 00:1A.x for USB2 controllers on Q35 conf: add virDomainDefAddController() qemu: define virDomainDevAddUSBController() qemu: auto-add a USB2 controller set for Q35 machines src/conf/domain_conf.c | 104 +++++++++++++++++---- src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 57 ++++++++++- src/qemu/qemu_domain.c | 14 ++- .../qemuxml2argv-q35-usb2-multi.args | 40 ++++++++ .../qemuxml2argv-q35-usb2-multi.xml | 47 ++++++++++ .../qemuxml2argv-q35-usb2-reorder.args | 40 ++++++++ .../qemuxml2argv-q35-usb2-reorder.xml | 47 ++++++++++ tests/qemuxml2argvdata/qemuxml2argv-q35-usb2.args | 30 ++++++ tests/qemuxml2argvdata/qemuxml2argv-q35-usb2.xml | 39 ++++++++ tests/qemuxml2argvdata/qemuxml2argv-q35.args | 5 + tests/qemuxml2argvtest.c | 22 +++++ .../qemuxml2xmlout-q35-usb2-multi.xml | 66 +++++++++++++ .../qemuxml2xmlout-q35-usb2-reorder.xml | 66 +++++++++++++ .../qemuxml2xmloutdata/qemuxml2xmlout-q35-usb2.xml | 46 +++++++++ tests/qemuxml2xmltest.c | 3 + 17 files changed, 606 insertions(+), 23 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-usb2-multi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-usb2-multi.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-usb2-reorder.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-usb2-reorder.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-usb2.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-usb2.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-usb2-multi.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-usb2-reorder.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-usb2.xml -- 2.4.3

2 14

[libvirt] [PATCH 1/2] cgroup: fix including sys/mount.h for OpenBSD
by Roman Bogorodskiy 11 Jan '16

11 Jan '16

From: Jasper Lievisse Adriaanse <jasper(a)openbsd.org> Include sys/param.h along with sys/mount.h to fix compilation on OpenBSD. Signed-off-by: Roman Bogorodskiy <bogorodskiy(a)gmail.com> --- src/util/vircgroup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index e39c4d1..e8e0875 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -27,7 +27,8 @@ #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R # include <mntent.h> #endif -#if defined HAVE_SYS_MOUNT_H +#if defined HAVE_SYS_PARAM_H && defined HAVE_SYS_MOUNT_H +# include <sys/param.h> # include <sys/mount.h> #endif #include <fcntl.h> -- 2.4.6

2 7

[libvirt] [PATCH v2 1/3] cgroup: don't include sys/mount.h if not needed
by Roman Bogorodskiy 11 Jan '16

11 Jan '16

From: Jasper Lievisse Adriaanse <jasper(a)openbsd.org> As cgroup implementation only works on Linux, it does not make much sense to include sys/mount.h if other requirements are not met, such as HAVE_MNTENT_H and HAVE_GETMNTENT_R. Also, it fixes build on OpenBSD that requires to include sys/param.h along with sys/mount.h. Signed-off-by: Roman Bogorodskiy <bogorodskiy(a)gmail.com> --- src/util/vircgroup.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index e39c4d1..78f519c 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -24,10 +24,9 @@ #include <config.h> #include <stdio.h> -#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R +#if defined HAVE_MNTENT_H && defined HAVE_SYS_MOUNT_H \ + && defined HAVE_GETMNTENT_R # include <mntent.h> -#endif -#if defined HAVE_SYS_MOUNT_H # include <sys/mount.h> #endif #include <fcntl.h> -- 2.4.6

3 6

[libvirt] [PATCH] vmx: Adapt to emptyBackingString for cdrom-image
by Michal Privoznik 11 Jan '16

11 Jan '16

https://bugzilla.redhat.com/show_bug.cgi?id=1266088 We are missing this value for cdrom-image device. Honestly, I have no idea whether it can happen for other disk devices too. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/vmx/vmx.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 568b2c7..baf27de 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2317,6 +2317,16 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con */ goto ignore; } + } else if (STREQ(fileName, "emptyBackingString")) { + if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Expecting VMX entry '%s' to be 'cdrom-image' " + "but found '%s'"), deviceType_name, deviceType); + goto cleanup; + } + + virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE); + ignore_value(virDomainDiskSetSource(*def, NULL)); } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid or not yet handled value '%s' " -- 2.4.10

3 2

[libvirt] [PATCH 0/5] Couple of wireshark fixes
by Michal Privoznik 11 Jan '16

11 Jan '16

Well, I've just updated wireshark on my system and encountered couple of compile errors while building libvirt. Here are the fixes. Fortunately, none of them requires us to increase the version number of wireshark that's required. However, I'd like to discuss how are we going to handle this. I mean, at wireshark they don't seem so committed to API stability as we are. Therefore this patch set. In the long term I don't see us adapting to every single API change, or do I? Although I am not sure what are our options here. Michal Privoznik (5): wireshark: s/proto_tree_add_text/proto_tree_add_item/ wireshark: s/ep_alloc/wmem_alloc/ wireshark: s/tvb_length/tvb_captured_length/ wireshark: Replace WIRESHARK_COMPAT with actual version comparison wireshark: Fix header of get_message_len() tools/wireshark/src/packet-libvirt.c | 27 ++++++++++++++------------- tools/wireshark/util/genxdrstub.pl | 7 +++++-- 2 files changed, 19 insertions(+), 15 deletions(-) -- 2.4.10

2 6

[libvirt] [PATCH 0/3] several cgroups/cpuset fixes
by Henning Schild 11 Jan '16

11 Jan '16

Hi, i already explained some of the cgroup problems in some detail so i will not do that again. https://www.redhat.com/archives/libvir-list/2015-October/msg00876.html I managed to solve some of the problems in the current codebase, and am now sharing the patches. But they are really just half of what i had to change to get libvirt to behave in a system with isolated cpus. Other changes/hacks i am not sending here because they do not work for the general case: - create machine.slice before starting libvirtd (smaller than root) ... and hope it wont grow - disabling cpuset.cpus inheritance in libvirtd - allowing only xml with fully specified cputune - set machine cpuset to (vcpupins | emulatorpin) I am not sure how useful the individual fixes are, i am sending them as concrete examples for the problems i described earlier. And i am hoping that will start a discussion. Henning Henning Schild (3): util: cgroups do not implicitly add task to new machine cgroup qemu: do not put a task into machine cgroup qemu cgroups: move new threads to new cgroup after cpuset is set up src/lxc/lxc_cgroup.c | 6 ++++++ src/qemu/qemu_cgroup.c | 23 ++++++++++++++--------- src/util/vircgroup.c | 22 ---------------------- 3 files changed, 20 insertions(+), 31 deletions(-) -- 2.4.10

2 23