April 2024 - Devel - libvirt List Archives

[RFC PATCH v1 0/5] node_device_udev: small improvements
by Marc Hartmayer 12 Apr '24

12 Apr '24

The first patch fixes a resource leak, the other patches are small improvements and locking improvements. Marc Hartmayer (5): node_device_udev: Remove the timeout if the data is disposed node_device_udev: Test for mdevctlTimeout != -1 node_device_udev: Add comments about locking node_device_udev: Take lock if driver->privateData is modified node_device_udev: Rename `th` to `udevThread` src/node_device/node_device_udev.c | 42 ++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 14 deletions(-) base-commit: e2a7dd3f7e9843b0c0753cf6b6d9792351f8c6e1 -- 2.34.1

2 12

[PATCH RFC 00/12] Support throttle block filters
by wucf＠linux.ibm.com 11 Apr '24

11 Apr '24

From: Chun Feng Wu <wucf(a)linux.ibm.com> Hi, I am thinking to leverage "throttle block filter" in QEMU to support more flexible I/O limits(e.g. tiered I/O groups), one sample provided by QEMU doc is: https://github.com/qemu/qemu/blob/master/docs/throttle.txt "For example, let's say that we have three different drives and we want to set I/O limits for each one of them and an additional set of limits for the combined I/O of all three drives." The implementation idea is to - Define throttle groups(limit) in domain - Define throttle filter to reference throttle group within disk - Within domain disk, throttle filters references multiple throttle groups to form filter chain to apply multiple limits in QEMU like above sample - Add new virsh cmds for throttle group management: throttlegroupset Add or update a throttling group. throttlegroupdel Delete a throttling group. throttlegroupinfo Get a throttling group. throttlegrouplist list all domain throttlegroups - Update "attach-disk" to add one more option "--throttle-groups" to apply throttle filters e.g. "virsh attach-disk $VM_ID ${DISK_PATH}/vm1_disk_2.qcow2 vdd --driver qemu --subdriver qcow2 --targetbus virtio --throttle-groups limit2,limit012" - I chose above semantics as I felt they're appropriate, if there are better ones please kindly suggest. This patchset includes: - Throttle group and throttle filter definition in patch 1 - New QMP processing to update and get throttle group in patch 2 - New API definition and implementation in patch 3 - Hotplug and qemuProcessLaunch flow implemenation in patch 4, 5 - Domain XML schema and doc(formatdomain.rst) change in patch 6 - Tests in patch 7, 8 - Virsh cmd implementation in patch 9 - Other enhencement/verification implementation in patch 10, 11, 12 From QMP perspective, the sample flow works this way: - Throttle group creation: virsh qemu-monitor-command 1 '{"execute":"object-add", "arguments":{"qom-type":"throttle-group","id":"limit0","limits":{"iops-total":200,"iops-read":0,"iops-total-max":200,"iops-total-max-length":1}}}' virsh qemu-monitor-command 1 '{"execute":"object-add", "arguments":{"qom-type":"throttle-group","id":"limit1","limits":{"iops-total":250,"iops-read":0,"iops-total-max":250,"iops-total-max-length":1}}}' virsh qemu-monitor-command 1 '{"execute":"object-add", "arguments":{"qom-type":"throttle-group","id":"limit2","limits":{"iops-total":300,"iops-read":0,"iops-total-max":300,"iops-total-max-length":1}}}' virsh qemu-monitor-command 1 '{"execute":"object-add", "arguments":{"qom-type":"throttle-group","id":"limit012","limits":{"iops-total":400,"iops-read":0,"iops-total-max":400,"iops-total-max-length":1}}}' - Chain up filters during attaching disk to apply two filters(limit0 and limit012): virsh qemu-monitor-command 1 '{"execute":"blockdev-add", "arguments": {"driver":"file","filename":"/virt/disks/vm1_disk_1.qcow2","node-name":"test-3-storage","auto-read-only":true,"discard":"unmap"}}' virsh qemu-monitor-command 1 '{"execute":"blockdev-add", "arguments":{"node-name":"test-3-format","read-only":false,"driver":"qcow2","file":"test-3-storage","backing":null}}' virsh qemu-monitor-command 1 '{"execute":"blockdev-add", "arguments":{"driver":"throttle","node-name":"libvirt-5-filter","throttle-group": "limit0","file":"test-3-format"}}' virsh qemu-monitor-command 1 '{"execute":"blockdev-add", "arguments": {"driver":"throttle","node-name":"libvirt-6-filter","throttle-group":"limit012","file":"libvirt-5-filter"}}' virsh qemu-monitor-command 1 '{"execute": "device_add", "arguments": {"driver":"virtio-blk-pci","scsi":false,"bus":"pci.0","addr":"0x5","drive":"libvirt-6-filter","id":"virtio-disk1"}}' BTW, I also got support from these guys(guyujie(a)linux.ibm.com, wuyx(a)linux.ibm.com, xinhaong(a)linux.ibm.com, commits under their names are included) to help do some enhancement/verification implementation, thanks a lot! Any comments/suggestions will be appriciated! Chun Feng Wu (9): config: Introduce ThrottleGroup and ThrottleFilter qemu: monitor: Add support for ThrottleGroup operations remote: New APIs for ThrottleGroup lifecycle management qemu: hotplug: Support hot attach block disk along with throttle filters qemu: command: Support throttle groups and filters during qemuProcessLaunch schema: Add new domain elements to support multiple throttle filters test: Test throttle group lifecycle APIs tests: Test qemuMonitorJSONGetThrottleGroup and qemuMonitorJSONUpdateThrottleGroup virsh: Add support for throttle group operations Hao Ning Xin (1): config: validate: Verify throttle group fields Yan Xiu Wu (1): config: validate: Use "iotune" and "throttlefilters" exclusively for specific disk Yu Jie Gu (1): remote: Define remote_domain_set_throttle_group_args and adjust throttle structs sequence docs/formatdomain.rst | 48 ++ include/libvirt/libvirt-domain.h | 29 + src/conf/domain_conf.c | 364 ++++++++++++ src/conf/domain_conf.h | 43 ++ src/conf/domain_validate.c | 115 ++-- src/conf/schemas/domaincommon.rng | 164 +++++- src/conf/virconftypes.h | 4 + src/driver-hypervisor.h | 22 + src/libvirt-domain.c | 188 +++++++ src/libvirt_private.syms | 9 + src/libvirt_public.syms | 7 + src/qemu/qemu_block.c | 121 ++++ src/qemu/qemu_block.h | 53 ++ src/qemu/qemu_command.c | 148 +++++ src/qemu/qemu_command.h | 9 + src/qemu/qemu_domain.c | 79 +++ src/qemu/qemu_domain.h | 10 + src/qemu/qemu_driver.c | 520 +++++++++++++++++ src/qemu/qemu_hotplug.c | 23 + src/qemu/qemu_monitor.c | 29 + src/qemu/qemu_monitor.h | 10 + src/qemu/qemu_monitor_json.c | 135 +++++ src/qemu/qemu_monitor_json.h | 13 + src/remote/remote_daemon_dispatch.c | 60 ++ src/remote/remote_driver.c | 46 ++ src/remote/remote_protocol.x | 50 +- src/remote_protocol-structs | 30 + src/test/test_driver.c | 380 +++++++++++++ tests/qemumonitorjsontest.c | 89 +++ .../qemustatusxml2xmldata/backup-pull-in.xml | 1 + .../blockjob-blockdev-in.xml | 1 + .../blockjob-mirror-in.xml | 1 + .../migration-in-params-in.xml | 1 + .../migration-out-nbd-bitmaps-in.xml | 1 + .../migration-out-nbd-out.xml | 1 + .../migration-out-nbd-tls-out.xml | 1 + .../migration-out-params-in.xml | 1 + tests/qemustatusxml2xmldata/modern-in.xml | 1 + tests/qemustatusxml2xmldata/upgrade-out.xml | 1 + .../qemustatusxml2xmldata/vcpus-multi-in.xml | 1 + tools/virsh-completer-domain.c | 62 ++ tools/virsh-completer-domain.h | 11 + tools/virsh-domain.c | 530 ++++++++++++++++++ 43 files changed, 3376 insertions(+), 36 deletions(-) -- 2.34.1

3 15

[PATCH 0/3] qemu: Add support for virtio sound model
by Rayhan Faizel 11 Apr '24

11 Apr '24

virtio-sound-pci and virtio-sound-device were recently introduced in QEMU 8.2.0. The full documentation of the virtio sound implementation in QEMU can be found here: https://www.qemu.org/docs/master/system/devices/virtio-snd.html Example: <sound model='virtio' streams='2'/> Rayhan Faizel (3): qemu_capabilities: Add QEMU_CAPS_DEVICE_VIRTIO_SOUND capability conf: Introduce support for virtio-sound devices qemu: Generate command line for sound devices with model 'virtio' docs/formatdomain.rst | 11 ++++- src/conf/domain_conf.c | 25 +++++++++++ src/conf/domain_conf.h | 4 ++ src/conf/domain_postparse.c | 13 +++++- src/conf/schemas/domaincommon.rng | 11 +++++ src/libxl/libxl_domain.c | 1 + src/qemu/qemu_capabilities.c | 3 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 24 +++++++++- src/qemu/qemu_domain_address.c | 9 ++++ src/qemu/qemu_validate.c | 8 ++++ .../caps_8.2.0_aarch64.xml | 1 + .../caps_8.2.0_armv7l.xml | 1 + .../caps_8.2.0_loongarch64.xml | 1 + .../qemucapabilitiesdata/caps_8.2.0_s390x.xml | 1 + .../caps_8.2.0_x86_64.xml | 1 + .../caps_9.0.0_x86_64.xml | 1 + .../arm-vexpressa9-virtio.aarch64-latest.args | 1 + .../arm-vexpressa9-virtio.aarch64-latest.xml | 3 ++ .../qemuxmlconfdata/arm-vexpressa9-virtio.xml | 3 +- .../sound-device-virtio.x86_64-latest.args | 36 +++++++++++++++ .../sound-device-virtio.x86_64-latest.xml | 44 +++++++++++++++++++ tests/qemuxmlconfdata/sound-device-virtio.xml | 28 ++++++++++++ tests/qemuxmlconftest.c | 1 + 24 files changed, 226 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxmlconfdata/sound-device-virtio.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/sound-device-virtio.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/sound-device-virtio.xml -- 2.34.1

1 3

[PATCH] qemu: Add sysusers config file for qemu & kvm user/groups
by tim＠siosm.fr 10 Apr '24

10 Apr '24

Install a systemd sysusers config file for the qemu & kvm user/groups. We can not use the sysusers_create_compat macro in the RPM specfile to create those users as we want to keep the specfile standalone and not relying on additionnal files. Update the specfile to make the commands closer to what is generated by the current macro. See: https://src.fedoraproject.org/rpms/libvirt/pull-request/22 See: https://gitlab.com/libvirt/libvirt/-/merge_requests/319 See: https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/ Based on previous work by: Peter Krempa <pkrempa(a)redhat.com> Signed-off-by: Timothée Ravier <tim(a)siosm.fr> --- libvirt.spec.in | 21 +++++++++++++-------- src/qemu/libvirt-qemu.sysusers.conf | 4 ++++ src/qemu/meson.build | 7 +++++++ 3 files changed, 24 insertions(+), 8 deletions(-) create mode 100644 src/qemu/libvirt-qemu.sysusers.conf diff --git a/libvirt.spec.in b/libvirt.spec.in index 8413e3c19a..a411ac6515 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1473,6 +1473,7 @@ chmod 600 $RPM_BUILD_ROOT%{_sysconfdir}/libvirt/nwfilter/*.xml %if ! %{with_qemu} rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/libvirtd_qemu.aug rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug +rm -f $RPM_BUILD_ROOT%{_sysusersdir}/libvirt-qemu.conf %endif %find_lang %{name} @@ -1834,16 +1835,19 @@ exit 0 %pre daemon-driver-qemu %libvirt_sysconfig_pre virtqemud %libvirt_systemd_unix_pre virtqemud + # We want soft static allocation of well-known ids, as disk images -# are commonly shared across NFS mounts by id rather than name; see -# https://fedoraproject.org/wiki/Packaging:UsersAndGroups -getent group kvm >/dev/null || groupadd -f -g 36 -r kvm -getent group qemu >/dev/null || groupadd -f -g 107 -r qemu -if ! getent passwd qemu >/dev/null; then - if ! getent passwd 107 >/dev/null; then - useradd -r -u 107 -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu +# are commonly shared across NFS mounts by id rather than name. +# See https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/ +# We can not use the sysusers_create_compat macro here as we want to keep the +# specfile standalone and not relying on additionnal files. +getent group 'kvm' >/dev/null || groupadd -f -g '36' -r 'kvm' || : +getent group 'qemu' >/dev/null || groupadd -f -g '107' -r 'qemu' || : +if ! getent passwd 'qemu' >/dev/null; then + if ! getent passwd '107' >/dev/null; then + useradd -r -u '107' -g 'qemu' -G 'kvm' -d '/' -s '/sbin/nologin' -c 'qemu user' 'qemu' || : else - useradd -r -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu + useradd -r -g 'qemu' -G 'kvm' -d '/' -s '/sbin/nologin' -c 'qemu user' 'qemu' || : fi fi exit 0 @@ -2246,6 +2250,7 @@ exit 0 %{_bindir}/virt-qemu-run %{_mandir}/man1/virt-qemu-run.1* %{_mandir}/man8/virtqemud.8* +%{_sysusersdir}/libvirt-qemu.conf %endif %if %{with_lxc} diff --git a/src/qemu/libvirt-qemu.sysusers.conf b/src/qemu/libvirt-qemu.sysusers.conf new file mode 100644 index 0000000000..3189191e73 --- /dev/null +++ b/src/qemu/libvirt-qemu.sysusers.conf @@ -0,0 +1,4 @@ +g kvm 36 +g qemu 107 +u qemu 107:qemu "qemu user" - - +m qemu kvm diff --git a/src/qemu/meson.build b/src/qemu/meson.build index 4c3e1dee78..7a0e908a66 100644 --- a/src/qemu/meson.build +++ b/src/qemu/meson.build @@ -160,6 +160,13 @@ if conf.has('WITH_QEMU') configuration: qemu_user_group_hack_conf, ) + # Install the sysuser config for the qemu driver + install_data( + 'libvirt-qemu.sysusers.conf', + install_dir: prefix / 'lib' / 'sysusers.d', + rename: [ 'libvirt-qemu.conf' ], + ) + virt_conf_files += qemu_conf virt_aug_files += files('libvirtd_qemu.aug') virt_test_aug_files += { -- 2.43.0

3 4

[RFC PATCH 0/2] One memory leak fix and one question
by Marc Hartmayer 10 Apr '24

10 Apr '24

Marc Hartmayer (2): node_device_conf: virNodeDeviceGetSCSITargetCaps: fix memory leak TODO virNodeDeviceUpdateCaps: checks missing? src/conf/node_device_conf.c | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) -- 2.34.1

2 10

[PATCH] qemusecuritytest: Call real virFileExists in mock
by Michal Privoznik 09 Apr '24

09 Apr '24

When I suggested to Jim to call real virFileExists() I forgot to also suggest calling init_syms(). Without it, real_virFileExists pointer might be left unset. And indeed, that's what we were seeing on FreeBSD. This effectively reverts commit 4b5cc57ed35dc24d11673dd3f04bfb8073c0340d. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Green pipeline: https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/1245457856 Okay, it doesn't test FreeBSD, but it tests x86_64-ubuntu-2204-clang which was also experiencing the failure: https://gitlab.com/libvirt/libvirt/-/jobs/6574951734 tests/qemusecuritymock.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c index dc8a893e9d..2dfd6c33a0 100644 --- a/tests/qemusecuritymock.c +++ b/tests/qemusecuritymock.c @@ -66,6 +66,7 @@ static int (*real_close)(int fd); static int (*real_setfilecon_raw)(const char *path, const char *context); static int (*real_getfilecon_raw)(const char *path, char **context); #endif +static bool (*real_virFileExists)(const char *file); /* Global mutex to avoid races */ @@ -123,6 +124,7 @@ init_syms(void) VIR_MOCK_REAL_INIT(setfilecon_raw); VIR_MOCK_REAL_INIT(getfilecon_raw); #endif + VIR_MOCK_REAL_INIT(virFileExists); /* Intentionally not calling init_hash() here */ } @@ -386,8 +388,11 @@ bool virFileExists(const char *path) { VIR_LOCK_GUARD lock = virLockGuardLock(&m); - if (getenv(ENVVAR) == NULL) - return access(path, F_OK) == 0; + if (getenv(ENVVAR) == NULL) { + init_syms(); + + return real_virFileExists(path); + } init_hash(); if (virHashHasEntry(chown_paths, path)) -- 2.43.2

3 2

[PATCH V2] security: Ensure file exists before attempting to restore label
by Jim Fehlig 09 Apr '24

09 Apr '24

When performing an install, it's common for tooling such as virt-install to remove the install kernel/initrd once they are successfully booted and the domain has been redefined to boot without them. After the installation is complete and the domain is rebooted/shutdown, the DAC and selinux security drivers attempt to restore labels on the now deleted files. It's harmles wrt functionality, but results in error messages such as Mar 08 12:40:37 virtqemud[5639]: internal error: child reported (status=125): unable to stat: /var/lib/libvirt/boot/vir> Mar 08 12:40:37 virtqemud[5639]: unable to stat: /var/lib/libvirt/boot/virtinst-yvp19moo-linux: No such file or directo> Mar 08 12:40:37 virtqemud[5639]: Unable to run security manager transaction Add a check for file existence to the virSecurity*RestoreFileLabel functions, and avoid relabeling if the file is no longer available. Skipping the restore caused failures in qemusecuritytest, which mocks stat, chown, etc as part of ensuring the security drivers properly restore labels. virFileExists is now mocked in qemusecuritymock.c to return true when passed a file previously seen by the mocked stat, chown, etc functions. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- src/security/security_dac.c | 3 +++ src/security/security_selinux.c | 2 ++ tests/qemusecuritymock.c | 18 ++++++++++++++++++ 3 files changed, 23 insertions(+) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 567be4bd23..4e850e219e 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -825,6 +825,9 @@ virSecurityDACRestoreFileLabelInternal(virSecurityManager *mgr, virStorageSourceIsLocalStorage(src)) path = src->path; + if (!virFileExists(path)) + return 0; + /* Be aware that this function might run in a separate process. * Therefore, any driver state changes would be thrown away. */ diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index b49af26e49..aaec34ff8b 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1488,6 +1488,8 @@ virSecuritySELinuxRestoreFileLabel(virSecurityManager *mgr, */ if (!path) return 0; + if (!virFileExists(path)) + return 0; VIR_INFO("Restoring SELinux context on '%s'", path); diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c index 80d59536b1..bb0a85544b 100644 --- a/tests/qemusecuritymock.c +++ b/tests/qemusecuritymock.c @@ -382,6 +382,24 @@ int virFileUnlock(int fd G_GNUC_UNUSED, } +bool virFileExists(const char *path G_GNUC_UNUSED) +{ + VIR_LOCK_GUARD lock = virLockGuardLock(&m); + + if (getenv(ENVVAR) == NULL) + return access(path, F_OK) == 0; + + init_hash(); + if (virHashHasEntry(chown_paths, path)) + return true; + + if (virHashHasEntry(selinux_paths, path)) + return true; + + return false; +} + + typedef struct _checkOwnerData checkOwnerData; struct _checkOwnerData { GHashTable *paths; -- 2.44.0

2 4

[PATCH] qemusecuritytest: Don't call real virFileExists in mock
by Jim Fehlig 08 Apr '24

08 Apr '24

Calling the real virFileExists in qemusecuritymock.c can cause a segfault in qemusecuritytest. No segfaults are noticed when calling access(2) instead of virFileExists. Fixes: 4ed5ade753d8f1136cdbf17ddfe1d9093bcd933d Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- Pushing as a build-breaker fix. tests/qemusecuritymock.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c index a7bb1f8bea..dc8a893e9d 100644 --- a/tests/qemusecuritymock.c +++ b/tests/qemusecuritymock.c @@ -66,7 +66,6 @@ static int (*real_close)(int fd); static int (*real_setfilecon_raw)(const char *path, const char *context); static int (*real_getfilecon_raw)(const char *path, char **context); #endif -static bool (*real_virFileExists)(const char *file); /* Global mutex to avoid races */ @@ -124,7 +123,6 @@ init_syms(void) VIR_MOCK_REAL_INIT(setfilecon_raw); VIR_MOCK_REAL_INIT(getfilecon_raw); #endif - VIR_MOCK_REAL_INIT(virFileExists); /* Intentionally not calling init_hash() here */ } @@ -389,7 +387,7 @@ bool virFileExists(const char *path) VIR_LOCK_GUARD lock = virLockGuardLock(&m); if (getenv(ENVVAR) == NULL) - return real_virFileExists(path); + return access(path, F_OK) == 0; init_hash(); if (virHashHasEntry(chown_paths, path)) -- 2.44.0

1 0

[PATCH] rpcgen: tests: Include stdint.h in test_demo.c
by Michal Privoznik 08 Apr '24

08 Apr '24

Since header file structure is a bit different on MacOS, it doesn't get uint64_t type declaration and thus test_demo.c must include it explicitly. This is proper solution anyway, because on Linux we're apparently relying on the header file sneaking through some other include. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/619 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- scripts/rpcgen/tests/test_demo.c | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/rpcgen/tests/test_demo.c b/scripts/rpcgen/tests/test_demo.c index ae19a674cf..e6ba7ddbc5 100644 --- a/scripts/rpcgen/tests/test_demo.c +++ b/scripts/rpcgen/tests/test_demo.c @@ -2,6 +2,7 @@ #include <rpc/types.h> #include <rpc/xdr.h> #include <stdbool.h> +#include <stdint.h> #ifdef __APPLE__ # define xdr_uint64_t xdr_u_int64_t -- 2.43.2

2 1

[RFC] virsysinfo: Try reading DMI table
by brett.holman＠canonical.com 08 Apr '24

08 Apr '24

This patch intends to add DMI support to libvirt for RISC-V and mips. This is based on commit ec6ce6363, which added ARM support. This is untested, as I've been unable to find hardware to test this on. src/util/virsysinfo.c | 2 ++ 1 file changed, 2 insertions(+) Cheers, Brett Holman P.S. This is my first post on this mailing list, I believe that I've followed

5 8