September 2020 - Devel - libvirt List Archives

[PATCH] util: add device name in errors from ethtool ioctls
by Daniel P. Berrangé 03 Sep '20

03 Sep '20

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/util/virnetdev.c | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 3040f129d1..e1a4cc2bef 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2849,7 +2849,7 @@ virNetDevRDMAFeature(const char *ifname, * Returns 0 on success, -1 on failure. */ static int -virNetDevSendEthtoolIoctl(int fd, struct ifreq *ifr) +virNetDevSendEthtoolIoctl(const char *ifname, int fd, struct ifreq *ifr) { int ret = -1; @@ -2857,13 +2857,13 @@ virNetDevSendEthtoolIoctl(int fd, struct ifreq *ifr) if (ret != 0) { switch (errno) { case EINVAL: /* kernel doesn't support SIOCETHTOOL */ - VIR_DEBUG("ethtool ioctl: invalid request"); + VIR_DEBUG("ethtool ioctl: invalid request on %s", ifname); break; case EOPNOTSUPP: /* kernel doesn't support specific feature */ - VIR_DEBUG("ethtool ioctl: request not supported"); + VIR_DEBUG("ethtool ioctl: request not supported on %s", ifname); break; default: - virReportSystemError(errno, "%s", _("ethtool ioctl error")); + virReportSystemError(errno, _("ethtool ioctl error on %s"), ifname); break; } } @@ -2888,10 +2888,10 @@ struct virNetDevEthtoolFeatureCmd { * Returns true if the feature is available, false otherwise. */ static bool -virNetDevFeatureAvailable(int fd, struct ifreq *ifr, struct ethtool_value *cmd) +virNetDevFeatureAvailable(const char *ifname, int fd, struct ifreq *ifr, struct ethtool_value *cmd) { ifr->ifr_data = (void*)cmd; - if (virNetDevSendEthtoolIoctl(fd, ifr) == 0 && + if (virNetDevSendEthtoolIoctl(ifname, fd, ifr) == 0 && cmd->data > 0) return true; return false; @@ -2899,7 +2899,8 @@ virNetDevFeatureAvailable(int fd, struct ifreq *ifr, struct ethtool_value *cmd) static void -virNetDevGetEthtoolFeatures(virBitmapPtr bitmap, +virNetDevGetEthtoolFeatures(const char *ifname, + virBitmapPtr bitmap, int fd, struct ifreq *ifr) { @@ -2941,13 +2942,13 @@ virNetDevGetEthtoolFeatures(virBitmapPtr bitmap, for (i = 0; i < G_N_ELEMENTS(ethtool_cmds); i++) { cmd.cmd = ethtool_cmds[i].cmd; - if (virNetDevFeatureAvailable(fd, ifr, &cmd)) + if (virNetDevFeatureAvailable(ifname, fd, ifr, &cmd)) ignore_value(virBitmapSetBit(bitmap, ethtool_cmds[i].feat)); } # if WITH_DECL_ETHTOOL_GFLAGS cmd.cmd = ETHTOOL_GFLAGS; - if (virNetDevFeatureAvailable(fd, ifr, &cmd)) { + if (virNetDevFeatureAvailable(ifname, fd, ifr, &cmd)) { for (i = 0; i < G_N_ELEMENTS(flags); i++) { if (cmd.data & flags[i].cmd) ignore_value(virBitmapSetBit(bitmap, flags[i].feat)); @@ -3133,19 +3134,21 @@ virNetDevSwitchdevFeature(const char *ifname G_GNUC_UNUSED, * Returns true if the feature is available, false otherwise. */ static bool -virNetDevGFeatureAvailable(int fd, +virNetDevGFeatureAvailable(const char *ifname, + int fd, struct ifreq *ifr, struct ethtool_gfeatures *cmd) { ifr->ifr_data = (void*)cmd; - if (virNetDevSendEthtoolIoctl(fd, ifr) == 0) + if (virNetDevSendEthtoolIoctl(ifname, fd, ifr) == 0) return !!FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active); return false; } static int -virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap, +virNetDevGetEthtoolGFeatures(const char *ifname, + virBitmapPtr bitmap, int fd, struct ifreq *ifr) { @@ -3157,13 +3160,14 @@ virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap, g_cmd->cmd = ETHTOOL_GFEATURES; g_cmd->size = GFEATURES_SIZE; - if (virNetDevGFeatureAvailable(fd, ifr, g_cmd)) + if (virNetDevGFeatureAvailable(ifname, fd, ifr, g_cmd)) ignore_value(virBitmapSetBit(bitmap, VIR_NET_DEV_FEAT_TXUDPTNL)); return 0; } # else static int -virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap G_GNUC_UNUSED, +virNetDevGetEthtoolGFeatures(const char *ifname G_GNUC_UNUSED, + virBitmapPtr bitmap G_GNUC_UNUSED, int fd G_GNUC_UNUSED, struct ifreq *ifr G_GNUC_UNUSED) { @@ -3228,7 +3232,7 @@ int virNetDevSetCoalesce(const char *ifname, ifr.ifr_data = (void *) &coal; - if (virNetDevSendEthtoolIoctl(fd, &ifr) < 0) { + if (virNetDevSendEthtoolIoctl(ifname, fd, &ifr) < 0) { virReportSystemError(errno, _("Cannot set coalesce info on '%s'"), ifname); @@ -3241,7 +3245,7 @@ int virNetDevSetCoalesce(const char *ifname, }; /* Don't fail if the update itself fails */ - if (virNetDevSendEthtoolIoctl(fd, &ifr) == 0) { + if (virNetDevSendEthtoolIoctl(ifname, fd, &ifr) == 0) { coalesce->rx_max_coalesced_frames = coal.rx_max_coalesced_frames; coalesce->rx_coalesce_usecs_irq = coal.rx_coalesce_usecs_irq; coalesce->rx_max_coalesced_frames_irq = coal.rx_max_coalesced_frames_irq; @@ -3307,9 +3311,9 @@ virNetDevGetFeatures(const char *ifname, if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) return -1; - virNetDevGetEthtoolFeatures(*out, fd, &ifr); + virNetDevGetEthtoolFeatures(ifname, *out, fd, &ifr); - if (virNetDevGetEthtoolGFeatures(*out, fd, &ifr) < 0) + if (virNetDevGetEthtoolGFeatures(ifname, *out, fd, &ifr) < 0) return -1; if (virNetDevRDMAFeature(ifname, out) < 0) -- 2.25.4

2 1

[PATCH v1 00/34] Rework building of domain's namespaces
by Michal Privoznik 03 Sep '20

03 Sep '20

There are couple of things happening in this series. The first patch fixes a bug. We are leaking /dev/mapper/control in QEMU. See the patch for detailed info. The second patch then cleans up code a bit. The third patch moves namespace handling code into a separate file. Patches 4 - 15 then prepare the code for switch to different approach in populating the namespace. Patches 16 - 26 then do the switch and finally, the rest of the patches drops and deduplicates some code. You can fetch the patches from here too: https://gitlab.com/MichalPrivoznik/libvirt/-/commits/dm_namespace/ Michal Prívozník (34): virDevMapperGetTargetsImpl: Close /dev/mapper/control in the end virDevMapperGetTargets: Don't ignore EBADF qemu: Separate out namespace handling code qemu_domain_namespace: Rename qemuDomainCreateNamespace() qemu_domain_namespace: Drop unused @cfg argument qemu_domain_namespace: Check for namespace enablement earlier qemuDomainNamespaceSetupHostdev: Create paths in one go qemuDomainAttachDeviceMknodHelper: Don't leak data->target qemu_domain_namespace.c: Rename qemuDomainAttachDeviceMknodData qemuDomainAttachDeviceMknodRecursive: Isolate bind mounted devices condition qemuDomainAttachDeviceMknodHelper: Create more files in a single go qemuDomainNamespaceMknodPaths: Create more files in one go qemuDomainNamespaceMknodPaths: Turn @paths into string list qemuDomainSetupDisk: Accept @src qemu_domain_namespace: Repurpose qemuDomainBuildNamespace() qemuDomainBuildNamespace: Populate basic /dev from daemon's namespace qemuDomainBuildNamespace: Populate disks from daemon's namespace qemuDomainBuildNamespace: Populate hostdevs from daemon's namespace qemuDomainBuildNamespace: Populate memory from daemon's namespace qemuDomainBuildNamespace: Populate chardevs from daemon's namespace qemuDomainBuildNamespace: Populate TPM from daemon's namespace qemuDomainBuildNamespace: Populate graphics from daemon's namespace qemuDomainBuildNamespace: Populate inputs from daemon's namespace qemuDomainBuildNamespace: Populate RNGs from daemon's namespace qemuDomainBuildNamespace: Populate loader from daemon's namespace qemuDomainBuildNamespace: Populate SEV from daemon's namespace qemu_domain_namespace: Drop unused functions qemuDomainDetachDeviceUnlink: Unlink paths in one go qemuDomainNamespaceUnlinkPaths: Turn @paths into string list qemuDomainNamespaceTeardownHostdev: Unlink paths in one go qemuDomainNamespaceTeardownMemory: Deduplicate code qemuDomainNamespaceTeardownChardev: Deduplicate code qemuDomainNamespaceTeardownRNG: Deduplicate code qemuDomainNamespaceTeardownInput: Deduplicate code po/POTFILES.in | 1 + src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_cgroup.c | 2 +- src/qemu/qemu_conf.c | 1 + src/qemu/qemu_domain.c | 1848 +----------------------------- src/qemu/qemu_domain.h | 57 - src/qemu/qemu_domain_namespace.c | 1610 ++++++++++++++++++++++++++ src/qemu/qemu_domain_namespace.h | 86 ++ src/qemu/qemu_driver.c | 1 + src/qemu/qemu_hotplug.c | 1 + src/qemu/qemu_process.c | 23 +- src/qemu/qemu_security.c | 1 + src/util/virdevmapper.c | 4 +- 13 files changed, 1727 insertions(+), 1910 deletions(-) create mode 100644 src/qemu/qemu_domain_namespace.c create mode 100644 src/qemu/qemu_domain_namespace.h -- 2.26.2

4 74

[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