July 2020 - Devel - libvirt List Archives

[PATCH 0/3] kbase: Port 'debuglogs' from wiki and convert directory to rst
by Peter Krempa 14 Nov '20

14 Nov '20

This patchset: 1) converts the 'kbase' directory page to RST 2) increases width of the container holding the kbase articles: before: https://libvirt.org/kbase.html after: https://pipo.sk.gitlab.io/-/libvirt/-/jobs/663385238/artifacts/website/kbas… 3) ports 'debuglogs' wiki article to the kbase: original: https://wiki.libvirt.org/page/DebugLogs converted: https://pipo.sk.gitlab.io/-/libvirt/-/jobs/663385238/artifacts/website/kbas… Note that this patchset requires Pavel's meson rewrite. Peter Krempa (3): docs: kbase: Convert 'kbase' article registry to RST docs: kbase: Make kbase article directory wider docs: kbase: Port 'debuglogs' document from libvirt's wiki docs/kbase.html.in | 54 -------------- docs/kbase.rst | 46 ++++++++++++ docs/kbase/debuglogs.rst | 148 +++++++++++++++++++++++++++++++++++++++ docs/kbase/meson.build | 1 + docs/libvirt.css | 8 +++ docs/meson.build | 2 +- 6 files changed, 204 insertions(+), 55 deletions(-) delete mode 100644 docs/kbase.html.in create mode 100644 docs/kbase.rst create mode 100644 docs/kbase/debuglogs.rst -- 2.26.2

6 13

Races / crashes in shutdown of libvirtd daemon
by Daniel P. Berrangé 13 Nov '20

13 Nov '20

We got a new BZ filed about a libvirtd crash in shutdown https://bugzilla.redhat.com/show_bug.cgi?id=1828207 We can see from the stack trace that the "interface" driver is in the middle of servicing an RPC call for virConnectListAllInterfaces() Meanwhile the libvirtd daemon is doing virObjectUnref(dmn) on the virNetDaemonPtr object. The fact that it is doing this unref, means that it must have already call virStateCleanup(), given the code sequence: /* Run event loop. */ virNetDaemonRun(dmn); ret = 0; virHookCall(VIR_HOOK_DRIVER_DAEMON, "-", VIR_HOOK_DAEMON_OP_SHUTDOWN, 0, "shutdown", NULL, NULL); cleanup: /* Keep cleanup order in inverse order of startup */ virNetDaemonClose(dmn); virNetlinkEventServiceStopAll(); if (driversInitialized) { /* NB: Possible issue with timing window between driversInitialized * setting if virNetlinkEventServerStart fails */ driversInitialized = false; virStateCleanup(); } virObjectUnref(adminProgram); virObjectUnref(srvAdm); virObjectUnref(qemuProgram); virObjectUnref(lxcProgram); virObjectUnref(remoteProgram); virObjectUnref(srv); virObjectUnref(dmn); Unless I'm missing something non-obvious, this cleanup code path is inherantly broken & racy. When virNetDaemonRun() returns the RPC worker threads are all still active. They are all liable to still be executing RPC calls, which means any of the drivers may be in use. So calling virStateCleanup() is an inherantly dangerous thing to do. There is the further complication that once we have exitted the main loop we may prevent the RPC calls from ever completing, as they may be waiting on an event to be dispatched. I know we're had various patch proposals in the past to improve the robustness of shutdown cleanup but I can't remember the outcome of the reviews. Hopefully people involved in those threads can jump in here... IMHO the key problem here is the virNetDeamonRun() method which just looks at the "quit" flag and immediately returns if it is set. This needs to be changed so that when it sees quit == true, it takes the following actions 1. Call virNetDaemonClose() to drop all RPC clients and thus prevent new RPC calls arriving 2. Flush any RPC calls which are queued but not yet assigned to a worker thread 3. Tell worker threads to exit after finishing their current job 4. Wait for all worker threads to exit 5. Now virNetDaemonRun may return At this point we can call virStateCleanup and the various other things, as we know no drivers are still active in RPC calls. Having said that, there could be background threads in the the drivers which are doing work that uses the event loop thread. So we probably need a virStateClose() method that we call from virNetDaemonRun, *after* all worker threads are gone, which would cleanup any background threads while the event loop is still running. The issue is that step 4 above ("Wait for all worker threads to exit") may take too long, or indeed never complete. To deal with this, it will need a timeout. In the remote_daemon.c cleanup code path, if there are still worker threads present, then we need to skip all cleanup and simply call _exit(0) to terminate the process with no attempt at cleanup, since it would be unsafe to try anything else. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

3 4

[libvirt PATCH v2 00/10] remote: introduce a custom netcat impl for ssh tunnelling
by Daniel P. Berrangé 27 Oct '20

27 Oct '20

We have long had a problem with use of netcat for ssh tunnelling because there's no guarantee the UNIX socket path the client builds will match the UNIX socket path the remote host uses. We don't even allow session mode SSH tunnelling for this reason. We also can't easily auto-spawn libvirtd in session mode. With the introduction of modular daemons we also have potential for two completely different UNIX socket paths even for system mode, and the client can't know which to use. The solution to all these problems is to introduce a custom netcat impl. Instead passing the UNIX socket path, we pass the libvirt driver URI. The custom netcat then decides which socket path to use based on the remote build host environment. We still have to support netcat for interoperability with legacy libvirt versions, but we can default to the new virt-nc. Daniel P. Berrangé (10): rpc: merge logic for generating remote SSH shell script remote: push logic for default netcat binary into common helper remote: split off enums into separate source file remote: split out function for parsing URI scheme remote: parse the remote transport string earlier remote: split out function for constructing socket path remote: extract logic for determining daemon to connect to remote: introduce virt-ssh-helper binary rpc: switch order of args in virNetClientNewSSH rpc: use new virt-ssh-helper binary for remote tunnelling build-aux/syntax-check.mk | 2 +- docs/uri.html.in | 24 +- libvirt.spec.in | 2 + po/POTFILES.in | 2 + src/libvirt_remote.syms | 1 + src/remote/Makefile.inc.am | 33 +++ src/remote/remote_driver.c | 323 ++++--------------------- src/remote/remote_sockets.c | 277 +++++++++++++++++++++ src/remote/remote_sockets.h | 70 ++++++ src/remote/remote_ssh_helper.c | 425 +++++++++++++++++++++++++++++++++ src/rpc/virnetclient.c | 166 ++++++++----- src/rpc/virnetclient.h | 29 ++- src/rpc/virnetsocket.c | 37 +-- src/rpc/virnetsocket.h | 4 +- tests/virnetsockettest.c | 12 +- 15 files changed, 1035 insertions(+), 372 deletions(-) create mode 100644 src/remote/remote_sockets.c create mode 100644 src/remote/remote_sockets.h create mode 100644 src/remote/remote_ssh_helper.c -- 2.26.2

2 14

[PATCH] qemu: clear residual QMP caps processes during QEMU driver initialization
by Bihong Yu 26 Sep '20

26 Sep '20

>From c328ff62b11d58553fd2032a85fd3295e009b3d3 Mon Sep 17 00:00:00 2001 From: Bihong Yu <yubihong(a)huawei.com> Date: Fri, 17 Jul 2020 16:55:12 +0800 Subject: [PATCH] qemu: clear residual QMP caps processes during QEMU driver initialization In some cases, the QMP capabilities processes possible residue. So we need to clear the residual QMP caps processes during starting libvirt. Signed-off-by:Bihong Yu <yubihong(a)huawei.com> --- src/qemu/qemu_driver.c | 2 ++ src/qemu/qemu_process.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_process.h | 2 ++ 3 files changed, 44 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d185666..d627fd7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -889,6 +889,8 @@ qemuStateInitialize(bool privileged, run_gid = cfg->group; } + qemuProcessQMPClear(cfg->libDir); + qemu_driver->qemuCapsCache = virQEMUCapsCacheNew(cfg->libDir, cfg->cacheDir, run_uid, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 70fc24b..d545e3e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8312,6 +8312,46 @@ static qemuMonitorCallbacks callbacks = { }; +/** + * qemuProcessQMPClear + * + * Try to kill residual QMP caps processes + */ +void +qemuProcessQMPClear(const char *libDir) +{ + virErrorPtr orig_err; + DIR *dirp = NULL; + struct dirent *dp; + + if (!virFileExists(libDir)) + return; + + if (virDirOpen(&dirp, libDir) < 0) + return; + + virErrorPreserveLast(&orig_err); + while (virDirRead(dirp, &dp, libDir) > 0) { + g_autofree char *qmp_uniqDir = NULL; + g_autofree char *qmp_pidfile = NULL; + + if (!STRPREFIX(dp->d_name, "qmp-")) + continue; + + qmp_uniqDir = g_strdup_printf("%s/%s", libDir, dp->d_name); + qmp_pidfile = g_strdup_printf("%s/%s", qmp_uniqDir, "qmp.pid"); + + ignore_value(virPidFileForceCleanupPath(qmp_pidfile)); + + if (qmp_uniqDir) + rmdir(qmp_uniqDir); + } + virErrorRestore(&orig_err); + + VIR_DIR_CLOSE(dirp); +} + + static void qemuProcessQMPStop(qemuProcessQMPPtr proc) { diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 15e67b9..b039e6c 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -233,6 +233,8 @@ qemuProcessQMPPtr qemuProcessQMPNew(const char *binary, gid_t runGid, bool forceTCG); +void qemuProcessQMPClear(const char *libDir); + void qemuProcessQMPFree(qemuProcessQMPPtr proc); int qemuProcessQMPStart(qemuProcessQMPPtr proc); -- 1.8.3.1

3 7

[PATCH v1] qemu: monitor: substitute missing model name for host-passthrough
by Collin Walling 24 Sep '20

24 Sep '20

When executing the hypervisor-cpu-compare/baseline commands and the XML file contains a CPU definition using host-passthrough and no model name, the commands will fail and return an error message from the QMP response. Let's fix this by checking for host-passthrough and a missing model name when converting a CPU definition to a JSON object. If these conditions are matched, then the JSON object will be constructed using "host" as the model name. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1850680 Signed-off-by: Collin Walling <walling(a)linux.ibm.com> --- src/qemu/qemu_monitor_json.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3070c1e6b3..448a3a9356 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5804,9 +5804,20 @@ qemuMonitorJSONMakeCPUModel(virCPUDefPtr cpu, { virJSONValuePtr model = virJSONValueNewObject(); virJSONValuePtr props = NULL; + const char *model_name = cpu->model; size_t i; - if (virJSONValueObjectAppendString(model, "name", cpu->model) < 0) + if (!model_name) { + if (cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) { + model_name = "host"; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cpu parameter is missing a model name")); + goto error; + } + } + + if (virJSONValueObjectAppendString(model, "name", model_name) < 0) goto error; if (cpu->nfeatures || !migratable) { -- 2.26.2

4 8

device compatibility interface for live migration with assigned devices
by Yan Zhao 15 Sep '20

15 Sep '20

hi folks, we are defining a device migration compatibility interface that helps upper layer stack like openstack/ovirt/libvirt to check if two devices are live migration compatible. The "devices" here could be MDEVs, physical devices, or hybrid of the two. e.g. we could use it to check whether - a src MDEV can migrate to a target MDEV, - a src VF in SRIOV can migrate to a target VF in SRIOV, - a src MDEV can migration to a target VF in SRIOV. (e.g. SIOV/SRIOV backward compatibility case) The upper layer stack could use this interface as the last step to check if one device is able to migrate to another device before triggering a real live migration procedure. we are not sure if this interface is of value or help to you. please don't hesitate to drop your valuable comments. (1) interface definition The interface is defined in below way: __ userspace /\ \ / \write / read \ ________/__________ ___\|/_____________ | migration_version | | migration_version |-->check migration --------------------- --------------------- compatibility device A device B a device attribute named migration_version is defined under each device's sysfs node. e.g. (/sys/bus/pci/devices/0000\:00\:02.0/$mdev_UUID/migration_version). userspace tools read the migration_version as a string from the source device, and write it to the migration_version sysfs attribute in the target device. The userspace should treat ANY of below conditions as two devices not compatible: - any one of the two devices does not have a migration_version attribute - error when reading from migration_version attribute of one device - error when writing migration_version string of one device to migration_version attribute of the other device The string read from migration_version attribute is defined by device vendor driver and is completely opaque to the userspace. for a Intel vGPU, string format can be defined like "parent device PCI ID" + "version of gvt driver" + "mdev type" + "aggregator count". for an NVMe VF connecting to a remote storage. it could be "PCI ID" + "driver version" + "configured remote storage URL" for a QAT VF, it may be "PCI ID" + "driver version" + "supported encryption set". (to avoid namespace confliction from each vendor, we may prefix a driver name to each migration_version string. e.g. i915-v1-8086-591d-i915-GVTg_V5_8-1) (2) backgrounds The reason we hope the migration_version string is opaque to the userspace is that it is hard to generalize standard comparing fields and comparing methods for different devices from different vendors. Though userspace now could still do a simple string compare to check if two devices are compatible, and result should also be right, it's still too limited as it excludes the possible candidate whose migration_version string fails to be equal. e.g. an MDEV with mdev_type_1, aggregator count 3 is probably compatible with another MDEV with mdev_type_3, aggregator count 1, even their migration_version strings are not equal. (assumed mdev_type_3 is of 3 times equal resources of mdev_type_1). besides that, driver version + configured resources are all elements demanding to take into account. So, we hope leaving the freedom to vendor driver and let it make the final decision in a simple reading from source side and writing for test in the target side way. we then think the device compatibility issues for live migration with assigned devices can be divided into two steps: a. management tools filter out possible migration target devices. Tags could be created according to info from product specification. we think openstack/ovirt may have vendor proprietary components to create those customized tags for each product from each vendor. e.g. for Intel vGPU, with a vGPU(a MDEV device) in source side, the tags to search target vGPU are like: a tag for compatible parent PCI IDs, a tag for a range of gvt driver versions, a tag for a range of mdev type + aggregator count for NVMe VF, the tags to search target VF may be like: a tag for compatible PCI IDs, a tag for a range of driver versions, a tag for URL of configured remote storage. b. with the output from step a, openstack/ovirt/libvirt could use our proposed device migration compatibility interface to make sure the two devices are indeed live migration compatible before launching the real live migration process to start stream copying, src device stopping and target device resuming. It is supposed that this step would not bring any performance penalty as -in kernel it's just a simple string decoding and comparing -in openstack/ovirt, it could be done by extending current function check_can_live_migrate_destination, along side claiming target resources.[1] [1] https://specs.openstack.org/openstack/nova-specs/specs/stein/approved/libvi… Thanks Yan

14 118

[PATCH 0/5] remove NVDIMM auto-alignment for pSeries guests
by Daniel Henrique Barboza 11 Sep '20

11 Sep '20

Hi, In [1] Andrea proposed that we should *not* auto-align down the NVDIMM size for pSeries guests. Instead we should error out and provide users with a suggested aligned value. This patch implements it. [1] https://www.redhat.com/archives/libvir-list/2020-July/msg01471.html Daniel Henrique Barboza (5): qemu_domain.c: make qemuDomainGetMemorySizeAlignment() public qemu_validate.c: add pSeries NVDIMM size alignment validation qemu_domain.c: do not auto-align ppc64 NVDIMMs qemu_domain.c: change qemuDomainMemoryDeviceAlignSize() return type Revert "formatdomain.html.in: mention pSeries NVDIMM 'align down' mechanic" docs/formatdomain.html.in | 6 +- src/qemu/qemu_domain.c | 57 ++----------------- src/qemu/qemu_domain.h | 7 ++- src/qemu/qemu_hotplug.c | 6 +- src/qemu/qemu_validate.c | 43 ++++++++++++-- ...ory-hotplug-nvdimm-ppc64.ppc64-latest.args | 2 +- .../memory-hotplug-nvdimm-ppc64.xml | 2 +- .../memory-hotplug-nvdimm-ppc64.xml | 2 +- 8 files changed, 55 insertions(+), 70 deletions(-) -- 2.26.2

3 14

[libvirt] [PATCH-for-4.2] hw/mips: Deprecate the r4k machine
by Philippe Mathieu-Daudé 10 Sep '20

10 Sep '20

The r4k machine was introduced in 2005 (6af0bf9c7) and its last logical change was in 2005 (9542611a6). After we can count 164 maintenance commits (QEMU API changes) with the exception of 1 fix in 2015 (memory leak, commit 3ad9fd5a). This machine was introduced as a proof of concept to run a MIPS CPU. 2 years later, the Malta machine was add (commit 5856de80) modeling a real platform. Note also this machine has no specification except 5 lines in the header of this file: * emulates a simple machine with ISA-like bus. * ISA IO space mapped to the 0x14000000 (PHYS) and * ISA memory at the 0x10000000 (PHYS, 16Mb in size). * All peripherial devices are attached to this "bus" with * the standard PC ISA addresses. It is time to deprecate this obsolete machine. Users are recommended to use the Malta board, which hardware is well documented. Signed-off-by: Philippe Mathieu-Daudé <philmd(a)redhat.com> --- qemu-deprecated.texi | 5 +++++ hw/mips/mips_r4k.c | 1 + MAINTAINERS | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi index 4b4b7425ac..05265b43c8 100644 --- a/qemu-deprecated.texi +++ b/qemu-deprecated.texi @@ -266,6 +266,11 @@ The 'scsi-disk' device is deprecated. Users should use 'scsi-hd' or @section System emulator machines +@subsection mips r4k platform (since 4.2) + +This machine type is very old and unmaintained. Users should use the 'malta' +machine type instead. + @subsection pc-0.12, pc-0.13, pc-0.14 and pc-0.15 (since 4.0) These machine types are very old and likely can not be used for live migration diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c index 70024235ae..0b79ad26cb 100644 --- a/hw/mips/mips_r4k.c +++ b/hw/mips/mips_r4k.c @@ -294,6 +294,7 @@ void mips_r4k_init(MachineState *machine) static void mips_machine_init(MachineClass *mc) { + mc->deprecation_reason = "use malta machine type instead"; mc->desc = "mips r4k platform"; mc->init = mips_r4k_init; mc->block_default_type = IF_IDE; diff --git a/MAINTAINERS b/MAINTAINERS index 5e5e3e52d6..3b3a88e264 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -972,7 +972,7 @@ F: hw/net/mipsnet.c R4000 M: Aurelien Jarno <aurelien(a)aurel32.net> R: Aleksandar Rikalo <aleksandar.rikalo(a)rt-rk.com> -S: Maintained +S: Obsolete F: hw/mips/mips_r4k.c Fulong 2E -- 2.21.0

8 21

[PATCH] client: fix memory leak in client msg
by Hao Wang 09 Sep '20

09 Sep '20

>From 3ad3fae4f2562a11bef8dcdd25b6a7e0b00d4e2c Mon Sep 17 00:00:00 2001 From: Hao Wang <wanghao232(a)huawei.com> Date: Sat, 18 Jul 2020 15:43:30 +0800 Subject: [PATCH] client: fix memory leak in client msg When closing client->waitDispatch in virNetClientIOEventLoopRemoveAll or virNetClientIOEventLoopRemoveDone, VIR_FREE() is called to free call->msg directly, resulting in leak of the memory call->msg->buffer points to. Use virNetMessageFree(call->msg) instead of VIR_FREE(call->msg). Signed-off-by: Hao Wang <wanghao232(a)huawei.com> --- src/rpc/virnetclient.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 441f1502a6..f899493b64 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1519,7 +1519,7 @@ static bool virNetClientIOEventLoopRemoveDone(virNetClientCallPtr call, if (call->expectReply) VIR_WARN("Got a call expecting a reply but without a waiting thread"); virCondDestroy(&call->cond); - VIR_FREE(call->msg); + virNetMessageFree(call->msg); VIR_FREE(call); } @@ -1546,7 +1546,7 @@ virNetClientIOEventLoopRemoveAll(virNetClientCallPtr call, VIR_DEBUG("Removing call %p", call); virCondDestroy(&call->cond); - VIR_FREE(call->msg); + virNetMessageFree(call->msg); VIR_FREE(call); return true; } -- 2.23.0

3 2

[PATCH v2 00/13] resolve hangs/crashes on libvirtd shutdown
by Nikolay Shirokovskiy 07 Sep '20

07 Sep '20

I keep qemu VM event loop exiting synchronously but add code to avoid deadlock that can be caused by this approach. I guess it is worth having synchronous exiting of threads in this case to avoid crashes. Patches that are already positively reviewed has appropriate 'Reviewed-by' lines. Changes from v1: - rename stateShutdown to state stateShutdownPrepare - introduce net daemon shutdown callbacks - make some adjustments in terms of qemu per VM's event loop thread finishing - factor out net server shutdown facilities into distinct patch - increase shutdown timeout from 15s to 30s Nikolay Shirokovskiy (13): libvirt: add stateShutdownPrepare/stateShutdownWait to drivers util: always initialize priority condition util: add stop/drain functions to thread pool rpc: don't unref service ref on socket behalf twice rpc: add virNetDaemonSetShutdownCallbacks rpc: add shutdown facilities to netserver rpc: finish all threads before exiting main loop qemu: don't shutdown event thread in monitor EOF callback vireventthread: exit thread synchronously on finalize qemu: avoid deadlock in qemuDomainObjStopWorker qemu: implement driver's shutdown/shutdown wait methods rpc: cleanup virNetDaemonClose method util: remove unused virThreadPoolNew macro scripts/check-drivername.py | 2 + src/driver-state.h | 8 ++++ src/libvirt.c | 42 ++++++++++++++++ src/libvirt_internal.h | 2 + src/libvirt_private.syms | 4 ++ src/libvirt_remote.syms | 2 +- src/qemu/qemu_domain.c | 18 +++++-- src/qemu/qemu_driver.c | 32 +++++++++++++ src/qemu/qemu_process.c | 3 -- src/remote/remote_daemon.c | 6 +-- src/rpc/virnetdaemon.c | 109 ++++++++++++++++++++++++++++++++++++------ src/rpc/virnetdaemon.h | 8 +++- src/rpc/virnetserver.c | 8 ++++ src/rpc/virnetserver.h | 1 + src/rpc/virnetserverservice.c | 1 - src/util/vireventthread.c | 1 + src/util/virthreadpool.c | 65 +++++++++++++++++-------- src/util/virthreadpool.h | 6 +-- 18 files changed, 267 insertions(+), 51 deletions(-) -- 1.8.3.1

3 29