September 2021 - Devel - libvirt List Archives

[PATCH 00/22] qemu: Fix mess with lifecycle actions and unbreak transient disks
by Peter Krempa 15 Sep '21

15 Sep '21

While investigating how to fix issue with transient disks breaking when -no-shutdown is not used I've ended up figuring out that lifecycle action handling in the qemu driver is very broken. Unbreak the handling by rejecting some actions which were never implemented, add support for 'set-action' qmp command and use it to update the 'reset' action in qemu and always use '-no-shutdown'. Peter Krempa (22): qemuMonitorJSONSetWatchdogAction: Use automatic memory clearing qemuDomainSetLifecycleAction: Add a note about argument range-check qemu: driver: Use 'qemuDomainSaveStatus' for saving status XML qemu: validate: Reformat header and purge unused includes qemu: Reject 'rename-restart' action for 'on_reboot'/'on_poweroff'/'on_crash' qemu: driver: Validate lifecycle actions in 'qemuDomainSetLifecycleAction' qemu: Reject 'preserve' action for 'on_reboot'/'on_poweroff'/'on_crash' qemu: Honor 'restart' action for 'on_poweroff' qemu: capablities: Detect presence of 'set-action' as QEMU_CAPS_SET_ACTION qemu: monitor: Implement monitor code for 'set-action' command qemuDomainAttachWatchdog: Use 'set-action' instead of 'watchdog-set-action' if supported qemuxml2argvtest: Add 'LATEST' version of 'misc-no-reboot' test case qemuDomainObjPrivate: Annotate 'allowReboot' field qemu: migration: Don't transfer 'allowReboot' flag qemu: domain: Remove qemuDomainIsUsingNoShutdown qemuDomainSetLifecycleAction: Forbid live update of 'on_reboot' qemuProcessHandleReset: Don't emulate lifecycle actions for RESET event qemuProcessLaunch: Setup handling of 'on_reboot' via QMP when starting the process qemu: command: Always use '-no-shutdown' qemu: process: Don't set 'allowReboot' when qemu supports 'set-action' qemuDomainSetLifecycleAction: Properly update 'onReboot' action in qemu qemu: process: Ignore 'RESET' event during startup docs/formatdomain.rst | 8 +- src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 14 +- src/qemu/qemu_domain.c | 16 -- src/qemu/qemu_domain.h | 11 +- src/qemu/qemu_driver.c | 201 +++++++++++------- src/qemu/qemu_hotplug.c | 54 ++++- src/qemu/qemu_migration.c | 5 - src/qemu/qemu_migration_cookie.c | 22 -- src/qemu/qemu_migration_cookie.h | 4 - src/qemu/qemu_monitor.c | 16 ++ src/qemu/qemu_monitor.h | 49 +++++ src/qemu/qemu_monitor_json.c | 95 ++++++++- src/qemu/qemu_monitor_json.h | 7 + src/qemu/qemu_process.c | 85 +++++--- src/qemu/qemu_validate.c | 51 +++++ src/qemu/qemu_validate.h | 31 +-- .../caps_6.0.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 + .../caps_6.0.0.x86_64.xml | 1 + .../caps_6.1.0.x86_64.xml | 1 + .../basic-xml2xml-out.xml | 1 - .../full-xml2xml-out.xml | 1 - .../modern-dom-out-dest.xml | 1 - .../modern-dom-out-source.xml | 1 - .../nbd-bitmaps-xml2xml-out.xml | 1 - tests/qemumonitorjsontest.c | 6 + .../misc-no-reboot.x86_64-latest.args | 35 +++ tests/qemuxml2argvtest.c | 1 + 30 files changed, 529 insertions(+), 194 deletions(-) create mode 100644 tests/qemuxml2argvdata/misc-no-reboot.x86_64-latest.args -- 2.31.1

4 27

[PATCH 0/1] qemu_tpm: Start swtpm(8) daemon with --terminate switch
by Nick Chevsky 15 Sep '21

15 Sep '21

libvirt expects the swtpm(8) daemon to auto-terminate along with QEMU. While that's already the case, it's currently happening for the wrong reason: swtpm's documented way of achieving this behavior is via the --terminate switch (which causes the daemon to shut down when the data channel connection drops), but libvirt isn't currently using this switch--and it should. The reason this currently works anyway, even without the --terminate switch, is two-fold: (1) When QEMU terminates gracefully, it sends command CMD_SHUTDOWN to swtpm which triggers a shutdown. Nothing wrong with this one. (2) When QEMU dies abruptly (e.g. SIGKILL, SIGSEGV) without issuing CMD_SHUTDOWN, swtpm should (a) shut down if the --terminate switch was given OR (b) stay alive if --terminate wasn't given. At the moment this isn't being respected, and swtpm unconditionally shuts down (regardless of whether --terminate was given or not) due to a bug in swtpm's connection handling logic [1]. libvirt currently relies on this incorrect and undocumented upstream behavior, trusting swtpm to shut itself down even when --terminate wasn't given, which is wrong and bound to break. The discussion [1] between swtpm's author and I shows that --terminate (a) is the proper way to achieve--and guarantee--the current behavior, (b) is innocuous to add since it won't alter existing behavior, (c) should've been used by libvirt all along, and (d) should be enforced by swtpm going forward. Since libvirt presently relies on swtpm's current (incorrect) behavior and we don't want to break libvirt, we need libvirt to start invoking swtpm with the --terminate switch ASAP so that the upstream bug can be fixed as soon as it's safe. Fixing the bug is the first step toward eventually enabling non-libvirt swtpm users to optionally run swtpm as a persistent service, allowing a VM to connect to and disconnect from it without the daemon dying. Proxmox VE, to which I also contribute, is already using --terminate in its (WIP) swtpm implementation. [1] https://github.com/stefanberger/swtpm/pull/509 -- Note that this already-merged PR addresses only one half of the bug; the other half (which will actually effect the change) remains on hold until libvirt implements --terminate. Nick Chevsky (1): qemu_tpm: Start swtpm(8) daemon with --terminate switch src/qemu/qemu_tpm.c | 2 ++ 1 file changed, 2 insertions(+) -- 2.30.2

2 4

[PATCH] libxl: Don't autostart domains on driver reload
by Jim Fehlig 14 Sep '21

14 Sep '21

When libxlAutostartDomain was introduced with commit fb92307f0d, one hunk mistakenly added a call site in libxlStateReload. Domains should not be autostarted when reloading the driver, so remove the offending hunk. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- src/libxl/libxl_driver.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index c5dbcaafa5..7ea157f9c4 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -825,10 +825,6 @@ libxlStateReload(void) libxl_driver->xmlopt, NULL, libxl_driver); - virDomainObjListForEach(libxl_driver->domains, false, - libxlAutostartDomain, - libxl_driver); - virObjectUnref(cfg); return 0; } -- 2.33.0

2 1

[PATCH] node_device_conf: Don't prealloc @vfs in virNodeDeviceGetPCISRIOVCaps()
by Michal Privoznik 14 Sep '21

14 Sep '21

The array of virtual functions @vfs in virNodeDeviceGetPCISRIOVCaps() is allocated twice: the first time during its declaration and the second time inside virPCIGetVirtualFunctions() which leads to a memleak: ==16691== 1,128 bytes in 47 blocks are definitely lost in loss record 1,771 of 1,803 ==16691== at 0x4844CC1: calloc (vg_replace_malloc.c:1117) ==16691== by 0x4E50070: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6800.3) ==16691== by 0x4A7B034: virNodeDeviceGetPCISRIOVCaps (node_device_conf.c:2649) ==16691== by 0x4A7B5E2: virNodeDeviceGetPCIDynamicCaps (node_device_conf.c:2762) ==16691== by 0xA7F6E18: udevProcessPCI (node_device_udev.c:418) Fixes: c97518d9b833a607f29b9bb02e3fbe74c011c088 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/conf/node_device_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index b4c1acb6a5..9bbff97ffd 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2646,7 +2646,7 @@ static int virNodeDeviceGetPCISRIOVCaps(const char *sysfsPath, virNodeDevCapPCIDev *pci_dev) { - g_autoptr(virPCIVirtualFunctionList) vfs = g_new0(virPCIVirtualFunctionList, 1); + g_autoptr(virPCIVirtualFunctionList) vfs = NULL; size_t i; int ret; -- 2.32.0

2 1

[PATCH] docs: Format @variable properly
by Michal Privoznik 14 Sep '21

14 Sep '21

When documenting our public API in some places we use '@' to refer to the variable. For instance: * This API tries to set guest time to the given value. The time * to set (@seconds and @nseconds) should be in seconds relative * to the Epoch of 1970-01-01 00:00:00 in UTC. However, when generating HTML documentation these tokens are copied verbatim. What we can do is drop the '@' character and wrap the variable in <code/> so that it is formatted properly. Due to the way we 'parse' docs a token might actually be slightly more than just '@variable'. For instance in the example above we will have the following tokens: '(@seconds' and '@nseconds)'. Thus we need to handle possible substring before and after variable. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- docs/newapi.xsl | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/newapi.xsl b/docs/newapi.xsl index 7ac8caa35d..e56a5f2a27 100644 --- a/docs/newapi.xsl +++ b/docs/newapi.xsl @@ -139,6 +139,12 @@ </a> <xsl:value-of select="substring-after($token, '>')"/> </xsl:when> + <xsl:when test="contains($token, '@')"> + <xsl:variable name="prologue" select="substring-before($token, '@')"/> + <xsl:value-of select="$prologue"/> + <code><xsl:value-of select="$stem"/></code> + <xsl:value-of select="substring($token, string-length($prologue) + string-length($stem) + 2)"/> + </xsl:when> <xsl:otherwise> <xsl:value-of select="$token"/> </xsl:otherwise> -- 2.32.0

2 1

[PATCH] libxl: Fix driver reload
by Jim Fehlig 14 Sep '21

14 Sep '21

On reload, the libxl driver calls virDomainObjListLoadAllConfigs to load all configs from /etc/libvirt/libxl/ but incorrectly passes 'true' for the liveStatus parameter, resulting in error messages such as libvirtd[21053]: XML error: unexpected root element <domain>, expecting <domstatus> libvirtd[21053]: Failed to load config for domain 'sles15sp3' Fix by not requesting live status when re-reading the persistent VM config files. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- src/libxl/libxl_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 6a3938ead4..c5dbcaafa5 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -821,7 +821,7 @@ libxlStateReload(void) virDomainObjListLoadAllConfigs(libxl_driver->domains, cfg->configDir, cfg->autostartDir, - true, + false, libxl_driver->xmlopt, NULL, libxl_driver); -- 2.33.0

2 2

question on vhost, limiting kernel threads and NPROC
by Mike Christie 14 Sep '21

14 Sep '21

Hi, The goal of this email is to try and figure how we want to track/limit the number of kernel threads created by vhost devices. Background: ----------- For vhost-scsi, we've hit a issue where the single vhost worker thread can't handle all IO the being sent from multiple queues. IOPs is stuck at around 500K. To fix this, we did this patchset: https://lore.kernel.org/linux-scsi/20210525180600.6349-1-michael.christie@o… which allows userspace to create N threads and map them to a dev's virtqueues. With this we can get around 1.4M IOPs. Problem: -------- While those patches were being reviewed, a concern about tracking all these new possible threads was raised here: https://lore.kernel.org/linux-scsi/YL45CfpHyzSEcAJv@stefanha-x1.localdomain/ To save you some time, the question is what does other kernel code using the kthread API do to track the number of kernel threads created on behalf of a userspace thread. The answer is they don't do anything so we will have to add that code. I started to do that here: https://lkml.org/lkml/2021/6/23/1233 where those patches would charge/check the vhost device owner's RLIMIT_NPROC value. But, the question of if we really want to do this has come up which is why I'm bugging lists like libvirt now. Question/Solution: ------------------ I'm bugging everyone so we can figure out: If we need to specifically track the number of kernel threads being made for the vhost kernel use case by the RLIMIT_NPROC limit? Or, is it ok to limit the number of devices with the RLIMIT_NOFILE limit. Then each device has a limit on the number of threads it can create.

4 4

[libvirt PATCH 0/2] ci: Add Debian 11 builds
by Andrea Bolognani 14 Sep '21

14 Sep '21

It's been out for a bit. Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/368763275 Andrea Bolognani (2): ci: Add Debian 11 builds ci: Regenerate configuration from manifest .../debian-11-cross-aarch64.Dockerfile | 126 +++++++++++++++++ .../debian-11-cross-armv6l.Dockerfile | 125 +++++++++++++++++ .../debian-11-cross-armv7l.Dockerfile | 126 +++++++++++++++++ ci/containers/debian-11-cross-i686.Dockerfile | 125 +++++++++++++++++ .../debian-11-cross-mips64el.Dockerfile | 125 +++++++++++++++++ .../debian-11-cross-mipsel.Dockerfile | 125 +++++++++++++++++ .../debian-11-cross-ppc64le.Dockerfile | 125 +++++++++++++++++ .../debian-11-cross-s390x.Dockerfile | 125 +++++++++++++++++ ci/containers/debian-11.Dockerfile | 106 ++++++++++++++ ci/gitlab.yml | 130 ++++++++++++++---- ci/manifest.yml | 42 +++++- 11 files changed, 1248 insertions(+), 32 deletions(-) create mode 100644 ci/containers/debian-11-cross-aarch64.Dockerfile create mode 100644 ci/containers/debian-11-cross-armv6l.Dockerfile create mode 100644 ci/containers/debian-11-cross-armv7l.Dockerfile create mode 100644 ci/containers/debian-11-cross-i686.Dockerfile create mode 100644 ci/containers/debian-11-cross-mips64el.Dockerfile create mode 100644 ci/containers/debian-11-cross-mipsel.Dockerfile create mode 100644 ci/containers/debian-11-cross-ppc64le.Dockerfile create mode 100644 ci/containers/debian-11-cross-s390x.Dockerfile create mode 100644 ci/containers/debian-11.Dockerfile -- 2.31.1

2 3

[PATCH] qemuxml2argvdata: Remove unused '.err' files
by Peter Krempa 14 Sep '21

14 Sep '21

These are no longer referenced by any existing test as of: os-firmware-invalid-type -> a9b1375d7d2f7d240dce09c5f8b62e568e386051 tseg-explicit-size -> 604990a1758bfdc302f3c576c5766c0763912dfd Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- .../qemuxml2argvdata/os-firmware-invalid-type.x86_64-latest.err | 1 - tests/qemuxml2argvdata/tseg-explicit-size.x86_64-2.10.0.err | 1 - 2 files changed, 2 deletions(-) delete mode 100644 tests/qemuxml2argvdata/os-firmware-invalid-type.x86_64-latest.err delete mode 100644 tests/qemuxml2argvdata/tseg-explicit-size.x86_64-2.10.0.err diff --git a/tests/qemuxml2argvdata/os-firmware-invalid-type.x86_64-latest.err b/tests/qemuxml2argvdata/os-firmware-invalid-type.x86_64-latest.err deleted file mode 100644 index c8174b1c8b..0000000000 --- a/tests/qemuxml2argvdata/os-firmware-invalid-type.x86_64-latest.err +++ /dev/null @@ -1 +0,0 @@ -unsupported configuration: firmware attribute and firmware type has to be the same diff --git a/tests/qemuxml2argvdata/tseg-explicit-size.x86_64-2.10.0.err b/tests/qemuxml2argvdata/tseg-explicit-size.x86_64-2.10.0.err deleted file mode 100644 index 82f8685a90..0000000000 --- a/tests/qemuxml2argvdata/tseg-explicit-size.x86_64-2.10.0.err +++ /dev/null @@ -1 +0,0 @@ -unsupported configuration: Setting TSEG size is not supported with this QEMU binary -- 2.31.1

2 1

[libvirt PATCH 0/2] docs: virtiofs: move legacy docs to the bottom
by Stefan Hajnoczi 14 Sep '21

14 Sep '21

The virtiofs kbase article includes a lot of information that is only relevant to old versions of QEMU and libvirt. Setting up virtiofs can seem intimidating but it's actually easier than the article lets on. Move the legacy information out of the way. Stefan Hajnoczi (2): docs: virtiofs: move legacy docs to the bottom docs: virtiofs: use the preferred virtiofs spelling docs/kbase/virtiofs.rst | 183 +++++++++++++++++++++------------------- 1 file changed, 97 insertions(+), 86 deletions(-) -- 2.31.1

3 8