- Devel - libvirt List Archives

[PATCH for v10.6.0 0/2] qemu: Two pstore improvements
by Michal Privoznik 30 Jul '24

30 Jul '24

These address Andrea's suggestion: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/EEI… I know we've just entered freeze, but pstore is a new feature and it'd be nice to have these in. The first one is technically a feature, but the second one can be viewed as a bug fix. Michal Prívozník (2): qemu: Autofill pstore path if missing qemu: Pre-create pstore device file src/qemu/qemu_domain.c | 27 +++++++++++++++++++++++- src/qemu/qemu_process.c | 46 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) -- 2.44.2

3 6

[PATCH] NEWS: Document features/improvements/bug fixes I've participated in
by Michal Privoznik 30 Jul '24

30 Jul '24

There are some features/improvements/bug fixes I've either contributed or reviewed/merged. Document them for upcoming release. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- NEWS.rst | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 17335b3f6e..1a320c5442 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -28,8 +28,42 @@ v10.6.0 (unreleased) * **Improvements** + * qemu: Set 'passt' net backend if 'default' is unsupported + + If QEMU is compiled without SLIRP support, and if domain XML allows it, + starting from this release libvirt will use passt as the default backend + instead. Also, supported backends are now reported in the domain + capabilities XML. + + * qemu: Require QEMU-5.2.0 or newer + + The minimal required version of QEMU was bumped to 5.2.0. + + * qemu: add a monitor to /proc/$pid when killing times out + + In cases when a QEMU process takes longer to be killed, libvirt might have + skipped cleaning up after it. But now a /proc/$pid watch is installed so + this does not happen ever again. + * **Bug fixes** + * virt-aa-helper: Allow RO access to /usr/share/edk2-ovmf + + When binary version of edk2 is distributed, the files reside under + /usr/share/edk2-ovmf. Allow virt-aa-helper to generate paths under that + directory. + + * virt-host-validate: Allow longer list of CPU flags + + During its run, virt-host-validate parses /proc/cpuinfo to learn about CPU + flags. But due to a bug it parsed only the first 1024 bytes worth of CPU + flags leading to unexpected results. The file is now parsed properly. + + * capabilities: Be more forgiving when decoding OEM strings + + On some systems, OEM strings are scattered in multiple sections. This + confused libvirt when generating capabilities XML. Not anymore. + v10.5.0 (2024-07-01) ==================== -- 2.44.2

3 2

Arm CCA instance creation fails
by Itaru Kitayama 30 Jul '24

30 Jul '24

Hi, With the below virt-install options, I am trying to import a Realm VM: […] export LIBVIRT_QEMU=/home/realm/qemu-system-aarch64 virt-install --machine=virt --arch=aarch64 --name=test8 --disk path=/var/lib/libvirt/images/jammy.qcow2,format=qcow2,device=disk,bus=virtio,cache=none --memory=2048 --vcpu=8 --nographic --check all=off --features acpi=off --import --os-variant=ubuntu22.04 --qemu-commandline="-M virt,confidential-guest-support=rme0,acpi=off,gic-version=3 -cpu host -object rme-guest,id=rme0,measurement-algo=sha512" --boot kernel=Image-cca,initrd=rootfs.cpio […] However, it fails with the messages: WARNING Disk /var/lib/libvirt/images/jammy.qcow2 is already in use by other guests ['test', 'tmp1', 'jammy']. Starting install... ERROR internal error: QEMU unexpectedly closed the monitor (vm='test8'): 2024-07-22T06:40:39.290847Z qemu-system-aarch64: could not load kernel '/home/realm/Image-cca' Domain installation does not appear to have been successful. If it was, you can restart your domain by running: virsh --connect qemu:///system start test8 otherwise, please restart your installation. The libvirt packages are from Ubuntu 24.04 LTS, but QEMU is the one Linaro folk maintain. Am I using the opions wrong for kernel direct boot scenario? Thanks, Itaru.

2 2

Entering freeze for libvirt-10.6.0
by Jiri Denemark 29 Jul '24

29 Jul '24

I have just tagged v10.6.0-rc1 in the repository and pushed signed tarballs to https://download.libvirt.org/ Please give the release candidate some testing and in case you find a serious issue which should have a fix in the upcoming release, feel free to reply to this thread to make sure the issue is more visible. If you have not done so yet, please update NEWS.rst to document any significant change you made since the last release. Thanks, Jirka

1 0

[PATCH 0/2] qemuProcessStop: Don't unlock domain during cleanup
by Michal Privoznik 26 Jul '24

26 Jul '24

Technically, this is a v2 of: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/C6QP… but it implements completely different approach, so I'm starting a new thread. Michal Prívozník (2): vireventthread: Introduce virEventThreadStop qemu: Use virEventThreadStop() in qemuProcessStop() src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 14 ++++++++++---- src/util/vireventthread.c | 22 ++++++++++++++++++++++ src/util/vireventthread.h | 2 ++ 4 files changed, 35 insertions(+), 4 deletions(-) -- 2.44.2

5 12

[PATCH RESEND 0/5] Introduce pstore device
by Michal Privoznik 26 Jul '24

26 Jul '24

This is a rebased version of: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/UAUK… which was a rebased version of: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/VWR6… Let's hope no more resends will be needed. Michal Prívozník (5): qemu_capabilities: Introduce QEMU_CAPS_DEVICE_ACPI_ERST conf: Introduce pstore device qemu: Build cmd line for pstore device security: Set seclabels for pstore device NEWS: Document pstore device addition NEWS.rst | 7 + docs/formatdomain.rst | 32 ++++ src/ch/ch_domain.c | 1 + src/conf/domain_conf.c | 153 ++++++++++++++++++ src/conf/domain_conf.h | 19 +++ src/conf/domain_postparse.c | 1 + src/conf/domain_validate.c | 30 ++++ src/conf/schemas/domaincommon.rng | 25 +++ src/conf/virconftypes.h | 2 + src/hyperv/hyperv_driver.c | 1 + src/libvirt_private.syms | 2 + src/libxl/libxl_driver.c | 6 + src/lxc/lxc_driver.c | 6 + src/qemu/qemu_alias.c | 10 ++ src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 52 ++++++ src/qemu/qemu_domain.c | 3 + src/qemu/qemu_domain_address.c | 11 ++ src/qemu/qemu_driver.c | 3 + src/qemu/qemu_hotplug.c | 5 + src/qemu/qemu_validate.c | 26 +++ src/security/security_dac.c | 10 ++ src/security/security_selinux.c | 9 ++ src/security/virt-aa-helper.c | 4 + src/test/test_driver.c | 1 + .../caps_7.0.0_aarch64+hvf.xml | 1 + .../caps_7.0.0_aarch64.xml | 1 + .../qemucapabilitiesdata/caps_7.0.0_ppc64.xml | 1 + .../caps_7.0.0_x86_64.xml | 1 + .../qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 1 + .../caps_7.1.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 1 + .../caps_7.2.0_x86_64+hvf.xml | 1 + .../caps_7.2.0_x86_64.xml | 1 + .../caps_8.0.0_x86_64.xml | 1 + .../caps_8.1.0_x86_64.xml | 1 + .../caps_8.2.0_aarch64.xml | 1 + .../caps_8.2.0_armv7l.xml | 1 + .../caps_8.2.0_loongarch64.xml | 1 + .../caps_8.2.0_x86_64.xml | 1 + .../caps_9.0.0_x86_64.xml | 1 + .../caps_9.1.0_x86_64.xml | 1 + .../pstore-acpi-erst.x86_64-latest.args | 38 +++++ .../pstore-acpi-erst.x86_64-latest.xml | 1 + tests/qemuxmlconfdata/pstore-acpi-erst.xml | 53 ++++++ tests/qemuxmlconftest.c | 1 + 47 files changed, 532 insertions(+) create mode 100644 tests/qemuxmlconfdata/pstore-acpi-erst.x86_64-latest.args create mode 120000 tests/qemuxmlconfdata/pstore-acpi-erst.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/pstore-acpi-erst.xml -- 2.44.2

3 9

[PATCH 0/4] Rework qemu internal active snapshots to use QMP
by Nikolai Barybin 26 Jul '24

26 Jul '24

These patches make use of QMP recently added snapshot-save/delete commands and reaps HMP savevm/deletevm. The usage of HMP commands are highly discouraged by QEMU. Nikolai Barybin (4): qemu monitor: add snaphot-save/delete QMP commands qemu blockjob: add snapshot-save/delete job types qemu snapshot: use QMP snapshot-save/delete for internal snapshots qemu monitor: reap qemu_monitor_text po/POTFILES | 1 - po/libvirt.pot | 18 ---- src/qemu/meson.build | 1 - src/qemu/qemu_block.c | 2 + src/qemu/qemu_blockjob.c | 6 +- src/qemu/qemu_blockjob.h | 2 + src/qemu/qemu_domain.c | 4 + src/qemu/qemu_monitor.c | 23 +++-- src/qemu/qemu_monitor.h | 16 +++- src/qemu/qemu_monitor_json.c | 66 +++++++++++++++ src/qemu/qemu_monitor_json.h | 13 +++ src/qemu/qemu_monitor_text.c | 88 ------------------- src/qemu/qemu_monitor_text.h | 29 ------- src/qemu/qemu_snapshot.c | 158 ++++++++++++++++++++++++++++++++--- 14 files changed, 267 insertions(+), 160 deletions(-) delete mode 100644 src/qemu/qemu_monitor_text.c delete mode 100644 src/qemu/qemu_monitor_text.h -- 2.43.5

4 12

[PATCH-for-9.1] target/riscv: Remove the deprecated 'any' CPU type
by Philippe Mathieu-Daudé 26 Jul '24

26 Jul '24

The 'any' CPU is deprecated since commit f57d5f8004b ("target/riscv: deprecate the 'any' CPU type"). Users are better off using the default CPUs or the 'max' CPU. Signed-off-by: Philippe Mathieu-Daudé <philmd(a)linaro.org> --- docs/about/deprecated.rst | 13 ------------- docs/about/removed-features.rst | 8 ++++++++ target/riscv/cpu-qom.h | 1 - target/riscv/cpu.c | 28 ---------------------------- 4 files changed, 8 insertions(+), 42 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 88f0f037865..0ac49b15b44 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -347,19 +347,6 @@ QEMU's ``vhost`` feature, which would eliminate the high latency costs under which the 9p ``proxy`` backend currently suffers. However as of to date nobody has indicated plans for such kind of reimplementation unfortunately. -RISC-V 'any' CPU type ``-cpu any`` (since 8.2) -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -The 'any' CPU type was introduced back in 2018 and has been around since the -initial RISC-V QEMU port. Its usage has always been unclear: users don't know -what to expect from a CPU called 'any', and in fact the CPU does not do anything -special that isn't already done by the default CPUs rv32/rv64. - -After the introduction of the 'max' CPU type, RISC-V now has a good coverage -of generic CPUs: rv32 and rv64 as default CPUs and 'max' as a feature complete -CPU for both 32 and 64 bit builds. Users are then discouraged to use the 'any' -CPU type starting in 8.2. - RISC-V CPU properties which start with capital 'Z' (since 8.2) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index fc7b28e6373..f3e9474a73e 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -850,6 +850,14 @@ The RISC-V no MMU cpus have been removed. The two CPUs: ``rv32imacu-nommu`` and ``rv64imacu-nommu`` can no longer be used. Instead the MMU status can be specified via the CPU ``mmu`` option when using the ``rv32`` or ``rv64`` CPUs. +RISC-V 'any' CPU type ``-cpu any`` (removed in 9.1) +''''''''''''''''''''''''''''''''''''''''''''''''''' + +The 'any' CPU type was introduced back in 2018 and was around since the +initial RISC-V QEMU port. Its usage was always been unclear: users don't know +what to expect from a CPU called 'any', and in fact the CPU does not do anything +special that isn't already done by the default CPUs rv32/rv64. + ``compat`` property of server class POWER CPUs (removed in 6.0) ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h index 3670cfe6d9a..4464c0fd7a3 100644 --- a/target/riscv/cpu-qom.h +++ b/target/riscv/cpu-qom.h @@ -29,7 +29,6 @@ #define RISCV_CPU_TYPE_SUFFIX "-" TYPE_RISCV_CPU #define RISCV_CPU_TYPE_NAME(name) (name RISCV_CPU_TYPE_SUFFIX) -#define TYPE_RISCV_CPU_ANY RISCV_CPU_TYPE_NAME("any") #define TYPE_RISCV_CPU_MAX RISCV_CPU_TYPE_NAME("max") #define TYPE_RISCV_CPU_BASE32 RISCV_CPU_TYPE_NAME("rv32") #define TYPE_RISCV_CPU_BASE64 RISCV_CPU_TYPE_NAME("rv64") diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index a90808a3bac..4bda754b013 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -438,27 +438,6 @@ static void set_satp_mode_default_map(RISCVCPU *cpu) } #endif -static void riscv_any_cpu_init(Object *obj) -{ - RISCVCPU *cpu = RISCV_CPU(obj); - CPURISCVState *env = &cpu->env; - riscv_cpu_set_misa_ext(env, RVI | RVM | RVA | RVF | RVD | RVC | RVU); - -#ifndef CONFIG_USER_ONLY - set_satp_mode_max_supported(RISCV_CPU(obj), - riscv_cpu_mxl(&RISCV_CPU(obj)->env) == MXL_RV32 ? - VM_1_10_SV32 : VM_1_10_SV57); -#endif - - env->priv_ver = PRIV_VERSION_LATEST; - - /* inherited from parent obj via riscv_cpu_init() */ - cpu->cfg.ext_zifencei = true; - cpu->cfg.ext_zicsr = true; - cpu->cfg.mmu = true; - cpu->cfg.pmp = true; -} - static void riscv_max_cpu_init(Object *obj) { RISCVCPU *cpu = RISCV_CPU(obj); @@ -1161,11 +1140,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev); Error *local_err = NULL; - if (object_dynamic_cast(OBJECT(dev), TYPE_RISCV_CPU_ANY) != NULL) { - warn_report("The 'any' CPU is deprecated and will be " - "removed in the future."); - } - cpu_exec_realizefn(cs, &local_err); if (local_err != NULL) { error_propagate(errp, local_err); @@ -2952,7 +2926,6 @@ static const TypeInfo riscv_cpu_type_infos[] = { .abstract = true, }, #if defined(TARGET_RISCV32) - DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_ANY, MXL_RV32, riscv_any_cpu_init), DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_MAX, MXL_RV32, riscv_max_cpu_init), DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE32, MXL_RV32, rv32_base_cpu_init), DEFINE_VENDOR_CPU(TYPE_RISCV_CPU_IBEX, MXL_RV32, rv32_ibex_cpu_init), @@ -2962,7 +2935,6 @@ static const TypeInfo riscv_cpu_type_infos[] = { DEFINE_BARE_CPU(TYPE_RISCV_CPU_RV32I, MXL_RV32, rv32i_bare_cpu_init), DEFINE_BARE_CPU(TYPE_RISCV_CPU_RV32E, MXL_RV32, rv32e_bare_cpu_init), #elif defined(TARGET_RISCV64) - DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_ANY, MXL_RV64, riscv_any_cpu_init), DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_MAX, MXL_RV64, riscv_max_cpu_init), DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE64, MXL_RV64, rv64_base_cpu_init), DEFINE_VENDOR_CPU(TYPE_RISCV_CPU_SIFIVE_E51, MXL_RV64, rv64_sifive_e_cpu_init), -- 2.45.2

3 3

[PATCH 0/5] virsysinfo: Be more forgiving when decoding OEM strings
by Michal Privoznik 25 Jul '24

25 Jul '24

See 4/5 for explanation. Michal Prívozník (5): vircommand: Initialize dryRunStatus to portable EXIT_SUCCESS instead of 0 virsysinfo: Trim newline when decoding OEM strings tests: Add HPE Apollo test case to sysinfotest virsysinfo: Be more forgiving when decoding OEM strings virsysinfo: Calculate OEM string index better src/util/vircommand.c | 2 +- src/util/virsysinfo.c | 36 +++- .../aarch64-hpe-apollosysinfo.data | 162 ++++++++++++++++++ .../aarch64-hpe-apollosysinfo.expect | 88 ++++++++++ tests/sysinfotest.c | 9 +- 5 files changed, 285 insertions(+), 12 deletions(-) create mode 100644 tests/sysinfodata/aarch64-hpe-apollosysinfo.data create mode 100644 tests/sysinfodata/aarch64-hpe-apollosysinfo.expect -- 2.44.2

2 7

[PATCH] vmx: Ensure unique disk targets when parsing
by Adam Julis 24 Jul '24

24 Jul '24

Disk targets were generated in virVMXParseConfig() with virVMXGenerateDiskTarget(). It works on combination of controller, fix offset, unit and prefix. While SCSI and SATA have same prefix "sd", function virVMXGenerateDiskTarget() could returned in some cases same targets. In this patch, after loaded SCSI and SATA disks to the def, it checks if in array exists any SATA disks after SCSI. If not, nothing happened. If yes, targets of all SATA disks are changed. Disk target is calculated as: last SCSI target value + n, where n is position of disk in array after last SCSI (1,2,..). Because assigned addresses of disks are generated from their indexes, for every changed SATA disk is called virDomainDiskDefAssignAddress() with the updated value. The corresponding tests have been modified to match the index changes. Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- src/vmx/vmx.c | 32 ++++++++++++++++++++++++ tests/vmx2xmldata/esx-in-the-wild-12.xml | 4 +-- tests/vmx2xmldata/esx-in-the-wild-8.xml | 4 +-- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 227744d062..9fdf0f1cd4 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1389,6 +1389,9 @@ virVMXParseConfig(virVMXContext *ctx, bool smbios_reflecthost = false; int controller; int bus; + int ndisk; + int last_scsi; + int offset = -1; int port; bool present; int scsi_virtualDev[4] = { -1, -1, -1, -1 }; @@ -1805,6 +1808,35 @@ virVMXParseConfig(virVMXContext *ctx, } } + /* now disks contain only SCSI and SATA, SATA could have same index (dst) as SCSI + * find last SCSI index in array and use it as offset for all SATA indexes + * (overwrite old values) + * finally, regenerate correct addresses, while it depends on the index */ + for (ndisk = 0; ndisk < def->ndisks; ndisk++) { + virDomainDiskDef *dsc = def->disks[ndisk]; + + if (dsc->bus == VIR_DOMAIN_DISK_BUS_SCSI) { + offset = virDiskNameToIndex(dsc->dst); + last_scsi = ndisk; + continue; + } + + if (offset > -1) { + VIR_FREE(def->disks[ndisk]->dst); + def->disks[ndisk]->dst = virIndexToDiskName(offset + ndisk - last_scsi, "sd"); + + if (virDomainDiskDefAssignAddress(NULL, def->disks[ndisk], def) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not assign address to disk '%1$s'"), + virDomainDiskGetSource(dsc)); + goto cleanup; + } + } + + else + break; + } + /* def:disks (ide) */ for (bus = 0; bus < 2; ++bus) { for (unit = 0; unit < 2; ++unit) { diff --git a/tests/vmx2xmldata/esx-in-the-wild-12.xml b/tests/vmx2xmldata/esx-in-the-wild-12.xml index 42184501d0..a7730845ee 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-12.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-12.xml @@ -21,9 +21,9 @@ <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> - <target dev='sda' bus='sata'/> + <target dev='sdb' bus='sata'/> <readonly/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> </disk> <controller type='scsi' index='0' model='vmpvscsi'/> <controller type='sata' index='0'/> diff --git a/tests/vmx2xmldata/esx-in-the-wild-8.xml b/tests/vmx2xmldata/esx-in-the-wild-8.xml index 47d22ced2a..d5356bda34 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-8.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-8.xml @@ -36,9 +36,9 @@ </disk> <disk type='file' device='cdrom'> <source file='[692eb778-2d4937fe] CentOS-4.7.ServerCD-x86_64.iso'/> - <target dev='sda' bus='sata'/> + <target dev='sdd' bus='sata'/> <readonly/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> + <address type='drive' controller='0' bus='0' target='0' unit='3'/> </disk> <controller type='scsi' index='0' model='vmpvscsi'/> <controller type='sata' index='0'/> -- 2.45.2

2 1

[libvirt PATCH] qemu: virtiofs: cache: use 'never' instead of 'none'
by Ján Tomko 24 Jul '24

24 Jul '24

The new option style renamed one of the cache modes. Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- src/qemu/qemu_virtiofs.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c index 703f1226a2..db987ddfe1 100644 --- a/src/qemu/qemu_virtiofs.c +++ b/src/qemu/qemu_virtiofs.c @@ -147,10 +147,19 @@ qemuVirtioFSBuildCommandLine(virQEMUDriverConfig *cfg, virCommandAddArg(cmd, "--shared-dir"); virCommandAddArg(cmd, fs->src->path); - if (fs->cache) { + switch (fs->cache) { + case VIR_DOMAIN_FS_CACHE_MODE_DEFAULT: + break; + case VIR_DOMAIN_FS_CACHE_MODE_NONE: + virCommandAddArg(cmd, "--cache"); + virCommandAddArg(cmd, "never"); + break; + case VIR_DOMAIN_FS_CACHE_MODE_ALWAYS: virCommandAddArg(cmd, "--cache"); virCommandAddArg(cmd, virDomainFSCacheModeTypeToString(fs->cache)); + break; } + if (fs->sandbox) { virCommandAddArg(cmd, "--sandbox"); virCommandAddArg(cmd, virDomainFSSandboxModeTypeToString(fs->sandbox)); -- 2.45.2

2 1

[PATCH] meson: correct git detection
by John Levon 24 Jul '24

24 Jul '24

The current "building from git" test uses "test -d .git"; however, that doesn't work when libvirt is used as a submodule, as in that case .git is a normal file. Use "test -e .git" instead. Signed-off-by: John Levon <john.levon(a)nutanix.com> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 3ab8e57e4a..231470e2ee 100644 --- a/meson.build +++ b/meson.build @@ -17,7 +17,7 @@ endif # figure out if we are building from git -git = run_command('test', '-d', '.git', check: false).returncode() == 0 +git = run_command('test', '-e', '.git', check: false).returncode() == 0 if git and not get_option('no_git') run_command('git', 'submodule', 'update', '--init', check: true) -- 2.34.1

2 1

[PATCH] virtiofs: rename member to 'openfiles' for clarity
by Adam Julis 24 Jul '24

24 Jul '24

New element 'openfiles' had confusing name. Since the patch with this new element wasn't propagate yet, old name ('rlimit_nofile') was changed. ... <binary> <openfiles max='122333'/> </binary> ... Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- docs/formatdomain.rst | 6 +++--- src/conf/domain_conf.c | 14 +++++++------- src/conf/domain_conf.h | 2 +- src/conf/schemas/domaincommon.rng | 4 ++-- src/qemu/qemu_virtiofs.c | 4 ++-- ... vhost-user-fs-fd-openfiles.x86_64-latest.args} | 0 .../vhost-user-fs-fd-openfiles.x86_64-latest.xml | 1 + ...d-rlimit.xml => vhost-user-fs-fd-openfiles.xml} | 2 +- .../vhost-user-fs-fd-rlimit.x86_64-latest.xml | 1 - tests/qemuxmlconftest.c | 2 +- 10 files changed, 18 insertions(+), 18 deletions(-) rename tests/qemuxmlconfdata/{vhost-user-fs-fd-rlimit.x86_64-latest.args => vhost-user-fs-fd-openfiles.x86_64-latest.args} (100%) create mode 120000 tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.x86_64-latest.xml rename tests/qemuxmlconfdata/{vhost-user-fs-fd-rlimit.xml => vhost-user-fs-fd-openfiles.xml} (97%) delete mode 120000 tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 10584dfe83..c2a6d0b910 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3734,10 +3734,10 @@ A directory on the host that can be accessed directly from the guest. The thread pool helps increase the number of requests in flight when used with storage that has a higher latency. However, it has an overhead, and so for fast, low latency filesystems, it may be best to turn it off. ( :since:`Since 8.5.0` ) - Element ``rlimit_profile`` accepts one attribute ``size`` which defines the + Element ``openfiles`` accepts one attribute ``max`` which defines the maximum number of file descriptors. Non-positive values are forbidden. - Although numbers greater than 1M are allowed, the virtiofsd documentation - states that in this case its set by virtiofsd to the 1M. ( :since:`Since 10.6.0` ) + The upper bound on the number of open files is implementation defined. + ( :since:`Since 10.6.0` ) ``source`` The resource on the host that is being accessed in the guest. The ``name`` attribute must be used with ``type='template'``, and the ``dir`` attribute diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6733857a3a..d1c59f7a91 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8868,7 +8868,7 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *queue_size = virXPathString("string(./driver/@queue)", ctxt); g_autofree char *binary = virXPathString("string(./binary/@path)", ctxt); g_autofree char *thread_pool_size = virXPathString("string(./binary/thread_pool/@size)", ctxt); - g_autofree char *rlimit_nofile = virXPathString("string(./binary/rlimit_nofile/@size)", ctxt); + g_autofree char *openfiles = virXPathString("string(./binary/openfiles/@max)", ctxt); xmlNodePtr binary_node = virXPathNode("./binary", ctxt); xmlNodePtr binary_lock_node = virXPathNode("./binary/lock", ctxt); xmlNodePtr binary_cache_node = virXPathNode("./binary/cache", ctxt); @@ -8892,11 +8892,11 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, goto error; } - if (rlimit_nofile && - virStrToLong_ull(rlimit_nofile, NULL, 10, &def->rlimit_nofile) < 0) { + if (openfiles && + virStrToLong_ull(openfiles, NULL, 10, &def->openfiles) < 0) { virReportError(VIR_ERR_XML_ERROR, - _("cannot parse rlimit_nofile '%1$s' for virtiofs"), - rlimit_nofile); + _("cannot parse openfiles '%1$s' for virtiofs"), + openfiles); goto error; } @@ -23424,8 +23424,8 @@ virDomainFSDefFormat(virBuffer *buf, if (def->thread_pool_size >= 0) virBufferAsprintf(&binaryBuf, "<thread_pool size='%d'/>\n", def->thread_pool_size); - if (def->rlimit_nofile > 0) - virBufferAsprintf(&binaryBuf, "<rlimit_nofile size='%lld'/>\n", def->rlimit_nofile); + if (def->openfiles > 0) + virBufferAsprintf(&binaryBuf, "<openfiles max='%lld'/>\n", def->openfiles); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8283493dfc..6972f6ae9b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -890,7 +890,7 @@ struct _virDomainFSDef { bool symlinksResolved; char *binary; unsigned long long queue_size; - unsigned long long rlimit_nofile; + unsigned long long openfiles; virTristateSwitch xattr; virDomainFSCacheMode cache; virTristateSwitch posix_lock; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index ab5374d5f0..8dae6416e9 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -3381,9 +3381,9 @@ </element> </optional> <optional> - <element name="rlimit_nofile"> + <element name="openfiles"> <optional> - <attribute name="size"> + <attribute name="max"> <data type="positiveInteger"/> </attribute> </optional> diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c index 703f1226a2..a8f2416273 100644 --- a/src/qemu/qemu_virtiofs.c +++ b/src/qemu/qemu_virtiofs.c @@ -194,8 +194,8 @@ qemuVirtioFSBuildCommandLine(virQEMUDriverConfig *cfg, if (fs->thread_pool_size >= 0) virCommandAddArgFormat(cmd, "--thread-pool-size=%i", fs->thread_pool_size); - if (fs->rlimit_nofile > 0) - virCommandAddArgFormat(cmd, "--rlimit-nofile=%llu", fs->rlimit_nofile); + if (fs->openfiles > 0) + virCommandAddArgFormat(cmd, "--rlimit-nofile=%llu", fs->openfiles); if (cfg->virtiofsdDebug) { if (virBitmapIsBitSet(fs->caps, QEMU_VHOST_USER_FS_FEATURE_SEPARATE_OPTIONS)) diff --git a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.args b/tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.x86_64-latest.args similarity index 100% rename from tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.args rename to tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.x86_64-latest.args diff --git a/tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.x86_64-latest.xml b/tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.x86_64-latest.xml new file mode 120000 index 0000000000..74761b0d0f --- /dev/null +++ b/tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.x86_64-latest.xml @@ -0,0 +1 @@ +vhost-user-fs-fd-openfiles.xml \ No newline at end of file diff --git a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.xml b/tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.xml similarity index 97% rename from tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.xml rename to tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.xml index 2983d3f275..3e925a8c8b 100644 --- a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.xml +++ b/tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.xml @@ -32,7 +32,7 @@ <cache mode='always'/> <sandbox mode='chroot'/> <thread_pool size='16'/> - <rlimit_nofile size='122333'/> + <openfiles max='122333'/> </binary> <idmap> <uid start='0' target='100000' count='65535'/> diff --git a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml b/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml deleted file mode 120000 index 68c4e8a482..0000000000 --- a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml +++ /dev/null @@ -1 +0,0 @@ -vhost-user-fs-fd-rlimit.xml \ No newline at end of file diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index cc984440ea..bf88bd2f8d 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2816,7 +2816,7 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST("launch-security-s390-pv", "s390x"); DO_TEST_CAPS_LATEST("vhost-user-fs-fd-memory"); - DO_TEST_CAPS_LATEST("vhost-user-fs-fd-rlimit"); + DO_TEST_CAPS_LATEST("vhost-user-fs-fd-openfiles"); DO_TEST_CAPS_LATEST("vhost-user-fs-hugepages"); DO_TEST_CAPS_LATEST_PARSE_ERROR("vhost-user-fs-readonly"); -- 2.45.2

2 1

[PATCH] qemu_hotplug: Do not allow absent values in rom settings
by Kristina Hanicova 24 Jul '24

24 Jul '24

If there are absent values in an already existing element specifying rom settings, we simply use the old ones. This behaviour is not desired, as users might think that deleting the element from XML would delete the setting (because the hotplug succeeds) - which does not happen. Because of that, we should not accept an interface without elements that cannot be changed. Therefore, we should not allow absent values for already existing rom setting during hotplug. Resolves: https://issues.redhat.com/browse/RHEL-7109 Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com> --- src/qemu/qemu_hotplug.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1f4620d833..7f158ddcd5 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3838,16 +3838,12 @@ qemuDomainChangeNet(virQEMUDriver *driver, /* device alias is checked already in virDomainDefCompatibleDevice */ - if (newdev->info.rombar == VIR_TRISTATE_SWITCH_ABSENT) - newdev->info.rombar = olddev->info.rombar; if (olddev->info.rombar != newdev->info.rombar) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("cannot modify network device rom bar setting")); goto cleanup; } - if (!newdev->info.romfile) - newdev->info.romfile = g_strdup(olddev->info.romfile); if (STRNEQ_NULLABLE(olddev->info.romfile, newdev->info.romfile)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("cannot modify network rom file")); @@ -3860,8 +3856,6 @@ qemuDomainChangeNet(virQEMUDriver *driver, goto cleanup; } - if (newdev->info.romenabled == VIR_TRISTATE_BOOL_ABSENT) - newdev->info.romenabled = olddev->info.romenabled; if (olddev->info.romenabled != newdev->info.romenabled) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("cannot modify network device rom enabled setting")); -- 2.45.1

2 1

[PATCH v3] qemu: add a monitor to /proc/$pid when killing times out
by Boris Fiuczynski 24 Jul '24

24 Jul '24

In cases when a QEMU process takes longer than the time sigterm and sigkill are issued to kill the process do not simply fail and leave the VM in state VIR_DOMAIN_SHUTDOWN until the daemon stops. Instead set up an fd on /proc/$pid and get notified when the QEMU process finally has terminated to cleanup the VM state. Resolves: https://issues.redhat.com/browse/RHEL-28819 Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com> --- src/qemu/qemu_domain.c | 8 +++ src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 18 ++++++ src/qemu/qemu_process.c | 124 ++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_process.h | 1 + 5 files changed, 148 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2134b11038..8147ff02fd 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1889,6 +1889,11 @@ qemuDomainObjPrivateFree(void *data) virChrdevFree(priv->devs); + if (priv->pidMonitored >= 0) { + virEventRemoveHandle(priv->pidMonitored); + priv->pidMonitored = -1; + } + /* This should never be non-NULL if we get here, but just in case... */ if (priv->mon) { VIR_ERROR(_("Unexpected QEMU monitor still active during domain deletion")); @@ -1934,6 +1939,8 @@ qemuDomainObjPrivateAlloc(void *opaque) priv->blockjobs = virHashNew(virObjectUnref); priv->fds = virHashNew(g_object_unref); + priv->pidMonitored = -1; + /* agent commands block by default, user can choose different behavior */ priv->agentTimeout = VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK; priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; @@ -11680,6 +11687,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event) case QEMU_PROCESS_EVENT_RESET: case QEMU_PROCESS_EVENT_NBDKIT_EXITED: case QEMU_PROCESS_EVENT_MONITOR_EOF: + case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED: case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index d777559119..a5092dd7f0 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -119,6 +119,7 @@ struct _qemuDomainObjPrivate { bool beingDestroyed; char *pidfile; + int pidMonitored; virDomainPCIAddressSet *pciaddrs; virDomainUSBAddressSet *usbaddrs; @@ -469,6 +470,7 @@ typedef enum { QEMU_PROCESS_EVENT_UNATTENDED_MIGRATION, QEMU_PROCESS_EVENT_RESET, QEMU_PROCESS_EVENT_NBDKIT_EXITED, + QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED, QEMU_PROCESS_EVENT_LAST } qemuProcessEventType; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9f3013e231..6b1e4084f6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4041,6 +4041,21 @@ processNbdkitExitedEvent(virDomainObj *vm, } +static void +processShutdownCompletedEvent(virQEMUDriver *driver, + virDomainObj *vm) +{ + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + return; + + if (virDomainObjIsActive(vm)) + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_UNKNOWN, + VIR_ASYNC_JOB_NONE, 0); + + virDomainObjEndJob(vm); +} + + static void qemuProcessEventHandler(void *data, void *opaque) { struct qemuProcessEvent *processEvent = data; @@ -4101,6 +4116,9 @@ static void qemuProcessEventHandler(void *data, void *opaque) case QEMU_PROCESS_EVENT_NBDKIT_EXITED: processNbdkitExitedEvent(vm, processEvent->data); break; + case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED: + processShutdownCompletedEvent(driver, vm); + break; case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 25dfd04272..c6f7d34168 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -25,6 +25,9 @@ #include <unistd.h> #include <signal.h> #include <sys/stat.h> +#if WITH_SYS_SYSCALL_H +# include <sys/syscall.h> +#endif #if defined(__linux__) # include <linux/capability.h> #elif defined(__FreeBSD__) @@ -8387,9 +8390,114 @@ qemuProcessCreatePretendCmdBuild(virDomainObj *vm, } +#if WITH_SYS_SYSCALL_H && defined(SYS_pidfd_open) +typedef struct { + virDomainObj *vm; + int pidfd; +} qemuProcessInShutdownEventData; + + +static qemuProcessInShutdownEventData* +qemuProcessInShutdownEventDataNew(virDomainObj *vm, int pidfd) +{ + qemuProcessInShutdownEventData *d = g_new(qemuProcessInShutdownEventData, 1); + d->vm = virObjectRef(vm); + d->pidfd = pidfd; + return d; +} + + +static void +qemuProcessInShutdownEventDataFree(qemuProcessInShutdownEventData *d) +{ + virObjectUnref(d->vm); + VIR_FORCE_CLOSE(d->pidfd); + g_free(d); +} + + +static void +qemuProcessInShutdownPidfdCb(int watch, + int fd, + int events G_GNUC_UNUSED, + void *opaque) +{ + qemuProcessInShutdownEventData *data = opaque; + virDomainObj *vm = data->vm; + + VIR_DEBUG("vm=%p name=%s pid=%lld fd=%d", + vm, vm->def->name, (long long)vm->pid, fd); + + virEventRemoveHandle(watch); + + virObjectLock(vm); + + VIR_INFO("QEMU process %lld finally completed termination", + (long long)vm->pid); + + QEMU_DOMAIN_PRIVATE(vm)->pidMonitored = -1; + qemuProcessEventSubmit(vm, QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED, + 0, 0, NULL); + + virObjectUnlock(vm); +} +#endif /* WITH_SYS_SYSCALL_H && defined(SYS_pidfd_open) */ + + +static int +qemuProcessInShutdownStartMonitor(virDomainObj *vm) +{ +#if WITH_SYS_SYSCALL_H && defined(SYS_pidfd_open) + qemuDomainObjPrivate *priv = vm->privateData; + qemuProcessInShutdownEventData *data; + int pidfd; + int ret = -1; + + VIR_DEBUG("vm=%p name=%s pid=%lld pidMonitored=%d", + vm, vm->def->name, (long long)vm->pid, + priv->pidMonitored); + + if (priv->pidMonitored >= 0) { + VIR_DEBUG("Monitoring qemu in-shutdown process %i already set up", vm->pid); + goto cleanup; + } + + pidfd = syscall(SYS_pidfd_open, vm->pid, 0); + if (pidfd < 0) { + if (errno == ESRCH) /* process has already terminated */ + ret = 1; + goto cleanup; + } + + data = qemuProcessInShutdownEventDataNew(vm, pidfd); + if ((priv->pidMonitored = virEventAddHandle(pidfd, + VIR_EVENT_HANDLE_READABLE, + qemuProcessInShutdownPidfdCb, + data, + (virFreeCallback)qemuProcessInShutdownEventDataFree)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to monitor qemu in-shutdown process %1$i"), + vm->pid); + qemuProcessInShutdownEventDataFree(data); + goto cleanup; + } + VIR_DEBUG("Monitoring qemu in-shutdown process %i for termination", vm->pid); + ret = 0; + + cleanup: + return ret; +#else /* !WITH_SYS_SYSCALL_H || !defined(SYS_pidfd_open) */ + VIR_DEBUG("Monitoring qemu process %i not implemented", vm->pid); + return -1; +#endif /* !WITH_SYS_SYSCALL_H || !defined(SYS_pidfd_open) */ +} + + int qemuProcessKill(virDomainObj *vm, unsigned int flags) { + int ret = -1; + VIR_DEBUG("vm=%p name=%s pid=%lld flags=0x%x", vm, vm->def->name, (long long)vm->pid, flags); @@ -8410,10 +8518,16 @@ qemuProcessKill(virDomainObj *vm, unsigned int flags) /* Request an extra delay of two seconds per current nhostdevs * to be safe against stalls by the kernel freeing up the resources */ - return virProcessKillPainfullyDelay(vm->pid, - !!(flags & VIR_QEMU_PROCESS_KILL_FORCE), - vm->def->nhostdevs * 2, - false); + ret = virProcessKillPainfullyDelay(vm->pid, + !!(flags & VIR_QEMU_PROCESS_KILL_FORCE), + vm->def->nhostdevs * 2, + false); + + if (ret < 0 && (flags & VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR)) + if (qemuProcessInShutdownStartMonitor(vm) == 1) + ret = 0; /* process termination detected */ + + return ret; } @@ -8438,7 +8552,7 @@ qemuProcessBeginStopJob(virDomainObj *vm, * cleared inside qemuProcessStop */ priv->beingDestroyed = true; - if (qemuProcessKill(vm, killFlags) < 0) + if (qemuProcessKill(vm, killFlags|VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR) < 0) goto error; /* Wake up anything waiting on domain condition */ diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index cb67bfcd2d..2324aeb7bd 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -180,6 +180,7 @@ typedef enum { VIR_QEMU_PROCESS_KILL_FORCE = 1 << 0, VIR_QEMU_PROCESS_KILL_NOWAIT = 1 << 1, VIR_QEMU_PROCESS_KILL_NOCHECK = 1 << 2, /* bypass the running vm check */ + VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR = 1 << 3, /* on error enable process monitor */ } virQemuProcessKillMode; int qemuProcessKill(virDomainObj *vm, unsigned int flags); -- 2.45.0

3 5

[PATCH v3] virsh: Provide completer for some pool-X-as commands
by Abhiram Tilak 24 Jul '24

24 Jul '24

From: notpua <atp(a)tutamail.com> Provides completers for auth-type and source-format commands for virsh pool-create-as and pool-define-as commands. Use Empty completers for options where completions are not required. Related Issue: https://gitlab.com/libvirt/libvirt/-/issues/9 Signed-off-by: Abhiram Tilak <atp.exp(a)gmail.com> --- Changes in v2: - Fix all formatting errors - Change some options using Empty completers, to use LocalPath completers. - Add completers for AdapterName and AdapterParent using information from node devices. Changes in v3: - Call virshNodeDeviceNameComplete with modified flags instead of duplicating the whole implementation. src/libvirt_private.syms | 2 ++ tools/virsh-completer-pool.c | 70 ++++++++++++++++++++++++++++++++++++ tools/virsh-completer-pool.h | 20 +++++++++++ tools/virsh-pool.c | 9 +++++ 4 files changed, 101 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c35366c9e1..6ba1e8e2c5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1123,6 +1123,8 @@ virStorageAuthDefCopy; virStorageAuthDefFormat; virStorageAuthDefFree; virStorageAuthDefParse; +virStorageAuthTypeFromString; +virStorageAuthTypeToString; virStorageFileFeatureTypeFromString; virStorageFileFeatureTypeToString; virStorageFileFormatTypeFromString; diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c index 3568bb985b..ba7855fdba 100644 --- a/tools/virsh-completer-pool.c +++ b/tools/virsh-completer-pool.c @@ -23,6 +23,7 @@ #include "virsh-completer-pool.h" #include "virsh-util.h" #include "conf/storage_conf.h" +#include "conf/node_device_conf.h" #include "virsh-pool.h" #include "virsh.h" @@ -106,3 +107,72 @@ virshPoolTypeCompleter(vshControl *ctl, return virshCommaStringListComplete(type_str, (const char **)tmp); } + + +char ** +virshPoolFormatCompleter(vshControl *ctl G_GNUC_UNUSED, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int flags) +{ + size_t i = 0; + size_t j = 0; + g_auto(GStrv) tmp = NULL; + size_t nformats = VIR_STORAGE_POOL_FS_LAST + VIR_STORAGE_POOL_NETFS_LAST + + VIR_STORAGE_POOL_DISK_LAST + VIR_STORAGE_POOL_LOGICAL_LAST; + + virCheckFlags(0, NULL); + + tmp = g_new0(char *, nformats + 1); + + /* Club all PoolFormats for completion */ + for (i = 0; i < VIR_STORAGE_POOL_FS_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatFileSystemTypeToString(i)); + + for (i = 0; i < VIR_STORAGE_POOL_NETFS_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatFileSystemNetTypeToString(i)); + + for (i = 1; i < VIR_STORAGE_POOL_DISK_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatDiskTypeToString(i)); + + for (i = 1; i < VIR_STORAGE_POOL_LOGICAL_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatLogicalTypeToString(i)); + + return g_steal_pointer(&tmp); +} + + +char ** +virshPoolAuthTypeCompleter(vshControl *ctl G_GNUC_UNUSED, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int flags) +{ + size_t i = 0; + g_auto(GStrv) tmp = NULL; + + virCheckFlags(0, NULL); + + tmp = g_new0(char *, VIR_STORAGE_AUTH_TYPE_LAST + 1); + + for (i = 0; i < VIR_STORAGE_AUTH_TYPE_LAST; i++) + tmp[i] = g_strdup(virStorageAuthTypeToString(i)); + + return g_steal_pointer(&tmp); +} + + +char ** +virshAdapterNameCompleter(vshControl *ctl, + const vshCmd *cmd, unsigned int flags) +{ + flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_HOST; + return virshNodeDeviceNameCompleter(ctl, cmd, flags); +} + + +char ** +virshAdapterParentCompleter(vshControl *ctl, + const vshCmd *cmd, unsigned int flags) +{ + flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS; + return virshNodeDeviceNameCompleter(ctl, cmd, flags); +} diff --git a/tools/virsh-completer-pool.h b/tools/virsh-completer-pool.h index bff3e5742b..eccc08a73f 100644 --- a/tools/virsh-completer-pool.h +++ b/tools/virsh-completer-pool.h @@ -40,3 +40,23 @@ char ** virshPoolTypeCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); + +char ** +virshPoolFormatCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + +char ** +virshPoolAuthTypeCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + +char ** +virshAdapterNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + +char ** +virshAdapterParentCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index f9aad8ded0..0cbd1417e6 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -80,31 +80,37 @@ {.name = "source-path", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshCompletePathLocalExisting, \ .help = N_("source path for underlying storage") \ }, \ {.name = "source-dev", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshCompletePathLocalExisting, \ .help = N_("source device for underlying storage") \ }, \ {.name = "source-name", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshCompleteEmpty, \ .help = N_("source name for underlying storage") \ }, \ {.name = "target", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshCompletePathLocalExisting, \ .help = N_("target for underlying storage") \ }, \ {.name = "source-format", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshPoolFormatCompleter, \ .help = N_("format for underlying storage") \ }, \ {.name = "auth-type", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshPoolAuthTypeCompleter, \ .help = N_("auth type to be used for underlying storage") \ }, \ {.name = "auth-username", \ @@ -126,6 +132,7 @@ {.name = "adapter-name", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshAdapterNameCompleter, \ .help = N_("adapter name to be used for underlying storage") \ }, \ {.name = "adapter-wwnn", \ @@ -141,6 +148,7 @@ {.name = "adapter-parent", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshAdapterParentCompleter, \ .help = N_("adapter parent scsi_hostN to be used for underlying vHBA storage") \ }, \ {.name = "adapter-parent-wwnn", \ @@ -161,6 +169,7 @@ {.name = "source-protocol-ver", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshCompleteEmpty, \ .help = N_("nfsvers value for NFS pool mount option") \ }, \ {.name = "source-initiator", \ -- 2.39.2

2 1

[PATCH v2] virsh: Provide completer for some pool-X-as commands
by Abhiram Tilak 23 Jul '24

23 Jul '24

Provides completers for auth-type and source-format commands for virsh pool-create-as and pool-define-as commands. Use Empty completers for options where completions are not required. Related Issue: https://gitlab.com/libvirt/libvirt/-/issues/9 Signed-off-by: Abhiram Tilak <atp.exp(a)gmail.com> --- Changes in v2: - Fix all formatting errors - Change some options using Empty completers, to use LocalPath completers. - Add completers for AdapterName and AdapterParent using information from node devices. src/libvirt_private.syms | 2 + tools/virsh-completer-pool.c | 128 +++++++++++++++++++++++++++++++++++ tools/virsh-completer-pool.h | 20 ++++++ tools/virsh-pool.c | 9 +++ 4 files changed, 159 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f0f7aa8654..fcb0ef7afe 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1117,6 +1117,8 @@ virStorageAuthDefCopy; virStorageAuthDefFormat; virStorageAuthDefFree; virStorageAuthDefParse; +virStorageAuthTypeFromString; +virStorageAuthTypeToString; virStorageFileFeatureTypeFromString; virStorageFileFeatureTypeToString; virStorageFileFormatTypeFromString; diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c index 3568bb985b..7db2a20347 100644 --- a/tools/virsh-completer-pool.c +++ b/tools/virsh-completer-pool.c @@ -23,6 +23,7 @@ #include "virsh-completer-pool.h" #include "virsh-util.h" #include "conf/storage_conf.h" +#include "conf/node_device_conf.h" #include "virsh-pool.h" #include "virsh.h" @@ -106,3 +107,130 @@ virshPoolTypeCompleter(vshControl *ctl, return virshCommaStringListComplete(type_str, (const char **)tmp); } + + +char ** +virshPoolFormatCompleter(vshControl *ctl G_GNUC_UNUSED, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int flags) +{ + size_t i = 0; + size_t j = 0; + g_auto(GStrv) tmp = NULL; + size_t nformats = VIR_STORAGE_POOL_FS_LAST + VIR_STORAGE_POOL_NETFS_LAST + + VIR_STORAGE_POOL_DISK_LAST + VIR_STORAGE_POOL_LOGICAL_LAST; + + virCheckFlags(0, NULL); + + tmp = g_new0(char *, nformats + 1); + + /* Club all PoolFormats for completion */ + for (i = 0; i < VIR_STORAGE_POOL_FS_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatFileSystemTypeToString(i)); + + for (i = 0; i < VIR_STORAGE_POOL_NETFS_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatFileSystemNetTypeToString(i)); + + for (i = 1; i < VIR_STORAGE_POOL_DISK_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatDiskTypeToString(i)); + + for (i = 1; i < VIR_STORAGE_POOL_LOGICAL_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatLogicalTypeToString(i)); + + return g_steal_pointer(&tmp); +} + + +char ** +virshPoolAuthTypeCompleter(vshControl *ctl G_GNUC_UNUSED, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int flags) +{ + size_t i = 0; + g_auto(GStrv) tmp = NULL; + + virCheckFlags(0, NULL); + + tmp = g_new0(char *, VIR_STORAGE_AUTH_TYPE_LAST + 1); + + for (i = 0; i < VIR_STORAGE_AUTH_TYPE_LAST; i++) + tmp[i] = g_strdup(virStorageAuthTypeToString(i)); + + return g_steal_pointer(&tmp); +} + + +char ** +virshAdapterNameCompleter(vshControl *ctl, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int flags) +{ + virshControl *priv = ctl->privData; + virNodeDevicePtr *devs = NULL; + int ndevs = 0; + size_t i = 0; + char **ret = NULL; + g_auto(GStrv) tmp = NULL; + + virCheckFlags(0, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_HOST; + if ((ndevs = virConnectListAllNodeDevices(priv->conn, &devs, flags)) < 0) + return NULL; + + tmp = g_new0(char *, ndevs + 1); + + for (i = 0; i < ndevs; i++) { + const char *name = virNodeDeviceGetName(devs[i]); + + tmp[i] = g_strdup(name); + } + + ret = g_steal_pointer(&tmp); + + for (i = 0; i < ndevs; i++) + virshNodeDeviceFree(devs[i]); + g_free(devs); + return ret; +} + + +char ** +virshAdapterParentCompleter(vshControl *ctl, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int flags) +{ + virshControl *priv = ctl->privData; + virNodeDevicePtr *devs = NULL; + int ndevs = 0; + size_t i = 0; + char **ret = NULL; + g_auto(GStrv) tmp = NULL; + + virCheckFlags(0, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS; + if ((ndevs = virConnectListAllNodeDevices(priv->conn, &devs, flags)) < 0) + return NULL; + + tmp = g_new0(char *, ndevs + 1); + + for (i = 0; i < ndevs; i++) { + const char *name = virNodeDeviceGetName(devs[i]); + + tmp[i] = g_strdup(name); + } + + ret = g_steal_pointer(&tmp); + + for (i = 0; i < ndevs; i++) + virshNodeDeviceFree(devs[i]); + g_free(devs); + return ret; +} diff --git a/tools/virsh-completer-pool.h b/tools/virsh-completer-pool.h index bff3e5742b..eccc08a73f 100644 --- a/tools/virsh-completer-pool.h +++ b/tools/virsh-completer-pool.h @@ -40,3 +40,23 @@ char ** virshPoolTypeCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); + +char ** +virshPoolFormatCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + +char ** +virshPoolAuthTypeCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + +char ** +virshAdapterNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + +char ** +virshAdapterParentCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index f9aad8ded0..0cbd1417e6 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -80,31 +80,37 @@ {.name = "source-path", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshCompletePathLocalExisting, \ .help = N_("source path for underlying storage") \ }, \ {.name = "source-dev", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshCompletePathLocalExisting, \ .help = N_("source device for underlying storage") \ }, \ {.name = "source-name", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshCompleteEmpty, \ .help = N_("source name for underlying storage") \ }, \ {.name = "target", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshCompletePathLocalExisting, \ .help = N_("target for underlying storage") \ }, \ {.name = "source-format", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshPoolFormatCompleter, \ .help = N_("format for underlying storage") \ }, \ {.name = "auth-type", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshPoolAuthTypeCompleter, \ .help = N_("auth type to be used for underlying storage") \ }, \ {.name = "auth-username", \ @@ -126,6 +132,7 @@ {.name = "adapter-name", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshAdapterNameCompleter, \ .help = N_("adapter name to be used for underlying storage") \ }, \ {.name = "adapter-wwnn", \ @@ -141,6 +148,7 @@ {.name = "adapter-parent", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshAdapterParentCompleter, \ .help = N_("adapter parent scsi_hostN to be used for underlying vHBA storage") \ }, \ {.name = "adapter-parent-wwnn", \ @@ -161,6 +169,7 @@ {.name = "source-protocol-ver", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshCompleteEmpty, \ .help = N_("nfsvers value for NFS pool mount option") \ }, \ {.name = "source-initiator", \ -- 2.39.2

3 2

[PATCH] qemuProcessStop: Don't unlock domain until most of the cleanup is done
by Michal Privoznik 23 Jul '24

23 Jul '24

Imagine two threads. Thread A is executing qemuProcessStop() and thread B is executing qemuDomainCreateXML(). To make things more interesting, the domain XML passed to qemuDomainCreateXML matches name + UUID of that the virDomainObj that qemuProcessStop() is currently working with. Here's what happens. 1) Thread A locks @vm, enters qemuProcessStop(). 2) Thread B parses given XML, calls virDomainObjListAdd() -> virDomainObjListAdd() -> virDomainObjListAddLocked() -> virDomainObjListFindByUUIDLocked(). Since there's a match on UUID, an virDomainObj object is returned and the thread proceeds to calling virObjectLock(). NB, it's the same object as in thread A. 3) Thread A sets vm->def->id = -1; this means that from this point on, virDomainObjIsActive() will return false. 4) Thread A calls qemuDomainObjStopWorker() which unlocks the @vm. 5) Thread B acquires the @vm lock and since virDomainObjIsActive() returns false, it proceeds to calling virDomainObjAssignDef() where vm->def is replaced. 6) Thread B then calls qemuProcessBeginJob() which unlocks the @vm temporarily. 7) Thread A, still in qemuDomainObjStopWorker() acquires @vm lock and proceeds with cleanup. 8) Thread A finds different definition than the one needing cleanup. In my testing I've seen stale pointers, e.g. vm->def->nets[0]->priv was NULL, which lead to a SIGSEGV as there's 'QEMU_DOMAIN_NETWORK_PRIVATE(net)->created' line when cleaning up nets. Your mileage may vary. Even if we did not crash, the plain fact that vm->def is changed in the middle of qemuProcessStop() means we might be cleaning up something else than intended. As a fix, I'm moving all lines that obviously touch vm->def before the domain object is unlocked. That left virHookCall(VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END) nearly next to virHookCall(VIR_HOOK_QEMU_OP_RELEASE, VIR_HOOK_SUBOP_END) which I figured is not something we want. So I've shuffled things a bit more. Fixes: 3865410e7f67ca4ec66e9a905e75f452762a97f0 Resolves: https://issues.redhat.com/browse/RHEL-49607 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_process.c | 122 ++++++++++++++++++++-------------------- 1 file changed, 62 insertions(+), 60 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 25dfd04272..9ea6c678b8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8530,6 +8530,18 @@ void qemuProcessStop(virQEMUDriver *driver, VIR_QEMU_PROCESS_KILL_FORCE| VIR_QEMU_PROCESS_KILL_NOCHECK)); + vm->pid = 0; + + /* now that we know it's stopped call the hook if present */ + if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { + g_autofree char *xml = qemuDomainDefFormatXML(driver, NULL, vm->def, 0); + + /* we can't stop the operation even if the script raised an error */ + ignore_value(virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name, + VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END, + NULL, xml, NULL)); + } + if (priv->agent) { g_clear_pointer(&priv->agent, qemuAgentClose); } @@ -8553,25 +8565,6 @@ void qemuProcessStop(virQEMUDriver *driver, qemuDBusStop(driver, vm); - /* Only after this point we can reset 'priv->beingDestroyed' so that - * there's no point at which the VM could be considered as alive between - * entering the destroy job and this point where the active "flag" is - * cleared. - */ - vm->def->id = -1; - priv->beingDestroyed = false; - - /* Wake up anything waiting on domain condition */ - virDomainObjBroadcast(vm); - - /* IMPORTANT: qemuDomainObjStopWorker() unlocks @vm in order to prevent - * deadlocks with the per-VM event loop thread. This MUST be done after - * marking the VM as dead */ - qemuDomainObjStopWorker(vm); - - if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) - driver->inhibitCallback(false, driver->inhibitOpaque); - /* Clear network bandwidth */ virDomainClearNetBandwidth(vm->def); @@ -8588,18 +8581,6 @@ void qemuProcessStop(virQEMUDriver *driver, } } - virPortAllocatorRelease(priv->nbdPort); - priv->nbdPort = 0; - - if (priv->monConfig) { - if (priv->monConfig->type == VIR_DOMAIN_CHR_TYPE_UNIX) - unlink(priv->monConfig->data.nix.path); - g_clear_pointer(&priv->monConfig, virObjectUnref); - } - - /* Remove the master key */ - qemuDomainMasterKeyRemove(priv); - ignore_value(virDomainChrDefForeach(vm->def, false, qemuProcessCleanupChardevDevice, @@ -8609,22 +8590,6 @@ void qemuProcessStop(virQEMUDriver *driver, /* Its namespace is also gone then. */ qemuDomainDestroyNamespace(driver, vm); - virFileDeleteTree(priv->libDir); - virFileDeleteTree(priv->channelTargetDir); - - /* Stop autodestroy in case guest is restarted */ - virCloseCallbacksDomainRemove(vm, NULL, qemuProcessAutoDestroy); - - /* now that we know it's stopped call the hook if present */ - if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { - g_autofree char *xml = qemuDomainDefFormatXML(driver, NULL, vm->def, 0); - - /* we can't stop the operation even if the script raised an error */ - ignore_value(virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name, - VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END, - NULL, xml, NULL)); - } - /* Reset Security Labels unless caller don't want us to */ if (!(flags & VIR_QEMU_PROCESS_STOP_NO_RELABEL)) qemuSecurityRestoreAllLabel(driver, vm, @@ -8672,8 +8637,6 @@ void qemuProcessStop(virQEMUDriver *driver, virResctrlAllocRemove(vm->def->resctrls[i]->alloc); } - qemuProcessRemoveDomainStatus(driver, vm); - /* Remove VNC and Spice ports from port reservation bitmap, but only if they were reserved by the driver (autoport=yes) */ @@ -8706,20 +8669,9 @@ void qemuProcessStop(virQEMUDriver *driver, } } - for (i = 0; i < vm->ndeprecations; i++) - g_free(vm->deprecations[i]); - g_clear_pointer(&vm->deprecations, g_free); - vm->ndeprecations = 0; - vm->taint = 0; - vm->pid = 0; - virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); for (i = 0; i < vm->def->niothreadids; i++) vm->def->iothreadids[i]->thread_id = 0; - /* clean up a possible backup job */ - if (priv->backup) - qemuBackupJobTerminate(vm, VIR_DOMAIN_JOB_STATUS_CANCELED); - /* Do this explicitly after vm->pid is reset so that security drivers don't * try to enter the domain's namespace which is non-existent by now as qemu * is no longer running. */ @@ -8753,6 +8705,56 @@ void qemuProcessStop(virQEMUDriver *driver, qemuSecurityReleaseLabel(driver->securityManager, vm->def); + /* Only after this point we can reset 'priv->beingDestroyed' so that + * there's no point at which the VM could be considered as alive between + * entering the destroy job and this point where the active "flag" is + * cleared. + */ + vm->def->id = -1; + priv->beingDestroyed = false; + + /* Wake up anything waiting on domain condition */ + virDomainObjBroadcast(vm); + + /* IMPORTANT: qemuDomainObjStopWorker() unlocks @vm in order to prevent + * deadlocks with the per-VM event loop thread. This MUST be done after + * marking the VM as dead */ + qemuDomainObjStopWorker(vm); + + if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) + driver->inhibitCallback(false, driver->inhibitOpaque); + + virPortAllocatorRelease(priv->nbdPort); + priv->nbdPort = 0; + + if (priv->monConfig) { + if (priv->monConfig->type == VIR_DOMAIN_CHR_TYPE_UNIX) + unlink(priv->monConfig->data.nix.path); + g_clear_pointer(&priv->monConfig, virObjectUnref); + } + + /* Remove the master key */ + qemuDomainMasterKeyRemove(priv); + + virFileDeleteTree(priv->libDir); + virFileDeleteTree(priv->channelTargetDir); + + /* Stop autodestroy in case guest is restarted */ + virCloseCallbacksDomainRemove(vm, NULL, qemuProcessAutoDestroy); + + qemuProcessRemoveDomainStatus(driver, vm); + + for (i = 0; i < vm->ndeprecations; i++) + g_free(vm->deprecations[i]); + g_clear_pointer(&vm->deprecations, g_free); + vm->ndeprecations = 0; + vm->taint = 0; + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); + + /* clean up a possible backup job */ + if (priv->backup) + qemuBackupJobTerminate(vm, VIR_DOMAIN_JOB_STATUS_CANCELED); + /* clear all private data entries which are no longer needed */ qemuDomainObjPrivateDataClear(priv); -- 2.44.2

3 4

[PATCH v2] virt-host-validate: Allow longer list of CPU flags
by Michal Privoznik 23 Jul '24

23 Jul '24

On various occasions, virt-host-validate parses /proc/cpuinfo to learn about CPU flags (see virHostValidateGetCPUFlags()). It does so, by reading the file line by line until the line with CPU flags is reached. Then the line is split into individual flags (using space as a delimiter) and the list of flags is then iterated over. This works, except for cases when the line with CPU flags is too long. Problem is - the line is capped at 1024 bytes and on newer CPUs (and newer kernels), the line can be significantly longer. I've seen a line that's ~1200 characters long (with 164 flags reported). Switch to unbounded read from the file (getline()). Resolves: https://issues.redhat.com/browse/RHEL-39969 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- v2 of: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/LK74… diff to v2: - Keep trimming of the optional newline tools/virt-host-validate-common.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index 591143c24d..63cc3dbe7b 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -106,21 +106,19 @@ virBitmap *virHostValidateGetCPUFlags(void) { FILE *fp; virBitmap *flags = NULL; + g_autofree char *line = NULL; + size_t linelen = 0; if (!(fp = fopen("/proc/cpuinfo", "r"))) return NULL; flags = virBitmapNew(VIR_HOST_VALIDATE_CPU_FLAG_LAST); - do { - char line[1024]; + while (getline(&line, &linelen, fp) > 0) { char *start; g_auto(GStrv) tokens = NULL; GStrv next; - if (!fgets(line, sizeof(line), fp)) - break; - /* The line we're interested in is marked differently depending * on the architecture, so check possible prefixes */ if (!STRPREFIX(line, "flags") && @@ -129,11 +127,9 @@ virBitmap *virHostValidateGetCPUFlags(void) !STRPREFIX(line, "facilities")) continue; - /* fgets() includes the trailing newline in the output buffer, - * so we need to clean that up ourselves. We can safely access - * line[strlen(line) - 1] because the checks above would cause - * us to skip empty strings */ - line[strlen(line) - 1] = '\0'; + /* getline() may include the trailing newline in the output + * buffer, so we need to clean that up ourselves. */ + virStringTrimOptionalNewline(line); /* Skip to the separator */ if (!(start = strchr(line, ':'))) @@ -153,7 +149,7 @@ virBitmap *virHostValidateGetCPUFlags(void) if ((value = virHostValidateCPUFlagTypeFromString(*next)) >= 0) ignore_value(virBitmapSetBit(flags, value)); } - } while (1); + } VIR_FORCE_FCLOSE(fp); -- 2.44.2

2 1

[PATCH] virt-host-validate: Allow longer list of CPU flags
by Michal Privoznik 23 Jul '24

23 Jul '24

On various occasions, virt-host-validate parses /proc/cpuinfo to learn about CPU flags (see virHostValidateGetCPUFlags()). It does so, by reading the file line by line until the line with CPU flags is reached. Then the line is split into individual flags (using space as a delimiter) and the list of flags is then iterated over. This works, except for cases when the line with CPU flags is too long. Problem is - the line is capped at 1024 bytes and on newer CPUs (and newer kernels), the line can be significantly longer. I've seen a line that's ~1200 characters long (with 164 flags reported). Switch to unbounded read from the file (getline()). Resolves: https://issues.redhat.com/browse/RHEL-39969 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tools/virt-host-validate-common.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index 591143c24d..e5fa6606d2 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -106,21 +106,19 @@ virBitmap *virHostValidateGetCPUFlags(void) { FILE *fp; virBitmap *flags = NULL; + g_autofree char *line = NULL; + size_t linelen = 0; if (!(fp = fopen("/proc/cpuinfo", "r"))) return NULL; flags = virBitmapNew(VIR_HOST_VALIDATE_CPU_FLAG_LAST); - do { - char line[1024]; + while (getline(&line, &linelen, fp) > 0) { char *start; g_auto(GStrv) tokens = NULL; GStrv next; - if (!fgets(line, sizeof(line), fp)) - break; - /* The line we're interested in is marked differently depending * on the architecture, so check possible prefixes */ if (!STRPREFIX(line, "flags") && @@ -129,12 +127,6 @@ virBitmap *virHostValidateGetCPUFlags(void) !STRPREFIX(line, "facilities")) continue; - /* fgets() includes the trailing newline in the output buffer, - * so we need to clean that up ourselves. We can safely access - * line[strlen(line) - 1] because the checks above would cause - * us to skip empty strings */ - line[strlen(line) - 1] = '\0'; - /* Skip to the separator */ if (!(start = strchr(line, ':'))) continue; @@ -153,7 +145,7 @@ virBitmap *virHostValidateGetCPUFlags(void) if ((value = virHostValidateCPUFlagTypeFromString(*next)) >= 0) ignore_value(virBitmapSetBit(flags, value)); } - } while (1); + } VIR_FORCE_FCLOSE(fp); -- 2.44.2

3 2

[PATCH] rpc: report error from filing to add timer
by Daniel P. Berrangé 22 Jul '24

22 Jul '24

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/rpc/virnetclientstream.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index 98034d737d..380b785869 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -725,6 +725,8 @@ int virNetClientStreamEventAddCallback(virNetClientStream *st, virNetClientStreamEventTimer, st, virObjectUnref)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to add timer to event loop")); virObjectUnref(st); goto cleanup; } -- 2.45.2

2 1

cpugroups for hyperv hypervisor
by Praveen K Paladugu 22 Jul '24

22 Jul '24

Hey folks, My team is working on exposing `cpugroups` to Libvirt while using 'hyperv' hypervisor with cloud-hypervisor(VMM). cpugroups are relevant in a specific configuration of hyperv called 'minroot'. In Minroot configuration, hypervisor artificially restricts Dom0 to run on a subset of cpus (Logical Processors). The rest of the cpus can be assigned to guests. cpugroups manage the CPUs assigned to guests and their scheduling properties. Initially this looks similar to `cpuset` (in cgroups), but the controls available with cpugroups don't map easily to those in cgroups. For example: * "IdleLPs" are the number of Logical Processors in a cpugroup, that should be reserved to a guest even if they are idle * "SchedulingPriority", the priority(values between 0..7) with which to schedule CPUs in a cpugroup. As controls like above don't easily map to anything in cgroups, using a driver specific element in Domain xml, to configure cpugroups seems like a right approach. For example: <ch:cpugroups> <idle_lps value='4'/> <scheduling_priority value='6'/> </ch:cpugroups> As cpugroups is only relevant while using minroot configuration on hyperv, I don't see any value in generalizing this setting. So, having some "ch" driver specific settings seems like a good approach to implement this feature. Question1: Do you see any concerns with this approach? The cpugroup settings can be applied/modified using sysfs interface or using a cmdline tool on the host. I see Libvirt uses both these mechanisms for various use cases. But, given a choice, sysfs based interface seems like a simpler approach to me. With sysfs interface Libvirt does not have to take install time dependencies on new tools. Question2: Of "sysfs" vs "cmdline tool" which is preferred, given a choice? Early feedback from the community will help us invest in the preferred choices sooner than later. Thanks for your consideration. References: * https://learn.microsoft.com/en-us/windows-server/virtualization/hyper-v/man… -- Regards, Praveen

2 2

[PATCH] conf: virtiofs: add rlimit_nofile element
by Adam Julis 22 Jul '24

22 Jul '24

Add an element to configure the rlimit nofile size: ... <binary> <rlimit_nofile size='122333'/> </binary> ... Non-positive values are forbidden in 'domaincommon.rng'. Added separate test file, created by modifying the 'vhost-user-fs-fd-memory.xml'. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/485 Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- docs/formatdomain.rst | 4 ++ src/conf/domain_conf.c | 12 +++++ src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 9 ++++ ...vhost-user-fs-fd-rlimit.x86_64-latest.args | 34 +++++++++++++ .../vhost-user-fs-fd-rlimit.x86_64-latest.xml | 50 +++++++++++++++++++ .../vhost-user-fs-fd-rlimit.xml | 50 +++++++++++++++++++ tests/qemuxmlconftest.c | 1 + 8 files changed, 161 insertions(+) create mode 100644 tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.xml diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index ca33a2a570..10584dfe83 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3734,6 +3734,10 @@ A directory on the host that can be accessed directly from the guest. The thread pool helps increase the number of requests in flight when used with storage that has a higher latency. However, it has an overhead, and so for fast, low latency filesystems, it may be best to turn it off. ( :since:`Since 8.5.0` ) + Element ``rlimit_profile`` accepts one attribute ``size`` which defines the + maximum number of file descriptors. Non-positive values are forbidden. + Although numbers greater than 1M are allowed, the virtiofsd documentation + states that in this case its set by virtiofsd to the 1M. ( :since:`Since 10.6.0` ) ``source`` The resource on the host that is being accessed in the guest. The ``name`` attribute must be used with ``type='template'``, and the ``dir`` attribute diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 32df908d95..6733857a3a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8868,6 +8868,7 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *queue_size = virXPathString("string(./driver/@queue)", ctxt); g_autofree char *binary = virXPathString("string(./binary/@path)", ctxt); g_autofree char *thread_pool_size = virXPathString("string(./binary/thread_pool/@size)", ctxt); + g_autofree char *rlimit_nofile = virXPathString("string(./binary/rlimit_nofile/@size)", ctxt); xmlNodePtr binary_node = virXPathNode("./binary", ctxt); xmlNodePtr binary_lock_node = virXPathNode("./binary/lock", ctxt); xmlNodePtr binary_cache_node = virXPathNode("./binary/cache", ctxt); @@ -8891,6 +8892,14 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, goto error; } + if (rlimit_nofile && + virStrToLong_ull(rlimit_nofile, NULL, 10, &def->rlimit_nofile) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("cannot parse rlimit_nofile '%1$s' for virtiofs"), + rlimit_nofile); + goto error; + } + if (binary) def->binary = virFileSanitizePath(binary); @@ -23415,6 +23424,9 @@ virDomainFSDefFormat(virBuffer *buf, if (def->thread_pool_size >= 0) virBufferAsprintf(&binaryBuf, "<thread_pool size='%d'/>\n", def->thread_pool_size); + if (def->rlimit_nofile > 0) + virBufferAsprintf(&binaryBuf, "<rlimit_nofile size='%lld'/>\n", def->rlimit_nofile); + } virDomainVirtioOptionsFormat(&driverAttrBuf, def->virtio); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0fcc4f1f9b..8283493dfc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -890,6 +890,7 @@ struct _virDomainFSDef { bool symlinksResolved; char *binary; unsigned long long queue_size; + unsigned long long rlimit_nofile; virTristateSwitch xattr; virDomainFSCacheMode cache; virTristateSwitch posix_lock; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 2d23fcf123..ab5374d5f0 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -3380,6 +3380,15 @@ </optional> </element> </optional> + <optional> + <element name="rlimit_nofile"> + <optional> + <attribute name="size"> + <data type="positiveInteger"/> + </attribute> + </optional> + </element> + </optional> </interleave> </element> </define> diff --git a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.args b/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.args new file mode 100644 index 0000000000..b4c2e3fe98 --- /dev/null +++ b/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.args @@ -0,0 +1,34 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-guest \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-guest/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-guest/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=guest,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-guest/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,acpi=off \ +-accel kvm \ +-cpu qemu64 \ +-m size=14680064k \ +-overcommit mem-lock=off \ +-smp 2,sockets=2,cores=1,threads=1 \ +-object '{"qom-type":"memory-backend-file","id":"ram-node0","mem-path":"/var/lib/libvirt/qemu/ram/-1-guest/ram-node0","share":true,"size":15032385536}' \ +-numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \ +-uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-chardev socket,id=chr-vu-fs0,path=/var/lib/libvirt/qemu/domain--1-guest/fs0-fs.sock \ +-device '{"driver":"vhost-user-fs-pci","id":"fs0","chardev":"chr-vu-fs0","queue-size":1027,"tag":"mount_tag","bus":"pci.0","addr":"0x2"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml b/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml new file mode 100644 index 0000000000..2983d3f275 --- /dev/null +++ b/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml @@ -0,0 +1,50 @@ +<domain type='kvm'> + <name>guest</name> + <uuid>126f2720-6f8e-45ab-a886-ec9277079a67</uuid> + <memory unit='KiB'>14680064</memory> + <currentMemory unit='KiB'>14680064</currentMemory> + <memoryBacking> + <source type='file'/> + <access mode='shared'/> + </memoryBacking> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + <numa> + <cell id='0' cpus='0-1' memory='14680064' unit='KiB' memAccess='shared'/> + </numa> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='pci' index='0' model='pci-root'/> + <filesystem type='mount' accessmode='passthrough'> + <driver type='virtiofs' queue='1027'/> + <binary xattr='on'> + <cache mode='always'/> + <sandbox mode='chroot'/> + <thread_pool size='16'/> + <rlimit_nofile size='122333'/> + </binary> + <idmap> + <uid start='0' target='100000' count='65535'/> + <gid start='0' target='100000' count='65535'/> + </idmap> + <source dir='/path'/> + <target dir='mount_tag'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </filesystem> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.xml b/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.xml new file mode 100644 index 0000000000..2983d3f275 --- /dev/null +++ b/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.xml @@ -0,0 +1,50 @@ +<domain type='kvm'> + <name>guest</name> + <uuid>126f2720-6f8e-45ab-a886-ec9277079a67</uuid> + <memory unit='KiB'>14680064</memory> + <currentMemory unit='KiB'>14680064</currentMemory> + <memoryBacking> + <source type='file'/> + <access mode='shared'/> + </memoryBacking> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + <numa> + <cell id='0' cpus='0-1' memory='14680064' unit='KiB' memAccess='shared'/> + </numa> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='pci' index='0' model='pci-root'/> + <filesystem type='mount' accessmode='passthrough'> + <driver type='virtiofs' queue='1027'/> + <binary xattr='on'> + <cache mode='always'/> + <sandbox mode='chroot'/> + <thread_pool size='16'/> + <rlimit_nofile size='122333'/> + </binary> + <idmap> + <uid start='0' target='100000' count='65535'/> + <gid start='0' target='100000' count='65535'/> + </idmap> + <source dir='/path'/> + <target dir='mount_tag'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </filesystem> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 389d31800b..2aaf6103db 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2814,6 +2814,7 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST("launch-security-s390-pv", "s390x"); DO_TEST_CAPS_LATEST("vhost-user-fs-fd-memory"); + DO_TEST_CAPS_LATEST("vhost-user-fs-fd-rlimit"); DO_TEST_CAPS_LATEST("vhost-user-fs-hugepages"); DO_TEST_CAPS_LATEST_PARSE_ERROR("vhost-user-fs-readonly"); -- 2.45.2

3 2

[PATCH v2 0/1] Expose availability of SEV-ES
by Takashi Kajinami 22 Jul '24

22 Jul '24

This introduces the new "model" field in sev elements so that clients can check whether SEV-ES, the 2nd generation of AMD SEV, is available in the taget hyprvisor. There is the maxESGuests field (along with the maxGuests field) but this field does not explain whether SEV-ES is actually enabled in KVM. Takashi Kajinami (1): Expose available AMD SEV models in domain capabilities Changes since v1: * Fixed one code path where available models are not added * Fixed missing update of "report" flag * Updated the documentation to explain the new model field in addition to the existing but undocumanted cpu0Id field Takashi Kajinami (1): Expose available AMD SEV models in domain capabilities docs/formatdomaincaps.rst | 5 ++ src/conf/domain_capabilities.c | 2 + src/conf/domain_capabilities.h | 1 + src/conf/domain_conf.c | 7 +++ src/conf/domain_conf.h | 8 ++++ src/qemu/qemu_capabilities.c | 84 +++++++++++++++++++++++++--------- 6 files changed, 85 insertions(+), 22 deletions(-) -- 2.43.0

2 3

[PATCH 0/2] Add Granite Rapids cpu model
by Tim Wiederhake 22 Jul '24

22 Jul '24

Also, update the sync tool with new features introduced in qemu. Tim Wiederhake (2): sync_qemu_models_i386.py: Add missing features cpu_map: Add GraniteRapids CPU model src/cpu_map/index.xml | 1 + src/cpu_map/meson.build | 1 + src/cpu_map/sync_qemu_models_i386.py | 5 + src/cpu_map/x86_GraniteRapids.xml | 200 ++++++++++++++++++ .../domaincapsdata/qemu_8.1.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_8.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_8.1.0.x86_64.xml | 1 + .../domaincapsdata/qemu_8.2.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_8.2.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_8.2.0.x86_64.xml | 1 + .../domaincapsdata/qemu_9.0.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_9.0.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_9.0.0.x86_64.xml | 1 + .../domaincapsdata/qemu_9.1.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_9.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_9.1.0.x86_64.xml | 1 + 16 files changed, 219 insertions(+) create mode 100644 src/cpu_map/x86_GraniteRapids.xml -- 2.43.0

2 3

[PATCH] cpu_map: Add libcpuinfo as optional data source
by Tim Wiederhake 22 Jul '24

22 Jul '24

This adds an option to use libcpuinfo [1] as data source for libvirt's list of x86 cpu features. This is purely optional and does not change the script's behavior if libcpuinfo is not installed. libcpuinfo is a cross-vendor, cross-architecture source for CPU related information that has the capability to replace libvirt's dependence on qemu's cpu feature list. [1] https://gitlab.com/twiederh/libcpuinfo Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com> --- src/cpu_map/libcpuinfo_aliases.xml | 75 +++++++++++++++++++++++++ src/cpu_map/sync_qemu_features_i386.py | 77 +++++++++++++++++++++++--- 2 files changed, 145 insertions(+), 7 deletions(-) create mode 100644 src/cpu_map/libcpuinfo_aliases.xml diff --git a/src/cpu_map/libcpuinfo_aliases.xml b/src/cpu_map/libcpuinfo_aliases.xml new file mode 100644 index 0000000000..75d243fead --- /dev/null +++ b/src/cpu_map/libcpuinfo_aliases.xml @@ -0,0 +1,75 @@ + + +<external_aliases> + <external_alias> + <type>feature</type> + <canonical>pclmulqdq</canonical> + <name>pclmuldq</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>ds-cpl</canonical> + <name>ds_cpl</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>sse4-1</canonical> + <name>sse4.1</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>sse4-2</canonical> + <name>sse4.2</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>tsc-adjust</canonical> + <name>tsc_adjust</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>lahf-lm</canonical> + <name>lahf_lm</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>cmp-legacy</canonical> + <name>cmp_legacy</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>nodeid-msr</canonical> + <name>nodeid_msr</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>perfctr-core</canonical> + <name>perfctr_core</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>perfctr-nb</canonical> + <name>perfctr_nb</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>fxsr-opt</canonical> + <name>fxsr_opt</name> + <domain>libvirt</domain> + </external_alias> +</external_aliases> diff --git a/src/cpu_map/sync_qemu_features_i386.py b/src/cpu_map/sync_qemu_features_i386.py index e4b1f7275a..3b3ad5a643 100755 --- a/src/cpu_map/sync_qemu_features_i386.py +++ b/src/cpu_map/sync_qemu_features_i386.py @@ -4,6 +4,11 @@ import argparse import os import re +try: + import pycpuinfo +except ImportError: + pycpuinfo = None + # features in qemu that we do not want in libvirt FEATURES_IGNORE = ( @@ -22,6 +27,7 @@ FEATURES_IGNORE = ( "kvm-steal-time", "kvmclock", "kvmclock-stable-bit", + "kvmclock2", "xstore", "xstore-en", @@ -295,6 +301,53 @@ def add_feature_qemu(query, data): add_feature_cpuid(eax, ecx, reg, bit, name) +def add_features_cpuinfo(): + def decode_bit(value): + for i in range(0, 64): + if value == (1 << i): + return i + + def decode_cpuid(v): + if v[0] != 0 and v[1] == 0 and v[2] == 0 and v[3] == 0: + reg, val = "eax", v[0] + if v[0] == 0 and v[1] != 0 and v[2] == 0 and v[3] == 0: + reg, val = "ebx", v[1] + if v[0] == 0 and v[1] == 0 and v[2] != 0 and v[3] == 0: + reg, val = "ecx", v[2] + if v[0] == 0 and v[1] == 0 and v[2] == 0 and v[3] != 0: + reg, val = "edx", v[3] + + return reg, decode_bit(val) + + x86 = pycpuinfo.Family.find("x86", "") + + for feature in pycpuinfo.features(): + if feature.family() != x86: + continue + + if list(feature.features()): + continue + + name = feature.name("libvirt") + if name in FEATURES_IGNORE: + continue + + cpuid = feature.extra_x86_cpuid() + if cpuid: + eax = cpuid[0] + ecx = cpuid[1] + if ecx == pycpuinfo.x86.CPUINFO_X86_CPUID_ECX_NONE: + ecx = None + reg, bit = decode_cpuid(cpuid[2:]) + add_feature_cpuid(eax, ecx, reg, bit, name) + + msr = feature.extra_x86_msr() + if msr: + index = msr[0] + bit = decode_bit(msr[1] | (msr[2] << 32)) + add_feature_msr(index, bit, name) + + # read the `feature_word_info` struct from qemu's cpu.c into a list of strings def read_cpu_c(path): pattern_comment = re.compile("/\\*.*?\\*/") @@ -450,6 +503,12 @@ def main(): nargs="?", type=os.path.realpath, ) + if pycpuinfo: + parser.add_argument( + "--libcpuinfo", + help="Use libcpuinfo as data source instead", + action="store_true", + ) parser.add_argument( "--output", "-o", @@ -459,14 +518,18 @@ def main(): ) args = parser.parse_args() - if not os.path.isdir(args.qemu): - parser.print_help() - exit("qemu source directory not found") + if pycpuinfo and args.libcpuinfo: + add_features_cpuinfo() + else: + if not os.path.isdir(args.qemu): + parser.print_help() + exit("qemu source directory not found") + + read_headers(args.qemu) + lines = read_cpu_c(args.qemu) + parse_feature_words(lines) + add_extra_features() - read_headers(args.qemu) - lines = read_cpu_c(args.qemu) - parse_feature_words(lines) - add_extra_features() write_output(args.output) print( -- 2.43.0

2 2

[PATCH] formatstorage: Document qcow2 default version change
by Peter Krempa 22 Jul '24

22 Jul '24

Based on discussion after commit f432114d9c was pushed it was pointed out that the documentation still mentions the older version. Fix the documentation to state the new version and introduce ambiguity for future updates. Fixes: f432114d9cf507a4047aa9dc1344b1c13356db08 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- Posting this documentation update to document what happened rather than introduce (almost pointless) complication in adding a config file which is unlikely to be ever used. docs/formatstorage.rst | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/formatstorage.rst b/docs/formatstorage.rst index 86e167d9cb..9d9a4143eb 100644 --- a/docs/formatstorage.rst +++ b/docs/formatstorage.rst @@ -700,10 +700,15 @@ host filesystem. It can contain the following child elements: Encryption <formatstorageencryption.html>`__ page for more information. ``compat`` Specify compatibility level. So far, this is only used for ``type='qcow2'`` - volumes. Valid values are ``0.10`` and ``1.1`` so far, specifying QEMU - version the images should be compatible with. If the ``feature`` element is - present, 1.1 is used. :since:`Since 1.1.0` If omitted, 0.10 is used. - :since:`Since 1.1.2` + volumes. Valid values are ``0.10`` (QCOW2 v2) and ``1.1`` (QCOW2 v3) so far. + The values were meant to specify QEMU version the images should be compatible + with. + + The default, if the ``feature`` element is present is ``1.1``. :since:`Since 1.1.0` + If ``feature`` is not present, ``0.10`` was used :since:`Since 1.1.2` and + :since:`Since 10.2.0` ``1.1`` is used as it's the default of ``qemu-img``. + + Any tool depending on a specific version should specify this field explicitly. ``nocow`` Turn off COW of the newly created volume. So far, this is only valid for a file image in btrfs file system. It will improve performance when the file -- 2.44.0

2 1

Plans for 10.6.0 release (freeze on Monday 29 Jul)
by Jiri Denemark 22 Jul '24

22 Jul '24

We are getting close to 10.6.0 release of libvirt. Since I'll be away from computers next week, I suggest moving the "last week in a month" freeze a bit and aim for the release on Monday 05 Aug after entering the freeze on Monday 29 Jul. There will be no RC2 this time. I hope this works for everyone. Jirka

1 0

[PATCH v2] qemu: add a monitor to /proc/$pid when killing times out
by Boris Fiuczynski 19 Jul '24

19 Jul '24

In cases when a QEMU process takes longer than the time sigterm and sigkill are issued to kill the process do not simply fail and leave the VM in state VIR_DOMAIN_SHUTDOWN until the daemon stops. Instead set up an fd on /proc/$pid and get notified when the QEMU process finally has terminated to cleanup the VM state. Resolves: https://issues.redhat.com/browse/RHEL-28819 Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com> --- src/qemu/qemu_domain.c | 8 +++ src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 18 +++++++ src/qemu/qemu_process.c | 115 ++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_process.h | 1 + 5 files changed, 139 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2134b11038..8147ff02fd 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1889,6 +1889,11 @@ qemuDomainObjPrivateFree(void *data) virChrdevFree(priv->devs); + if (priv->pidMonitored >= 0) { + virEventRemoveHandle(priv->pidMonitored); + priv->pidMonitored = -1; + } + /* This should never be non-NULL if we get here, but just in case... */ if (priv->mon) { VIR_ERROR(_("Unexpected QEMU monitor still active during domain deletion")); @@ -1934,6 +1939,8 @@ qemuDomainObjPrivateAlloc(void *opaque) priv->blockjobs = virHashNew(virObjectUnref); priv->fds = virHashNew(g_object_unref); + priv->pidMonitored = -1; + /* agent commands block by default, user can choose different behavior */ priv->agentTimeout = VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK; priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; @@ -11680,6 +11687,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event) case QEMU_PROCESS_EVENT_RESET: case QEMU_PROCESS_EVENT_NBDKIT_EXITED: case QEMU_PROCESS_EVENT_MONITOR_EOF: + case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED: case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index d777559119..a5092dd7f0 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -119,6 +119,7 @@ struct _qemuDomainObjPrivate { bool beingDestroyed; char *pidfile; + int pidMonitored; virDomainPCIAddressSet *pciaddrs; virDomainUSBAddressSet *usbaddrs; @@ -469,6 +470,7 @@ typedef enum { QEMU_PROCESS_EVENT_UNATTENDED_MIGRATION, QEMU_PROCESS_EVENT_RESET, QEMU_PROCESS_EVENT_NBDKIT_EXITED, + QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED, QEMU_PROCESS_EVENT_LAST } qemuProcessEventType; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9f3013e231..6b1e4084f6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4041,6 +4041,21 @@ processNbdkitExitedEvent(virDomainObj *vm, } +static void +processShutdownCompletedEvent(virQEMUDriver *driver, + virDomainObj *vm) +{ + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + return; + + if (virDomainObjIsActive(vm)) + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_UNKNOWN, + VIR_ASYNC_JOB_NONE, 0); + + virDomainObjEndJob(vm); +} + + static void qemuProcessEventHandler(void *data, void *opaque) { struct qemuProcessEvent *processEvent = data; @@ -4101,6 +4116,9 @@ static void qemuProcessEventHandler(void *data, void *opaque) case QEMU_PROCESS_EVENT_NBDKIT_EXITED: processNbdkitExitedEvent(vm, processEvent->data); break; + case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED: + processShutdownCompletedEvent(driver, vm); + break; case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 25dfd04272..86ca495a20 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -25,6 +25,7 @@ #include <unistd.h> #include <signal.h> #include <sys/stat.h> +#include <sys/syscall.h> #if defined(__linux__) # include <linux/capability.h> #elif defined(__FreeBSD__) @@ -8387,9 +8388,107 @@ qemuProcessCreatePretendCmdBuild(virDomainObj *vm, } +typedef struct { + virDomainObj *vm; + int pidfd; +} qemuProcessInShutdownEventData; + + +static qemuProcessInShutdownEventData* +qemuProcessInShutdownEventDataNew(virDomainObj *vm, int pidfd) +{ + qemuProcessInShutdownEventData *d = g_new(qemuProcessInShutdownEventData, 1); + d->vm = virObjectRef(vm); + d->pidfd = pidfd; + return d; +} + + +static void +qemuProcessInShutdownEventDataFree(qemuProcessInShutdownEventData *d) +{ + virObjectUnref(d->vm); + VIR_FORCE_CLOSE(d->pidfd); + g_free(d); +} + + +static void +qemuProcessInShutdownPidfdCb(int watch, + int fd, + int events G_GNUC_UNUSED, + void *opaque) +{ + qemuProcessInShutdownEventData *data = opaque; + virDomainObj *vm = data->vm; + + VIR_DEBUG("vm=%p name=%s pid=%lld fd=%d", + vm, vm->def->name, (long long)vm->pid, fd); + + virEventRemoveHandle(watch); + + virObjectLock(vm); + + VIR_INFO("QEMU process %lld finally completed termination", + (long long)vm->pid); + + QEMU_DOMAIN_PRIVATE(vm)->pidMonitored = -1; + qemuProcessEventSubmit(vm, QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED, + 0, 0, NULL); + + virObjectUnlock(vm); +} + + +static int +qemuProcessInShutdownStartMonitor(virDomainObj *vm) +{ + qemuDomainObjPrivate *priv = vm->privateData; + qemuProcessInShutdownEventData *data; + int pidfd; + int ret = -1; + + VIR_DEBUG("vm=%p name=%s pid=%lld pidMonitored=%d", + vm, vm->def->name, (long long)vm->pid, + priv->pidMonitored); + + if (priv->pidMonitored >= 0) { + VIR_DEBUG("Monitoring qemu in-shutdown process %i already set up", vm->pid); + goto cleanup; + } + + pidfd = syscall(SYS_pidfd_open, vm->pid, 0); + if (pidfd < 0) { + if (errno == ESRCH) /* process has already terminated */ + ret = 1; + goto cleanup; + } + + data = qemuProcessInShutdownEventDataNew(vm, pidfd); + if ((priv->pidMonitored = virEventAddHandle(pidfd, + VIR_EVENT_HANDLE_READABLE, + qemuProcessInShutdownPidfdCb, + data, + (virFreeCallback)qemuProcessInShutdownEventDataFree)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to monitor qemu in-shutdown process %1$i"), + vm->pid); + qemuProcessInShutdownEventDataFree(data); + goto cleanup; + } + VIR_DEBUG("Monitoring qemu in-shutdown process %i for termination", vm->pid); + ret = 0; + + cleanup: + return ret; +} + + int qemuProcessKill(virDomainObj *vm, unsigned int flags) { + int ret = -1; + VIR_DEBUG("vm=%p name=%s pid=%lld flags=0x%x", vm, vm->def->name, (long long)vm->pid, flags); @@ -8410,10 +8509,16 @@ qemuProcessKill(virDomainObj *vm, unsigned int flags) /* Request an extra delay of two seconds per current nhostdevs * to be safe against stalls by the kernel freeing up the resources */ - return virProcessKillPainfullyDelay(vm->pid, - !!(flags & VIR_QEMU_PROCESS_KILL_FORCE), - vm->def->nhostdevs * 2, - false); + ret = virProcessKillPainfullyDelay(vm->pid, + !!(flags & VIR_QEMU_PROCESS_KILL_FORCE), + vm->def->nhostdevs * 2, + false); + + if (ret < 0 && (flags & VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR)) + if (qemuProcessInShutdownStartMonitor(vm) == 1) + ret = 0; /* process termination detected */ + + return ret; } @@ -8438,7 +8543,7 @@ qemuProcessBeginStopJob(virDomainObj *vm, * cleared inside qemuProcessStop */ priv->beingDestroyed = true; - if (qemuProcessKill(vm, killFlags) < 0) + if (qemuProcessKill(vm, killFlags|VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR) < 0) goto error; /* Wake up anything waiting on domain condition */ diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index cb67bfcd2d..2324aeb7bd 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -180,6 +180,7 @@ typedef enum { VIR_QEMU_PROCESS_KILL_FORCE = 1 << 0, VIR_QEMU_PROCESS_KILL_NOWAIT = 1 << 1, VIR_QEMU_PROCESS_KILL_NOCHECK = 1 << 2, /* bypass the running vm check */ + VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR = 1 << 3, /* on error enable process monitor */ } virQemuProcessKillMode; int qemuProcessKill(virDomainObj *vm, unsigned int flags); -- 2.45.0

2 2

[PATCH] security: AppArmor allow write when os loader readonly=no
by Miroslav Los 19 Jul '24

19 Jul '24

Since libvirt commit 3ef9b51b10e52886e8fe8d75e36d0714957616b7, the pflash storage for the os loader file follows its read-only flag, and qemu tries to open the file for writing if set so. This patches virt-aa-helper to generate the VM's AppArmor rules that allow this, using the same domain definition flag and default. Signed-off-by: Miroslav Los <mirlos(a)cisco.com> --- src/security/virt-aa-helper.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 0374581f07..2f57664a4c 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1001,9 +1001,14 @@ get_files(vahControl * ctl) if (vah_add_file(&buf, ctl->def->os.slic_table, "r") != 0) goto cleanup; - if (ctl->def->os.loader && ctl->def->os.loader->path) - if (vah_add_file(&buf, ctl->def->os.loader->path, "rk") != 0) + if (ctl->def->os.loader && ctl->def->os.loader->path) { + bool readonly = false; + virTristateBoolToBool(ctl->def->os.loader->readonly, &readonly); + if (vah_add_file(&buf, + ctl->def->os.loader->path, + readonly ? "rk" : "rwk") != 0) goto cleanup; + } if (ctl->def->os.loader && ctl->def->os.loader->nvram) { if (storage_source_add_files(ctl->def->os.loader->nvram, &buf, 0) < 0) -- 2.25.1

4 8

[PATCH 0/2] Warn on pthread errors
by Tim Wiederhake 18 Jul '24

18 Jul '24

libvirt currently exhibits undefined behavior due to pthread mutex misuse, e.g. destroying a locked mutex or attempting to unlock an already unlocked mutex. Add a warning if such a case is detected, so we can start on fixing the issues. Tim Wiederhake (2): virMutex: Warn on error DO NOT MERGE: virMutex: Fail loudly src/util/virthread.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) -- 2.43.0

3 8

[PATCH] qemu: add a monitor to /proc/$pid when killing times out
by Boris Fiuczynski 17 Jul '24

17 Jul '24

In cases when a QEMU process takes longer than the time sigterm and sigkill are issued to kill the process do not simply fail and leave the VM in state VIR_DOMAIN_SHUTDOWN until the daemon stops. Instead set up an fd on /proc/$pid and get notified when the QEMU process finally has terminated to cleanup the VM state. Resolves: https://issues.redhat.com/browse/RHEL-28819 Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com> --- src/qemu/qemu_domain.c | 8 +++ src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 18 ++++++ src/qemu/qemu_process.c | 127 ++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_process.h | 1 + 5 files changed, 151 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2134b11038..96f4e41a11 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1889,6 +1889,11 @@ qemuDomainObjPrivateFree(void *data) virChrdevFree(priv->devs); + if (priv->watchPid >= 0) { + virEventRemoveHandle(priv->watchPid); + priv->watchPid = -1; + } + /* This should never be non-NULL if we get here, but just in case... */ if (priv->mon) { VIR_ERROR(_("Unexpected QEMU monitor still active during domain deletion")); @@ -1934,6 +1939,8 @@ qemuDomainObjPrivateAlloc(void *opaque) priv->blockjobs = virHashNew(virObjectUnref); priv->fds = virHashNew(g_object_unref); + priv->watchPid = -1; + /* agent commands block by default, user can choose different behavior */ priv->agentTimeout = VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK; priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; @@ -11680,6 +11687,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event) case QEMU_PROCESS_EVENT_RESET: case QEMU_PROCESS_EVENT_NBDKIT_EXITED: case QEMU_PROCESS_EVENT_MONITOR_EOF: + case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED: case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index d777559119..e5366c6e8c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -119,6 +119,7 @@ struct _qemuDomainObjPrivate { bool beingDestroyed; char *pidfile; + int watchPid; virDomainPCIAddressSet *pciaddrs; virDomainUSBAddressSet *usbaddrs; @@ -469,6 +470,7 @@ typedef enum { QEMU_PROCESS_EVENT_UNATTENDED_MIGRATION, QEMU_PROCESS_EVENT_RESET, QEMU_PROCESS_EVENT_NBDKIT_EXITED, + QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED, QEMU_PROCESS_EVENT_LAST } qemuProcessEventType; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9f3013e231..6b1e4084f6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4041,6 +4041,21 @@ processNbdkitExitedEvent(virDomainObj *vm, } +static void +processShutdownCompletedEvent(virQEMUDriver *driver, + virDomainObj *vm) +{ + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + return; + + if (virDomainObjIsActive(vm)) + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_UNKNOWN, + VIR_ASYNC_JOB_NONE, 0); + + virDomainObjEndJob(vm); +} + + static void qemuProcessEventHandler(void *data, void *opaque) { struct qemuProcessEvent *processEvent = data; @@ -4101,6 +4116,9 @@ static void qemuProcessEventHandler(void *data, void *opaque) case QEMU_PROCESS_EVENT_NBDKIT_EXITED: processNbdkitExitedEvent(vm, processEvent->data); break; + case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED: + processShutdownCompletedEvent(driver, vm); + break; case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 25dfd04272..d6dbd7ba53 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -25,6 +25,7 @@ #include <unistd.h> #include <signal.h> #include <sys/stat.h> +#include <sys/syscall.h> #if defined(__linux__) # include <linux/capability.h> #elif defined(__FreeBSD__) @@ -8387,9 +8388,119 @@ qemuProcessCreatePretendCmdBuild(virDomainObj *vm, } +typedef struct { + virDomainObj *vm; + int pidfd; +} qemuProcessInShutdownEventData; + + +static qemuProcessInShutdownEventData* +qemuProcessInShutdownEventDataNew(virDomainObj *vm, int pidfd) +{ + qemuProcessInShutdownEventData *d = g_new(qemuProcessInShutdownEventData, 1); + d->vm = virObjectRef(vm); + d->pidfd = pidfd; + return d; +} + + +static void +qemuProcessInShutdownEventDataFree(qemuProcessInShutdownEventData *d) +{ + virObjectUnref(d->vm); + VIR_FORCE_CLOSE(d->pidfd); + g_free(d); +} + + +static void +qemuProcessInShutdownStopMonitor(virDomainObj *vm) +{ + qemuDomainObjPrivate *priv = vm->privateData; + + VIR_DEBUG("vm=%p name=%s pid=%lld watchPid=%d", + vm, vm->def->name, (long long)vm->pid, + priv->watchPid); + + virObjectLock(vm); + if (priv->watchPid >= 0) { + virEventRemoveHandle(priv->watchPid); + priv->watchPid = -1; + } + virObjectUnlock(vm); +} + + +static void +qemuProcessInShutdownPidfdCb(int watch G_GNUC_UNUSED, + int fd, + int events G_GNUC_UNUSED, + void *opaque) +{ + qemuProcessInShutdownEventData *data = opaque; + virDomainObj *vm = data->vm; + + VIR_DEBUG("vm=%p name=%s pid=%lld fd=%d", + vm, vm->def->name, (long long)vm->pid, fd); + + VIR_DEBUG("QEMU process %lld finally completed termination", + (long long)vm->pid); + qemuProcessInShutdownStopMonitor(vm); + + qemuProcessEventSubmit(vm, QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED, + 0, 0, NULL); +} + + +static int +qemuProcessInShutdownStartMonitor(virDomainObj *vm) +{ + qemuDomainObjPrivate *priv = vm->privateData; + qemuProcessInShutdownEventData *data; + int pidfd; + int ret = -1; + + VIR_DEBUG("vm=%p name=%s pid=%lld watchPid=%d", + vm, vm->def->name, (long long)vm->pid, + priv->watchPid); + + if (priv->watchPid >= 0) { + VIR_DEBUG("Monitoring qemu in-shutdown process %i already set up", vm->pid); + goto cleanup; + } + + pidfd = syscall(SYS_pidfd_open, vm->pid, 0); + if (pidfd < 0) { + if (errno == ESRCH) /* process has already terminated */ + ret = 1; + goto cleanup; + } + + data = qemuProcessInShutdownEventDataNew(vm, pidfd); + if ((priv->watchPid = virEventAddHandle(pidfd, + VIR_EVENT_HANDLE_READABLE, + qemuProcessInShutdownPidfdCb, + data, + (virFreeCallback)qemuProcessInShutdownEventDataFree)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to monitor qemu in-shutdown process %1$i"), + vm->pid); + qemuProcessInShutdownEventDataFree(data); + goto cleanup; + } + VIR_DEBUG("Monitoring qemu in-shutdown process %i for termination", vm->pid); + ret = 0; + + cleanup: + return ret; +} + + int qemuProcessKill(virDomainObj *vm, unsigned int flags) { + int ret = -1; + VIR_DEBUG("vm=%p name=%s pid=%lld flags=0x%x", vm, vm->def->name, (long long)vm->pid, flags); @@ -8410,10 +8521,16 @@ qemuProcessKill(virDomainObj *vm, unsigned int flags) /* Request an extra delay of two seconds per current nhostdevs * to be safe against stalls by the kernel freeing up the resources */ - return virProcessKillPainfullyDelay(vm->pid, - !!(flags & VIR_QEMU_PROCESS_KILL_FORCE), - vm->def->nhostdevs * 2, - false); + ret = virProcessKillPainfullyDelay(vm->pid, + !!(flags & VIR_QEMU_PROCESS_KILL_FORCE), + vm->def->nhostdevs * 2, + false); + + if (ret < 0 && (flags & VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR)) + if (qemuProcessInShutdownStartMonitor(vm) == 1) + ret = 0; /* process termination detected */ + + return ret; } @@ -8438,7 +8555,7 @@ qemuProcessBeginStopJob(virDomainObj *vm, * cleared inside qemuProcessStop */ priv->beingDestroyed = true; - if (qemuProcessKill(vm, killFlags) < 0) + if (qemuProcessKill(vm, killFlags|VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR) < 0) goto error; /* Wake up anything waiting on domain condition */ diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index cb67bfcd2d..2324aeb7bd 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -180,6 +180,7 @@ typedef enum { VIR_QEMU_PROCESS_KILL_FORCE = 1 << 0, VIR_QEMU_PROCESS_KILL_NOWAIT = 1 << 1, VIR_QEMU_PROCESS_KILL_NOCHECK = 1 << 2, /* bypass the running vm check */ + VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR = 1 << 3, /* on error enable process monitor */ } virQemuProcessKillMode; int qemuProcessKill(virDomainObj *vm, unsigned int flags); -- 2.45.0

1 0

[PATCH] qemu: virtiofs: format --rlimit-nofile
by Adam Julis 17 Jul '24

17 Jul '24

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/485 Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- src/qemu/qemu_virtiofs.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c index 0e3c7dbb58..703f1226a2 100644 --- a/src/qemu/qemu_virtiofs.c +++ b/src/qemu/qemu_virtiofs.c @@ -194,6 +194,9 @@ qemuVirtioFSBuildCommandLine(virQEMUDriverConfig *cfg, if (fs->thread_pool_size >= 0) virCommandAddArgFormat(cmd, "--thread-pool-size=%i", fs->thread_pool_size); + if (fs->rlimit_nofile > 0) + virCommandAddArgFormat(cmd, "--rlimit-nofile=%llu", fs->rlimit_nofile); + if (cfg->virtiofsdDebug) { if (virBitmapIsBitSet(fs->caps, QEMU_VHOST_USER_FS_FEATURE_SEPARATE_OPTIONS)) virCommandAddArgList(cmd, "--log-level", "debug", NULL); -- 2.45.2

2 1

[PATCH] vmx: Be even more lax when trying to comprehend serial ports
by Martin Kletzander 16 Jul '24

16 Jul '24

So much can happen in the fileName field of the VMX that the easiest thing is to silently report a serial type="null". This effectively reverts commits de81bdb8d4cd and 62c53db0421a, but keeps the test files to show the fix is still in place. There is one instance where an error gets reset, but since that is a rare case on its own and on top of that does not happen in any of our long-running daemons with a logfile that might get monitored it should be fine to leave it there. Resolves: https://issues.redhat.com/browse/RHEL-32182 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/vmx/vmx.c | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index e5bc2d793c66..227744d06258 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2975,9 +2975,6 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port, char fileName_name[48] = ""; g_autofree char *fileName = NULL; - char vspc_name[48] = ""; - g_autofree char *vspc = NULL; - char network_endPoint_name[48] = ""; g_autofree char *network_endPoint = NULL; @@ -3000,7 +2997,6 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port, VMX_BUILD_NAME(startConnected); VMX_BUILD_NAME(fileType); VMX_BUILD_NAME(fileName); - VMX_BUILD_NAME(vspc); VMX_BUILD_NAME_EXTRA(network_endPoint, "network.endPoint"); /* vmx:present */ @@ -3030,10 +3026,6 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port, if (virVMXGetConfigString(conf, fileName_name, &fileName, true) < 0) goto cleanup; - /* vmx:fileName -> def:data.file.path */ - if (virVMXGetConfigString(conf, vspc_name, &vspc, true) < 0) - goto cleanup; - /* vmx:network.endPoint -> def:data.tcp.listen */ if (virVMXGetConfigString(conf, network_endPoint_name, &network_endPoint, true) < 0) { @@ -3065,21 +3057,25 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port, (*def)->target.port = port; (*def)->source->type = VIR_DOMAIN_CHR_TYPE_PIPE; (*def)->source->data.file.path = g_steal_pointer(&fileName); - } else if (STRCASEEQ(fileType, "network") && (vspc || !fileName || STREQ(fileName, ""))) { - (*def)->target.port = port; - (*def)->source->type = VIR_DOMAIN_CHR_TYPE_NULL; } else if (STRCASEEQ(fileType, "network")) { (*def)->target.port = port; (*def)->source->type = VIR_DOMAIN_CHR_TYPE_TCP; - if (!(parsedUri = virURIParse(fileName))) - goto cleanup; + if (!(parsedUri = virURIParse(fileName))) { + /* + * Ignore anything we cannot parse since there are many variations + * that could lead to unusable or non-representable serial ports + * which are very commonly seen and the main consumer of this driver + * (virt-v2v) ignores them anyway, so let's at least not error out. + */ + virResetLastError(); + (*def)->source->type = VIR_DOMAIN_CHR_TYPE_NULL; + return 0; + } if (parsedUri->port == 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("VMX entry '%1$s' doesn't contain a port part"), - fileName_name); - goto cleanup; + (*def)->source->type = VIR_DOMAIN_CHR_TYPE_NULL; + return 0; } (*def)->source->data.tcp.host = g_strdup(parsedUri->server); -- 2.45.1

2 1

[PATCH] ci: Refresh generated files
by Michal Privoznik 15 Jul '24

15 Jul '24

This is supposed to unstuck FreeBSD as it switched to Python-3.11. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- ci/cirrus/freebsd-13.vars | 2 +- ci/cirrus/freebsd-14.vars | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/cirrus/freebsd-13.vars b/ci/cirrus/freebsd-13.vars index 278b5834f6..f24b921300 100644 --- a/ci/cirrus/freebsd-13.vars +++ b/ci/cirrus/freebsd-13.vars @@ -11,6 +11,6 @@ MAKE='/usr/local/bin/gmake' NINJA='/usr/local/bin/ninja' PACKAGING_COMMAND='pkg' PIP3='/usr/local/bin/pip-3.8' -PKGS='augeas bash-completion ca_root_nss ccache codespell cppi curl cyrus-sasl diffutils fusefs-libs gettext git glib gmake gnugrep gnutls gsed libpcap libpciaccess libssh libssh2 libxml2 libxslt meson ninja perl5 pkgconf polkit py39-black py39-docutils py39-flake8 py39-pytest python3 qemu readline yajl' +PKGS='augeas bash-completion ca_root_nss ccache codespell cppi curl cyrus-sasl diffutils fusefs-libs gettext git glib gmake gnugrep gnutls gsed libpcap libpciaccess libssh libssh2 libxml2 libxslt meson ninja perl5 pkgconf polkit py311-black py311-docutils py311-flake8 py311-pytest python3 qemu readline yajl' PYPI_PKGS='' PYTHON='/usr/local/bin/python3' diff --git a/ci/cirrus/freebsd-14.vars b/ci/cirrus/freebsd-14.vars index 278b5834f6..f24b921300 100644 --- a/ci/cirrus/freebsd-14.vars +++ b/ci/cirrus/freebsd-14.vars @@ -11,6 +11,6 @@ MAKE='/usr/local/bin/gmake' NINJA='/usr/local/bin/ninja' PACKAGING_COMMAND='pkg' PIP3='/usr/local/bin/pip-3.8' -PKGS='augeas bash-completion ca_root_nss ccache codespell cppi curl cyrus-sasl diffutils fusefs-libs gettext git glib gmake gnugrep gnutls gsed libpcap libpciaccess libssh libssh2 libxml2 libxslt meson ninja perl5 pkgconf polkit py39-black py39-docutils py39-flake8 py39-pytest python3 qemu readline yajl' +PKGS='augeas bash-completion ca_root_nss ccache codespell cppi curl cyrus-sasl diffutils fusefs-libs gettext git glib gmake gnugrep gnutls gsed libpcap libpciaccess libssh libssh2 libxml2 libxslt meson ninja perl5 pkgconf polkit py311-black py311-docutils py311-flake8 py311-pytest python3 qemu readline yajl' PYPI_PKGS='' PYTHON='/usr/local/bin/python3' -- 2.44.2

2 1

[PATCH 0/2] qemu: Add support for pauth Arm CPU feature
by Andrea Bolognani 15 Jul '24

15 Jul '24

Andrea Bolognani (2): cpu_map: Add pauth Arm CPU feature tests: Add coverage for pauth Arm CPU feature src/cpu_map/arm_features.xml | 3 ++ ...aarch64-features-pauth.aarch64-latest.args | 31 +++++++++++++++++++ .../aarch64-features-pauth.aarch64-latest.xml | 28 +++++++++++++++++ .../aarch64-features-pauth.xml | 17 ++++++++++ tests/qemuxmlconftest.c | 1 + 5 files changed, 80 insertions(+) create mode 100644 tests/qemuxmlconfdata/aarch64-features-pauth.aarch64-latest.args create mode 100644 tests/qemuxmlconfdata/aarch64-features-pauth.aarch64-latest.xml create mode 100644 tests/qemuxmlconfdata/aarch64-features-pauth.xml -- 2.45.2

2 7

[PATCH] docs: use real examples for QEMU cli passthrough
by Daniel P. Berrangé 15 Jul '24

15 Jul '24

User feedback has shown that the examples are not clear enough to illustrate the cli passthrough concept in action. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- docs/drvqemu.rst | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/drvqemu.rst b/docs/drvqemu.rst index 1ee4b1e366..e52d9ffd47 100644 --- a/docs/drvqemu.rst +++ b/docs/drvqemu.rst @@ -595,8 +595,11 @@ Example: <emulator>/usr/bin/qemu-system-x86_64</emulator> </devices> <qemu:commandline> - <qemu:arg value='-newarg'/> - <qemu:env name='QEMU_ENV' value='VAL'/> + <qemu:arg value='-device'/> + <qemu:arg value='ipmi-bmc-sim,id=bmc0'/> + <qemu:arg value='-device'/> + <qemu:arg value='smbus-impi,bmc=bmc0'/> + <qemu:env name='QEMU_MODULE_DIR' value='/usr/lib64/qemu'/> </qemu:commandline> </domain> -- 2.45.1

2 1

[PATCH v3 00/17] hw/sd/sdcard: Accumulation of cleanups and fixes
by Philippe Mathieu-Daudé 11 Jul '24

11 Jul '24

Since v2: - Tested-by from Cédric recorded - more patches added :S Since v1: - various patches merged, few more added Various SD card cleanups and fixes accumulated over the years. Various have been useful to help integrating eMMC support (which will come later). Full series for testing: https://gitlab.com/philmd/qemu/-/tags/emmc-v4 Cédric Le Goater (1): hw/sd/sdcard: Introduce definitions for EXT_CSD register Philippe Mathieu-Daudé (16): hw/sd/sdcard: Deprecate support for spec v1.10 hw/sd/sdcard: Use spec v3.01 by default hw/sd/sdcard: Track last command used to help logging hw/sd/sdcard: Trace block offset in READ/WRITE data accesses hw/sd/sdcard: Trace requested address computed by sd_req_get_address() hw/sd/sdcard: Do not store vendor data on block drive (CMD56) hw/sd/sdcard: Send WRITE_PROT bits MSB first (CMD30) hw/sd/sdcard: Send NUM_WR_BLOCKS bits MSB first (ACMD22) hw/sd/sdcard: Use READY_FOR_DATA definition instead of magic value hw/sd/sdcard: Assign SDCardStates enum values hw/sd/sdcard: Simplify sd_inactive_state handling hw/sd/sdcard: Restrict SWITCH_FUNCTION to sd_transfer_state (CMD6) hw/sd/sdcard: Add direct reference to SDProto in SDState hw/sd/sdcard: Extract sd_blk_len() helper tests/qtest: Disable npcm7xx_sdhci tests using hardcoded RCA hw/sd/sdcard: Generate random RCA value docs/about/deprecated.rst | 6 ++ hw/sd/sdmmc-internal.h | 97 +++++++++++++++++++++ hw/sd/sd.c | 145 ++++++++++++++++++------------- tests/qtest/npcm7xx_sdhci-test.c | 7 ++ hw/sd/trace-events | 6 +- 5 files changed, 199 insertions(+), 62 deletions(-) -- 2.41.0

4 34

[PATCH] qemu: Don't leave beingDestroyed=true on inactive domain
by Jiri Denemark 11 Jul '24

11 Jul '24

Recent commit v10.4.0-87-gd9935a5c4f made a reasonable change to only reset beingDestroyed back to false when vm->def->id is reset to make sure other code can detect a domain is (about to become) inactive. It even added a comment saying any caller of qemuProcessBeginStopJob is supposed to call qemuProcessStop to clear beingDestroyed. But not every caller really does so because they first call qemuProcessBeginStopJob and then check whether a domain is still running. If not the qemuProcessStop call is skipped leaving beingDestroyed=true. In case of a persistent domain this may block incoming migrations of such domain as the migration code would think the domain died unexpectedly (even though it's still running). The qemuProcessBeginStopJob function is a wrapper around virDomainObjBeginJob, but virDomainObjEndJob was used directly for cleanup. This patch introduces a new qemuProcessEndStopJob wrapper around virDomainObjEndJob to properly undo everything qemuProcessBeginStopJob did. https://issues.redhat.com/browse/RHEL-43309 Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_process.c | 20 ++++++++++++++++---- src/qemu/qemu_process.h | 1 + 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f898d85667..9f3013e231 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2102,7 +2102,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, endjob: if (ret == 0) qemuDomainRemoveInactive(driver, vm, 0, false); - virDomainObjEndJob(vm); + qemuProcessEndStopJob(vm); cleanup: virDomainObjEndAPI(&vm); @@ -3888,7 +3888,7 @@ processMonitorEOFEvent(virQEMUDriver *driver, endjob: qemuDomainRemoveInactive(driver, vm, 0, false); - virDomainObjEndJob(vm); + qemuProcessEndStopJob(vm); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7cbe521a6e..25dfd04272 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8421,7 +8421,8 @@ qemuProcessKill(virDomainObj *vm, unsigned int flags) * qemuProcessBeginStopJob: * * Stop all current jobs by killing the domain and start a new one for - * qemuProcessStop. + * qemuProcessStop. The caller has to make sure qemuProcessEndStopJob is + * called to properly cleanup the job. */ int qemuProcessBeginStopJob(virDomainObj *vm, @@ -8448,8 +8449,9 @@ qemuProcessBeginStopJob(virDomainObj *vm, goto error; /* priv->beingDestroyed is deliberately left set to 'true' here. Caller - * is supposed to call qemuProcessStop, which will reset it after - * 'vm->def->id' is set to -1 */ + * is supposed to call qemuProcessStop (which will reset it after + * 'vm->def->id' is set to -1) and/or qemuProcessEndStopJob to do proper + * cleanup. */ return 0; error: @@ -8458,6 +8460,16 @@ qemuProcessBeginStopJob(virDomainObj *vm, } +void +qemuProcessEndStopJob(virDomainObj *vm) +{ + if (!virDomainObjIsActive(vm)) + QEMU_DOMAIN_PRIVATE(vm)->beingDestroyed = false; + + virDomainObjEndJob(vm); +} + + void qemuProcessStop(virQEMUDriver *driver, virDomainObj *vm, virDomainShutoffReason reason, @@ -8800,7 +8812,7 @@ qemuProcessAutoDestroy(virDomainObj *dom, qemuDomainRemoveInactive(driver, dom, 0, false); - virDomainObjEndJob(dom); + qemuProcessEndStopJob(dom); virObjectEventStateQueue(driver->domainEventState, event); } diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index c1ea949215..cb67bfcd2d 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -169,6 +169,7 @@ typedef enum { int qemuProcessBeginStopJob(virDomainObj *vm, virDomainJob job, bool forceKill); +void qemuProcessEndStopJob(virDomainObj *vm); void qemuProcessStop(virQEMUDriver *driver, virDomainObj *vm, virDomainShutoffReason reason, -- 2.45.2

2 1

[PATCH] virt-host-validate: Rework calling of driver validation
by Michal Privoznik 11 Jul '24

11 Jul '24

It all started with me looking at the --help output which also printed "bhyve" as supported hypervisor type. Well, it's not on my Linux machine. To resolve this, I'm just creating a static array of { "$driver", callback() } pairs and iterating over it. The array is then initialized at compile time with supported drivers. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tools/virt-host-validate.c | 80 +++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 35 deletions(-) diff --git a/tools/virt-host-validate.c b/tools/virt-host-validate.c index 426648a5d3..365b8acd92 100644 --- a/tools/virt-host-validate.c +++ b/tools/virt-host-validate.c @@ -29,6 +29,7 @@ #include "internal.h" #include "virerror.h" #include "virgettext.h" +#include "virglibutil.h" #include "virt-host-validate-common.h" #if WITH_QEMU @@ -44,26 +45,58 @@ # include "virt-host-validate-ch.h" #endif +typedef struct _virValidateCallbacks virValidateCallbacks; +struct _virValidateCallbacks { + const char *name; + int (*callback)(void); +}; + +static virValidateCallbacks validateCallbacks[] = { +#if WITH_QEMU + { "qemu", virHostValidateQEMU }, +#endif +#if WITH_LXC + { "lxc", virHostValidateLXC }, +#endif +#if WITH_BHYVE + { "bhyve", virHostValidateBhyve }, +#endif +#if WITH_CH + { "ch", virHostValidateCh }, +#endif +}; + static void show_help(FILE *out, const char *argv0) { + g_autofree char *hvs = NULL; + char *hvs_list[G_N_ELEMENTS(validateCallbacks) + 1] = { }; + size_t i; + + for (i = 0; i < G_N_ELEMENTS(validateCallbacks); i++) { + hvs_list[i] = g_strdup_printf(" - %1$s", validateCallbacks[i].name); + } + + hvs = g_strjoinv("\n", hvs_list); + + for (i = 0; i < G_N_ELEMENTS(validateCallbacks); i++) { + g_free(hvs_list[i]); + } + fprintf(out, _("\n" "syntax: %1$s [OPTIONS] [HVTYPE]\n" "\n" " Hypervisor types:\n" "\n" - " - qemu\n" - " - lxc\n" - " - bhyve\n" - " - ch\n" + "%2$s\n" "\n" " Options:\n" " -h, --help Display command line help\n" " -v, --version Display command version\n" " -q, --quiet Don't display progress information\n" "\n"), - argv0); + argv0, hvs); } static void @@ -87,6 +120,7 @@ main(int argc, char **argv) int ret = EXIT_SUCCESS; bool quiet = false; bool usedHvname = false; + size_t i; if (virGettextInitialize() < 0 || virErrorInitialize() < 0) { @@ -126,37 +160,13 @@ main(int argc, char **argv) virValidateSetQuiet(quiet); -#if WITH_QEMU - if (!hvname || STREQ(hvname, "qemu")) { - usedHvname = true; - if (virHostValidateQEMU() < 0) - ret = EXIT_FAILURE; + for (i = 0; i < G_N_ELEMENTS(validateCallbacks); i++) { + if (!hvname || STREQ(hvname, validateCallbacks[i].name)) { + usedHvname = true; + if (validateCallbacks[i].callback() < 0) + ret = EXIT_FAILURE; + } } -#endif - -#if WITH_LXC - if (!hvname || STREQ(hvname, "lxc")) { - usedHvname = true; - if (virHostValidateLXC() < 0) - ret = EXIT_FAILURE; - } -#endif - -#if WITH_BHYVE - if (!hvname || STREQ(hvname, "bhyve")) { - usedHvname = true; - if (virHostValidateBhyve() < 0) - ret = EXIT_FAILURE; - } -#endif - -#if WITH_CH - if (!hvname || STREQ(hvname, "ch")) { - usedHvname = true; - if (virHostValidateCh() < 0) - ret = EXIT_FAILURE; - } -#endif if (hvname && !usedHvname) { fprintf(stderr, _("%1$s: unsupported hypervisor name %2$s\n"), -- 2.44.2

2 1

[PATCH] virt-host-validate: Drop extra "PASS"
by Michal Privoznik 11 Jul '24

11 Jul '24

If virt-host-validate is ran on a SEV-SNP capable machine, an extra "PASS" is printed out. This is because virHostValidateAMDSev() prints "PASS" and then returns 1 (indicating success) which in turn makes the caller (virHostValidateSecureGuests()) print "PASS" again. Just drop the extra printing in the caller and let virHostValidateAMDSev() do all the printing. Fixes: 1a8f646f291775d2423ce4e4df62ad69f06ab827 Resolves: https://issues.redhat.com/browse/RHEL-46868 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tools/virt-host-validate-common.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index a29a5b6d5f..86db4815c3 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -488,11 +488,7 @@ int virHostValidateSecureGuests(const char *hvname, return VIR_VALIDATE_FAILURE(level); } } else if (hasAMDSev) { - int rc = virHostValidateAMDSev(hvname, level); - - if (rc > 0) - virValidatePass(); - return rc; + return virHostValidateAMDSev(hvname, level); } virValidateFail(level, -- 2.44.2

2 1

[PATCH v1 0/1] Add support for Vfio Stats in Libvirt
by Kshitij Jha 10 Jul '24

10 Jul '24

Hi, This is a solution to add support for data transferred by Vfio devices during the Live Migration in Libvirt. Currently libvirt only supports a few essential stats like memory transferred, dirty rate and number of iteration etc. returned as part of virDomainGetJobStats for each iteration. However it does not return stats for Vfio devices like data transferred by the device during the migration With this change virDomainGetJobStats will be able to support Vfio stats. Regards Kshitij Jha Kshitij Jha (1): Include support for Vfio stats during Migration include/libvirt/libvirt-domain.h | 9 +++++++++ src/qemu/qemu_domainjob.c | 6 ++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 12 ++++++++++++ 4 files changed, 28 insertions(+) -- 2.22.3

2 2

[PATCH] network: allow "modify" option for DNS-Txt records
by Adam Julis 10 Jul '24

10 Jul '24

The "modify" command allows to replace an existing record (its text value). The primary key is the name of the record. If duplicity or missing record detected, throw error. Tests in networkxml2xmlupdatetest.c contain replacements of an existing DNS-text record and failure due to non-existing record. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/639 Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- src/conf/network_conf.c | 18 ++++++++----- .../dns-txt-record-modify-fail.xml | 1 + .../dns-txt-record-modify-success.xml | 1 + .../nat-network-dns-txt-modify-ok.xml | 26 +++++++++++++++++++ tests/networkxml2xmlupdatetest.c | 9 +++++++ 5 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 tests/networkxml2xmlupdatein/dns-txt-record-modify-fail.xml create mode 100644 tests/networkxml2xmlupdatein/dns-txt-record-modify-success.xml create mode 100644 tests/networkxml2xmlupdateout/nat-network-dns-txt-modify-ok.xml diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index fc387f9566..dd362b6ab2 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3351,12 +3351,6 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def, bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST || command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST); - if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("DNS TXT records cannot be modified, only added or deleted")); - goto cleanup; - } - if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "txt") < 0) goto cleanup; @@ -3395,6 +3389,18 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def, virNetworkDNSTxtDefClear(&dns->txts[foundIdx]); VIR_DELETE_ELEMENT(dns->txts, foundIdx, dns->ntxts); + } else if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { + + if (foundIdx == dns->ntxts) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("couldn't locate a matching DNS TXT record in network %1$s"), + def->name); + goto cleanup; + } + + VIR_FREE(dns->txts[foundIdx].value); + dns->txts[foundIdx].value = g_strdup(txt.value); + } else { virNetworkDefUpdateUnknownCommand(command); goto cleanup; diff --git a/tests/networkxml2xmlupdatein/dns-txt-record-modify-fail.xml b/tests/networkxml2xmlupdatein/dns-txt-record-modify-fail.xml new file mode 100644 index 0000000000..75ed475fe1 --- /dev/null +++ b/tests/networkxml2xmlupdatein/dns-txt-record-modify-fail.xml @@ -0,0 +1 @@ +<txt name='notexisted' value='modified example'/> diff --git a/tests/networkxml2xmlupdatein/dns-txt-record-modify-success.xml b/tests/networkxml2xmlupdatein/dns-txt-record-modify-success.xml new file mode 100644 index 0000000000..e16c352253 --- /dev/null +++ b/tests/networkxml2xmlupdatein/dns-txt-record-modify-success.xml @@ -0,0 +1 @@ +<txt name='example' value='modified example'/> diff --git a/tests/networkxml2xmlupdateout/nat-network-dns-txt-modify-ok.xml b/tests/networkxml2xmlupdateout/nat-network-dns-txt-modify-ok.xml new file mode 100644 index 0000000000..4b4dda094a --- /dev/null +++ b/tests/networkxml2xmlupdateout/nat-network-dns-txt-modify-ok.xml @@ -0,0 +1,26 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward dev='eth1' mode='nat'> + <interface dev='eth1'/> + </forward> + <bridge name='virbr0' stp='on' delay='0'/> + <dns> + <txt name='example' value='modified example'/> + </dns> + <ip address='192.168.122.1' netmask='255.255.255.0'> + <dhcp> + <range start='192.168.122.2' end='192.168.122.254'/> + <host mac='00:16:3e:77:e2:ed' name='a.example.com' ip='192.168.122.10'/> + <host mac='00:16:3e:3e:a9:1a' name='b.example.com' ip='192.168.122.11'/> + </dhcp> + </ip> + <ip family='ipv4' address='192.168.123.1' netmask='255.255.255.0'> + </ip> + <ip family='ipv6' address='2001:db8:ac10:fe01::1' prefix='64'> + </ip> + <ip family='ipv6' address='2001:db8:ac10:fd01::1' prefix='64'> + </ip> + <ip family='ipv4' address='10.24.10.1'> + </ip> +</network> diff --git a/tests/networkxml2xmlupdatetest.c b/tests/networkxml2xmlupdatetest.c index 59e6ce98e5..875cede035 100644 --- a/tests/networkxml2xmlupdatetest.c +++ b/tests/networkxml2xmlupdatetest.c @@ -306,6 +306,15 @@ mymain(void) "dns-txt-record-snowman", "nat-network-dns-txt-record", VIR_NETWORK_UPDATE_COMMAND_DELETE); + DO_TEST("modify-dns-txt-record", + "dns-txt-record-modify-success", + "nat-network-dns-txt-record", + "nat-network-dns-txt-modify-ok", + VIR_NETWORK_UPDATE_COMMAND_MODIFY); + DO_TEST_FAIL("modify-missing-dns-txt-record", + "dns-txt-record-modify-fail", + "nat-network-dns-txt-record", + VIR_NETWORK_UPDATE_COMMAND_MODIFY); section = VIR_NETWORK_SECTION_DNS_SRV; -- 2.45.2

2 1

[PATCH] network: allow "modify" option for DNS hostname
by Adam Julis 10 Jul '24

10 Jul '24

The "modify" command allows you to replace an existing record (its hostname, sub-elements). IP address acts as the primary key. If it is not found, the attempt ends with an error message. If the XML contains a duplicate address, it will select the last one. Tests in networkxml2xmlupdatetest.c contain replacements of an existing DNS-Host record and failure due to non-existing record. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/639 Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- src/conf/network_conf.c | 28 ++++++++++++++----- .../dns-host-modify-not-existing.xml | 4 +++ .../dns-host-modify.xml | 5 ++++ .../nat-network-dns-hosts-modified.xml | 28 +++++++++++++++++++ tests/networkxml2xmlupdatetest.c | 9 ++++++ 5 files changed, 67 insertions(+), 7 deletions(-) create mode 100644 tests/networkxml2xmlupdatein/dns-host-modify-not-existing.xml create mode 100644 tests/networkxml2xmlupdatein/dns-host-modify.xml create mode 100644 tests/networkxml2xmlupdateout/nat-network-dns-hosts-modified.xml diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index f5ccf4bd12..2a541cd5b0 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3138,19 +3138,13 @@ virNetworkDefUpdateDNSHost(virNetworkDef *def, unsigned int fflags G_GNUC_UNUSED) { size_t i, j, k; - int foundIdx = -1, ret = -1; + int foundIdx = -1, ret = -1, foundIdxModify = -1; virNetworkDNSDef *dns = &def->dns; virNetworkDNSHostDef host = { 0 }; bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST || command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST); int foundCt = 0; - if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("DNS HOST records cannot be modified, only added or deleted")); - goto cleanup; - } - if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "host") < 0) goto cleanup; @@ -3163,6 +3157,12 @@ virNetworkDefUpdateDNSHost(virNetworkDef *def, if (virSocketAddrEqual(&host.ip, &dns->hosts[i].ip)) foundThisTime = true; + /* modify option required index of matching ip-address, the loop under + * this comment could affect results of found index foundThisTime, + * so the foundIdxModify is there used instead */ + if (foundThisTime) + foundIdxModify = i; + /* when adding we want to only check duplicates of address since having * multiple addresses with the same hostname is a legitimate configuration */ if (!isAdd) { @@ -3213,6 +3213,20 @@ virNetworkDefUpdateDNSHost(virNetworkDef *def, virNetworkDNSHostDefClear(&dns->hosts[foundIdx]); VIR_DELETE_ELEMENT(dns->hosts, foundIdx, dns->nhosts); + } else if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { + + if (foundCt == 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("couldn't locate a matching DNS HOST record in network %1$s"), + def->name); + goto cleanup; + } + + virNetworkDNSHostDefClear(&dns->hosts[foundIdxModify]); + + memcpy(&dns->hosts[foundIdxModify], &host, sizeof(virNetworkDNSHostDef)); + memset(&host, 0, sizeof(virNetworkDNSHostDef)); + } else { virNetworkDefUpdateUnknownCommand(command); goto cleanup; diff --git a/tests/networkxml2xmlupdatein/dns-host-modify-not-existing.xml b/tests/networkxml2xmlupdatein/dns-host-modify-not-existing.xml new file mode 100644 index 0000000000..357fccd110 --- /dev/null +++ b/tests/networkxml2xmlupdatein/dns-host-modify-not-existing.xml @@ -0,0 +1,4 @@ +<host ip='192.168.122.333'> + <hostname>shared</hostname> + <hostname>names</hostname> +</host> diff --git a/tests/networkxml2xmlupdatein/dns-host-modify.xml b/tests/networkxml2xmlupdatein/dns-host-modify.xml new file mode 100644 index 0000000000..78b9fd88a6 --- /dev/null +++ b/tests/networkxml2xmlupdatein/dns-host-modify.xml @@ -0,0 +1,5 @@ +<host ip='192.168.122.2'> + <hostname>Another</hostname> + <hostname>decent</hostname> + <hostname>names</hostname> +</host> diff --git a/tests/networkxml2xmlupdateout/nat-network-dns-hosts-modified.xml b/tests/networkxml2xmlupdateout/nat-network-dns-hosts-modified.xml new file mode 100644 index 0000000000..8fcaad15d1 --- /dev/null +++ b/tests/networkxml2xmlupdateout/nat-network-dns-hosts-modified.xml @@ -0,0 +1,28 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid> + <forward dev='eth0' mode='nat'> + <interface dev='eth0'/> + </forward> + <bridge name='virbr0' stp='on' delay='0'/> + <dns forwardPlainNames='no'> + <host ip='192.168.122.122'> + <hostname>pudding</hostname> + </host> + <host ip='192.168.122.1'> + <hostname>host</hostname> + <hostname>gateway</hostname> + </host> + <host ip='192.168.122.2'> + <hostname>Another</hostname> + <hostname>decent</hostname> + <hostname>names</hostname> + </host> + <host ip='fd8f:1391:3a82:150::c0a8:9603'> + <hostname>shared</hostname> + <hostname>names</hostname> + </host> + </dns> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxml2xmlupdatetest.c b/tests/networkxml2xmlupdatetest.c index afe2b1f574..383cbf85ce 100644 --- a/tests/networkxml2xmlupdatetest.c +++ b/tests/networkxml2xmlupdatetest.c @@ -276,6 +276,15 @@ mymain(void) "nat-network-dns-hosts", "nat-network-no-hosts", VIR_NETWORK_UPDATE_COMMAND_DELETE); + DO_TEST("modify-dns-host", + "dns-host-modify", + "nat-network-dns-hosts", + "nat-network-dns-hosts-modified", + VIR_NETWORK_UPDATE_COMMAND_MODIFY); + DO_TEST_FAIL("modify-dns-host-not-existing", + "dns-host-modify-not-existing", + "nat-network-dns-hosts", + VIR_NETWORK_UPDATE_COMMAND_MODIFY); section = VIR_NETWORK_SECTION_DNS_TXT; -- 2.45.2

2 1

[PATCH] domain_conf: comment not match the code below
by Adam Julis 10 Jul '24

10 Jul '24

The outdated comment refers to a non-existent member in the virDomainObj structure. Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7033b4e9fe..115862bbde 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4449,7 +4449,7 @@ virDomainObjGetDefs(virDomainObj *vm, * @live: set to true if live config was returned (may be omitted) * * Helper function to resolve @flags and return the correct domain pointer - * object. This function returns one of @vm->def or @vm->persistentDef + * object. This function returns one of @vm->def or @vm->newDef * according to @flags. @live is set to true if the live vm config will be * returned. This helper should be used only in APIs that guarantee * that @flags contains exactly one of VIR_DOMAIN_AFFECT_LIVE or @@ -4489,7 +4489,7 @@ virDomainObjGetOneDefState(virDomainObj *vm, * @flags: for virDomainModificationImpact * * Helper function to resolve @flags and return the correct domain pointer - * object. This function returns one of @vm->def or @vm->persistentDef + * object. This function returns one of @vm->def or @vm->newDef * according to @flags. This helper should be used only in APIs that guarantee * that @flags contains exactly one of VIR_DOMAIN_AFFECT_LIVE or * VIR_DOMAIN_AFFECT_CONFIG and not both. -- 2.45.2

2 1

[PATCH] virt-aa-helper: Drop needless comments
by Michal Privoznik 09 Jul '24

09 Jul '24

When generating paths for a domain specific AppArmor profile each path undergoes a validation where it's matched against an array of well known prefixes (among other things). Now, for OVMF/AAVMF/... images we have a list and some entries have comments to which type of image the entry belongs to. For instance: "/usr/share/OVMF/", /* for OVMF images */ "/usr/share/AAVMF/", /* for AAVMF images */ But these comments are pretty useless. The path itself already gives away the image type. Drop them. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/security/virt-aa-helper.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index a3f85d26b0..c1e89dc6cf 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -475,15 +475,15 @@ valid_path(const char *path, const bool readonly) "/initrd", "/initrd.img", "/usr/share/edk2/", - "/usr/share/edk2-ovmf/", /* for OVMF images */ - "/usr/share/OVMF/", /* for OVMF images */ - "/usr/share/ovmf/", /* for OVMF images */ - "/usr/share/AAVMF/", /* for AAVMF images */ + "/usr/share/edk2-ovmf/", + "/usr/share/OVMF/", + "/usr/share/ovmf/", + "/usr/share/AAVMF/", "/usr/share/qemu-efi/", /* for AAVMF images */ - "/usr/share/qemu-efi-aarch64/", /* for AAVMF images */ + "/usr/share/qemu-efi-aarch64/", "/usr/share/qemu/", /* SUSE path for OVMF and AAVMF images */ - "/usr/lib/u-boot/", /* u-boot loaders for qemu */ - "/usr/lib/riscv64-linux-gnu/opensbi" /* RISC-V SBI implementation */ + "/usr/lib/u-boot/", + "/usr/lib/riscv64-linux-gnu/opensbi", }; /* override the above with these */ const char * const override[] = { -- 2.44.2

2 1

[libvirt PATCHv2 0/5] qemu: do not use deprecated options for new virtiofsd (virtiofs epopee)
by Ján Tomko 09 Jul '24

09 Jul '24

Also, allow migration if we think we can. Ján Tomko (5): tests: vhostuser: add virtiofsd json descriptor tests: qemuxmlconf: adjust test case to new virtiofsd qemu: fill capabilities for virtiofsd qemu: do not use deprecated options for new virtiofsd qemu: migration: allow migration for virtiofs src/conf/domain_conf.h | 1 + src/qemu/qemu_migration.c | 9 +- src/qemu/qemu_vhost_user.c | 60 ++++++++++++ src/qemu/qemu_vhost_user.h | 12 +++ src/qemu/qemu_virtiofs.c | 92 +++++++++++++------ .../libexec/qemu/vhost-user/test-virtiofsd | 10 ++ .../qemu/vhost-user/50-qemu-virtiofsd.json | 5 + tests/qemuvhostusertest.c | 1 + .../vhost-user-fs-fd-memory.xml | 3 +- 9 files changed, 161 insertions(+), 32 deletions(-) create mode 100755 tests/qemuvhostuserdata/usr/libexec/qemu/vhost-user/test-virtiofsd create mode 100644 tests/qemuvhostuserdata/usr/share/qemu/vhost-user/50-qemu-virtiofsd.json -- 2.45.2

2 7

[PATCH v3 0/2] security_manager: Fix security manager deadlock after libvirtd fork()
by hongmianquan 09 Jul '24

09 Jul '24

We have the stack security driver, which internally manages other security drivers, just call them "top" and "nested". We call virSecurityStackPreFork() to lock the top one, and it also locks and then unlocks the nested drivers prior to fork. Then in qemuSecurityPostFork(), it unlocks the top one, but not the nested ones. Thus, if one of the nested drivers ("dac" or "selinux") is still locked, it will cause a deadlock. If we always surround nested locks with top lock, it is always secure. Because we have got top lock before fork child libvirtd. However, it is not always the case in the current code, We discovered this case: the nested list obtained through the qemuSecurityGetNested() will be locked directly for subsequent use, such as in virQEMUDriverCreateCapabilities(), where the nested list is locked using qemuSecurityGetDOI, but the top one is not locked beforehand. The problem stack is as follows: libvirtd thread1 libvirtd thread2 child libvirtd | | | | | | virsh capabilities qemuProcessLanuch | | | | | lock top | | | | lock nested | | | | | | fork------------------->|(nested lock held by thread1) | | | | | | unlock nested unlock top unlock top | | qemuSecuritySetSocketLabel | | lock nested (deadlock) v3 changes: Made modifications based on Michal's comments - ensured matching qemuSecurityStackLock() and qemuSecurityStackUnlock() - modify the correct order in libvirt_private.syms - split the code streamlining part into a separate patch hongmianquan (2): security_manager: Ensure top lock is acquired before nested locks security_manager: Remove redundant qemuSecurityGetNested() call src/libvirt_private.syms | 2 ++ src/qemu/qemu_conf.c | 13 +++++++++++-- src/qemu/qemu_driver.c | 21 +++++++++++++-------- src/qemu/qemu_security.h | 2 ++ src/security/security_manager.c | 22 ++++++++++++++++++++++ src/security/security_manager.h | 2 ++ 6 files changed, 52 insertions(+), 10 deletions(-) -- 2.20.1

3 5

[PATCH 0/1] qemuDomainChangeNet: check virtio options for non-virtio models
by Miroslav Los 09 Jul '24

09 Jul '24

Hi, in our project, we create all of our domain interfaces regardless of model with a <driver><host csum='off'/></driver>. The documentation is not clear as to which models support offloading or not, and the domain validates. In libvirtd, since <driver> is present, this causes each network device to have its virDomainVirtioOptions *virtio to be non-NULL, with all members ABSENT, which is also deemed valid. However, clients receive the interface elements without the driver. When the modified value is sent back via updateDeviceFlags, the candidate's virtio attribute is thus NULL. The validation in qemuDomainChangeNet requires both structs to be equal or both NULL, which is violated by such a request. This can be seen also with how virsh domif-setlink operates: virsh # domiflist 5 Interface Type Source Model MAC -------------------------------------------------------- - udp - e1000 52:54:00:1c:10:42 - udp - e1000 52:54:00:09:29:9c - udp - e1000 52:54:00:1d:a6:8d - udp - e1000 52:54:00:07:4a:83 52:54:00:1c:10:42 up virsh # domif-setlink 5 52:54:00:1c:10:42 down error: Failed to update interface link state error: Operation not supported: cannot modify virtio network device driver options A workaround is to add an empty <driver> in the update request, which works until libvirt is restarted. The driver element on a non-virtio interface is not present when the domain is reloaded, thus the pointer is NULL in olddev, causing the inverse issue and same error. Our current workaround is to retry with the empty driver element only if the first call fails, but we think libvirt should handle this struct more consistently. This patch expands the check to the non-virtio case, where the olddev is assumed to be valid, and newdev's virtio options are validated to be NULL or all-ABSENT. Since the driver element is normally preserved for virtio models, the stricter equality condition is left unchanged in that case. I did not investigate further if the discrepancy between NULL and all-ABSENT values for virDomainVirtioOptions can cause a problem elsewhere, or in other device kinds. Though I do wonder if the options handling should be improved. One possibility is that non-virtio devices keep the value NULL, e.g. after validating that the driver element does not contain any of the options, whereas virtio devices always create the struct even with no driver element present on input. Another option is that NULL pointer is treated the same as all-ABSENT struct, e.g. in virDomainCheckVirtioOptionsAreAbsent and virDomainVirtioOptionsCheckABIStability; the former does that while the latter does not. These functions would also need to become callable from the qemu driver. Miroslav Los (1): qemuDomainChangeNet: check virtio options for non-virtio models src/qemu/qemu_hotplug.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) -- 2.25.1

2 2

[PATCH] virt-aa-helper: Allow RO access to /usr/share/edk2-ovmf
by Michal Privoznik 09 Jul '24

09 Jul '24

When binary version of edk2 is distributed, the files reside under /usr/share/edk2-ovmf as can be seen from Gentoo's ebuild [1]. Allow virt-aa-helper to generate paths under that dir. 1: https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-firmware/edk2-ovmf-bin/e… Resolves: https://bugs.gentoo.org/911786 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/security/virt-aa-helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 402cbd9602..076b98a1d7 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -475,6 +475,7 @@ valid_path(const char *path, const bool readonly) "/initrd", "/initrd.img", "/usr/share/edk2/", + "/usr/share/edk2-ovmf/", "/usr/share/OVMF/", /* for OVMF images */ "/usr/share/ovmf/", /* for OVMF images */ "/usr/share/AAVMF/", /* for AAVMF images */ -- 2.44.2

3 2