April 2021 - Devel - libvirt List Archives

[libvirt PATCH 0/3] Fix some memory leaks
by Tim Wiederhake 16 Apr '21

16 Apr '21

These memory leaks were found by llvm's address sanitizer asan. Tim Wiederhake (3): xenParseHypervisorFeatures: Fix memory leak virDomainFeaturesDefParse: Fix memory leak virQEMUCapsSetHostModel: Fix memory leak src/conf/domain_conf.c | 2 +- src/libxl/xen_common.c | 1 + src/qemu/qemu_capabilities.c | 8 ++++++++ 3 files changed, 10 insertions(+), 1 deletion(-) -- 2.26.2

3 7

[libvirt PATCH] spec: Merge -bash-completion and -admin into -client
by Andrea Bolognani 16 Apr '21

16 Apr '21

The former is ridiculously tiny and doesn't have any use on its own, so it hardly warrants the overhead of an additional package; the latter is also very small and, just like virsh, is something that you likely want to have available on any virtualization host to help with management and debugging tasks. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- libvirt.spec.in | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index f9af330186..80a12a307a 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -901,7 +901,6 @@ Requires: %{name}-libs = %{version}-%{release} Requires: gettext # Needed by virt-pki-validate script. Requires: gnutls-utils -Requires: %{name}-bash-completion = %{version}-%{release} %description client The client binaries needed to access the virtualization @@ -918,20 +917,6 @@ Requires: cyrus-sasl-gssapi %description libs Shared libraries for accessing the libvirt daemon. -%package admin -Summary: Set of tools to control libvirt daemon -Requires: %{name}-libs = %{version}-%{release} -Requires: %{name}-bash-completion = %{version}-%{release} - -%description admin -The client side utilities to control the libvirt daemon. - -%package bash-completion -Summary: Bash completion script - -%description bash-completion -Bash completion script stub. - %if %{with_wireshark} %package wireshark Summary: Wireshark dissector plugin for libvirt RPC transactions @@ -1872,10 +1857,12 @@ exit 0 %files client %{_mandir}/man1/virsh.1* +%{_mandir}/man1/virt-admin.1* %{_mandir}/man1/virt-xml-validate.1* %{_mandir}/man1/virt-pki-validate.1* %{_mandir}/man1/virt-host-validate.1* %{_bindir}/virsh +%{_bindir}/virt-admin %{_bindir}/virt-xml-validate %{_bindir}/virt-pki-validate %{_bindir}/virt-host-validate @@ -1886,8 +1873,9 @@ exit 0 %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp %endif +%{_datadir}/bash-completion/completions/vsh %{_datadir}/bash-completion/completions/virsh - +%{_datadir}/bash-completion/completions/virt-admin %{_unitdir}/libvirt-guests.service %config(noreplace) %{_sysconfdir}/sysconfig/libvirt-guests @@ -1911,14 +1899,6 @@ exit 0 %{_datadir}/libvirt/test-screenshot.png -%files admin -%{_mandir}/man1/virt-admin.1* -%{_bindir}/virt-admin -%{_datadir}/bash-completion/completions/virt-admin - -%files bash-completion -%{_datadir}/bash-completion/completions/vsh - %if %{with_wireshark} %files wireshark %{wireshark_plugindir}/libvirt.so -- 2.26.3

4 5

[libvirt PATCHv1 0/4] qemu: support unmanaged virtiofsd
by Ján Tomko 16 Apr '21

16 Apr '21

Connect the vhost-user-fs device to a socket path configured in the XML. Starting and stopping the virtiofsd daemon on that socket is the user's responsibility. Ján Tomko (4): conf: fs: fill out accessmode in post-parse conf: fs: allow missing accessmode in the formatter conf: add socket for virtiofs filesystems qemu: add socket for virtiofs filesystems docs/schemas/domaincommon.rng | 11 +++++ src/conf/domain_conf.c | 43 +++++++++++++------ src/conf/domain_conf.h | 2 + src/qemu/qemu_extdevice.c | 10 +++-- src/qemu/qemu_validate.c | 17 +++++--- tests/qemuxml2argvdata/vhost-user-fs-sock.xml | 39 +++++++++++++++++ .../vhost-user-fs-sock.x86_64-latest.xml | 1 + tests/qemuxml2xmltest.c | 1 + 8 files changed, 102 insertions(+), 22 deletions(-) create mode 100644 tests/qemuxml2argvdata/vhost-user-fs-sock.xml create mode 120000 tests/qemuxml2xmloutdata/vhost-user-fs-sock.x86_64-latest.xml -- 2.29.2

2 6

[PATCH 0/2] conf: Convert 'tray_status' and 'startupPolicy' of virDomainDiskDef to enums
by Peter Krempa 16 Apr '21

16 Apr '21

I was testing how virXMLPropEnum works when used on converted types, so I've converted these. Peter Krempa (2): conf: domain: Convert virDomainDiskDef's 'tray_status' to virDomainDiskTray conf: domain: Convert virDomainDiskDef's 'startupPolicy' to virDomainStartupPolicy src/conf/domain_conf.c | 31 +++++++------------------------ src/conf/domain_conf.h | 4 ++-- 2 files changed, 9 insertions(+), 26 deletions(-) -- 2.30.2

2 4

[libvirt PATCH] nodedev: Fix possible NULL pointer dereference on vfiogroup opening
by Erik Skultety 16 Apr '21

16 Apr '21

Coverity report: 1193 g_autofree char *vfiogroup = 1194 virMediatedDeviceGetIOMMUGroupDev(def->caps->data.mdev.uuid); >>> CID 317619: Null pointer dereferences (NULL_RETURNS) >>> Dereferencing a pointer that might be "NULL" "vfiogroup" when calling "open". [Note: The source code implementation of the function has been overridden by a builtin model.] 1195 VIR_AUTOCLOSE fd = open(vfiogroup, O_RDONLY); This patch shuffles the declarations in the affected 'if' block a bit to make it more readable after adding the NULL pointer condition. Note that error is not reported in this patch, because if @vfiogroup is NULL, then it must have been a system error which was already reported by the called function. Don't get confused by virMediatedDeviceGetIOMMUGroupDev returning NULL on an empty UUID, mdevs will always have one. Signed-off-by: Erik Skultety <eskultet(a)redhat.com> --- src/node_device/node_device_driver.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index e33d4a965b..7289322050 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1190,10 +1190,16 @@ nodeDeviceDestroy(virNodeDevicePtr device) * to be opened by one user at a time. So if we get EBUSY when opening * the group, we infer that the device is in use and therefore we * shouldn't try to remove the device. */ - g_autofree char *vfiogroup = - virMediatedDeviceGetIOMMUGroupDev(def->caps->data.mdev.uuid); - VIR_AUTOCLOSE fd = open(vfiogroup, O_RDONLY); g_autofree char *errmsg = NULL; + g_autofree char *vfiogroup = NULL; + const char *uuidstr = def->caps->data.mdev.uuid; + VIR_AUTOCLOSE fd = -1; + + vfiogroup = virMediatedDeviceGetIOMMUGroupDev(uuidstr); + if (!vfiogroup) + goto cleanup; + + fd = open(vfiogroup, O_RDONLY); if (fd < 0 && errno == EBUSY) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 2.30.2

2 1

[libvirt PATCH v6 0/8] Refactor XML parsing boilerplate code
by Tim Wiederhake 16 Apr '21

16 Apr '21

This series lays the groundwork for replacing some recurring boilerplate code in src/conf/ regarding the extraction of XML attribute values. For an on / off attribute, the boilerplate code looks roughly like this, g_autofree char *str = NULL; if (str = virXMLPropString(node, "x")) { int val; if ((val = virTristateBoolTypeFromString(str)) <= 0) { virReportError(...) return -1; } def->x = val; } with some variations regarding how `str` is free'd in case of later re-use, the exact error message for invalid values, whether or not `VIR_TRISTATE_(SWITCH|BOOL)_ABSENT` is explicitly checked for (`val < 0` vs. `val <= 0`), whether an intermediate variable is used or the value is assigned directly, and in some cases the conditions in the two if-blocks are merged. After the refactoring, the above code block looks like this: if (virXMLPropTristateBool(node, "x", VIR_XML_PROP_OPTIONAL, &def->x) < 0) return -1; Similar functions are introduced for integer valued attributes, unsigned integer valued attributes and enum valued attributes. Patches #6, #7, and #8 demonstrate the application of these function and stand representative of more patches that I did not sent along yet as to not drown the mailing list in spam. These patches remove a total of ~ 1000 lines of code and fix some places, where e.g. `<foo bar="default"/>` would incorrectly be accepted as virXMLTristateBool. As an added benefit, this refactoring makes the error messages for invalid values in these XML attributes much more consistent: $ git diff master | grep "^-.*_(\"" | wc -l 239 $ git diff master | grep "^+.*_(\"" | wc -l 21 V1: https://listman.redhat.com/archives/libvir-list/2021-March/msg00853.html V2: https://listman.redhat.com/archives/libvir-list/2021-March/msg00994.html V3: https://listman.redhat.com/archives/libvir-list/2021-March/msg01066.html V4: https://listman.redhat.com/archives/libvir-list/2021-April/msg00209.html V5: https://listman.redhat.com/archives/libvir-list/2021-April/msg00232.html Changes since V5: * Applied changes requested in https://listman.redhat.com/archives/libvir-list/2021-April/msg00658.html Cheers, Tim Tim Wiederhake (8): virxml: Add virXMLPropTristateBool virxml: Add virXMLPropTristateSwitch virxml: Add virXMLPropInt virxml: Add virXMLPropUInt virxml: Add virXMLPropEnum virNetworkForwardNatDefParseXML: Use virXMLProp* virDomainIOThreadIDDefParseXML: Use virXMLProp* virCPUDefParseXML: Use virXMLProp* src/conf/cpu_conf.c | 14 +-- src/conf/domain_conf.c | 14 +-- src/conf/network_conf.c | 16 +-- src/libvirt_private.syms | 5 + src/util/virxml.c | 249 +++++++++++++++++++++++++++++++++++++++ src/util/virxml.h | 42 +++++++ 6 files changed, 305 insertions(+), 35 deletions(-) -- 2.26.2

2 9

[PATCH 0/5] util: xml: cleanup virxml.h and virXMLParseHelper
by Peter Krempa 16 Apr '21

16 Apr '21

Peter Krempa (5): util: virxml: Fix formatting of virxml.h virXMLParseHelper: Sync argument names between declaration and definition util: xml: Register autoptr cleanup function for 'xmlParserCtxt' virXMLParseHelper: Rework error reporting virXMLParseHelper: Refactor cleanup src/util/virxml.c | 45 ++++++----- src/util/virxml.h | 193 +++++++++++++++++++++++++++------------------- 2 files changed, 137 insertions(+), 101 deletions(-) -- 2.30.2

2 6

[libvirt][PATCH v5 0/3] introduce 'restrictive' mode in numatune
by Luyao Zhong 16 Apr '21

16 Apr '21

Before this patch set, numatune only has three memory modes: static, interleave and prefered. These memory policies are ultimately set by mbind() system call. Memory policy could be 'hard coded' into the kernel, but none of above policies fit our requirment under this case. mbind() support default memory policy, but it requires a NULL nodemask. So obviously setting allowed memory nodes is cgroups' mission under this case. So we introduce a new option for mode in numatune named 'restrictive'. <numatune> <memory mode="restrictive" nodeset="1-4,^3"/> <memnode cellid="0" mode="restrictive" nodeset="1"/> <memnode cellid="2" mode="restrictive" nodeset="2"/> </numatune> The config above means we only use cgroups to restrict the allowed memory nodes and not setting any specific memory policies explicitly. For this new "restrictive" mode, there is a concrete use case about a new feature in kernel but not merged yet, we call it memory tiering. (https://lwn.net/Articles/802544/) If memory tiering is enabled on host, DRAM is top tier memory, and PMEM(persistent memory) is second tier memory, PMEM is shown as numa node without cpu. Pages can be migrated between DRAM node and PMEM node based on DRAM pressure and how cold/hot they are. *this memory policy* is implemented in kernel. So we need a default mode here, but from libvirt's perspective, the "defaut" mode is "strict", it's not MPOL_DEFAULT (https://man7.org/linux/man-pages/man2/mbind.2.html) defined in kernel. And to make memory tiering works well, cgroups setting is necessary, since it restricts that the pages can only be migrated between the DRAM and PMEM nodes that we specified (NUMA affinity support). Just using cgroups with multiple nodes in the nodeset makes kernel decide on which node (out of those in the restricted set) to allocate on, but specifying "strict" basically allocates it sequentially (on the first one until it is full, then on the next one and so on). In a word, if a user requires default mode(MPOL_DEFAULT), that means they want kernel decide the memory allocation and also want the cgroups to restrict memory nodes, "restrictive" mode will be useful. BR, Luyao Luyao Zhong (3): docs: add docs for 'restrictive' option for mode in numatune schema: add 'restrictive' config option for mode in numatune qemu: add parser and formatter for 'restrictive' mode in numatune docs/formatdomain.rst | 7 +++- docs/schemas/domaincommon.rng | 2 + include/libvirt/libvirt-domain.h | 1 + src/conf/numa_conf.c | 9 ++++ src/qemu/qemu_command.c | 6 ++- src/qemu/qemu_process.c | 27 ++++++++++++ src/util/virnuma.c | 3 ++ .../numatune-memnode-invalid-mode.err | 1 + .../numatune-memnode-invalid-mode.xml | 33 +++++++++++++++ ...emnode-restrictive-mode.x86_64-latest.args | 38 +++++++++++++++++ .../numatune-memnode-restrictive-mode.xml | 33 +++++++++++++++ tests/qemuxml2argvtest.c | 2 + ...memnode-restrictive-mode.x86_64-latest.xml | 41 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 14 files changed, 201 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/numatune-memnode-invalid-mode.err create mode 100644 tests/qemuxml2argvdata/numatune-memnode-invalid-mode.xml create mode 100644 tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.xml create mode 100644 tests/qemuxml2xmloutdata/numatune-memnode-restrictive-mode.x86_64-latest.xml -- 2.25.4

3 5

[libvirt PATCH v5 0/8] Refactor XML parsing boilerplate code
by Tim Wiederhake 16 Apr '21

16 Apr '21

This series lays the groundwork for replacing some recurring boilerplate code in src/conf/ regarding the extraction of XML attribute values. For an on / off attribute, the boilerplate code looks roughly like this, g_autofree char *str = NULL; if (str = virXMLPropString(node, "x")) { int val; if ((val = virTristateBoolTypeFromString(str)) <= 0) { virReportError(...) return -1; } def->x = val; } with some variations regarding how `str` is free'd in case of later re-use, the exact error message for invalid values, whether or not `VIR_TRISTATE_(SWITCH|BOOL)_ABSENT` is explicitly checked for (`val < 0` vs. `val <= 0`), whether an intermediate variable is used or the value is assigned directly, and in some cases the conditions in the two if-blocks are merged. After the refactoring, the above code block looks like this: if (virXMLPropTristateBool(node, "x", VIR_XML_PROP_OPTIONAL, &def->x) < 0) return -1; Similar functions are introduced for integer valued attributes, unsigned integer valued attributes and enum valued attributes. Patches #6, #7, and #8 demonstrate the application of these function and stand representative of more patches that I did not sent along yet as to not drown the mailing list in spam. These patches remove a total of ~ 1000 lines of code and fix some places, where e.g. `<foo bar="default"/>` would incorrectly be accepted as virXMLTristateBool. As an added benefit, this refactoring makes the error messages for invalid values in these XML attributes much more consistent: $ git diff master | grep "^-.*_(\"" | wc -l 239 $ git diff master | grep "^+.*_(\"" | wc -l 21 V1: https://listman.redhat.com/archives/libvir-list/2021-March/msg00853.html V2: https://listman.redhat.com/archives/libvir-list/2021-March/msg00994.html V3: https://listman.redhat.com/archives/libvir-list/2021-March/msg01066.html V4: https://listman.redhat.com/archives/libvir-list/2021-April/msg00209.html Changes since V4: * Added custom error message for virXMLProp(Int|UInt) when the attribute value is 0 and VIR_XML_PROP_NONZERO is specified. Cheers, Tim Tim Wiederhake (8): virxml: Add virXMLPropTristateBool virxml: Add virXMLPropTristateSwitch virxml: Add virXMLPropInt virxml: Add virXMLPropUInt virxml: Add virXMLPropEnum virNetworkForwardNatDefParseXML: Use virXMLProp* virDomainIOThreadIDDefParseXML: Use virXMLProp* virCPUDefParseXML: Use virXMLProp* src/conf/cpu_conf.c | 14 +- src/conf/domain_conf.c | 14 +- src/conf/network_conf.c | 16 +-- src/libvirt_private.syms | 5 + src/util/virxml.c | 267 +++++++++++++++++++++++++++++++++++++++ src/util/virxml.h | 31 +++++ 6 files changed, 312 insertions(+), 35 deletions(-) -- 2.26.2

2 18

[libvirt PATCH 00/14] Use virTristateXXX for more struct members
by Tim Wiederhake 16 Apr '21

16 Apr '21

This is a preparation step for some refactoring of the XML parser, see https://listman.redhat.com/archives/libvir-list/2021-March/msg01066.html Many libvirt structs have members that are of type `int` but actually are virTristateBool or virTristateSwitch. Fix this to increase type safety. Note that the comments on `virStorageAdapterFCHost::managed` and `virStoragePoolSourceDevice::part_separator` (patches #12 and #13) were actually wrong. Cheers, Tim Tim Wiederhake (14): conf: Use virTristateXXX in virStorageSource conf: Use virTristateXXX in virStorageSourceNVMeDef conf: Use virTristateXXX in virDomainDeviceInfo conf: Use virTristateXXX in virDomainDiskDef conf: Use virTristateXXX in virDomainActualNetDef conf: Use virTristateXXX in virDomainNetDef conf: Use virTristateXXX in virDomainChrSourceDef conf: Use virTristateXXX in virDomainGraphicsDef conf: Use virTristateXXX in virDomainMemballoonDef conf: Use virTristateXXX in virDomainLoaderDef conf: Use virTristateXXX in virDomainDef conf: Use virTristateXXX in virStorageAdapterFCHost conf: Use virTristateXXX in virStoragePoolSourceDevice conf: Use virTristateXXX in virPCIDeviceAddress src/conf/device_conf.c | 16 +-- src/conf/device_conf.h | 4 +- src/conf/domain_conf.c | 201 +++++++++++++++++++------------- src/conf/domain_conf.h | 28 ++--- src/conf/storage_adapter_conf.c | 4 +- src/conf/storage_adapter_conf.h | 2 +- src/conf/storage_conf.c | 7 +- src/conf/storage_conf.h | 2 +- src/conf/storage_source_conf.h | 4 +- src/qemu/qemu_command.c | 3 +- src/qemu/qemu_hotplug.c | 2 +- src/util/virpci.h | 2 +- 12 files changed, 162 insertions(+), 113 deletions(-) -- 2.26.2

2 16