December 2019 - Devel - libvirt List Archives

[libvirt] [PATCH v4 0/4] Rewrite virGetUser*Directory() functions using g_get_*_dir()
by Fabiano Fidêncio 18 Dec '19

18 Dec '19

By rewriting virGetUser*Directory() functions using g_get_*_dir() functions allows us to drop all the different implementations we keep, as GLib already takes care of those for us. Changes since v3: https://www.redhat.com/archives/libvir-list/2019-December/msg01123.html - Used g_build_filename() instead of g_strdup_printf(); Changes since v2: https://www.redhat.com/archives/libvir-list/2019-December/msg01064.html - Addressed comments made by Cole about: - virGetUserDirectory() should return $HOME; - virGetUser*Directory() don't append "libvirt" to the returned path on Windows; Changes since v1: https://www.redhat.com/archives/libvir-list/2019-December/msg01055.html - Don't check for the return of g_get_*_dir(), as it cannot be NULL; Fabiano Fidêncio (4): util: Rewrite virGetUserDirectory() using g_get_home_dir() util: Rewrite virGetUserConfigDirectory() using g_get_user_config_dir() util: Rewrite virGetUserCacheDirectory() using g_get_user_cache_dir() util: Rewrite virGetUserRuntimeDirectory() using g_get_user_runtime_dir() src/util/virutil.c | 137 ++++++++++----------------------------------- 1 file changed, 31 insertions(+), 106 deletions(-) -- 2.23.0

2 5

[libvirt] [PATCH v2 0/4] Rewrite virGetUser*Directory() functions using g_get_*_dir()
by Fabiano Fidêncio 18 Dec '19

18 Dec '19

By rewriting virGetUser*Directory() functions using g_get_*_dir() functions allows us to drop all the different implementations we keep, as GLib already takes care of those for us. Changes since v1: https://www.redhat.com/archives/libvir-list/2019-December/msg01055.html - Don't check for the return of g_get_*_dir(), as it cannot be NULL; Fabiano Fidêncio (4): util: Rewrite virGetUserDirectory() using g_get_home_dir() util: Rewrite virGetUserConfigDirectory() using g_get_user_config_dir() util: Rewrite virGetUserCacheDirectory() using g_get_user_cache_dir() util: Rewrite virGetUserRuntimeDirectory() using g_get_user_runtime_dir() src/util/virutil.c | 125 +++++++-------------------------------------- 1 file changed, 19 insertions(+), 106 deletions(-) -- 2.23.0

4 12

[libvirt] [PATCH v3 0/1] qemu_command: tidy up qemuBuildHostdevCommandLine loop
by Daniel Henrique Barboza 18 Dec '19

18 Dec '19

This fella got buried a few months ago. Re-sending as a v3 due to rebase changes. changes from previous version [1]: - rebased to master at 6c17606b7c [1] https://www.redhat.com/archives/libvir-list/2019-October/msg00457.html Daniel Henrique Barboza (1): qemu_command: tidy up qemuBuildHostdevCommandLine loop src/qemu/qemu_command.c | 52 +++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 20 deletions(-) -- 2.23.0

2 2

[libvirt] [PATCH] qemu: store the emulator name in the capabilities XML
by Daniel P. Berrangé 18 Dec '19

18 Dec '19

We don't need this for any functional purpose, but when debugging hosts it is useful to know what binary a given capabilities XML document is associated with. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/qemu/qemu_capabilities.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 2223589058..7d47fa4d02 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3852,6 +3852,7 @@ virQEMUCapsParseSEVInfo(virQEMUCapsPtr qemuCaps, xmlXPathContextPtr ctxt) * Parsing a doc that looks like * * <qemuCaps> + * <emulator>/some/path</emulator> * <qemuctime>234235253</qemuctime> * <selfctime>234235253</selfctime> * <selfvers>1002016</selfvers> @@ -3895,6 +3896,18 @@ virQEMUCapsLoadCache(virArch hostArch, goto cleanup; } + if (!(str = virXPathString("string(./emulator)", ctxt))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing emulator in QEMU capabilities cache")); + goto cleanup; + } + if (!STREQ(str, qemuCaps->binary)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Expected caps for '%s' but saw '%s'"), + qemuCaps->binary, str); + goto cleanup; + } + VIR_FREE(str); if (virXPathLongLong("string(./qemuctime)", ctxt, &l) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing qemuctime in QEMU capabilities XML")); @@ -4232,6 +4245,8 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps) virBufferAddLit(&buf, "<qemuCaps>\n"); virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<emulator>%s</emulator>\n", + qemuCaps->binary); virBufferAsprintf(&buf, "<qemuctime>%llu</qemuctime>\n", (long long)qemuCaps->ctime); virBufferAsprintf(&buf, "<selfctime>%llu</selfctime>\n", -- 2.23.0

3 3

[libvirt] [PATCH] conf: fix populating of fake NUMA in multi-node hosts
by Daniel P. Berrangé 18 Dec '19

18 Dec '19

If the host OS doesn't have NUMA present, we fallback to populating fake NUMA info and the code thus assumes only a single NUMA node. Unfortunately we also fallback to fake NUMA if numactl-devel was not present, and in this case we can still have multiple NUMA nodes. In this case we create all CPUs, but only the CPUs in the first node have any data filled in, resulting in capabilities like: <topology> <cells num='1'> <cell id='0'> <memory unit='KiB'>15977572</memory> <cpus num='48'> <cpu id='0' socket_id='0' core_id='0' siblings='0'/> <cpu id='1' socket_id='0' core_id='0' siblings='1'/> <cpu id='2' socket_id='0' core_id='1' siblings='2'/> <cpu id='3' socket_id='0' core_id='1' siblings='3'/> <cpu id='4' socket_id='0' core_id='2' siblings='4'/> <cpu id='5' socket_id='0' core_id='2' siblings='5'/> <cpu id='6' socket_id='0' core_id='3' siblings='6'/> <cpu id='7' socket_id='0' core_id='3' siblings='7'/> <cpu id='8' socket_id='0' core_id='4' siblings='8'/> <cpu id='9' socket_id='0' core_id='4' siblings='9'/> <cpu id='10' socket_id='0' core_id='5' siblings='10'/> <cpu id='11' socket_id='0' core_id='5' siblings='11'/> <cpu id='0'/> <cpu id='0'/> <cpu id='0'/> <cpu id='0'/> <cpu id='0'/> <cpu id='0'/> <cpu id='0'/> <cpu id='0'/> <cpu id='0'/> <cpu id='0'/> <cpu id='0'/> </cpus> </cell> </cells> </topology> With this new code we get something slightly less broken <topology> <cells num='4'> <cell id='0'> <memory unit='KiB'>15977572</memory> <cpus num='12'> <cpu id='0' socket_id='0' core_id='0' siblings='0-1'/> <cpu id='1' socket_id='0' core_id='0' siblings='0-1'/> <cpu id='2' socket_id='0' core_id='1' siblings='2-3'/> <cpu id='3' socket_id='0' core_id='1' siblings='2-3'/> <cpu id='4' socket_id='0' core_id='2' siblings='4-5'/> <cpu id='5' socket_id='0' core_id='2' siblings='4-5'/> <cpu id='6' socket_id='0' core_id='3' siblings='6-7'/> <cpu id='7' socket_id='0' core_id='3' siblings='6-7'/> <cpu id='8' socket_id='0' core_id='4' siblings='8-9'/> <cpu id='9' socket_id='0' core_id='4' siblings='8-9'/> <cpu id='10' socket_id='0' core_id='5' siblings='10-11'/> <cpu id='11' socket_id='0' core_id='5' siblings='10-11'/> </cpus> </cell> <cell id='0'> <memory unit='KiB'>15977572</memory> <cpus num='12'> <cpu id='12' socket_id='0' core_id='0' siblings='12-13'/> <cpu id='13' socket_id='0' core_id='0' siblings='12-13'/> <cpu id='14' socket_id='0' core_id='1' siblings='14-15'/> <cpu id='15' socket_id='0' core_id='1' siblings='14-15'/> <cpu id='16' socket_id='0' core_id='2' siblings='16-17'/> <cpu id='17' socket_id='0' core_id='2' siblings='16-17'/> <cpu id='18' socket_id='0' core_id='3' siblings='18-19'/> <cpu id='19' socket_id='0' core_id='3' siblings='18-19'/> <cpu id='20' socket_id='0' core_id='4' siblings='20-21'/> <cpu id='21' socket_id='0' core_id='4' siblings='20-21'/> <cpu id='22' socket_id='0' core_id='5' siblings='22-23'/> <cpu id='23' socket_id='0' core_id='5' siblings='22-23'/> </cpus> </cell> </cells> </topology> The topology at least now reflects what 'virsh nodeinfo' reports. The main bug is that the CPU "id" values won't match what the Linux host actually uses. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/conf/capabilities.c | 67 ++++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 31 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 81a2004dba..2a183d7070 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -1593,7 +1593,7 @@ virCapabilitiesHostNUMAInitFake(virCapsHostNUMAPtr caps) virNodeInfo nodeinfo; virCapsHostNUMACellCPUPtr cpus; int ncpus; - int s, c, t; + int n, s, c, t; int id, cid; int onlinecpus G_GNUC_UNUSED; bool tmp; @@ -1602,47 +1602,52 @@ virCapabilitiesHostNUMAInitFake(virCapsHostNUMAPtr caps) return -1; ncpus = VIR_NODEINFO_MAXCPUS(nodeinfo); - onlinecpus = nodeinfo.cpus; - if (VIR_ALLOC_N(cpus, ncpus) < 0) - return -1; - id = cid = 0; - for (s = 0; s < nodeinfo.sockets; s++) { - for (c = 0; c < nodeinfo.cores; c++) { - for (t = 0; t < nodeinfo.threads; t++) { - if (virHostCPUGetOnline(id, &tmp) < 0) - goto error; - if (tmp) { - cpus[cid].id = id; - cpus[cid].socket_id = s; - cpus[cid].core_id = c; - if (!(cpus[cid].siblings = virBitmapNew(ncpus))) + id = 0; + for (n = 0; n < nodeinfo.nodes; n++) { + int nodecpus = nodeinfo.sockets * nodeinfo.cores * nodeinfo.threads; + cid = 0; + + if (VIR_ALLOC_N(cpus, nodecpus) < 0) + return -1; + + for (s = 0; s < nodeinfo.sockets; s++) { + for (c = 0; c < nodeinfo.cores; c++) { + g_autoptr(virBitmap) siblings = virBitmapNew(ncpus); + for (t = 0; t < nodeinfo.threads; t++) + ignore_value(virBitmapSetBit(siblings, id + t)); + + for (t = 0; t < nodeinfo.threads; t++) { + if (virHostCPUGetOnline(id, &tmp) < 0) goto error; - ignore_value(virBitmapSetBit(cpus[cid].siblings, id)); - cid++; + if (tmp) { + cpus[cid].id = id; + cpus[cid].socket_id = s; + cpus[cid].core_id = c; + if (!(cpus[cid].siblings = virBitmapNew(ncpus))) + goto error; + virBitmapCopy(cpus[cid].siblings, siblings); + cid++; + } + + id++; } - - id++; } } - } - virCapabilitiesHostNUMAAddCell(caps, 0, - nodeinfo.memory, -#ifdef __linux__ - onlinecpus, cpus, -#else - ncpus, cpus, -#endif - 0, NULL, - 0, NULL); + virCapabilitiesHostNUMAAddCell(caps, 0, + nodeinfo.memory, + cid, cpus, + 0, NULL, + 0, NULL); + } return 0; error: - for (; id >= 0; id--) - virBitmapFree(cpus[id].siblings); + for (; cid >= 0; cid--) + virBitmapFree(cpus[cid].siblings); VIR_FREE(cpus); return -1; } -- 2.23.0

2 1

[libvirt] [PATCH 0/5] Various memleak fixes
by Michal Privoznik 18 Dec '19

18 Dec '19

*** BLURB HERE *** Michal Prívozník (5): domaincapstest: Don't leak cpu definitions testutilsxen: Avoid double free of driver caps virCapabilitiesHostNUMAUnref: Accept NULL qemu: Reoder cleanup in qemuStateCleanup() qemu: Don't leak hostcpu or hostnuma on driver cleanup src/conf/capabilities.c | 3 +++ src/qemu/qemu_driver.c | 45 +++++++++++++++++------------------------ tests/domaincapstest.c | 2 +- tests/testutilsxen.c | 1 - 4 files changed, 23 insertions(+), 28 deletions(-) -- 2.24.1

2 8

[libvirt] [PATCH] conf: avoid mem leak re-allocating fake NUMA capabilities
by Daniel P. Berrangé 18 Dec '19

18 Dec '19

The 'caps' object is already allocated when the fake NUMA initialization takes place. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/conf/capabilities.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 4fac59e6f7..81a2004dba 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -1628,9 +1628,6 @@ virCapabilitiesHostNUMAInitFake(virCapsHostNUMAPtr caps) } } - caps = g_new0(virCapsHostNUMA, 1); - caps->cells = g_ptr_array_new_with_free_func( - (GDestroyNotify)virCapabilitiesFreeHostNUMACell); virCapabilitiesHostNUMAAddCell(caps, 0, nodeinfo.memory, #ifdef __linux__ -- 2.23.0

2 1

[libvirt] [PATCH 0/3] qemu: fix crash bugs in snapshot revert
by Pavel Mores 18 Dec '19

18 Dec '19

The aim of this series is to fix https://bugzilla.redhat.com/show_bug.cgi?id=1610207 however before getting to that we first need to fix an unrelated (and much more recent) bug in patch 1. We clean up the fix in patch 2 by converting the whole function to the new allocation idioms. The actual reported bug is then fixed in patch 3. Pavel Mores (3): qemu: fix crash due to freeing an uninitialised pointer qemu: use g_autofree instead of VIR_FREE in qemuMonitorTextCreateSnapshot() qemu: fix concurrency crash bug in snapshot revert src/qemu/qemu_driver.c | 17 ++++++++++++++--- src/qemu/qemu_monitor_text.c | 20 ++++++-------------- 2 files changed, 20 insertions(+), 17 deletions(-) -- 2.21.0

3 7

[libvirt] [PATCH 0/3] Some coverity adjustments
by John Ferlan 18 Dec '19

18 Dec '19

I upgraded to f31 and it resulted in an essentially hosed Coverity build/analysis environment with the following message during cov-emit processing (a preprocessing of sorts): "/usr/include/glib-2.0/glib/gspawn.h", line 76: error #67: expected a "}" G_SPAWN_ERROR_2BIG GLIB_DEPRECATED_ENUMERATOR_IN_2_32_FOR(G_SPAWN_ERROR_TOO_BIG) = G_SPAWN_ERROR_TOO_BIG, ^ So instead, I'm using a guest to run Coverity "when I remember/can". I also found that my f31 environment doesn't like building w/ docs as I get the following messages while running the convert command: ... usr/bin/mv: cannot stat '/tmp/magick-1191987h12h27ex0lZD.svg': No such file or directory GEN kbase.html.tmp convert: delegate failed `'uniconvertor' '%i' '%o.svg'; /usr/bin/mv '%o.svg' '%o'' @ error/delegate.c/InvokeDelegate/1958. convert: unable to open file `/tmp/magick-1191987OqYJwrq8isaG': No such file or directory @ error/constitute.c/ReadImage/605. convert: no images defined `migration-managed-p2p.png' @ error/convert.c/ConvertImageCommand/3235. .... I haven't followed along as closely as I used to, but my vpath env uses obj as a subdirectory of my main git tree/target. Whether the new build env has anything to do with it or it's just f31, I haven't been able to determine. Beyond these 3 patches here - there is one other adjustment that is necessary to build libvirt under Coverity and that's removing the ATTRIBUTE_NONNULL(2) from the virDomainDefFormat definition in src/conf/domain_conf.h. This was added in commit 92d412149 which also included two calls to virDomainDefFormat with NULL as the 2nd argument (hyperv_driver.c and security_apparmor.c); however, the commit message notes preparation for future work, so I'll keep a hack for that local for now at least. The virsh change below is innocuous yes, but it showed up in a coverity analysis because it wasn't sure if the resulting variables could point to the same address and if they did, then there was a possible use after free because the @source is free'd even though the @target_node is later referenced. The patch here avoids that and provides a slight adjustment to not search for either node by name if it was already found. Whether there's a weird latent issue because <source> can be repeated while <target> cannot be is something I suppose a reviewer can warn me about ;-). John Ferlan (3): conf: Fix ATTRIBUTE_NONNULL usages vbox: Reset @ret after xmlFreeNode virsh: Adjust logic checks in virshUpdateDiskXML src/conf/domain_conf.h | 15 ++++++--------- src/vbox/vbox_snapshot_conf.c | 1 + tools/virsh-domain.c | 5 ++--- 3 files changed, 9 insertions(+), 12 deletions(-) -- 2.23.0

2 4

[libvirt] [PATCH] test: qemucaps: Refresh x86_64 caps probe data for the qemu-4.2 release
by Peter Krempa 18 Dec '19

18 Dec '19

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- Pushed as trivial. tests/qemucapabilitiesdata/caps_4.2.0.x86_64.replies | 10 +++++----- tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.replies index ae07ffdb49..b9481b6f85 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.replies @@ -17,11 +17,11 @@ { "return": { "qemu": { - "micro": 92, - "minor": 1, + "micro": 0, + "minor": 2, "major": 4 }, - "package": "v4.2.0-rc2-19-g2061735ff0" + "package": "v4.2.0" }, "id": "libvirt-2" } @@ -23614,7 +23614,7 @@ "kvm-steal-time": true, "kvmclock": true, "vmx-zero-len-inject": false, - "pschange-mc-no": false, + "pschange-mc-no": true, "vmx-rdrand-exit": true, "lwp": false, "amd-ssbd": false, @@ -23930,7 +23930,7 @@ "kvm-steal-time": true, "kvmclock": true, "vmx-zero-len-inject": false, - "pschange-mc-no": false, + "pschange-mc-no": true, "vmx-rdrand-exit": true, "lwp": false, "amd-ssbd": false, diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml index 31302c9a7b..7d886d9a87 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml @@ -217,10 +217,10 @@ <flag name='blockdev-file-dynamic-auto-read-only'/> <flag name='savevm-monitor-nodes'/> <flag name='drive-nvme'/> - <version>4001092</version> + <version>4002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> - <package>v4.2.0-rc2-19-g2061735ff0</package> + <package>v4.2.0</package> <arch>x86_64</arch> <hostCPU type='kvm' model='base' migratability='yes'> <property name='vmx-entry-load-rtit-ctl' type='boolean' value='false'/> @@ -416,7 +416,7 @@ <property name='kvm-steal-time' type='boolean' value='true' migratable='yes'/> <property name='kvmclock' type='boolean' value='true' migratable='yes'/> <property name='vmx-zero-len-inject' type='boolean' value='false'/> - <property name='pschange-mc-no' type='boolean' value='false'/> + <property name='pschange-mc-no' type='boolean' value='true' migratable='yes'/> <property name='vmx-rdrand-exit' type='boolean' value='true' migratable='yes'/> <property name='lwp' type='boolean' value='false'/> <property name='amd-ssbd' type='boolean' value='false'/> -- 2.23.0

1 0