September 2017 - Devel - libvirt List Archives

[libvirt] Questions about function virPCIDeviceIsBehindSwitchLackingACS in virpci.c
by Wuzongyong (Euler Dept) 21 Sep '17

21 Sep '17

Hi, In function virPCIDeviceIsBehindSwitchLackingACS, I noticed that(line 8): 1 if (virPCIDeviceGetParent(dev, &parent) < 0) 2 return -1; 3 if (!parent) { 4 /* if we have no parent, and this is the root bus, ACS doesn't come 5 * into play since devices on the root bus can't P2P without going 6 * through the root IOMMU. 7 */ 8 if (dev->address.bus == 0) { 9 return 0; 10 } else { 11 virReportError(VIR_ERR_INTERNAL_ERROR, 12 _("Failed to find parent device for %s"), 13 dev->name); 14 return -1; 15 } 16 } Why we just return 0 only if device's bus is 0? In my server, I can see a root bus which bus number is greater than 0, see the results(just a part) after I run lspci -t: +-[0000:80]-+-02.0-[81-83]--+-00.0 | | \-00.1 | +-05.0 | +-05.1 | +-05.2 | \-05.4 +-[0000:7f]-+-08.0 | +-08.2 | +-08.3 | + . . . | \-1f.2 \-[0000:00]-+-00.0 +-01.0-[01]----00.0 +-02.0-[02]--+-00.0 | +-00.1 | +-00.2 | \-00.3 +-02.2-[03]-- +-03.0-[04-0b]----00.0-[05-0b]--+-08.0-[06-08]----00.0 | \-10.0-[09-0b]----00.0 +-05.0 +-05.1 +-05.2 +-05.4 +-11.0 +-11.4 +-16.0 +-16.1 +-1a.0 If I assign the device 0000:81:00.0 to a VM, I get "Failed to find parent device". I think I should get no error with return value 0 just like bus number is 0, because bus 80 is the root bus as well in my case. In the <<Intel C610 Series Chipset and Intel X99 Chipset Platform Controller Hub(PCH)>> Datasheet, I found that(Chapter 9.1): For some server platforms, it may be desirable to have multiple PCHs in the system Which means some PCH's may reside on a bus greater than 0. So, is this a bug? Thanks, Zongyong Wu

4 6

[libvirt] [PATCH] iohelper: avoid calling read() with misaligned buffers for O_DIRECT
by Daniel P. Berrange 20 Sep '17

20 Sep '17

The iohelper currently calls saferead() to get data from the underlying file. This has a problem with O_DIRECT when hitting end-of-file. saferead() is asked to read 1MB, but the first read() it does may return only a few KB, so it'll try another read() to fill the remaining buffer. Unfortunately the buffer pointer passed into this 2nd read() is likely not aligned to the extent that O_DIRECT requires, so rather than seeing '0' for end-of-file, we'll get -1 + EINVAL due to misaligned buffer. The way the iohelper is currently written, it already handles getting short reads, so there is actually no need to use saferead() at all. We can simply call read() directly. The benefit of this is that we can now write() the data immediately so when we go into the subsequent reads() we'll always have a correctly aligned buffer. Technically the file position ought to be aligned for O_DIRECT too, but this does not appear to matter when at end-of-file. Tested-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/util/iohelper.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/iohelper.c b/src/util/iohelper.c index fe15a92e6..5416d4506 100644 --- a/src/util/iohelper.c +++ b/src/util/iohelper.c @@ -109,7 +109,9 @@ runIO(const char *path, int fd, int oflags) while (1) { ssize_t got; - if ((got = saferead(fdin, buf, buflen)) < 0) { + if ((got = read(fdin, buf, buflen)) < 0) { + if (errno == EINTR) + continue; virReportSystemError(errno, _("Unable to read %s"), fdinname); goto cleanup; } -- 2.13.5

2 1

[libvirt] [PATCH v2] iohelper: fix reading with O_DIRECT
by Nikolay Shirokovskiy 20 Sep '17

20 Sep '17

saferead is not suitable for direct reads. If file size is not multiple of align size then we get EINVAL on the read(2) that is supposed to return 0 because read buffer will not be aligned at this point. Let's not read again after partial read and check that we read everything by comparing the number of total bytes read against file size. --- Diff from v1: - a couple of cosmetic changes src/util/iohelper.c | 42 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/src/util/iohelper.c b/src/util/iohelper.c index fe15a92..9aa6ae0 100644 --- a/src/util/iohelper.c +++ b/src/util/iohelper.c @@ -56,6 +56,7 @@ runIO(const char *path, int fd, int oflags) unsigned long long total = 0; bool direct = O_DIRECT && ((oflags & O_DIRECT) != 0); off_t end = 0; + off_t cur; #if HAVE_POSIX_MEMALIGN if (posix_memalign(&base, alignMask + 1, buflen)) { @@ -78,10 +79,22 @@ runIO(const char *path, int fd, int oflags) fdoutname = "stdout"; /* To make the implementation simpler, we give up on any * attempt to use O_DIRECT in a non-trivial manner. */ - if (direct && ((end = lseek(fd, 0, SEEK_CUR)) != 0)) { - virReportSystemError(end < 0 ? errno : EINVAL, "%s", - _("O_DIRECT read needs entire seekable file")); - goto cleanup; + if (direct) { + if ((cur = lseek(fd, 0, SEEK_CUR)) != 0) { + virReportSystemError(cur < 0 ? errno : EINVAL, "%s", + _("O_DIRECT read needs entire seekable file")); + goto cleanup; + } + + if ((end = lseek(fd, 0, SEEK_END)) < 0) { + virReportSystemError(errno, "%s", _("can not seek file end")); + goto cleanup; + } + + if (lseek(fd, cur, SEEK_SET) < 0) { + virReportSystemError(errno, "%s", _("can not seek file begin")); + goto cleanup; + } } break; case O_WRONLY: @@ -109,7 +122,26 @@ runIO(const char *path, int fd, int oflags) while (1) { ssize_t got; - if ((got = saferead(fdin, buf, buflen)) < 0) { + /* in case of O_DIRECT we cannot read again to check for EOF + * after partial buffer read as it is done in saferead */ + if (direct && fdin == fd && end - total < buflen) { + if (total == end) + break; + + while ((got = read(fd, buf, buflen)) < 0 && errno == EINTR) + ; + + if (got < end - total) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to read last chunk from %s"), + fdinname); + goto cleanup; + } + } else { + got = saferead(fdin, buf, buflen); + } + + if (got < 0) { virReportSystemError(errno, _("Unable to read %s"), fdinname); goto cleanup; } -- 1.8.3.1

2 9

[libvirt] [PATCH] vhost-user: add support reconnect for vhost-user ports
by ZhiPeng Lu 20 Sep '17

20 Sep '17

For vhost-user ports, Open vSwitch acts as the server and QEMU the client. When OVS crashed or restart, QEMU shoule be reconnect to OVS. Signed-off-by: ZhiPeng Lu <lu.zhipeng(a)zte.com.cn> --- docs/formatdomain.html.in | 6 +++-- docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 28 ++++++++++++++++++++-- .../qemuxml2argv-net-vhostuser-multiq.args | 2 +- .../qemuxml2argv-net-vhostuser-multiq.xml | 2 +- 5 files changed, 37 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 8ca7637..ffe45c2 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5660,7 +5660,7 @@ qemu-kvm -net nic,model=? /dev/null </interface> <interface type='vhostuser'> <mac address='52:54:00:3b:83:1b'/> - <source type='unix' path='/tmp/vhost2.sock' mode='client'/> + <source type='unix' path='/tmp/vhost2.sock' mode='client' reconnect='10'/> <model type='virtio'/> <driver queues='5'/> </interface> @@ -5675,7 +5675,9 @@ qemu-kvm -net nic,model=? /dev/null Both <code>mode='server'</code> and <code>mode='client'</code> are supported. vhost-user requires the virtio model type, thus the - <code><model></code> element is mandatory. + <code><model></code> element is mandatory. <code>reconnect</code> + is an optional element,which configures reconnect timeout if the + connection is lost. </p> <h5><a id="elementNwfilter">Traffic filtering with NWFilter</a></h5> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 06c5a91..82f30ae 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2383,6 +2383,11 @@ <value>client</value> </choice> </attribute> + <optional> + <attribute name="reconnect"> + <ref name='positiveInteger'/> + </attribute> + </optional> <empty/> </element> <ref name="interface-options"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2fc1fc3..f9c3b35 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10176,6 +10176,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, char *vhostuser_mode = NULL; char *vhostuser_path = NULL; char *vhostuser_type = NULL; + char *vhostuser_reconnect = NULL; char *trustGuestRxFilters = NULL; char *vhost_path = NULL; virNWFilterHashTablePtr filterparams = NULL; @@ -10262,11 +10263,12 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } } else if (!vhostuser_path && !vhostuser_mode && !vhostuser_type - && def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && - virXMLNodeNameEqual(cur, "source")) { + && !vhostuser_reconnect && def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER + && virXMLNodeNameEqual(cur, "source")) { vhostuser_type = virXMLPropString(cur, "type"); vhostuser_path = virXMLPropString(cur, "path"); vhostuser_mode = virXMLPropString(cur, "mode"); + vhostuser_reconnect = virXMLPropString(cur, "reconnect"); } else if (!def->virtPortProfile && virXMLNodeNameEqual(cur, "virtualport")) { if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) { @@ -10478,6 +10480,11 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, "type='vhostuser'/>")); goto error; } + if (vhostuser_reconnect != NULL && STREQ(vhostuser_mode, "server")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'reconnect' attribute unsupported " + "'server' mode for <interface type='vhostuser'>")); + } if (VIR_ALLOC(def->data.vhostuser) < 0) goto error; @@ -10490,6 +10497,17 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, def->data.vhostuser->data.nix.listen = true; } else if (STREQ(vhostuser_mode, "client")) { def->data.vhostuser->data.nix.listen = false; + if (vhostuser_reconnect != NULL) { + def->data.vhostuser->data.nix.reconnect.enabled = true; + if (virStrToLong_ui(vhostuser_reconnect, NULL, 0, + &def->data.vhostuser->data.nix.reconnect.timeout) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("vhost-user reconnect attribute is invalid")); + vhostuser_reconnect = NULL; + def->data.vhostuser->data.nix.reconnect.enabled = false; + goto error; + } + } } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Wrong <source> 'mode' attribute " @@ -10937,6 +10955,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(vhostuser_type); VIR_FREE(vhostuser_path); VIR_FREE(vhostuser_mode); + VIR_FREE(vhostuser_reconnect); VIR_FREE(ifname); VIR_FREE(ifname_guest); VIR_FREE(ifname_guest_actual); @@ -22928,6 +22947,11 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " mode='%s'", def->data.vhostuser->data.nix.listen ? "server" : "client"); + if (def->data.vhostuser->data.nix.reconnect.enabled == true) { + virBufferAsprintf(buf, " reconnect='%u'", + def->data.vhostuser->data.nix.reconnect.timeout); + } + sourceLines++; } break; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args index b69ebd8..996828f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args @@ -32,7 +32,7 @@ addr=0x4 \ -netdev socket,listen=:2015,id=hostnet2 \ -device rtl8139,netdev=hostnet2,id=net2,mac=52:54:00:95:db:c0,bus=pci.0,\ addr=0x5 \ --chardev socket,id=charnet3,path=/tmp/vhost2.sock \ +-chardev socket,id=charnet3,path=/tmp/vhost2.sock,reconnect=10 \ -netdev vhost-user,chardev=charnet3,queues=4,id=hostnet3 \ -device virtio-net-pci,mq=on,vectors=10,netdev=hostnet3,id=net3,\ mac=52:54:00:ee:96:6d,bus=pci.0,addr=0x6 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.xml index d5c42fe..7eb6fa0 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.xml @@ -40,7 +40,7 @@ </interface> <interface type='vhostuser'> <mac address='52:54:00:ee:96:6d'/> - <source type='unix' path='/tmp/vhost2.sock' mode='client'/> + <source type='unix' path='/tmp/vhost2.sock' mode='client' reconnect='10'/> <model type='virtio'/> <driver queues='4'/> </interface> -- 1.8.3.1

3 4

[libvirt] [PATCH v2 0/2] qemu: Guarantee correct ordering for controllers
by Andrea Bolognani 20 Sep '17

20 Sep '17

Changes from [v1] * ensure the controllers are ordered correctly from the moment they are added to the guest configuration rather than waiting for the QEMU command line to be generated. [v1] https://www.redhat.com/archives/libvir-list/2017-September/msg00084.html Andrea Bolognani (2): conf: Add USB companion controllers with virDomainControllerInsert() conf: Add all controllers using virDomainControllerInsert() src/conf/domain_conf.c | 61 +++++++++++++++------- .../qemuxml2argv-channel-virtio-default.args | 2 +- .../qemuxml2argv-channel-virtio-unix.args | 2 +- .../qemuxml2argv-chardev-reconnect.args | 2 +- .../qemuxml2argv-pci-autoadd-idx.args | 16 +++--- .../qemuxml2argv-pseries-many-buses-2.args | 4 +- .../qemuxml2xmlout-aarch64-virtio-pci-default.xml | 6 +-- ...2xmlout-aarch64-virtio-pci-manual-addresses.xml | 6 +-- .../qemuxml2xmlout-channel-virtio-auto.xml | 2 +- ...qemuxml2xmlout-hostdev-scsi-autogen-address.xml | 6 +-- .../qemuxml2xmlout-pci-autoadd-idx.xml | 22 ++++---- .../qemuxml2xmlout-pseries-many-buses-2.xml | 6 +-- .../qemuxml2xmlout-pseries-phb-default-missing.xml | 8 +-- 13 files changed, 82 insertions(+), 61 deletions(-) -- 2.13.5

2 4

[libvirt] [PATCH] news: remove kernel version reference from switchdev entry
by Ján Tomko 20 Sep '17

20 Sep '17

The functionality was added in 4.8, but due to a rename of the DEVLINK_CMD_ESWITCH_GET constant in the kernel headers, the headers from kernel 4.11 are required by the libvirt code. Remove the reference from the news entry, since it could be misleading. --- docs/news.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/news.xml b/docs/news.xml index c48869c55..aab812b25 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -56,7 +56,7 @@ <description> Allow querying the NIC interface capabilities for the availability of switchdev offloading (also known as - kernel-forward-plane-offload introduced in kernel 4.8). + kernel-forward-plane-offload). </description> </change> </section> -- 2.13.0

2 1

[libvirt] [PATCH v2] util: Fix configure.ac check for DEVLINK_CMD_ESWITCH_GET
by John Ferlan 20 Sep '17

20 Sep '17

Turns out the mechanism of providing multiple definitions to check via AC_CHECK_DECLS in order to determine whether HAVE_DECL_DEVLINK should be set resulted in a false positive since as long as one of the defs was true, then the HAVE_DECL_DEVLINK got defined. But we cannot just check for the symbol we most care about due to a complication in a kernel 4.11 change that altered the name of the symbol from DEVLINK_CMD_ESWITCH_MODE_GET to DEVLINK_CMD_ESWITCH_GET in the newer kernel and added a #define for the former name with an ominous comment/caveat that the old format is obsolete should never be used. However, if we only looked for the new symbol, that would leave kernels 4.8 through 4.10 unable to use the command even though it exists under an old name. So, rather than checking for whether the devlink.h on the system has multiple symbols, let's only check for whether the command we want is defined. This patch checks for both symbol names in order to allow usage and then adds a check to see if the new name doesn't exist after linux/devlink.h is include, then to use #define of the new name to the old name - essentially the reverse logic that the 4.11 header file has. Note that we do not need to check for other symbols as well since they already existed prior to when the various ESWITCH symbols were added. Other defintions include DEVLINK_GENL_VERSION, DEVLINK_GENL_NAME, DEVLINK_ATTR_MAX, DEVLINK_ATTR_BUS_NAME, and DEVLINK_ATTR_DEV_NAME. The DEVLINK_ATTR_ESWITCH_MODE and DEVLINK_ESWITCH_MODE_SWITCHDEV were added at the same time as DEVLINK_CMD_ESWITCH_MODE_GET, but it's only DEVLINK_CMD_ESWITCH_MODE_GET that changed in 4.11. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- v1: https://www.redhat.com/archives/libvir-list/2017-September/msg00543.html Just to make it official - here's what I described in the v1 followup which got the ACK if comment from Andrea: https://www.redhat.com/archives/libvir-list/2017-September/msg00591.html Still, I won't push this yet until it too gets an official blessing. configure.ac | 8 ++++++-- src/util/virnetdev.c | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index c9509c7f9..73ae7fdd5 100644 --- a/configure.ac +++ b/configure.ac @@ -630,12 +630,16 @@ fi dnl dnl check for kernel headers required by devlink dnl +dnl kernel 4.8 introduced DEVLINK_CMD_ESWITCH_MODE_GET, but that was +dnl later changed in kernel 4.11 to be just DEVLINK_CMD_ESWITCH_GET, so +dnl for "completeness" let's allow HAVE_DECL_DEVLINK to be define if +dnl either is defined. if test "$with_linux" = "yes"; then AC_CHECK_HEADERS([linux/devlink.h]) - AC_CHECK_DECLS([DEVLINK_GENL_VERSION, DEVLINK_GENL_NAME, DEVLINK_ATTR_MAX, DEVLINK_CMD_ESWITCH_GET, DEVLINK_ATTR_BUS_NAME, DEVLINK_ATTR_DEV_NAME, DEVLINK_ATTR_ESWITCH_MODE, DEVLINK_ESWITCH_MODE_SWITCHDEV], + AC_CHECK_DECLS([DEVLINK_CMD_ESWITCH_GET, DEVLINK_CMD_ESWITCH_MODE_GET], [AC_DEFINE([HAVE_DECL_DEVLINK], [1], - [whether devlink declarations are available])], + [whether devlink CMD_ESWITCH_GET is available])], [], [[#include <linux/devlink.h>]]) fi diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 040693925..435ca5562 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -61,6 +61,9 @@ #if HAVE_DECL_DEVLINK # include <linux/devlink.h> +# ifndef DEVLINK_CMD_ESWITCH_GET +# define DEVLINK_CMD_ESWITCH_GET DEVLINK_CMD_ESWITCH_MODE_GET +# endif #endif #ifndef IFNAMSIZ -- 2.13.5

3 3

[libvirt] [PATCH] Stop linking tests/commandhelper to libvirt code
by Daniel P. Berrange 20 Sep '17

20 Sep '17

The commandhelper binary is a helper for commandtest that validates what file handles were inherited. For this to work reliably we must not have any libraries that leak file descriptors into commandhelper. Unfortunately some versions of gnutls will intentionally open file handles at library load time via a constructor function. We previously hacked around this in commit 4cbc15d037e1cd8abf5c4aa6acc30d83ae13e34d Author: Martin Kletzander <mkletzan(a)redhat.com> Date: Fri May 2 09:55:52 2014 +0200 tests: don't fail with newer gnutls gnutls-3.3.0 and newer leaves 2 FDs open in order to be backwards compatible when it comes to chrooted binaries [1]. Linking commandhelper with gnutls then leaves these two FDs open and commandtest fails thanks to that. This patch does not link commandhelper with libvirt.la, but rather only the utilities making the test pass. Based on suggestion from Daniel [2]. [1] http://lists.gnutls.org/pipermail/gnutls-help/2014-April/003429.html [2] https://www.redhat.com/archives/libvir-list/2014-April/msg01119.html That fix relied on fact that while libvirt.so linked with gnutls, libvirt_util.la did not link to it. With the introduction of the util/vircrypto.c file that assumption is no longer valid. We must not link to libvirt_util.la at all - only gnulib and libc can (hopefully) be relied on not to open random file descriptors in constructors. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- cfg.mk | 8 ++++---- tests/Makefile.am | 6 ++++-- tests/commandhelper.c | 29 +++++++++++++---------------- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/cfg.mk b/cfg.mk index 56cb14b..f2a053f 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1111,7 +1111,7 @@ $(srcdir)/src/admin/admin_client.h: $(srcdir)/src/admin/admin_protocol.x exclude_file_name_regexp--sc_avoid_strcase = ^tools/vsh\.h$$ _src1=libvirt-stream|qemu/qemu_monitor|util/vir(command|file|fdstream)|xen/xend_internal|rpc/virnetsocket|lxc/lxc_controller|locking/lock_daemon|logging/log_daemon -_test1=shunloadtest|virnettlscontexttest|virnettlssessiontest|vircgroupmock +_test1=shunloadtest|virnettlscontexttest|virnettlssessiontest|vircgroupmock|commandhelper exclude_file_name_regexp--sc_avoid_write = \ ^(src/($(_src1))|daemon/libvirtd|tools/virsh-console|tests/($(_test1)))\.c$$ @@ -1146,10 +1146,10 @@ exclude_file_name_regexp--sc_prohibit_asprintf = \ ^(cfg\.mk|bootstrap.conf$$|examples/|src/util/virstring\.[ch]$$|tests/vircgroupmock\.c$$) exclude_file_name_regexp--sc_prohibit_strdup = \ - ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup)mock.c$$) + ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup)mock.c|tests/commandhelper\.c$$) exclude_file_name_regexp--sc_prohibit_close = \ - (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\.c|tests/vir.+mock\.c)$$) + (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\.c|tests/vir.+mock\.c|tests/commandhelper\.c)$$) exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ (^tests/(qemuhelp|virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$) @@ -1173,7 +1173,7 @@ exclude_file_name_regexp--sc_prohibit_select = \ ^cfg\.mk$$ exclude_file_name_regexp--sc_prohibit_raw_allocation = \ - ^(docs/hacking\.html\.in|src/util/viralloc\.[ch]|examples/.*|tests/(securityselinuxhelper|(vircgroup|nss)mock)\.c|tools/wireshark/src/packet-libvirt\.c)$$ + ^(docs/hacking\.html\.in|src/util/viralloc\.[ch]|examples/.*|tests/(securityselinuxhelper|(vircgroup|nss)mock|commandhelper)\.c|tools/wireshark/src/packet-libvirt\.c)$$ exclude_file_name_regexp--sc_prohibit_readlink = \ ^src/(util/virutil|lxc/lxc_container)\.c$$ diff --git a/tests/Makefile.am b/tests/Makefile.am index 8138885..0b2305d 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -946,12 +946,14 @@ commandtest_SOURCES = \ commandtest.c testutils.h testutils.c commandtest_LDADD = $(LDADDS) +# Must not link to any libvirt modules - libc / gnulib only +# otherwise external libraries might unexpectedly leak +# file descriptors into commandhelper invalidating the +# test logic assumptions commandhelper_SOURCES = \ commandhelper.c commandhelper_LDADD = \ $(NO_INDIRECT_LDFLAGS) \ - $(PROBES_O) \ - ../src/libvirt_util.la \ $(GNULIB_LIBS) commandhelper_LDFLAGS = -static diff --git a/tests/commandhelper.c b/tests/commandhelper.c index 015efda..e9e6353 100644 --- a/tests/commandhelper.c +++ b/tests/commandhelper.c @@ -28,11 +28,6 @@ #include <sys/stat.h> #include "internal.h" -#include "virutil.h" -#include "viralloc.h" -#include "virfile.h" -#include "testutils.h" -#include "virstring.h" #ifndef WIN32 @@ -50,11 +45,13 @@ static int envsort(const void *a, const void *b) char *bkey; int ret; - ignore_value(VIR_STRNDUP_QUIET(akey, astr, aeq - astr)); - ignore_value(VIR_STRNDUP_QUIET(bkey, bstr, beq - bstr)); + if (!(akey = strndup(astr, aeq - astr))) + abort(); + if (!(bkey = strndup(bstr, beq - bstr))) + abort(); ret = strcmp(akey, bkey); - VIR_FREE(akey); - VIR_FREE(bkey); + free(akey); + free(bkey); return ret; } @@ -80,8 +77,8 @@ int main(int argc, char **argv) { origenv++; } - if (VIR_ALLOC_N_QUIET(newenv, n) < 0) - goto cleanup; + if (!(newenv = malloc(sizeof(*newenv) * n))) + abort(); origenv = environ; n = i = 0; @@ -120,7 +117,7 @@ int main(int argc, char **argv) { STREQ(cwd + strlen(cwd) - strlen("/commanddata"), "/commanddata")) strcpy(cwd, ".../commanddata"); fprintf(log, "CWD:%s\n", cwd); - VIR_FREE(cwd); + free(cwd); fprintf(log, "UMASK:%04o\n", umask(0)); @@ -144,9 +141,9 @@ int main(int argc, char **argv) { goto cleanup; if (got == 0) break; - if (safewrite(STDOUT_FILENO, buf, got) != got) + if (write(STDOUT_FILENO, buf, got) != got) goto cleanup; - if (safewrite(STDERR_FILENO, buf, got) != got) + if (write(STDERR_FILENO, buf, got) != got) goto cleanup; } @@ -158,8 +155,8 @@ int main(int argc, char **argv) { ret = EXIT_SUCCESS; cleanup: - VIR_FORCE_FCLOSE(log); - VIR_FREE(newenv); + fclose(log); + free(newenv); return ret; } -- 2.5.5

2 2

[libvirt] [PATCH v2] Fix libvirtd crash in qemuDomainGetTLSObjects
by Ashish Mittal 20 Sep '17

20 Sep '17

Passing a NULL value for the argument secAlias to the function qemuDomainGetTLSObjects causes a segmentation fault. Thread 3 "libvirtd" received signal SIGSEGV, Segmentation fault. 0x00007f97c9c42a3d in qemuDomainGetTLSObjects (..., secAlias=0x0) at qemu/qemu_hotplug.c:1736 Changed code to not dereference a NULL secAlias. Signed-off-by: Ashish Mittal <ashmit602(a)gmail.com> --- src/qemu/qemu_hotplug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7dd6e5f..9ecdf0a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1643,7 +1643,8 @@ qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps, } if (qemuBuildTLSx509BackendProps(tlsCertdir, tlsListen, tlsVerify, - *secAlias, qemuCaps, tlsProps) < 0) + secAlias ? *secAlias : NULL, qemuCaps, + tlsProps) < 0) return -1; if (!(*tlsAlias = qemuAliasTLSObjFromSrcAlias(srcAlias))) -- 2.5.5

4 3

[libvirt] [PATCH] Fix libvirtd crash in qemuDomainGetTLSObjects
by Ashish Mittal 20 Sep '17

20 Sep '17

Passing a NULL value for the argument secAlias to the function qemuDomainGetTLSObjects causes a segmentation fault. Thread 3 "libvirtd" received signal SIGSEGV, Segmentation fault. 0x00007f97c9c42a3d in qemuDomainGetTLSObjects (...,secAlias=0x0) at qemu/qemu_hotplug.c:1736 Changed code to not dereference a NULL secAlias. Signed-off-by: Ashish Mittal <ashmit602(a)gmail.com> --- src/qemu/qemu_hotplug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7dd6e5f..df58ecc 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1643,7 +1643,8 @@ qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps, } if (qemuBuildTLSx509BackendProps(tlsCertdir, tlsListen, tlsVerify, - *secAlias, qemuCaps, tlsProps) < 0) + **secAlias ? *secAlias : NULL, qemuCaps, + tlsProps) < 0) return -1; if (!(*tlsAlias = qemuAliasTLSObjFromSrcAlias(srcAlias))) -- 2.5.5

3 3