[libvirt] Bug in RPC code causes failure to start LXC container using virDomainCreateXMLWithFiles
by Ben Gray
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 */
9 years, 4 months
[libvirt] DNS for IPv6 addresses?
by Yaniv Kaul
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.
9 years, 4 months
[libvirt] [PATCH 0/5] logging fixes
by Laine Stump
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
9 years, 4 months
[libvirt] [PATCH] qemu: use enum when setting PCI "multi" value, not 0 or 1
by Laine Stump
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
9 years, 4 months
[libvirt] [PATCH 0/5] auto-add USB2 controller set for Q35
by Laine Stump
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
9 years, 4 months
[libvirt] [PATCH 1/2] cgroup: fix including sys/mount.h for OpenBSD
by Roman Bogorodskiy
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
9 years, 4 months
[libvirt] [PATCH v2 1/3] cgroup: don't include sys/mount.h if not needed
by Roman Bogorodskiy
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
9 years, 4 months
[libvirt] [PATCH] vmx: Adapt to emptyBackingString for cdrom-image
by Michal Privoznik
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
9 years, 4 months
[libvirt] [PATCH 0/5] Couple of wireshark fixes
by Michal Privoznik
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
9 years, 4 months
[libvirt] [PATCH 0/3] several cgroups/cpuset fixes
by Henning Schild
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
9 years, 4 months
