- Devel - libvirt List Archives

[PATCH v2 0/2] Fix broken build with readline
by Michal Privoznik 03 Sep '20

03 Sep '20

This is a v2 of: https://www.redhat.com/archives/libvir-list/2020-September/msg00107.html diff to v1: - Patch 1/2 is completely new, it removes include of readline from virsh.c and virt-admin.c, so that patch 2/2 can be unchanged. Michal Prívozník (2): vsh: Introduce wrapper for add_history() vsh: Define HAVE_STDARG_H before including readline tools/virsh.c | 10 ++-------- tools/virt-admin.c | 10 ++-------- tools/vsh.c | 16 ++++++++++++++++ tools/vsh.h | 2 ++ 4 files changed, 22 insertions(+), 16 deletions(-) -- 2.26.2

4 6

[libvirt PATCH] remote: adapt augeas test case for dynamic polkit config change
by Daniel P. Berrangé 03 Sep '20

03 Sep '20

We need to use @default_auth@ in the augeas test case to match its use in the main libvirtd.conf.in file. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- Pushed as a CI fix src/remote/test_libvirtd.aug.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/remote/test_libvirtd.aug.in b/src/remote/test_libvirtd.aug.in index 1e17397af6..56c4487a01 100644 --- a/src/remote/test_libvirtd.aug.in +++ b/src/remote/test_libvirtd.aug.in @@ -14,8 +14,8 @@ module Test_@DAEMON_NAME@ = { "unix_sock_rw_perms" = "0770" } { "unix_sock_admin_perms" = "0700" } { "unix_sock_dir" = "@runstatedir@/libvirt" } - { "auth_unix_ro" = "none" } - { "auth_unix_rw" = "none" } + { "auth_unix_ro" = "@default_auth@" } + { "auth_unix_rw" = "@default_auth@" } @CUT_ENABLE_IP@ { "auth_tcp" = "sasl" } { "auth_tls" = "none" } -- 2.26.2

1 0

[PATCH] network: drop use of dummy tap device in bridges
by Daniel P. Berrangé 03 Sep '20

03 Sep '20

A long time ago we introduced a dummy tap device (e.g. virbr0-nic) that we attached to the bridge device created for virtual networks: commit 5754dbd56d4738112a86776c09e810e32f7c3224 Author: Laine Stump <laine(a)redhat.com> Date: Wed Feb 9 03:28:12 2011 -0500 Give each virtual network bridge its own fixed MAC address This was a hack to workaround a Linux kernel bug where it would not honour any attempt to set a MAC address on a bridge. Instead the bridge would adopt the numerically lowest MAC address of all NICs attached to the bridge. This lead to the MAC addrss of the bridge changing over time as NICs were attached/detached. The Linux bug was actually fixed 3 years before the libvirt workaround was added in: commit 92c0574f11598c8036f81e27d2e8bdd6eed7d76d Author: Stephen Hemminger <shemminger(a)vyatta.com> Date: Tue Jun 17 16:10:06 2008 -0700 bridge: make bridge address settings sticky Normally, the bridge just chooses the smallest mac address as the bridge id and mac address of bridge device. But if the administrator has explictly set the interface address then don't change it. Signed-off-by: Stephen Hemminger <shemminger(a)vyatta.com> Signed-off-by: David S. Miller <davem(a)davemloft.net> but libvirt needed to support RHEL-5 kernels at that time, so none the less added the workaround. We have long since dropped support for RHEL-5 vintage distros, so there's no reason to keep the dummy tap device for the purpose of setting the bridge MAC address. Later the dummy TAP device was used for a second purpose related to IPv6 DAD (Duplicate Address Detection) in: commit db488c79173b240459c7754f38c3c6af9b432970 Author: Benjamin Cama <benoar(a)dolka.fr> Date: Wed Sep 26 21:02:20 2012 +0200 network: fix dnsmasq/radvd binding to IPv6 on recent kernels This was again dealing with a regression in the Linux kernel, where if there were no devices attached to the bridge in the UP state, IPv6 DAD would not be performed. The virbr0-nic was attached but in the DOWN state, so the above libvirt fix tenporarily brought the NIC online. The Linux commit causing the problem was in v2.6.38 commit 1faa4356a3bd89ea11fb92752d897cff3a20ec0e Author: stephen hemminger <shemminger(a)vyatta.com> Date: Mon Mar 7 08:34:06 2011 +0000 bridge: control carrier based on ports online A short while later Linux was tweaked so that DAD would still occur if the bridge had no attached devices at all in 3.1: commit b64b73d7d0c480f75684519c6134e79d50c1b341 Author: stephen hemminger <shemminger(a)vyatta.com> Date: Mon Oct 3 18:14:45 2011 +0000 bridge: leave carrier on for empty bridge IOW, the only reason we need the DAD hack of bringing virbr0-nic online is because virbr0-nic exists. Once it doesn't exist, then we hit the "empty bridge" case which works in Linux. We can rely on distros having Linux kernel >= 3.1, so both things that the virbr0-nic are doing are redundant. Fixes https://gitlab.com/libvirt/libvirt/-/issues/53 Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/network/bridge_driver.c | 58 +++++-------------------------------- 1 file changed, 8 insertions(+), 50 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index b016d86b9f..5c00befc16 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2247,8 +2247,7 @@ networkAddAddrToBridge(virNetworkObjPtr obj, static int -networkStartHandleMACTableManagerMode(virNetworkObjPtr obj, - const char *macTapIfName) +networkStartHandleMACTableManagerMode(virNetworkObjPtr obj) { virNetworkDefPtr def = virNetworkObjGetDef(obj); const char *brname = def->bridge; @@ -2257,12 +2256,6 @@ networkStartHandleMACTableManagerMode(virNetworkObjPtr obj, def->macTableManager == VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT) { if (virNetDevBridgeSetVlanFiltering(brname, true) < 0) return -1; - if (macTapIfName) { - if (virNetDevBridgePortSetLearning(brname, macTapIfName, false) < 0) - return -1; - if (virNetDevBridgePortSetUnicastFlood(brname, macTapIfName, false) < 0) - return -1; - } } return 0; } @@ -2330,10 +2323,8 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, virErrorPtr save_err = NULL; virNetworkIPDefPtr ipdef; virNetDevIPRoutePtr routedef; - g_autofree char *macTapIfName = NULL; virMacMapPtr macmap; g_autofree char *macMapFile = NULL; - int tapfd = -1; bool dnsmasqStarted = false; bool devOnline = false; bool firewalRulesAdded = false; @@ -2360,29 +2351,6 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, if (virNetDevBridgeCreate(def->bridge, &def->mac) < 0) return -1; - if (def->mac_specified) { - /* To set a mac for the bridge, we need to define a dummy tap - * device, set its mac, then attach it to the bridge. As long - * as its mac address is lower than any other interface that - * gets attached, the bridge will always maintain this mac - * address. - */ - macTapIfName = networkBridgeDummyNicName(def->bridge); - if (!macTapIfName) - goto error; - /* Keep tun fd open and interface up to allow for IPv6 DAD to happen */ - if (virNetDevTapCreateInBridgePort(def->bridge, - &macTapIfName, &def->mac, - NULL, NULL, &tapfd, 1, NULL, NULL, - VIR_TRISTATE_BOOL_NO, - NULL, def->mtu, NULL, - VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE | - VIR_NETDEV_TAP_CREATE_IFUP | - VIR_NETDEV_TAP_CREATE_PERSIST) < 0) { - goto error; - } - } - if (!(macMapFile = virMacMapFileName(driver->dnsmasqStateDir, def->bridge)) || !(macmap = virMacMapNew(macMapFile))) @@ -2426,7 +2394,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, goto error; } - if (networkStartHandleMACTableManagerMode(obj, macTapIfName) < 0) + if (networkStartHandleMACTableManagerMode(obj) < 0) goto error; /* Bring up the bridge interface */ @@ -2482,15 +2450,6 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, if (v6present && networkWaitDadFinish(obj) < 0) goto error; - /* DAD has finished, dnsmasq is now bound to the - * bridge's IPv6 address, so we can set the dummy tun down. - */ - if (tapfd >= 0) { - if (virNetDevSetOnline(macTapIfName, false) < 0) - goto error; - VIR_FORCE_CLOSE(tapfd); - } - if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0) goto error; @@ -2514,16 +2473,11 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, def->forward.type != VIR_NETWORK_FORWARD_OPEN) networkRemoveFirewallRules(def); - if (macTapIfName) { - VIR_FORCE_CLOSE(tapfd); - ignore_value(virNetDevTapDelete(macTapIfName, NULL)); - } virNetworkObjUnrefMacMap(obj); ignore_value(virNetDevBridgeDelete(def->bridge)); virErrorRestore(&save_err); - /* coverity[leaked_handle] - 'tapfd' is not leaked */ return -1; } @@ -2555,9 +2509,13 @@ networkShutdownNetworkVirtual(virNetworkDriverStatePtr driver, if (dnsmasqPid > 0) kill(dnsmasqPid, SIGTERM); + /* We no longer create a dummy NIC, but if we've upgraded + * from old libvirt, we still need to delete any dummy NIC + * that might exist. Keep this logic around for a while... + */ if (def->mac_specified) { g_autofree char *macTapIfName = networkBridgeDummyNicName(def->bridge); - if (macTapIfName) + if (macTapIfName && virNetDevExists(macTapIfName)) ignore_value(virNetDevTapDelete(macTapIfName, NULL)); } @@ -2597,7 +2555,7 @@ networkStartNetworkBridge(virNetworkObjPtr obj) if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0) goto error; - if (networkStartHandleMACTableManagerMode(obj, NULL) < 0) + if (networkStartHandleMACTableManagerMode(obj) < 0) goto error; return 0; -- 2.25.4

2 3

[libvirt PATCH] remote: use SocketMode=0600 when polkit is not compiled
by Daniel P. Berrangé 03 Sep '20

03 Sep '20

The systemd .socket unit files we ship for libvirt daemons use SocketMode=0666 on the assumption that libvirt is built with polkit which provides access control. Some people, however, may have explicitly turned off polkit at build time and not realize that leaves them insecure unless they also change the SocketMode. This addresses that problem by making the SocketMode default to 0600 when polkit is disabled at compile time. Note we cannot automatically fix the case where the user compiles polkit, but then overrides the libvirtd.conf defaults to disable polkit. This is what lead to CVE-2020-15708 in Ubuntu 20.10. We can at least improve the inline comments in the config file to give a clearer warning though, which may have helped avoid the mistaken config. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/meson.build | 11 ++++++++++ src/remote/libvirtd.conf.in | 40 ++++++++++++++++++++++++++--------- src/remote/libvirtd.socket.in | 2 +- 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/meson.build b/src/meson.build index 5d8deaf548..897b5ecbca 100644 --- a/src/meson.build +++ b/src/meson.build @@ -713,6 +713,12 @@ foreach data : virt_daemon_confs daemon_conf.set('DAEMON_NAME_UC', name_uc) # to silence meson warning about missing 'CONFIG' in the configuration_data daemon_conf.set('CONFIG', '@CONFIG@') + if conf.has('WITH_POLKIT') + daemon_conf.set('default_auth', 'polkit') + else + daemon_conf.set('default_auth', 'none') + endif + if data.get('with_ip', false) conf_in = libvirtd_conf_tmp @@ -792,6 +798,11 @@ if conf.has('WITH_LIBVIRTD') unit_conf.set('service', unit['service']) unit_conf.set('sockprefix', unit['sockprefix']) unit_conf.set('deps', unit.get('deps', '')) + if conf.has('WITH_POLKIT') + unit_conf.set('mode', '0666') + else + unit_conf.set('mode', '0600') + endif configure_file( input: unit['service_in'], diff --git a/src/remote/libvirtd.conf.in b/src/remote/libvirtd.conf.in index 2607fbad86..ae6207bf54 100644 --- a/src/remote/libvirtd.conf.in +++ b/src/remote/libvirtd.conf.in @@ -127,6 +127,8 @@ # # Authentication. # +# There are the following choices available: +# # - none: do not perform auth checks. If you can connect to the # socket you are allowed. This is suitable if there are # restrictions on connecting to the socket (eg, UNIX @@ -144,21 +146,39 @@ # full read/write access (aka sudo like), while anyone # is allowed read/only access. # + # Set an authentication scheme for UNIX read-only sockets +# # By default socket permissions allow anyone to connect # -# To restrict monitoring of domains you may wish to enable -# an authentication mechanism here -#auth_unix_ro = "none" +# If libvirt was compiled without support for 'polkit', then +# no access control checks are done, but libvirt still only +# allows execution of APIs which don't change state. +# +# If libvirt was compiled with support for 'polkit', then +# the libvirt socket will perform a check with polkit after +# connections. The default policy still allows any local +# user access. +# +# To restrict monitoring of domains you may wish to either +# enable 'sasl' here, or change the polkit policy definition. +#auth_unix_ro = "@default_auth@" -# Set an authentication scheme for UNIX read-write sockets -# By default socket permissions only allow root. If PolicyKit -# support was compiled into libvirt, the default will be to -# use 'polkit' auth. +# Set an authentication scheme for UNIX read-write sockets. +# +# If libvirt was compiled without support for 'polkit', then +# the systemd .socket files will use SocketMode=0600 by default +# thus only allowing root user to connect, and 'auth_unix_rw' +# will default to 'none'. +# +# If libvirt was compiled with support for 'polkit', then +# the systemd .socket files will use SocketMode=0666 which +# allows any user to connect and 'auth_iunix_rw' will default +# to 'polkit'. If you disable use of 'polkit' here, then it +# is essential to change the systemd SocketMode parameter +# back to 0600, to avoid an insecure configuration. # -# If the unix_sock_rw_perms are changed you may wish to enable -# an authentication mechanism here -#auth_unix_rw = "none" +#auth_unix_rw = "@default_auth@" @CUT_ENABLE_IP@ # Change the authentication scheme for TCP sockets. diff --git a/src/remote/libvirtd.socket.in b/src/remote/libvirtd.socket.in index df36df2125..85b4aa800a 100644 --- a/src/remote/libvirtd.socket.in +++ b/src/remote/libvirtd.socket.in @@ -8,7 +8,7 @@ Before=@service@.service # when using systemd version < 227 ListenStream=@runstatedir@/libvirt/@sockprefix@-sock Service=@service@.service -SocketMode=0666 +SocketMode=@mode@ [Install] WantedBy=sockets.target -- 2.25.4

3 3

[PATCH] storage: only fallocate when allocation matches capacity
by Christian Ehrhardt 03 Sep '20

03 Sep '20

In c9ec7088 "storage: extend preallocation flags support for qemu-img" the option to fallocate was added and meant to be active when (quote): "the XML described storage <allocation> matches its <capacity>" Up until recently 81a3042a12 "storage_util: fix qemu-img sparse allocation" the compared allocation size was an order of magnitude too small, but still it does use fallocate too often unless capacity>allocation. This change fixes the comparison to match the intended description of the feature. Fixes: c9ec7088c7a3f4cd26bb471f1f243931fff6f4f9 Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1759454 Fixes: https://bugs.launchpad.net/ubuntu/focal/+source/libvirt/+bug/1847105 Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com> --- src/storage/storage_util.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index cf82ea0a87..85bed76863 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -710,10 +710,10 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr encinfo, virQEMUBuildQemuImgKeySecretOpts(&buf, encinfo, info->secretAlias); if (info->preallocate) { - if (info->size_arg > info->allocation) - virBufferAddLit(&buf, "preallocation=metadata,"); - else + if (info->size_arg == info->allocation) virBufferAddLit(&buf, "preallocation=falloc,"); + else + virBufferAddLit(&buf, "preallocation=metadata,"); } if (info->nocow) -- 2.28.0

4 8

[libvirt PATCH v2] meson: add -Wall and -Wextra explicitly
by Pavel Hrdina 03 Sep '20

03 Sep '20

If someone runs `meson setup --buildtype plain` meson ignores warning_level=2 that is in our meson.build file. The implication is that Meson will not automatically add -Wall which enables -Wformat. This breaks building libvirt from git with the buildtype set to plain. There is an issue reported [1] to not ignore warning_level silently and the change to ignore it was done by upstream commit [2]. This change makes Meson print warnings about using warning_level which as described above doesn't work for all build types. [1] <https://github.com/mesonbuild/meson/issues/7399> [2] <https://github.com/mesonbuild/meson/commit/8ee1c9a07a3a35e3ed262fbc358fd86c…> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- Chages in v2: - add -Wall and -Wextra explicitly regardless of the buildtype meson.build | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/meson.build b/meson.build index 1aad385ad1..1eadea33bf 100644 --- a/meson.build +++ b/meson.build @@ -228,6 +228,7 @@ cc_flags += [ '-Waddress', '-Waddress-of-packed-member', '-Waggressive-loop-optimizations', + '-Wall', '-Wattribute-warning', '-Wattributes', '-Wbool-compare', @@ -257,6 +258,7 @@ cc_flags += [ '-Wempty-body', '-Wendif-labels', '-Wexpansion-to-defined', + '-Wextra', '-Wformat-contains-nul', '-Wformat-extra-args', '-Wformat-nonliteral', @@ -503,10 +505,6 @@ cc_flags_disabled = [ # In meson this is specified using 'c_std=gnu99' in project() function. '-std=gnu99', - # In meson this is specified using 'warning_level=2' in project() function. - '-Wall', - '-Wextra', - # don't care about C++ compiler compat '-Wc++-compat', '-Wabi', -- 2.26.2

2 1

[libvirt PATCH] tests: change confusing macro agument names
by Jonathon Jongsma 03 Sep '20

03 Sep '20

Rather than use the names "fial" and "kep", use "fail" and "keep". In the DO_TEST() macro, to prevent the preprocessor replacing the struct member names during assignment, use the names "fail_" and "keep_" instead. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- tests/qemuhotplugtest.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 1e18820a2b..b023c49a69 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -641,28 +641,28 @@ mymain(void) } -#define DO_TEST(file, ACTION, dev, fial, kep, ...) \ +#define DO_TEST(file, ACTION, dev, fail_, keep_, ...) \ do { \ const char *my_mon[] = { __VA_ARGS__, NULL}; \ const char *name = file " " #ACTION " " dev; \ data.action = ACTION; \ data.domain_filename = file; \ data.device_filename = dev; \ - data.fail = fial; \ + data.fail = fail_; \ data.mon = my_mon; \ - data.keep = kep; \ + data.keep = keep_; \ if (virTestRun(name, testQemuHotplug, &data) < 0) \ ret = -1; \ } while (0) -#define DO_TEST_ATTACH(file, dev, fial, kep, ...) \ - DO_TEST(file, ATTACH, dev, fial, kep, __VA_ARGS__) +#define DO_TEST_ATTACH(file, dev, fail, keep, ...) \ + DO_TEST(file, ATTACH, dev, fail, keep, __VA_ARGS__) -#define DO_TEST_DETACH(file, dev, fial, kep, ...) \ - DO_TEST(file, DETACH, dev, fial, kep, __VA_ARGS__) +#define DO_TEST_DETACH(file, dev, fail, keep, ...) \ + DO_TEST(file, DETACH, dev, fail, keep, __VA_ARGS__) -#define DO_TEST_UPDATE(file, dev, fial, kep, ...) \ - DO_TEST(file, UPDATE, dev, fial, kep, __VA_ARGS__) +#define DO_TEST_UPDATE(file, dev, fail, keep, ...) \ + DO_TEST(file, UPDATE, dev, fail, keep, __VA_ARGS__) #define QMP_OK "{\"return\": {}}" -- 2.26.2

2 1

[PATCH] vsh: Define HAVE_STDARG_H before including readline
by Michal Privoznik 03 Sep '20

03 Sep '20

As it turned out my previous commits which switched from HAVE_ to WITH_ and dropped stdarg.h detection were a bit too aggressive. Because of reasons described in 9ea3424a178 we need to define HAVE_STDARG_H before including readline otherwise macos build fails. Honestly, I still don't fully understand the problem so I am not going to bother you with "explanation". Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tools/vsh.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/vsh.c b/tools/vsh.c index 11f493f969..d063a7f8cb 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -30,6 +30,10 @@ #include <signal.h> #if WITH_READLINE +/* In order to have proper rl_message declaration with older + * versions of readline, we have to declare this. See 9ea3424a178 + * for more info. */ +# define HAVE_STDARG_H # include <readline/readline.h> # include <readline/history.h> #endif -- 2.26.2

3 2

[PATCH] meson: Check for stdarg.h
by Michal Privoznik 03 Sep '20

03 Sep '20

As it turns out, one of my previous commits in which I removed checking for stdarg.h was too aggressive. Long story short, the readline public headers rely on stdarg.h and what is worse, they expect us to declare the autotools style of macro (HAVE_STDARG_H) if the header file exists. If we don't do it then compiling virsh on macos fails. See 9ea3424a178 for more info. Fixes: 85808b73846f93d656b4c81b6ebddd2dc3881bf6 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- meson.build | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/meson.build b/meson.build index 1aad385ad1..98f7545495 100644 --- a/meson.build +++ b/meson.build @@ -1333,8 +1333,12 @@ if readline_dep.found() endif endif - # We need this to avoid compilation issues with modern compilers. - # See 9ea3424a178 for a more detailed explanation + # We need both of these hacks to avoid compilation issues with modern + # compilers. See 9ea3424a178 for a more detailed explanation. + if cc.has_header('stdarg.h') + conf.set('HAVE_STDARG_H', 1) + endif + readline_dep = declare_dependency( compile_args: [ '-D_FUNCTION_DEF' ], dependencies: [ readline_dep ], -- 2.26.2

4 4

[PATCH] rpm: Fix conditional for defining %_vpath_builddir for RHEL <= 7
by Neal Gompa 03 Sep '20

03 Sep '20

The conditional was incorrectly overriding %_vpath_builddir when %rhel is not defined, which led to surprising behavior when the global %_vpath_builddir path is set on Fedora already. Signed-off-by: Neal Gompa <ngompa13(a)gmail.com> --- libvirt.spec.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index bb74443484..4b9e04ae61 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -32,7 +32,7 @@ %endif # On RHEL 7 and older macro _vpath_builddir is not defined. -%if 0%{?rhel} <= 7 +%if 0%{?rhel} && 0%{?rhel} <= 7 %define _vpath_builddir %{_target_platform} %endif -- 2.26.2

2 2

[PATCH v2] qemu_validate: Only allow none address for watchdog ib700
by Han Han 03 Sep '20

03 Sep '20

Since QEMU 1.5.3, the ib700 watchdog device has no options for address, and not address in device tree: $ /usr/libexec/qemu-kvm -version QEMU emulator version 1.5.3 (qemu-kvm-1.5.3-175.el7), Copyright (c) 2003-2008 Fabrice Bellard $ /usr/libexec/qemu-kvm -device ib700,\? $ virsh qemu-monitor-command seabios --hmp info qtree|grep ib700 -A 2 dev: ib700, id "watchdog0" dev: isa-serial, id "serial0" index = 0 So only allow it to use none address. Fixes: 8a54cc1d08a333283c9cfc3fd7788be2642ca71a Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1509908 Signed-off-by: Han Han <hhan(a)redhat.com> --- src/qemu/qemu_validate.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 488f258d00..402f81a3cd 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1637,10 +1637,9 @@ qemuValidateDomainWatchdogDef(const virDomainWatchdogDef *dev, break; case VIR_DOMAIN_WATCHDOG_MODEL_IB700: - if (dev->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && - dev->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA) { + if (dev->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("%s model of watchdog can go only on ISA bus"), + _("%s model of watchdog does not support configuring the address"), virDomainWatchdogModelTypeToString(dev->model)); return -1; } -- 2.28.0

3 2

[PATCH v2] qemu: Do not silently allow non-available timers on non-x86 systems
by Thomas Huth 03 Sep '20

03 Sep '20

libvirt currently silently allows <timer name="kvmclock"/> and some other timer tags in the guest XML definition for timers that do not exist on non-x86 systems. We should not silently ignore these tags since the users might not get what they expected otherwise. Note: The error is only generated if the timer is marked with present="yes" - otherwise we would suddenly refuse XML definitions that worked without problems before. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1754887 Signed-off-by: Thomas Huth <thuth(a)redhat.com> --- v2: Check also for timer->present == 1 src/qemu/qemu_validate.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 488f258d00..561e7b12c7 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -371,6 +371,18 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, case VIR_DOMAIN_TIMER_NAME_TSC: case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: + if (!ARCH_IS_X86(def->os.arch) && timer->present == 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Configuring the '%s' timer is not supported " + "for virtType=%s arch=%s machine=%s guests"), + virDomainTimerNameTypeToString(timer->name), + virDomainVirtTypeToString(def->virtType), + virArchToString(def->os.arch), + def->os.machine); + return -1; + } + break; + case VIR_DOMAIN_TIMER_NAME_LAST: break; -- 2.18.1

5 5

[PATCH] qemu_driver.c: Fix domfsinfo for non-PCI device information from guest agent
by Thomas Huth 03 Sep '20

03 Sep '20

qemuAgentFSInfoToPublic() currently only sets the devAlias for PCI devices. However, the QEMU guest agent could also provide the device name in the "dev" field of the response for other devices instead (well, at least after fixing another problem in the current QEMU guest agent...). So if creating the devAlias from the PCI information failed, let's fall back to the name provided by the guest agent. This helps to fix the empty "Target" fields that occur when running "virsh domfsinfo" on s390x where CCW devices are used for the guest instead of PCI devices. Also add a proper debug message here in case we completely failed to set the device alias, since this problem here was very hard to debug: The only two error messages that I've seen were "Unable to get filesystem information" and "Unable to encode message payload" - which only indicates that something went wrong in the RPC call. No debug message indicated the real problem, so I had to learn the hard way why the RPC call failed (it apparently does not like devAlias left to be NULL) and where the real problem comes from. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1755075 Signed-off-by: Thomas Huth <thuth(a)redhat.com> --- src/qemu/qemu_driver.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d185666ed8..e45c61ee20 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21935,14 +21935,17 @@ qemuAgentFSInfoToPublic(qemuAgentFSInfoPtr agent, qemuAgentDiskInfoPtr agentdisk = agent->disks[i]; virDomainDiskDefPtr diskDef; - if (!(diskDef = virDomainDiskByAddress(vmdef, - &agentdisk->pci_controller, - agentdisk->bus, - agentdisk->target, - agentdisk->unit))) - continue; - - ret->devAlias[i] = g_strdup(diskDef->dst); + diskDef = virDomainDiskByAddress(vmdef, + &agentdisk->pci_controller, + agentdisk->bus, + agentdisk->target, + agentdisk->unit); + if (diskDef != NULL) + ret->devAlias[i] = g_strdup(diskDef->dst); + else if (agentdisk->devnode != NULL) + ret->devAlias[i] = g_strdup(agentdisk->devnode); + else + VIR_DEBUG("Missing devnode name for '%s'.", ret->mountpoint); } return ret; -- 2.18.1

2 5

[RESEND][PATCH] migration: fix xml file residual during vm crash with migration
by zhengchuan 02 Sep '20

02 Sep '20

>From 935ec812b822ca978631e72bb9b9a5d00df24a42 Mon Sep 17 00:00:00 2001 From: Zheng Chuan <zhengchuan(a)huawei.com> Date: Mon, 27 Jul 2020 14:39:05 +0800 Subject: [PATCH] migration: fix xml file residual during vm crash with migration when migration is cancelled (such as kill -9 vmpid in Src, etc), it could do virDomainSaveStatus() to save xml file after qemuProcessStop(), which results in xml residulal. Fix it by that do not do virDomainSaveStatus() if vm is not active. Signed-off-by: Zheng Chuan <zhengchuan(a)huawei.com> --- src/qemu/qemu_migration.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 2c7bf34..d2804ab 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3073,6 +3073,9 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver, qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, jobPriv->migParams, priv->job.apiFlags); + if (!virDomainObjIsActive(vm)) + goto done; + if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) VIR_WARN("Failed to save status on vm %s", vm->def->name); } -- 1.8.3.1

3 2

[PATCH] fix vm schizencephaly when heartbeat stoped
by Yilu Lin 02 Sep '20

02 Sep '20

>From e28cb2a03a670e4c0e7641f68f9d9f3accb00ae0 Mon Sep 17 00:00:00 2001 From: Yilu Lin <linyilu(a)huawei.com> Date: Tue, 4 Aug 2020 02:42:00 -0400 Subject: [PATCH] fix vm schizencephaly when heartbeat stoped Signed-off-by: Yilu Lin <linyilu(a)huawei.com> If keepalive messages lost in finish step, vm maybe schizencephaly. Shutdown src vm for protection. --- src/qemu/qemu_migration.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 2c7bf34..b8296ba 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4136,6 +4136,8 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, int cookieoutlen = 0; int ret = -1; virErrorPtr orig_err = NULL; + virErrorPtr finish_err = NULL; + bool living = true; bool cancelled = true; virStreamPtr st = NULL; unsigned long destflags; @@ -4394,7 +4396,16 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, * The lock manager plugins should take care of * safety in this scenario. */ - cancelled = ddomain == NULL; + if (!cancelled && !ddomain) + finish_err = virSaveLastError(); + + if (finish_err && finish_err->message && + strstr(finish_err->message, "received hangup / error event on socket")) { + living = false; + VIR_ERROR(_("keepalive messages lost in finish step, shutdown src vm for protection")); + } else { + cancelled = ddomain == NULL; + } /* If finish3 set an error, and we don't have an earlier * one we need to preserve it in case confirm3 overwrites @@ -4427,10 +4438,17 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, virObjectUnref(ddomain); ret = 0; } else { - ret = -1; + if (!living) + ret = 0; + else + ret = -1; } virObjectUnref(st); + if (finish_err) { + virSetError(finish_err); + virFreeError(finish_err); + } virErrorRestore(&orig_err); VIR_FREE(uri_out); -- 2.19.1

2 1

[PATCH 0/5] Prefer WITH_* prefix for #if conditionals
by Michal Privoznik 02 Sep '20

02 Sep '20

2 8

[PATCH v2 0/9] qemu: Allow migration over UNIX sockets
by Martin Kletzander 02 Sep '20

02 Sep '20

v2: - Allow TLS and parallel migration as well - Use simpler unix socket URIs KubeVirt would like to use this feature. For more information see individual commits and changes in manpages and documentation. Resolves: https://bugzilla.redhat.com/1638889 Martin Kletzander (9): qemu: Use g_autofree in qemuMigrationSrcConnect qemu: Rework qemuMigrationSrcConnect virsh: Reuse existing variable when parsing migrate --disks-port qemu: Rework starting NBD server for migration tests: Add simple test for virDomainMigrateCheckNotLocal qemu: Allow NBD migration over UNIX socket peer2peer migration: allow connecting to local sockets qemu: Allow migration over UNIX socket news: qemu: Allow migration over UNIX sockets NEWS.rst | 6 + docs/manpages/virsh.rst | 33 ++- docs/migration.html.in | 33 +++ include/libvirt/libvirt-domain.h | 13 ++ scripts/apibuild.py | 1 + src/libvirt-domain.c | 11 +- src/libvirt_internal.h | 2 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 33 ++- src/qemu/qemu_migration.c | 354 +++++++++++++++++++++++-------- src/qemu/qemu_migration.h | 3 + src/qemu/qemu_migration_cookie.c | 3 +- src/qemu/qemu_migration_params.c | 9 + src/qemu/qemu_migration_params.h | 3 + src/qemu/qemu_monitor.c | 15 ++ src/qemu/qemu_monitor.h | 4 + src/remote/remote_driver.c | 8 +- src/util/viruri.c | 30 +++ src/util/viruri.h | 2 + tests/meson.build | 1 + tests/virmigtest.c | 91 ++++++++ tools/virsh-domain.c | 19 +- 22 files changed, 570 insertions(+), 105 deletions(-) create mode 100644 tests/virmigtest.c -- 2.28.0

4 24

[libvirt PATCH 0/3] AUTHORS: Convert to reStructuredText
by Andrea Bolognani 02 Sep '20

02 Sep '20

Plus some other adjustments. Andrea Bolognani (3): AUTHORS: Convert to reStructuredText AUTHORS: Some small tweaks AUTHORS: Remove Emacs file variables AUTHORS.in | 103 ----------------------------------- AUTHORS.rst.in | 99 +++++++++++++++++++++++++++++++++ libvirt.spec.in | 2 +- meson.build | 4 +- scripts/meson-gen-authors.py | 2 +- 5 files changed, 103 insertions(+), 107 deletions(-) delete mode 100644 AUTHORS.in create mode 100644 AUTHORS.rst.in -- 2.26.2

2 7

[PATCH] build-aux: use GNU sed for syntax-check on FreeBSD
by Roman Bogorodskiy 02 Sep '20

02 Sep '20

BSD sed(1) and GNU sed(1) syntax are not compatible, and as syntax-check.mk uses the GNU flavor, set SED variable to 'gsed' by default. Signed-off-by: Roman Bogorodskiy <bogorodskiy(a)gmail.com> --- build-aux/syntax-check.mk | 8 ++++++++ 1 file changed, 8 insertions(+) I'm not sure if this requires a more comprehensive solution. I have mixed feeling about this. If we try to just use gsed like in this patch, it'll fail because we don't require gsed to be installed. OTOH, an alternative solution like checking for gsed in meson.build, and probably even generation of some files with variables to be sourced by .mk files feels like too much of a hassle, esp. in this context. diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 6eb59cf90e..bbfcb63152 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -28,7 +28,15 @@ ME := build-aux/syntax-check.mk # ignoring the module description. AWK ?= awk GREP ?= grep +# FreeBSD (and probably some other OSes too) ships own version of sed(1), not +# compatible with the GNU sed. GNU sed is available as gsed(1), so use this +# instead +UNAME := $(shell uname) +ifeq ($(UNAME),FreeBSD) +SED ?= gsed +else SED ?= sed +endif # Helper variables. _empty = -- 2.27.0

2 3

[libvirt PATCH] meson: add -Wall and -Wextra explicitly for buildtype=plain
by Pavel Hrdina 02 Sep '20

02 Sep '20

If someone runs `meson setup --buildtype plain` meson ignores warning_level=2 that is in our meson.build file. The implication is that Meson will not automatically add -Wall which enables -Wformat. This breaks building libvirt from git with the buildtype set to plain. There is an issue reported [1] to not ignore warning_level silently and the change to ignore it was done by upstream commit [2]. [1] <https://github.com/mesonbuild/meson/issues/7399> [2] <https://github.com/mesonbuild/meson/commit/8ee1c9a07a3a35e3ed262fbc358fd86c…> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- meson.build | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/meson.build b/meson.build index e193166a9b..f38785eeae 100644 --- a/meson.build +++ b/meson.build @@ -503,10 +503,6 @@ cc_flags_disabled = [ # In meson this is specified using 'c_std=gnu99' in project() function. '-std=gnu99', - # In meson this is specified using 'warning_level=2' in project() function. - '-Wall', - '-Wextra', - # don't care about C++ compiler compat '-Wc++-compat', '-Wabi', @@ -573,6 +569,21 @@ cc_flags_disabled = [ '-Wsuggest-attribute=malloc', ] +# In meson this is specified using 'warning_level=2' in project() function. +# However, meson silently ignores 'warning_level' option with 'buildtype=plain' +# so we have to enable them explicitly. +if get_option('buildtype') == 'plain' + cc_flags += [ + '-Wall', + '-Wextra', + ] +else + cc_flags_disabled += [ + '-Wall', + '-Wextra', + ] +endif + foreach flag : cc_flags_disabled if cc_flags.contains(flag) error('@0@ is disabled but listed in cc_flags'.format(flag)) -- 2.26.2

3 3

[libvirt PATCH] util: virnetdevtap: stats: fix txdrop on FreeBSD
by Ján Tomko 02 Sep '20

02 Sep '20

For older FreeBSD, we needed an ifdef guard to use if_data.ifi_oqdrops, which was introduced by: commit 61bbdbb94ce3e2f5e969c9bddb443427db07bf61 Implement interface stats for BSD But when we dropped the check because we deprecated building on FreeBSD-10 in: commit 83131d9714db7ee77ab220186b6b0d8b6c22b09e configure: drop check for unsupported FreeBSD We started building the wrong side of the ifdef. Signed-off-by: Ján Tomko <jtomko(a)redhat.com> Fixes: 83131d9714db7ee77ab220186b6b0d8b6c22b09e --- src/util/virnetdevtap.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 9f5e535180..cab5ef0ced 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -974,20 +974,12 @@ virNetDevTapInterfaceStats(const char *ifname, stats->rx_bytes = ifd->ifi_obytes; stats->rx_packets = ifd->ifi_opackets; stats->rx_errs = ifd->ifi_oerrors; -# ifdef HAVE_STRUCT_IF_DATA_IFI_OQDROPS stats->rx_drop = ifd->ifi_oqdrops; -# else - stats->rx_drop = 0; -# endif } else { stats->tx_bytes = ifd->ifi_obytes; stats->tx_packets = ifd->ifi_opackets; stats->tx_errs = ifd->ifi_oerrors; -# ifdef HAVE_STRUCT_IF_DATA_IFI_OQDROPS stats->tx_drop = ifd->ifi_oqdrops; -# else - stats->tx_drop = 0; -# endif stats->rx_bytes = ifd->ifi_ibytes; stats->rx_packets = ifd->ifi_ipackets; stats->rx_errs = ifd->ifi_ierrors; -- 2.26.2

2 1

GSoC 2020 Salt Virt Advanced Performance Tuning Support
by Guoqing Li 02 Sep '20

02 Sep '20

Hello Libvirt community This is my first time working on a GSoC project with Libvirt. SaltStack is an open-source project that facilitates configuration management for IT infrastructures and Virt is a module of SaltStack. The following 9 PRs summerizes the work I have done during this summer. The goal is to make existing Libvirt API domain properties configurable in Salt Virt in order to support advanced performance tuning. 5 PRs have been merged so far and another 4 are under reviewing. Although all basic unit tests have passed, I believe more rigorous tests are needed before using these features in production. I will spend more time later on adding more tests. Acknowledgements =============== Sincere thanks to my mentors Cedric Bosdonnat (cbosdo), Pedro Algarvio (s0undt3ch) and Tyler Jones (Akm0d) for their patient support and advice. Thank Wayne Werner (waynew), Daniel Wozniak (dwoz) Megan Wilhite (ch3ll) from SaltStack community for being very responsive on my questions and code reviews. and Libvirt organisation for providing this opportunity for me to learn and grow. PRs =============== CPU tuning and io threads allocation (reviewing) https://github.com/saltstack/salt/pull/58196 Memory backing (reviewing) https://github.com/saltstack/salt/pull/58167 CPU model, topology and NUMA node tuning (reviewing) https://github.com/saltstack/salt/pull/57879 Memory tuning (reviewing) https://github.com/saltstack/salt/pull/57636 Polish parameter doc (merged) https://github.com/saltstack/salt/pull/57086 Map null to None in sls file (merged) https://github.com/saltstack/salt/pull/57451 Firmware auto select (merged) https://github.com/saltstack/salt/pull/57396 Boot VMs with UEFI (merged) https://github.com/saltstack/salt/pull/56613 Boot parameter update fix (merged) https://github.com/saltstack/salt/pull/56562 Kind regards, Guoqing LI

1 0

[libvirt PATCH v2] Adds e1000e/vmxnet3 Vnet_hdr suuport
by Patrick Magauran 02 Sep '20

02 Sep '20

Changes from Original: Moved Comparison to qemuInterfaceIsVnetCompatModel in qemu_interface.c per the recommendation of Daniel Berrangé ----- Libvirt bases its decision about whether to apply the vnet_hdr flag to the tap interface on whether or not the selected model is VirtIO. Originally, VirtIO was the only model to support the vnet_hdr in QEMU; however, the e1000e & vmxnet3 adapters also support it(seemingly from introduction based on commits). This passes the whole packet to the host, reducing emulation overhead and improving performance. Signed-off-by: Patrick Magauran <patmagauran.j(a)gmail.com> --- src/qemu/qemu_interface.c | 15 +++++++++++---- src/qemu/qemu_interface.h | 1 + 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c index ffec992596..229bb299aa 100644 --- a/src/qemu/qemu_interface.c +++ b/src/qemu/qemu_interface.c @@ -230,6 +230,13 @@ qemuInterfaceStopDevices(virDomainDefPtr def) return 0; } +bool qemuInterfaceIsVnetCompatModel(const virDomainNetDef *net) +{ + return (virDomainNetIsVirtioModel(net) || + net->model == VIR_DOMAIN_NET_MODEL_E1000E || + net->model == VIR_DOMAIN_NET_MODEL_VMXNET3); +} + /** * qemuInterfaceDirectConnect: @@ -255,7 +262,7 @@ qemuInterfaceDirectConnect(virDomainDefPtr def, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned int macvlan_create_flags = VIR_NETDEV_MACVLAN_CREATE_WITH_TAP; - if (virDomainNetIsVirtioModel(net)) + if (qemuInterfaceIsVnetCompatModel(net)) macvlan_create_flags |= VIR_NETDEV_MACVLAN_VNET_HDR; if (virNetDevMacVLanCreateWithVPortProfile(net->ifname, @@ -417,7 +424,7 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def, } } - if (virDomainNetIsVirtioModel(net)) + if (qemuInterfaceIsVnetCompatModel(net)) tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR; if (net->managed_tap == VIR_TRISTATE_BOOL_NO) { @@ -436,7 +443,7 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def, if (virNetDevMacVLanTapOpen(net->ifname, tapfd, tapfdSize) < 0) goto cleanup; if (virNetDevMacVLanTapSetup(tapfd, tapfdSize, - virDomainNetIsVirtioModel(net)) < 0) { + qemuInterfaceIsVnetCompatModel(net)) < 0) { goto cleanup; } } else { @@ -559,7 +566,7 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def, template_ifname = true; } - if (virDomainNetIsVirtioModel(net)) + if (qemuInterfaceIsVnetCompatModel(net)) tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR; if (driver->privileged) { diff --git a/src/qemu/qemu_interface.h b/src/qemu/qemu_interface.h index 0464b903d7..9e3f61e8e0 100644 --- a/src/qemu/qemu_interface.h +++ b/src/qemu/qemu_interface.h @@ -58,3 +58,4 @@ int qemuInterfaceOpenVhostNet(virDomainDefPtr def, qemuSlirpPtr qemuInterfacePrepareSlirp(virQEMUDriverPtr driver, virDomainNetDefPtr net); +bool qemuInterfaceIsVnetCompatModel(const virDomainNetDef *net); -- 2.26.2

3 2

[PATCH] conf: Don't check virTristateBool < 0
by Jim Fehlig 02 Sep '20

02 Sep '20

Commit 9d15647dcb introduced a check of virtTristateBool < 0, which causes a build failure in some environments ../dist-unpack/libvirt-6.8.0/src/conf/domain_conf.c:8144:78: error: result of comparison of unsigned enum expression < 0 is always false [-Werror,-Wtautological-unsigned-enum-zero-compare] if ((def->writeFiltering = virTristateBoolTypeFromString(filtering)) < 0) { Fix it by assigning the return value of virTristateBoolTypeFromString to a temp int variable, which allows checking for unknown values before assigning to the virtTristateBool variable. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- Perhaps considered trivial and could be pushed under the build-breaker rule, but I'd prefer a review since I've botched trivial patches in the past :-). src/conf/domain_conf.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7d177a5562..5b224ccd78 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8141,12 +8141,15 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node, xmlNodePtr cur; if ((filtering = virXMLPropString(node, "writeFiltering"))) { - if ((def->writeFiltering = virTristateBoolTypeFromString(filtering)) < 0) { + int filterval; + + if ((filterval = virTristateBoolTypeFromString(filtering)) < 0) { virReportError(VIR_ERR_XML_ERROR, _("unknown pci writeFiltering setting '%s'"), filtering); return -1; } + def->writeFiltering = filterval; } cur = node->children; -- 2.28.0

1 1

[libvirt PATCH] conf: fix enum conversion
by Ján Tomko 02 Sep '20

02 Sep '20

../src/conf/domain_conf.c:8144:78: error: result of comparison of unsigned enum expression < 0 is always false [-Werror,-Wtautological-unsigned-enum-zero-compare] if ((def->writeFiltering = virTristateBoolTypeFromString(filtering)) < 0) { ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~ Signed-off-by: Ján Tomko <jtomko(a)redhat.com> Fixes: 9d15647dcb96831c93ac8c1d67c47265b5ed9072 --- Pushed as a build-breaker fix. src/conf/domain_conf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7d177a5562..72ac4f4191 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8141,12 +8141,14 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node, xmlNodePtr cur; if ((filtering = virXMLPropString(node, "writeFiltering"))) { - if ((def->writeFiltering = virTristateBoolTypeFromString(filtering)) < 0) { + int val; + if ((val = virTristateBoolTypeFromString(filtering)) < 0) { virReportError(VIR_ERR_XML_ERROR, _("unknown pci writeFiltering setting '%s'"), filtering); return -1; } + def->writeFiltering = val; } cur = node->children; -- 2.26.2

1 0