November 2024 - Devel - libvirt List Archives

[PATCH 0/3] hw/sd/sdcard: Deprecate support for spec v1.10
by Philippe Mathieu-Daudé 02 Sep '25

02 Sep '25

Deprecate SD spec v1.10, use v3.01 as new default. Philippe Mathieu-Daudé (3): hw/sd/sdcard: Deprecate support for spec v1.10 hw/sd/sdcard: Use spec v3.01 by default hw/sd/sdcard: Remove support for spec v1.10 docs/about/removed-features.rst | 5 +++++ include/hw/sd/sd.h | 1 - hw/sd/sd.c | 14 +++----------- 3 files changed, 8 insertions(+), 12 deletions(-) -- 2.41.0

2 10

[PATCH rfcv4 00/13] LIBVIRT: X86: TDX support
by Zhenzhong Duan 02 Apr '25

02 Apr '25

Hi, This series brings libvirt the x86 TDX support. * What's TDX? TDX stands for Trust Domain Extensions which isolates VMs from the virtual-machine manager (VMM)/hypervisor and any other software on the platform. To support TDX, multiple software components, not only KVM but also QEMU, guest Linux and virtual bios, need to be updated. For more details, please check link[1]. This patchset is another software component to extend libvirt to support TDX, with which one can start a TDX guest from high level rather than running qemu directly. * Misc As QEMU use a software emulated way to reset guest which isn't supported by TDX guest for security reason. We simulate reboot for TDX guest by kill and create a new one in FakeReboot framework. Complete code can be found at [2], matching qemu code can be found at [3]. There is a 'debug' property for tdx-guest object which isn't in matching qemu[3] yet. I keep them intentionally as they will be implemented in qemu as extention series of [3]. * Test start/stop/reboot with virsh stop/reboot trigger in guest stop with on_poweroff=destroy/restart reboot with on_reboot=destroy/restart * Patch organization - patch 1-4: Support query of TDX capabilities. - patch 5-8: Add TDX type to launchsecurity framework. - patch 9-11: Add reboot support to TDX guest - patch 12-13: Add test and docs TODO: - update QEMU capabilities data in tests, depending on qemu TDX merged beforehand - add reconnect logic in virsh command [1] https://lore.kernel.org/kvm/cover.1708933498.git.isaku.yamahata@intel.com [2] https://github.com/intel/libvirt-tdx/commits/tdx_for_upstream_rfcv4 [3] https://github.com/intel/qemu-tdx/tree/tdx-qemu-upstream-v5 Thanks Zhenzhong Changelog: rfcv4: - add a check to tools/virt-host-validate-qemu.c (Daniel) - remove check of q35 (Daniel) - model 'SocktetAddress' QAPI in xml schema (Daniel) - s/Quote-Generation-Service/quoteGenerationService/ (Daniel) - define bits in tdx->policy and add validating logic (Daniel) - presume QEMU choose split kernel irqchip for TDX guest by default (Daniel) - utilize existing FakeReboot framework to do reboot for TDX guest (Daniel) - drop patch11 'conf: Add support to keep same domid for hard reboot' (Daniel) - add test in tests/ to validate parsing and formatting logic (Daniel) - add doc in docs/formatdomain.rst (Daniel) - add R-B rfcv3: - Change to generate qemu cmdline with -bios - drop firmware auto match as -bios is used - add a hard reboot method to reboot TDX guest rfcv3: https://www.mail-archive.com/devel@lists.libvirt.org/msg00385.html rfcv2: - give up using qmp cmd and check TDX directly on host for TDX capabilities. - use launchsecurity framework to support TDX - use <os>.<loader> for general loader - add auto firmware match feature for TDX A example TDVF fimware description file 70-edk2-x86_64-tdx.json: { "description": "UEFI firmware for x86_64, supporting Intel TDX", "interface-types": [ "uefi" ], "mapping": { "device": "generic", "filename": "/usr/share/OVMF/OVMF_CODE-tdx.fd" }, "targets": [ { "architecture": "x86_64", "machines": [ "pc-q35-*" ] } ], "features": [ "intel-tdx", "verbose-dynamic" ], "tags": [ ] } rfcv2: https://www.mail-archive.com/libvir-list@redhat.com/msg219378.html Zhenzhong Duan (13): tools: Secure guest check for Intel in virt-host-validate qemu: Check if INTEL Trust Domain Extention support is enabled qemu: Add TDX capability conf: expose TDX feature in domain capabilities conf: add tdx as launch security type qemu: Add command line and validation for TDX type qemu: force special parameters enabled for TDX guest Add Intel TDX Quote Generation Service(QGS) support qemu: add FakeReboot support for TDX guest qemu: Support reboot command in guest qemu: Avoid duplicate FakeReboot for secure guest Add test cases for Intel TDX docs: domain: Add documentation for Intel TDX guest docs/formatdomain.rst | 68 ++++ docs/formatdomaincaps.rst | 1 + src/conf/domain_capabilities.c | 1 + src/conf/domain_capabilities.h | 1 + src/conf/domain_conf.c | 312 ++++++++++++++++++ src/conf/domain_conf.h | 75 +++++ src/conf/schemas/domaincaps.rng | 9 + src/conf/schemas/domaincommon.rng | 135 ++++++++ src/conf/virconftypes.h | 2 + src/qemu/qemu_capabilities.c | 36 +- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 139 ++++++++ src/qemu/qemu_firmware.c | 1 + src/qemu/qemu_monitor.c | 28 +- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c | 6 +- src/qemu/qemu_namespace.c | 1 + src/qemu/qemu_process.c | 75 +++++ src/qemu/qemu_validate.c | 44 +++ ...unch-security-tdx-qgs-fd.x86_64-latest.xml | 77 +++++ .../launch-security-tdx-qgs-fd.xml | 30 ++ ...ch-security-tdx-qgs-inet.x86_64-latest.xml | 77 +++++ .../launch-security-tdx-qgs-inet.xml | 30 ++ ...ch-security-tdx-qgs-unix.x86_64-latest.xml | 77 +++++ .../launch-security-tdx-qgs-unix.xml | 30 ++ ...h-security-tdx-qgs-vsock.x86_64-latest.xml | 77 +++++ .../launch-security-tdx-qgs-vsock.xml | 30 ++ tests/qemuxmlconftest.c | 24 ++ tools/virt-host-validate-common.c | 22 +- tools/virt-host-validate-common.h | 1 + 30 files changed, 1407 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxmlconfdata/launch-security-tdx-qgs-fd.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/launch-security-tdx-qgs-fd.xml create mode 100644 tests/qemuxmlconfdata/launch-security-tdx-qgs-inet.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/launch-security-tdx-qgs-inet.xml create mode 100644 tests/qemuxmlconfdata/launch-security-tdx-qgs-unix.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/launch-security-tdx-qgs-unix.xml create mode 100644 tests/qemuxmlconfdata/launch-security-tdx-qgs-vsock.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/launch-security-tdx-qgs-vsock.xml -- 2.34.1

6 45

[PATCH 0/3] Full boot order support on s390x
by Boris Fiuczynski 11 Mar '25

11 Mar '25

This series adds on s390x full boot order support which has been introduced recently in QEMU with the PR https://lore.kernel.org/qemu-devel/20241023131710.906748-1-thuth@redhat.com/ The replies and xml files are removed from the patch in this series and are available in https://gitlab.com/fiuczy/libvirt/-/commits/fullbootorder Boris Fiuczynski (3): qemu: capabilities: Add QEMU_CAPS_VIRTIO_CCW_DEVICE_LOADPARM tests: add capabilities for QEMU 9.2.0 on s390x qemu: command: add multi boot device support on s390x src/qemu/qemu_capabilities.c | 8 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 40 +- src/qemu/qemu_command.h | 6 +- src/qemu/qemu_hotplug.c | 6 +- tests/domaincapsdata/qemu_9.2.0.s390x.xml | 311 + .../caps_5.2.0_s390x.replies | 321 +- .../caps_6.0.0_s390x.replies | 321 +- .../caps_8.1.0_s390x.replies | 321 +- .../caps_8.2.0_s390x.replies | 321 +- .../caps_9.1.0_s390x.replies | 321 +- .../caps_9.2.0_s390x.replies | 36741 ++++++++++++++++ .../qemucapabilitiesdata/caps_9.2.0_s390x.xml | 3752 ++ .../machine-loadparm-hostdev.s390x-9.1.0.args | 33 + .../machine-loadparm-hostdev.s390x-9.1.0.xml | 33 + ...machine-loadparm-hostdev.s390x-latest.args | 4 +- ...-multiple-disks-nets-s390.s390x-9.1.0.args | 40 + ...m-multiple-disks-nets-s390.s390x-9.1.0.xml | 51 + ...multiple-disks-nets-s390.s390x-latest.args | 8 +- ...machine-loadparm-net-s390.s390x-9.1.0.args | 34 + .../machine-loadparm-net-s390.s390x-9.1.0.xml | 32 + ...achine-loadparm-net-s390.s390x-latest.args | 4 +- .../machine-loadparm-s390.s390x-9.1.0.args | 34 + .../machine-loadparm-s390.s390x-9.1.0.xml | 33 + .../machine-loadparm-s390.s390x-latest.args | 4 +- tests/qemuxmlconftest.c | 4 + 26 files changed, 42664 insertions(+), 120 deletions(-) create mode 100644 tests/domaincapsdata/qemu_9.2.0.s390x.xml create mode 100644 tests/qemucapabilitiesdata/caps_9.2.0_s390x.replies create mode 100644 tests/qemucapabilitiesdata/caps_9.2.0_s390x.xml create mode 100644 tests/qemuxmlconfdata/machine-loadparm-hostdev.s390x-9.1.0.args create mode 100644 tests/qemuxmlconfdata/machine-loadparm-hostdev.s390x-9.1.0.xml create mode 100644 tests/qemuxmlconfdata/machine-loadparm-multiple-disks-nets-s390.s390x-9.1.0.args create mode 100644 tests/qemuxmlconfdata/machine-loadparm-multiple-disks-nets-s390.s390x-9.1.0.xml create mode 100644 tests/qemuxmlconfdata/machine-loadparm-net-s390.s390x-9.1.0.args create mode 100644 tests/qemuxmlconfdata/machine-loadparm-net-s390.s390x-9.1.0.xml create mode 100644 tests/qemuxmlconfdata/machine-loadparm-s390.s390x-9.1.0.args create mode 100644 tests/qemuxmlconfdata/machine-loadparm-s390.s390x-9.1.0.xml -- 2.45.0

2 11

[PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and deprecate
by Philippe Mathieu-Daudé 10 Mar '25

10 Mar '25

We are trying to unify all qemu-system-FOO to a single binary. In order to do that we need to remove QAPI target specific code. @dump-skeys is only available on qemu-system-s390x. This series rename it as @dump-s390-skey, making it available on other binaries. We take care of backward compatibility via deprecation. Philippe Mathieu-Daudé (4): hw/s390x: Introduce the @dump-s390-skeys QMP command hw/s390x: Introduce the 'dump_s390_skeys' HMP command hw/s390x: Deprecate the HMP 'dump_skeys' command hw/s390x: Deprecate the QMP @dump-skeys command docs/about/deprecated.rst | 5 +++++ qapi/misc-target.json | 5 +++++ qapi/misc.json | 18 ++++++++++++++++++ include/monitor/hmp.h | 1 + hw/s390x/s390-skeys-stub.c | 24 ++++++++++++++++++++++++ hw/s390x/s390-skeys.c | 19 +++++++++++++++++-- hmp-commands.hx | 17 +++++++++++++++-- hw/s390x/meson.build | 5 +++++ 8 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 hw/s390x/s390-skeys-stub.c -- 2.41.0

7 24

[libvirt] [PATCH] Fix python error reporting for some storage operations
by Cole Robinson 07 Mar '25

07 Mar '25

In the python bindings, all vir* classes expect to be passed a virConnect object when instantiated. Before the storage stuff, these classes were only instantiated in virConnect methods, so the generator is hardcoded to pass 'self' as the connection instance to these classes. Problem is there are some methods that return pool or vol instances which aren't called from virConnect: you can lookup a storage volume's associated pool, and can lookup volumes from a pool. In these cases passing 'self' doesn't give the vir* instance a connection, so when it comes time to raise an exception crap hits the fan. Rather than rework the generator to accomodate this edge case, I just fixed the init functions for virStorage* to pull the associated connection out of the passed value if it's not a virConnect instance. Thanks, Cole diff --git a/python/generator.py b/python/generator.py index 01a17da..c706b19 100755 --- a/python/generator.py +++ b/python/generator.py @@ -962,8 +962,12 @@ def buildWrappers(): list = reference_keepers[classname] for ref in list: classes.write(" self.%s = None\n" % ref[1]) - if classname in [ "virDomain", "virNetwork", "virStoragePool", "virStorageVol" ]: + if classname in [ "virDomain", "virNetwork" ]: classes.write(" self._conn = conn\n") + elif classname in [ "virStorageVol", "virStoragePool" ]: + classes.write(" self._conn = conn\n" + \ + " if not isinstance(conn, virConnect):\n" + \ + " self._conn = conn._conn\n") classes.write(" if _obj != None:self._o = _obj;return\n") classes.write(" self._o = None\n\n"); destruct=None

4 3

[RFC 0/4] meson: Enable -Wundef
by Andrea Bolognani 27 Feb '25

27 Feb '25

A few days ago I have posted a patch[1] that addresses an issue introduced when a meson check was dropped but some uses of the corresponding WITH_ macro were not removed at the same time. That got me thinking about what we can do to prevent such scenarios from happening again in the future. I have come up with something that I think would be effective, but since applying the approach throughout the entire codebase would require a non-trivial amount of work, I figured I'd ask for feedback before embarking on it. The idea is that there are two types of macros we can use for conditional compilation: external ones, coming from the OS or other libraries, and internal ones, which are the result of meson tests. The external ones (e.g. SIOCSIFFLAGS, __APPLE__) are usually only defined if they apply, so it is correct to check for their presence with #ifdef. Using #if will also work, as undefined macros evaluate to zero, but it's not good practice to use them that way. If -Wundef has been passed to the compiler, those incorrect uses will be reported (only on platforms where they are not defined, of course). The internal ones (e.g. WITH_QEMU, WITH_STRUCT_IFREQ) are similar, but in this case we control their definition. This means that using means that the feature is not available on the machine we're building on, but it could also mean that we've removed the meson check and forgot to update all users of the macro. In this case, -Wundef would work 100% reliably to detect the issue: if the meson check doesn't exist, neither will the macro, regardless of what platform we're building on. So the approach I'm suggesting is to use a syntax-check rule to ensure that internal macros are only ever checked with #if instead of Of course this requires a full sweep to fix all cases in which we're not already doing things according to the proposal. Should be fairly easy, if annoying. A couple of examples are included here for demonstration purposes. The bigger impact is going to be on the build system. Right now we generally only define WITH_ macros if the check passed, but that will have to change and the result is going to be quite a bit of additional meson code I'm afraid. Thoughts? [1] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/SKJ… Andrea Bolognani (4): configmake: Check for WIN32 correctly meson: Always define WITH_*_DECL macros syntax-check: Ensure WITH_ macros are used correctly meson: Enable -Wundef build-aux/syntax-check.mk | 5 +++++ configmake.h.in | 2 +- meson.build | 3 +++ tests/virmockstathelpers.c | 28 ++++++++++++++-------------- 4 files changed, 23 insertions(+), 15 deletions(-) -- 2.43.2

4 8

[PATCH v5 00/18] *** qemu: block: Support block disk along with throttle filters ***
by Harikumar R 12 Feb '25

12 Feb '25

*** BLURB HERE *** Chun Feng Wu (17): schema: Add new domain elements to support multiple throttle groups schema: Add new domain elements to support multiple throttle filters config: Introduce ThrottleGroup and corresponding XML parsing config: Introduce ThrottleFilter and corresponding XML parsing qemu: monitor: Add support for ThrottleGroup operations tests: Test qemuMonitorJSONGetThrottleGroup and qemuMonitorJSONUpdateThrottleGroup remote: New APIs for ThrottleGroup lifecycle management qemu: Refactor qemuDomainSetBlockIoTune to extract common methods qemu: Implement qemu driver for throttle API qemu: helper: throttle filter nodename and preparation processing qemu: block: Support block disk along with throttle filters config: validate: Verify iotune, throttle group and filter qemuxmlconftest: Add 'throttlefilter' tests test_driver: Test throttle group lifecycle APIs virsh: Refactor iotune options for re-use virsh: Add support for throttle group operations virsh: Add option "throttle-groups" to "attach_disk" Harikumar Rajkumar (1): tests: Test qemuxmlactivetestThrottleGroup docs/formatdomain.rst | 47 ++ docs/manpages/virsh.rst | 135 +++- include/libvirt/libvirt-domain.h | 21 + src/conf/domain_conf.c | 398 ++++++++++ src/conf/domain_conf.h | 45 ++ src/conf/domain_validate.c | 119 ++- src/conf/schemas/domaincommon.rng | 293 ++++---- src/conf/virconftypes.h | 4 + src/driver-hypervisor.h | 22 + src/libvirt-domain.c | 174 +++++ src/libvirt_private.syms | 8 + src/libvirt_public.syms | 7 + src/qemu/qemu_block.c | 136 ++++ src/qemu/qemu_block.h | 49 ++ src/qemu/qemu_command.c | 180 +++++ src/qemu/qemu_command.h | 6 + src/qemu/qemu_domain.c | 73 +- src/qemu/qemu_driver.c | 619 +++++++++++++--- src/qemu/qemu_hotplug.c | 33 + src/qemu/qemu_monitor.c | 34 + src/qemu/qemu_monitor.h | 14 + src/qemu/qemu_monitor_json.c | 134 ++++ src/qemu/qemu_monitor_json.h | 14 + src/remote/remote_daemon_dispatch.c | 44 ++ src/remote/remote_driver.c | 40 ++ src/remote/remote_protocol.x | 48 +- src/remote_protocol-structs | 28 + src/test/test_driver.c | 452 ++++++++---- tests/qemumonitorjsontest.c | 86 +++ .../throttlefilter-in.xml | 392 ++++++++++ .../throttlefilter-out.xml | 393 ++++++++++ tests/qemuxmlactivetest.c | 1 + .../throttlefilter-invalid.x86_64-latest.err | 1 + .../throttlefilter-invalid.xml | 89 +++ .../throttlefilter.x86_64-latest.args | 55 ++ .../throttlefilter.x86_64-latest.xml | 105 +++ tests/qemuxmlconfdata/throttlefilter.xml | 95 +++ tests/qemuxmlconftest.c | 2 + tools/virsh-completer-domain.c | 82 +++ tools/virsh-completer-domain.h | 16 + tools/virsh-domain.c | 680 ++++++++++++++---- 41 files changed, 4649 insertions(+), 525 deletions(-) create mode 100644 tests/qemustatusxml2xmldata/throttlefilter-in.xml create mode 100644 tests/qemustatusxml2xmldata/throttlefilter-out.xml create mode 100644 tests/qemuxmlconfdata/throttlefilter-invalid.x86_64-latest.err create mode 100644 tests/qemuxmlconfdata/throttlefilter-invalid.xml create mode 100644 tests/qemuxmlconfdata/throttlefilter.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/throttlefilter.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/throttlefilter.xml -- 2.39.5 (Apple Git-154)

4 51

[PATCH v3 00/17] hw/microblaze: Allow running cross-endian vCPUs
by Philippe Mathieu-Daudé 05 Feb '25

05 Feb '25

Missing review: 4 (new) & 10 Since v2: - Addressed Richard's review comments Since v1: - Make device endianness configurable (Edgar) - Convert more Xilinx devices - Avoid preprocessor #if (Richard) - Add R-b tags Make machines endianness-agnostic, allowing to run a big-endian vCPU on the little-endian 'qemu-system-microblazeel' binary, and a little endian one on the big-endian 'qemu-system-microblaze' binary. Tests added, following combinations covered: - little-endian vCPU using little-endian binary (in-tree) - little-endian vCPU using big-endian binary (new) - big-endian vCPU using little-endian binary (new) - big-endian vCPU using big-endian binary (in-tree) To make a target endian-agnostic we need to remove the MO_TE uses. In order to do that, we propagate the MemOp from earlier in the call stack, or we extract it from the vCPU env (on MicroBlaze the CPU endianness is exposed by the 'ENDI' bit). Next step: Look at unifying binaries. Please review, Phil. Philippe Mathieu-Daudé (17): hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit hw/microblaze: Propagate CPU endianness to microblaze_load_kernel() hw/intc/xilinx_intc: Make device endianness configurable RFC hw/net/xilinx_ethlite: Simplify by having configurable endianness RFC hw/timer/xilinx_timer: Allow down to 8-bit memory access hw/timer/xilinx_timer: Make device endianness configurable hw/char/xilinx_uartlite: Make device endianness configurable hw/ssi/xilinx_spi: Make device endianness configurable hw/ssi/xilinx_spips: Make device endianness configurable hw/arm/xlnx-zynqmp: Use &error_abort for programming errors target/microblaze: Explode MO_TExx -> MO_TE | MO_xx target/microblaze: Set MO_TE once in do_load() / do_store() target/microblaze: Introduce mo_endian() helper target/microblaze: Consider endianness while translating code hw/microblaze: Support various endianness for s3adsp1800 machines tests/functional: Explicit endianness of microblaze assets tests/functional: Add microblaze cross-endianness tests hw/microblaze/boot.h | 4 +- include/hw/ssi/xilinx_spips.h | 1 + target/microblaze/cpu.h | 7 +++ hw/arm/xilinx_zynq.c | 1 + hw/arm/xlnx-zynqmp.c | 40 +++++-------- hw/char/xilinx_uartlite.c | 31 ++++++---- hw/intc/xilinx_intc.c | 50 ++++++++++++---- hw/microblaze/boot.c | 8 +-- hw/microblaze/petalogix_ml605_mmu.c | 4 +- hw/microblaze/petalogix_s3adsp1800_mmu.c | 58 ++++++++++++++++--- hw/microblaze/xlnx-zynqmp-pmu.c | 2 +- hw/net/xilinx_ethlite.c | 42 +++++++++----- hw/ppc/virtex_ml507.c | 1 + hw/ssi/xilinx_spi.c | 24 +++++--- hw/ssi/xilinx_spips.c | 36 +++++++----- hw/timer/xilinx_timer.c | 33 +++++++---- target/microblaze/translate.c | 49 ++++++++++------ .../functional/test_microblaze_s3adsp1800.py | 27 ++++++++- .../test_microblazeel_s3adsp1800.py | 25 +++++++- 19 files changed, 309 insertions(+), 134 deletions(-) -- 2.45.2

4 27

Re: [PATCH] Сheck snapshot disk is not NULL when searching it in the VM config
by Peter Krempa 29 Jan '25

29 Jan '25

On Mon, May 20, 2024 at 14:48:47 +0000, Efim Shevrin via Devel wrote: > Hello, > > > If vmdisk is NULL, shouldn't this function (qemuSnapshotDeleteValidate()) return an error? > > I think this qemuSnapshotDeleteValidate should not return an error. > > It seems to me that when vmdisk is NULL, this does not invalidate > the snapshot itself, but indicates that the config has changed since > the snapshot was done. And if the VM config has changed, this adds evidence that the snapshot should be deleted, > because the snapshot does not reflect the real vm config. > > Since we do not have an analogue of the --force option for deleting a snapshot, in the case when qemuSnapshotDeleteValidate returns > an error when vmdisk is NULL, we will never delete a snapshot which has invalid disk. Snapshot deletion does have something that can be considered force and that is the '--metadata' option that removes just the snapshot definition (metadata) and doesn't touch the disk images. > > Similarly, disk can be NULL too > Thank you for the comment regarding the disk variable. I`ve reworked patch. > > When creating a snapshot of a VM with multiple hard disks, > the snapshot takes into account the presence of all disks > in the system. If, over time, one of the disks is deleted, > the snapshot will continue to store knowledge of the deleted disk. > This results in the fact that at the moment of deleting the snapshot, > at the validation stage, a disk from the snapshot will be searched which > is not in the VM configuration. As a result, vmdisk variable will > be equal to NULL. Dereferencing a null pointer at the time of calling > virStorageSourceIsSameLocation(vmdisk->src, disk->src) > will result in SIGSEGV. Crashing is obviously not okay ... > Also, the disk variable can also be equal to NULL and this > requires to check that disk != NULL before calling the > virStorageSourceIsSameLocation function to avoid SIGSEGV. .. but going ahead with the snapshot deletion isn't always okay either. The disk isn't referenced by the VM so the disk state can't be merged, while the state would be merged for any other disk. When reverting back to a previous snapshot, which is still referencing the older state of the disk which was removed from the VM, the VM would see that the image state of disks that were present at deletion would contain the merged state, but only a partial state for the disk which was later removed.

3 3

[PATCH] qemuSnapshotDeleteValidate: Fix crash when disk is not found in VM definition
by kaihuan 29 Jan '25

29 Jan '25

qemuDomainDiskByName() can return a NULL pointer on failure. But this returned value in qemuSnapshotDeleteValidate is not checked.It will make libvirtd crash. Signed-off-by: kaihuan <jungleman759(a)gmail.com> --- src/qemu/qemu_snapshot.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 18b2e478f6..bcbd913073 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -4242,8 +4242,19 @@ qemuSnapshotDeleteValidate(virDomainObj *vm, virDomainDiskDef *vmdisk = NULL; virDomainDiskDef *disk = NULL; - vmdisk = qemuDomainDiskByName(vm->def, snapDisk->name); - disk = qemuDomainDiskByName(snapdef->parent.dom, snapDisk->name); + if (!(vmdisk = qemuDomainDiskByName(vm->def, snapDisk->name))) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("disk '%1$s' referenced by snapshot '%2$s' not found in the current definition"), + snapDisk->name, snap->def->name); + return -1; + } + + if (!(disk = qemuDomainDiskByName(snapdef->parent.dom, snapDisk->name))) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("disk '%1$s' referenced by snapshot '%2$s' not found in the VM definition of the deleted snapshot"), + snapDisk->name, snap->def->name); + return -1; + } if (!virStorageSourceIsSameLocation(vmdisk->src, disk->src)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, -- 2.33.1.windows.1

3 3