January 2025 - Devel - libvirt List Archives

[PATCH 0/5] Drop support for VirtualBox-6.1
by Michal Privoznik 31 Jan '25

31 Jan '25

This was initiated by the following issue: https://gitlab.com/libvirt/libvirt/-/issues/681 and I tried to add support for VBOX-7.1 but unfortunately got hit by a bug in VBOX which renders the way we initialize C bindings useless. I've reported the bug here: https://www.virtualbox.org/ticket/22224 but it didn't get any attention. Michal Prívozník (5): vbox: Use g_autofree in tryLoadOne() vbox: Report an error when VBox CAPI initialization fails vbox: Drop support for VirtualBox-6.1.x vbox: Drop code supporting old VBox version NEWS: Document VBOX-6.1 removal NEWS.rst | 5 + src/vbox/meson.build | 1 - src/vbox/vbox_CAPI_v6_1.h | 32896 -------------------------------- src/vbox/vbox_V6_1.c | 13 - src/vbox/vbox_XPCOMCGlue.c | 5 +- src/vbox/vbox_XPCOMCGlue.h | 2 +- src/vbox/vbox_common.h | 4 +- src/vbox/vbox_storage.c | 4 +- src/vbox/vbox_tmpl.c | 30 +- src/vbox/vbox_uniformed_api.h | 1 - 10 files changed, 16 insertions(+), 32945 deletions(-) delete mode 100644 src/vbox/vbox_CAPI_v6_1.h delete mode 100644 src/vbox/vbox_V6_1.c -- 2.45.2

2 6

[PATCH V2 0/3] qemu: Improve opening and verifying save images
by Jim Fehlig 31 Jan '25

31 Jan '25

V2 of https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/DAT… Changes in V2: * Move unlinking corrupt save images from qemuSaveImageOpen to the only caller using that functionality * Add a function to read save image header * Correctly position file pointer of save image for QEMU Jim Fehlig (3): qemu: Move unlinking corrupt save image file to caller qemu: Decompose qemuSaveImageOpen qemu: Check for valid save image format when verifying image header src/qemu/qemu_driver.c | 46 ++++--- src/qemu/qemu_saveimage.c | 268 +++++++++++++++++++++++--------------- src/qemu/qemu_saveimage.h | 21 ++- src/qemu/qemu_snapshot.c | 9 +- 4 files changed, 207 insertions(+), 137 deletions(-) -- 2.43.0

2 4

[PATCH 0/2] qemu: Improve opening and verifying save images
by Jim Fehlig 31 Jan '25

31 Jan '25

qemuSaveImageOpen does a lot of stuff. When bypass-cache is specified, it creates a virFileWrapperFd, which is not cleaned up in the subsequent failure paths. This results in errors from the iohelper, which can overwrite the actual error. E.g. consider libvirt attempting to restore an image saved in an unknown format # virsh restore --bypass-cache /data/test.sav.sparse error: Failed to restore domain from /data/test.sav.sparse error: internal error: Child process (LIBVIRT_LOG_OUTPUTS=1:stderr /usr/lib64/libvirt/libvirt_iohelper /data/test.sav.sparse 0) unexpected fatal signal 13 When not using the iohelper, and not creating a virFileWrapperFd, we see the real error: # virsh restore /data/test.sav.sparse error: Failed to restore domain from /data/test.sav.sparse error: operation failed: Invalid compressed save format 6 Although that error highlights a spot I missed when removing the "compression" implications around the 'foo_image_format' settings in qemu.conf with commit bd6d7ebf622 :-). IMO, the error message would be best fixed by checking for valid values when reading the save image metadata. Patch 1 decomposes qemuSaveImageOpen to allow for better error handling and more flexibility. Patch 2 checks for a valid format when checking the other header fields. Jim Fehlig (2): qemu: Decompose qemuSaveImageOpen qemu: Check for valid save image format when verifying image header src/qemu/qemu_driver.c | 37 +++++++-------- src/qemu/qemu_saveimage.c | 95 +++++++++++++++++++++++++-------------- src/qemu/qemu_saveimage.h | 16 ++++--- src/qemu/qemu_snapshot.c | 9 ++-- 4 files changed, 95 insertions(+), 62 deletions(-) -- 2.43.0

2 8

[PATCH 0/2] finish sysusers support
by Daniel P. Berrangé 31 Jan '25

31 Jan '25

This fixes another missing sysusers file and changes the RPM spec to take account of new RPM processing of sysusers files at install time. Daniel P. Berrangé (2): tools: add sysusers file to create 'virtlogin' group rpm: disable account creation for Fedora >= 42 libvirt.spec.in | 19 +++++++++++++++++++ tools/libvirt-login-shell.sysusers.conf | 1 + tools/meson.build | 7 +++++++ 3 files changed, 27 insertions(+) create mode 100644 tools/libvirt-login-shell.sysusers.conf -- 2.47.1

2 6

[RFC PATCH 0/5] qemu: Route hostdevs to multiple nested SMMUs
by Nathan Chen 31 Jan '25

31 Jan '25

Hi, This is a draft solution for supporting multiple vSMMU instances in a qemu VM. Based on discussions/suggestions received for a previous RFC by Nicolin here[0], the association of vSMMUs to VFIO devices in VM PCIe topology should be moved out of qemu into libvirt. In addition, the nested SMMU nodes should be passed to qemu as pluggable devices. To address these changes, this patch series introduces a new "nestedSmmuv3" IOMMU model and "nestedSmmuv3" device type. Upon specifying the nestedSmmuv3 IOMMU model, nestedSmmuv3 devices will be auto-added to the VM definition based on the available SMMU nodes in the host's sysfs. The nestedSmmuv3 devices will each be attached to a separate PXB controller, and VFIO devices will be routed to PXBs based on their association with host SMMU nodes. This will maintain a VM PCIe topology that allows for multiple nested SMMUs per Nicolin's original qemu patch series in [0] and Shameer's work in [1] to remove VM topology changes from qemu and allow the nested SMMUs to be specified as pluggable devices. For instance, if we specify the nestedSmmuv3 IOMMU model and a hostdev for passthrough: <devices> <hostdev mode='subsystem' type='pci' managed='no'> <source> <address domain='0x0009' bus='0x01' slot='0x00' function='0x0'/> </source> </hostdev> <iommu model='nestedSmmuv3'/> </devices> Libvirt will scan sysfs and populate the VM definition with controllers and nestedSmmuv3 devices based on host config. So if /sys/bus/pci/devices/0009:01:00.0/iommu is a symlink to the host SMMU node represented by /sys/devices/platform/arm-smmu-v3.8.auto/iommu/smmu3.0x0000000016000000 and there are 3 host SMMU nodes under /sys/class/iommu/, we'll see three auto-added nestedSmmuv3 devices, each routed to a pcie-expander-bus controller. Then the hostdev will be routed to a PXB controller that has a matching host SMMU node associated with it: <devices> ... <controller type='pci' index='1' model='pcie-expander-bus'> <model name='pxb-pcie'/> <target busNr='254'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </controller> <controller type='pci' index='2' model='pcie-expander-bus'> <model name='pxb-pcie'/> <target busNr='251'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </controller> <controller type='pci' index='3' model='pcie-expander-bus'> <model name='pxb-pcie'/> <target busNr='249'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </controller> <controller type='pci' index='4' model='pcie-root-port'> <model name='pcie-root-port'/> <target chassis='7' port='0x8'/> <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> </controller> <hostdev mode='subsystem' type='pci' managed='no'> <source> <address domain='0x0009' bus='0x01' slot='0x00' function='0x0'/> </source> <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/> </hostdev> <iommu model='nestedSmmuv3'/> <nestedSmmuv3> <name>smmu3.0x0000000012000000</name> <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> </nestedSmmuv3> <nestedSmmuv3> <name>smmu3.0x0000000016000000</name> <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> </nestedSmmuv3> <nestedSmmuv3> <name>smmu3.0x0000000011000000</name> <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> </nestedSmmuv3> <iommu model='nestedSmmuv3'/> </devices> TODO: - No DMA mapping can found by UEFI when specifying multiple passthrough devices in the VM definition, and VM boot is subsequently blocked. We need to investigate this for the next revision, but we don't encounter this issue when passing through a single device. We'll include iommufd support in the next revision to narrow down whether the required fix would be outside of libvirt. - Shameer's qemu branch specifies nestedSmmuv3 bus number with "pci-bus" instead of "bus", so the libvirt compilation test args and qemu args in qemuBuildPCINestedSmmuv3DevProps() need to be modified to match this revision of qemu. It will be reverted to using "bus" in the next qemu revision. - This patchset decrements PXB busNr based on how many devices are attached downstream, and the libvirt documentation states we must reserve busNr for the PXB itself in addition to any devices attached downstream. When I launch a VM and a PXB has a pcie-root-port and hostdev attached downstream, busNrs 253, 252, and 251 are reserved. But the PXB itself already has a bus number assigned via the <address/> attribute, and I see 253 and 252 assigned to the hostdev and pcie-root-port in the VM but not 251. Should we decrement busNr based on libvirt documentation or do we only need two busNrs 253 and 252 in the example here? This series is on Github: https://github.com/NathanChenNVIDIA/libvirt/tree/nested-smmuv3-12-05-24 Thanks, Nathan [0] https://lore.kernel.org/qemu-devel/cover.1719361174.git.nicolinc@nvidia.com/ [1] https://lore.kernel.org/qemu-devel/20241108125242.60136-1-shameerali.koloth… Signed-off-by: Nathan Chen <nathanc(a)nvidia.com> Nathan Chen (5): conf: Add a nestedSmmuv3 IOMMU model qemu: Implement and auto-add a nestedSmmuv3 device type qemu: Create PXBs and auto-assign VFIO devs and nested SMMUs qemu: Update PXB busNr for nestedSmmuv3 controllers qemu: Add test case for specifying multiple nested SMMUs docs/formatdomain.rst | 25 ++- src/ch/ch_domain.c | 1 + src/conf/domain_addr.c | 26 ++- src/conf/domain_addr.h | 4 +- src/conf/domain_conf.c | 188 +++++++++++++++++ src/conf/domain_conf.h | 15 ++ src/conf/domain_postparse.c | 1 + src/conf/domain_validate.c | 24 +++ src/conf/schemas/domaincommon.rng | 17 ++ src/conf/virconftypes.h | 2 + src/libvirt_private.syms | 2 + src/lxc/lxc_driver.c | 6 + src/qemu/qemu_command.c | 64 +++++- src/qemu/qemu_command.h | 4 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_domain_address.c | 193 ++++++++++++++++++ src/qemu/qemu_driver.c | 3 + src/qemu/qemu_hotplug.c | 5 + src/qemu/qemu_postparse.c | 1 + src/qemu/qemu_validate.c | 16 ++ src/test/test_driver.c | 4 + tests/meson.build | 1 + .../iommu-nestedsmmuv3.aarch64-latest.args | 38 ++++ .../iommu-nestedsmmuv3.aarch64-latest.xml | 61 ++++++ tests/qemuxmlconfdata/iommu-nestedsmmuv3.xml | 29 +++ tests/qemuxmlconftest.c | 4 +- tests/schemas/device.rng.in | 1 + tests/virnestedsmmuv3mock.c | 57 ++++++ 28 files changed, 788 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxmlconfdata/iommu-nestedsmmuv3.aarch64-latest.args create mode 100644 tests/qemuxmlconfdata/iommu-nestedsmmuv3.aarch64-latest.xml create mode 100644 tests/qemuxmlconfdata/iommu-nestedsmmuv3.xml create mode 100644 tests/virnestedsmmuv3mock.c -- 2.34.1

4 18

[PATCH v2 00/15] qemu: Store I/O error messages for disks
by Peter Krempa 30 Jan '25

30 Jan '25

v2: - ACK'd patches were pushed - the QEMU 'qom-path' regression is no longer mentioned - it's worker around by prefering lookup via node-name - the patch is justified without the need to mention it - 'qom-path' is now properly used for lookup of the disk - I/O error messages are now stored in virStorageSource - the messages can be viewed via virDomainGetMessages() - various refactors to make the addition clean added - news added Peter Krempa (15): qemu: Handle quirks of 'device' field of BLOCK_IO_ERROR event in monitor code qemu: Rename 'diskAlias' to 'device' in qemu IO error event handling qemuProcessHandleIOError: Rename local variables qemuMonitorJSONHandleIOError: Do not munge 'reason' field of IO error event qemuProcessHandleIOError: Prefer lookup by node name qemuMonitorJSONHandleIOError: Propagate new 'qom-path' field virStorageSource: Add fields for storing last I/O error message qemuProcessHandleIOError: Populate I/O error reason to virStorageSource qemuProcessHandleIOError: Log IO errors in the VM log file libxlDomainGetMessages: Add existing flags to 'virCheckFlags' virDomainObjGetMessages: Refactor using GPtrArray virDomainGetMessages: Introduce VIR_DOMAIN_MESSAGE_IOERRORS include: libvirt-domain: Reword documentation for @reason of VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON include: libvirt-domain: Add 'hypervisor-message' @reason of VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON NEWS: Mention preserving I/O error messages for qemu VMs NEWS.rst | 6 +++ include/libvirt/libvirt-domain.h | 19 ++++++-- src/conf/domain_conf.c | 80 +++++++++++++++++++++----------- src/conf/domain_conf.h | 9 +++- src/conf/storage_source_conf.c | 5 ++ src/conf/storage_source_conf.h | 6 +++ src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 13 +++++- src/qemu/qemu_driver.c | 24 +++++++++- src/qemu/qemu_monitor.c | 6 ++- src/qemu/qemu_monitor.h | 8 +++- src/qemu/qemu_monitor_json.c | 23 ++++++--- src/qemu/qemu_process.c | 61 +++++++++++++++--------- src/test/test_driver.c | 12 ++++- 14 files changed, 202 insertions(+), 71 deletions(-) -- 2.48.1

2 32

[PATCH] glibcompat: Updating "backport" 'g_string_replace'
by Adam Julis 30 Jan '25

30 Jan '25

Update the vir_g_string_replace with following commits from glib: c9e48947e gstring: Fix a heap buffer overflow in the new g_string_replace() code e8517e777 remove quadratic behavior in g_string_replace Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- src/util/glibcompat.c | 125 +++++++++++++++++++++++++++++++++++------- 1 file changed, 105 insertions(+), 20 deletions(-) diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c index bcb666992a..47e3edef13 100644 --- a/src/util/glibcompat.c +++ b/src/util/glibcompat.c @@ -65,7 +65,7 @@ /** * Adapted (to pass syntax check) from 'g_string_replace' from - * glib-2.81.1. Drop once minimum glib is bumped to 2.68. + * glib-2.83.3. Drop once minimum glib is bumped to 2.68. * * g_string_replace: * @string: a #GString @@ -94,35 +94,120 @@ vir_g_string_replace(GString *string, const gchar *replace, guint limit) { - gsize f_len, r_len, pos; - gchar *cur, *next; - guint n = 0; + GString *new_string = NULL; + gsize f_len, r_len, new_len; + gchar *cur, *next, *first, *dst; + guint n; g_return_val_if_fail(string != NULL, 0); g_return_val_if_fail(find != NULL, 0); g_return_val_if_fail(replace != NULL, 0); + first = strstr(string->str, find); + + if (first == NULL) + return 0; + + new_len = string->len; f_len = strlen(find); r_len = strlen(replace); - cur = string->str; - while ((next = strstr(cur, find)) != NULL) { - pos = next - string->str; - g_string_erase(string, pos, f_len); - g_string_insert(string, pos, replace); - cur = string->str + pos + r_len; - n++; - /* Only match the empty string once at any given position, to - * avoid infinite loops */ - if (f_len == 0) { - if (cur[0] == '\0') - break; - else - cur++; + /* It removes a lot of branches and possibility for infinite loops if we + * handle the case of an empty @find string separately. */ + if (G_UNLIKELY(f_len == 0)) { + size_t i; + if (limit == 0 || limit > string->len) { + if (string->len > G_MAXSIZE - 1) + g_error("inserting in every position in string would overflow"); + + limit = string->len + 1; + } + + if (r_len > 0 && + (limit > G_MAXSIZE / r_len || + limit * r_len > G_MAXSIZE - string->len)) + g_error("inserting in every position in string would overflow"); + + new_len = string->len + limit * r_len; + new_string = g_string_sized_new(new_len); + for (i = 0; i < limit; i++) { + g_string_append_len(new_string, replace, r_len); + if (i < string->len) + g_string_append_c(new_string, string->str[i]); } - if (n == limit) - break; + if (limit < string->len) + g_string_append_len(new_string, string->str + limit, string->len - limit); + + g_free(string->str); + string->allocated_len = new_string->allocated_len; + string->len = new_string->len; + string->str = g_string_free(g_steal_pointer(&new_string), FALSE); + + return limit; } + /* Potentially do two passes: the first to calculate the length of the new string, + * new_len, if it’s going to be longer than the original string; and the second to + * do the replacements. The first pass is skipped if the new string is going to be + * no longer than the original. + * + * The second pass calls various g_string_insert_len() (and similar) methods + * which would normally potentially reallocate string->str, and hence + * invalidate the cur/next/first/dst pointers. Because we’ve pre-calculated + * the new_len and do all the string manipulations on new_string, that + * shouldn’t happen. This means we scan `string` while modifying + * `new_string`. */ + do { + dst = first; + cur = first; + n = 0; + while ((next = strstr(cur, find)) != NULL) { + n++; + + if (r_len <= f_len) { + memmove(dst, cur, next - cur); + dst += next - cur; + memcpy(dst, replace, r_len); + dst += r_len; + } else { + if (new_string == NULL) { + new_len += r_len - f_len; + } else { + g_string_append_len(new_string, cur, next - cur); + g_string_append_len(new_string, replace, r_len); + } + } + cur = next + f_len; + + if (n == limit) + break; + } + + /* Append the trailing characters from after the final instance of @find + * in the input string. */ + if (r_len <= f_len) { + /* First pass skipped. */ + gchar *end = string->str + string->len; + memmove(dst, cur, end - cur); + end = dst + (end - cur); + *end = 0; + string->len = end - string->str; + break; + } else { + if (new_string == NULL) { + /* First pass. */ + new_string = g_string_sized_new(new_len); + g_string_append_len(new_string, string->str, first - string->str); + } else { + /* Second pass. */ + g_string_append_len(new_string, cur, (string->str + string->len) - cur); + g_free(string->str); + string->allocated_len = new_string->allocated_len; + string->len = new_string->len; + string->str = g_string_free(g_steal_pointer(&new_string), FALSE); + break; + } + } + } while (1); return n; } -- 2.47.1

2 1

[PATCH] remote: add sysusers file to create 'libvirt' group
by Daniel P. Berrangé 30 Jan '25

30 Jan '25

We previously added a sysusers file, but missed the 'libvirt' group. This group is referenced in the polkit rules, so we should be registering that too. It must be done in a separate sysusers file, however, since it is common to all daemons. Fixes: a2c3e390f7bedf36f4ddc544d09fe3b8772c5c6f Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- libvirt.spec.in | 1 + src/remote/libvirt.sysusers.conf | 1 + src/remote/meson.build | 7 +++++++ 3 files changed, 9 insertions(+) create mode 100644 src/remote/libvirt.sysusers.conf diff --git a/libvirt.spec.in b/libvirt.spec.in index a82c366334..5c5d36966d 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2110,6 +2110,7 @@ exit 0 %{_datadir}/polkit-1/actions/org.libvirt.unix.policy %{_datadir}/polkit-1/actions/org.libvirt.api.policy %{_datadir}/polkit-1/rules.d/50-libvirt.rules +%{_sysusersdir}/libvirt.conf %dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/ %attr(0755, root, root) %{_libexecdir}/libvirt_iohelper %attr(0755, root, root) %{_bindir}/virt-ssh-helper diff --git a/src/remote/libvirt.sysusers.conf b/src/remote/libvirt.sysusers.conf new file mode 100644 index 0000000000..50c6716cce --- /dev/null +++ b/src/remote/libvirt.sysusers.conf @@ -0,0 +1 @@ +g libvirt - diff --git a/src/remote/meson.build b/src/remote/meson.build index ea063ed6cc..e503263266 100644 --- a/src/remote/meson.build +++ b/src/remote/meson.build @@ -310,6 +310,13 @@ if conf.has('WITH_REMOTE') ) endif + # Install the sysuser config for the daemon polkit rules + install_data( + 'libvirt.sysusers.conf', + install_dir: sysusersdir, + rename: [ 'libvirt.conf' ], + ) + virt_helpers += { 'name': 'virt-ssh-helper', 'sources': [ -- 2.47.1

2 2

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

30 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 30 Jan '25

30 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

[PATCH] Support IDE/SATA disk 'product' parameter
by Adam Julis 30 Jan '25

30 Jan '25

Since we supported 'product' parameter for SCSI, just expanded existing solution makes IDE/SATA parameter works too. QEMU requires parameter 'model' in case of IDE/SATA (instead of 'product'), so the process of making JSON object is slightly modified for that. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/697 Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- docs/formatdomain.rst | 7 ++-- src/qemu/qemu_command.c | 11 +++++- src/qemu/qemu_validate.c | 14 ++++++-- ...disk-product-build-error.x86_64-latest.err | 1 + .../disk-scsi-disk-product-build-error.xml | 34 +++++++++++++++++++ ...-disk-vendor-build-error.x86_64-latest.err | 1 + ... => disk-scsi-disk-vendor-build-error.xml} | 0 ...csi-disk-vpd-build-error.x86_64-latest.err | 1 - .../disk-scsi-disk-vpd.x86_64-latest.args | 4 +-- .../disk-scsi-disk-vpd.x86_64-latest.xml | 7 ++-- tests/qemuxmlconfdata/disk-scsi-disk-vpd.xml | 4 +-- tests/qemuxmlconftest.c | 3 +- 12 files changed, 71 insertions(+), 16 deletions(-) create mode 100644 tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.x86_64-latest.err create mode 100644 tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.xml create mode 100644 tests/qemuxmlconfdata/disk-scsi-disk-vendor-build-error.x86_64-latest.err rename tests/qemuxmlconfdata/{disk-scsi-disk-vpd-build-error.xml => disk-scsi-disk-vendor-build-error.xml} (100%) delete mode 100644 tests/qemuxmlconfdata/disk-scsi-disk-vpd-build-error.x86_64-latest.err diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 60bee8bd4f..c93a321401 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3551,12 +3551,13 @@ paravirtualized driver is specified via the ``disk`` element. :since:`Since 0.10.1` ``vendor`` If present, this element specifies the vendor of a virtual hard disk or - CD-ROM device. It must not be longer than 8 printable characters. - :since:`Since 1.0.1` + CD-ROM device. It must not be longer than 8 printable characters. Only for + devices using 'scsi' ``bus``. :since:`Since 1.0.1` ``product`` If present, this element specifies the product of a virtual hard disk or CD-ROM device. It must not be longer than 16 printable characters. - :since:`Since 1.0.1` + Only for devices using 'scsi' (:since:`Since 1.0.1`), 'sata' or 'ide' ``bus``. + :since:`Since 11.0.0` ``address`` If present, the ``address`` element ties the disk to a given slot of a controller (the actual ``<controller>`` device can often be inferred by diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dcb9c4934e..5c38858f5d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1628,6 +1628,11 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, else driver = "ide-hd"; + if (virJSONValueObjectAdd(&props, + "S:model", disk->product, + NULL) < 0) + return NULL; + break; case VIR_DOMAIN_DISK_BUS_SCSI: @@ -1654,6 +1659,11 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, } } + if (virJSONValueObjectAdd(&props, + "S:product", disk->product, + NULL) < 0) + return NULL; + break; case VIR_DOMAIN_DISK_BUS_VIRTIO: { @@ -1803,7 +1813,6 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, "A:wwn", &wwn, "p:rotation_rate", disk->rotation_rate, "S:vendor", disk->vendor, - "S:product", disk->product, "T:removable", removable, "S:write-cache", qemuOnOffAuto(writeCache), "p:cyls", disk->geometry.cylinders, diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index aaa056379e..f0be236533 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2947,10 +2947,20 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, } } - if (disk->vendor || disk->product) { + if (disk->vendor) { if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Only scsi disk supports vendor and product")); + _("Only scsi disk supports vendor")); + return -1; + } + } + + if (disk->product) { + if ((disk->bus != VIR_DOMAIN_DISK_BUS_IDE) && + (disk->bus != VIR_DOMAIN_DISK_BUS_SATA) && + (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only ide, sata and scsi disk supports product")); return -1; } } diff --git a/tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.x86_64-latest.err b/tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.x86_64-latest.err new file mode 100644 index 0000000000..93dfac0d1e --- /dev/null +++ b/tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.x86_64-latest.err @@ -0,0 +1 @@ +unsupported configuration: Only ide, sata and scsi disk supports product diff --git a/tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.xml b/tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.xml new file mode 100644 index 0000000000..da2fc59da3 --- /dev/null +++ b/tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <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> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='sda' bus='virtio'/> + <product>ST3146707LC</product> + </disk> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='sdb' bus='scsi'/> + <vendor>SEAGATE</vendor> + <product>ST3567807GD</product> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='scsi' index='0' model='virtio-scsi'/> + <controller type='scsi' index='1' model='lsilogic'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxmlconfdata/disk-scsi-disk-vendor-build-error.x86_64-latest.err b/tests/qemuxmlconfdata/disk-scsi-disk-vendor-build-error.x86_64-latest.err new file mode 100644 index 0000000000..88bd9e5468 --- /dev/null +++ b/tests/qemuxmlconfdata/disk-scsi-disk-vendor-build-error.x86_64-latest.err @@ -0,0 +1 @@ +unsupported configuration: Only scsi disk supports vendor diff --git a/tests/qemuxmlconfdata/disk-scsi-disk-vpd-build-error.xml b/tests/qemuxmlconfdata/disk-scsi-disk-vendor-build-error.xml similarity index 100% rename from tests/qemuxmlconfdata/disk-scsi-disk-vpd-build-error.xml rename to tests/qemuxmlconfdata/disk-scsi-disk-vendor-build-error.xml diff --git a/tests/qemuxmlconfdata/disk-scsi-disk-vpd-build-error.x86_64-latest.err b/tests/qemuxmlconfdata/disk-scsi-disk-vpd-build-error.x86_64-latest.err deleted file mode 100644 index f70b7a774f..0000000000 --- a/tests/qemuxmlconfdata/disk-scsi-disk-vpd-build-error.x86_64-latest.err +++ /dev/null @@ -1 +0,0 @@ -unsupported configuration: Only scsi disk supports vendor and product diff --git a/tests/qemuxmlconfdata/disk-scsi-disk-vpd.x86_64-latest.args b/tests/qemuxmlconfdata/disk-scsi-disk-vpd.x86_64-latest.args index 4234a7e677..1d3aaf3819 100644 --- a/tests/qemuxmlconfdata/disk-scsi-disk-vpd.x86_64-latest.args +++ b/tests/qemuxmlconfdata/disk-scsi-disk-vpd.x86_64-latest.args @@ -30,9 +30,9 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -device '{"driver":"virtio-scsi-pci","id":"scsi0","bus":"pci.0","addr":"0x2"}' \ -device '{"driver":"lsi","id":"scsi1","bus":"pci.0","addr":"0x3"}' \ -blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-2-storage","read-only":true}' \ --device '{"driver":"scsi-cd","bus":"scsi0.0","channel":0,"scsi-id":0,"lun":0,"device_id":"drive-scsi0-0-0-0","drive":"libvirt-2-storage","id":"scsi0-0-0-0","vendor":"SEAGATE","product":"ST3146707LC"}' \ +-device '{"model":"ST3146707LC","driver":"ide-cd","bus":"ide.0","unit":0,"drive":"libvirt-2-storage","id":"ide0-0-0"}' \ -blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest2","node-name":"libvirt-1-storage","read-only":true}' \ --device '{"driver":"scsi-hd","bus":"scsi1.0","scsi-id":0,"device_id":"drive-scsi1-0-0","drive":"libvirt-1-storage","id":"scsi1-0-0","bootindex":1,"vendor":"SEA GATE","product":"ST67 807GD"}' \ +-device '{"product":"ST67 807GD","driver":"scsi-hd","bus":"scsi1.0","scsi-id":0,"device_id":"drive-scsi1-0-0","drive":"libvirt-1-storage","id":"scsi1-0-0","bootindex":1}' \ -audiodev '{"id":"audio1","driver":"none"}' \ -device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x4"}' \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ diff --git a/tests/qemuxmlconfdata/disk-scsi-disk-vpd.x86_64-latest.xml b/tests/qemuxmlconfdata/disk-scsi-disk-vpd.x86_64-latest.xml index 4b23fbfcfe..39148f6ce7 100644 --- a/tests/qemuxmlconfdata/disk-scsi-disk-vpd.x86_64-latest.xml +++ b/tests/qemuxmlconfdata/disk-scsi-disk-vpd.x86_64-latest.xml @@ -20,9 +20,8 @@ <disk type='block' device='cdrom'> <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='sda' bus='scsi'/> + <target dev='sda' bus='ide'/> <readonly/> - <vendor>SEAGATE</vendor> <product>ST3146707LC</product> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> @@ -31,7 +30,6 @@ <source dev='/dev/HostVG/QEMUGuest2'/> <target dev='sdb' bus='scsi'/> <readonly/> - <vendor>SEA GATE</vendor> <product>ST67 807GD</product> <address type='drive' controller='1' bus='0' target='0' unit='0'/> </disk> @@ -45,6 +43,9 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </controller> <controller type='pci' index='0' model='pci-root'/> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <audio id='1' type='none'/> diff --git a/tests/qemuxmlconfdata/disk-scsi-disk-vpd.xml b/tests/qemuxmlconfdata/disk-scsi-disk-vpd.xml index 36dd2a89ba..e3665d3afa 100644 --- a/tests/qemuxmlconfdata/disk-scsi-disk-vpd.xml +++ b/tests/qemuxmlconfdata/disk-scsi-disk-vpd.xml @@ -16,9 +16,8 @@ <emulator>/usr/bin/qemu-system-x86_64</emulator> <disk type='block' device='cdrom'> <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='sda' bus='scsi'/> + <target dev='sda' bus='ide'/> <readonly/> - <vendor>SEAGATE</vendor> <product>ST3146707LC</product> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> @@ -26,7 +25,6 @@ <source dev='/dev/HostVG/QEMUGuest2'/> <target dev='sdb' bus='scsi'/> <readonly/> - <vendor>SEA GATE</vendor> <product>ST67 807GD</product> <address type='drive' controller='1' bus='0' target='0' unit='0'/> </disk> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 21b56dc94e..083b0ab7f6 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -1620,7 +1620,8 @@ mymain(void) DO_TEST_CAPS_LATEST("disk-scsi-disk-split"); DO_TEST_CAPS_LATEST("disk-scsi-disk-wwn"); DO_TEST_CAPS_LATEST("disk-scsi-disk-vpd"); - DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-scsi-disk-vpd-build-error"); + DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-scsi-disk-vendor-build-error"); + DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-scsi-disk-product-build-error"); DO_TEST_CAPS_LATEST("controller-virtio-scsi"); DO_TEST_CAPS_LATEST("controller-scsi-auto"); DO_TEST_CAPS_LATEST("disk-sata-device"); -- 2.47.0

2 5

[libvirt PATCH] qemu_snapshot: allow reverting to external disk only snapshot
by Pavel Hrdina 29 Jan '25

29 Jan '25

When snapshot is created with disk-only flag it is always external snapshot without memory state. Historically when there was not support to revert external snapshots this produced error message. error: Failed to revert snapshot s1 error: internal error: Invalid target domain state 'disk-snapshot'. Refusing snapshot reversion Now we can simply consider this as reverting to offline snapshot as the possible damage to file system is already done at the point of snapshot creation. Resolves: https://issues.redhat.com/browse/RHEL-21549 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/qemu/qemu_snapshot.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 0cac0c4146..7964f70553 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2606,6 +2606,7 @@ qemuSnapshotRevert(virDomainObj *vm, case VIR_DOMAIN_SNAPSHOT_SHUTDOWN: case VIR_DOMAIN_SNAPSHOT_SHUTOFF: case VIR_DOMAIN_SNAPSHOT_CRASHED: + case VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT: ret = qemuSnapshotRevertInactive(vm, snapshot, snap, driver, cfg, &inactiveConfig, @@ -2617,8 +2618,6 @@ qemuSnapshotRevert(virDomainObj *vm, _("qemu doesn't support reversion of snapshot taken in PMSUSPENDED state")); goto endjob; - case VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT: - /* Rejected earlier as an external snapshot */ case VIR_DOMAIN_SNAPSHOT_NOSTATE: case VIR_DOMAIN_SNAPSHOT_BLOCKED: case VIR_DOMAIN_SNAPSHOT_LAST: -- 2.43.0

2 1

[PATCH pushed 0/3] Patches from MR
by Peter Krempa 29 Jan '25

29 Jan '25

I've reviewed and fixed some patches from MR's: Assaf Admi (1): docs: Document that 'vcpu.delay' statistic is in nanoseconds https://gitlab.com/libvirt/libvirt/-/merge_requests/352 Bronek Kozicki (1): tools: Move libvirt-guests lock file out of subsys directory https://gitlab.com/libvirt/libvirt/-/merge_requests/448 Philipp Schuster (1): virsh: pool-list: remove unnecessary spaces to align behavior https://gitlab.com/libvirt/libvirt/-/merge_requests/452 docs/manpages/virsh.rst | 2 +- src/libvirt-domain.c | 2 +- tools/libvirt-guests.sh.in | 2 +- tools/virsh-pool.c | 7 +------ 4 files changed, 4 insertions(+), 9 deletions(-) -- 2.48.1

1 3

[PATCH v2 0/2] qemu: Improve block I/O throttle API errors
by Fabian Leditzky 29 Jan '25

29 Jan '25

This is v2 of: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/PNHO… Changes since v1: - Report more accurate errors instead of suppressing the error message when the drive is empty. - Add check for this condition earlier in the API code. Fabian Leditzky (2): qemu: Check empty drives in block I/O throttle API qemu: Clarify block I/O throttle JSON parsing errors src/qemu/qemu_driver.c | 8 ++++++++ src/qemu/qemu_monitor_json.c | 11 +++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) -- 2.48.1

2 4

[PATCH] build: Bump minimul glib2 version to 2.66.0
by Peter Krempa 29 Jan '25

29 Jan '25

Per our supported platforms the minimum available versions are: CentOS Stream 9: 2.68.4 Debian 11: 2.66.8 Fedora 39: 2.78.6 openSUSE Leap 15.6: 2.78.6 Ubuntu 24.04: 2.72.4 FreeBSD: 2.80.5 Bump to 2.66 which is limited by Debian 11. While ideally we'd bump to 2.68 which would give us 'g_strv_builder' and friends 2.66 is enough for g_ptr_array_steal() which can be used to emulate the former with almost no extra code. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- libvirt.spec.in | 2 +- meson.build | 2 +- src/libvirt_private.syms | 4 -- src/qemu/qemu_agent.c | 2 +- src/qemu/qemu_monitor.c | 2 +- src/util/glibcompat.c | 94 ---------------------------------------- src/util/glibcompat.h | 18 -------- src/util/vireventglib.c | 12 ++--- 8 files changed, 10 insertions(+), 126 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index f397b95b79..c5585a72c0 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -357,7 +357,7 @@ BuildRequires: gcc %if %{with_libxl} BuildRequires: xen-devel %endif -BuildRequires: glib2-devel >= 2.58 +BuildRequires: glib2-devel >= 2.66 BuildRequires: libxml2-devel BuildRequires: readline-devel BuildRequires: pkgconfig(bash-completion) >= 2.0 diff --git a/meson.build b/meson.build index 1b0b717901..52fef8c0fb 100644 --- a/meson.build +++ b/meson.build @@ -1004,7 +1004,7 @@ else endif endif -glib_version = '2.58.0' +glib_version = '2.66.0' glib_dep = dependency('glib-2.0', version: '>=' + glib_version) gobject_dep = dependency('gobject-2.0', version: '>=' + glib_version) if host_machine.system() == 'windows' diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ee90fb2b84..2fe0a07944 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1879,10 +1879,6 @@ virStorageSourceUpdatePhysicalSize; # util/glibcompat.h -vir_g_fsync; -vir_g_source_unref; -vir_g_strdup_printf; -vir_g_strdup_vprintf; vir_g_string_replace; diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 22359f8518..43fca86f10 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -448,7 +448,7 @@ qemuAgentUnregister(qemuAgent *agent) { if (agent->watch) { g_source_destroy(agent->watch); - vir_g_source_unref(agent->watch, agent->context); + g_source_unref(agent->watch); agent->watch = NULL; } } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index ec2f166785..e0b1bf1d37 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -745,7 +745,7 @@ qemuMonitorUnregister(qemuMonitor *mon) { if (mon->watch) { g_source_destroy(mon->watch); - vir_g_source_unref(mon->watch, mon->context); + g_source_unref(mon->watch); mon->watch = NULL; } } diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c index 98dcfab389..bcb666992a 100644 --- a/src/util/glibcompat.c +++ b/src/util/glibcompat.c @@ -63,100 +63,6 @@ #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdeprecated-declarations" -#undef g_fsync -#undef g_strdup_printf -#undef g_strdup_vprintf - - -/* Drop when min glib >= 2.63.0 */ -gint -vir_g_fsync(gint fd) -{ -#ifdef G_OS_WIN32 - return _commit(fd); -#else - return fsync(fd); -#endif -} - - -/* Due to a bug in glib, g_strdup_printf() nor g_strdup_vprintf() - * abort on OOM. It's fixed in glib's upstream. Provide our own - * implementation until the fix gets distributed. */ -char * -vir_g_strdup_printf(const char *msg, ...) -{ - va_list args; - char *ret; - va_start(args, msg); - ret = g_strdup_vprintf(msg, args); - if (!ret) - abort(); - va_end(args); - return ret; -} - - -char * -vir_g_strdup_vprintf(const char *msg, va_list args) -{ - char *ret; - ret = g_strdup_vprintf(msg, args); - if (!ret) - abort(); - return ret; -} - - -/* - * If the last reference to a GSource is released in a non-main - * thread we're exposed to a race condition that causes a - * crash: - * - * https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1358 - * - * Thus we're using an idle func to release our ref... - * - * ...but this imposes a significant performance penalty on - * I/O intensive workloads which are sensitive to the iterations - * of the event loop, so avoid the workaround if we know we have - * new enough glib. - * - * The function below is used from a header file definition. - * - * Drop when min glib >= 2.64.0 - */ -#if GLIB_CHECK_VERSION(2, 64, 0) -void vir_g_source_unref(GSource *src, GMainContext *ctx G_GNUC_UNUSED) -{ - g_source_unref(src); -} -#else - -static gboolean -virEventGLibSourceUnrefIdle(gpointer data) -{ - GSource *src = data; - - g_source_unref(src); - - return FALSE; -} - -void vir_g_source_unref(GSource *src, GMainContext *ctx) -{ - GSource *idle = g_idle_source_new(); - - g_source_set_callback(idle, virEventGLibSourceUnrefIdle, src, NULL); - - g_source_attach(idle, ctx); - - g_source_unref(idle); -} - -#endif - - /** * Adapted (to pass syntax check) from 'g_string_replace' from * glib-2.81.1. Drop once minimum glib is bumped to 2.68. diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h index 474ff95bc5..a3d01089e6 100644 --- a/src/util/glibcompat.h +++ b/src/util/glibcompat.h @@ -42,24 +42,6 @@ #endif /* GLib < 2.67.0 */ - -gint vir_g_fsync(gint fd); -char *vir_g_strdup_printf(const char *msg, ...) - G_GNUC_PRINTF(1, 2); -char *vir_g_strdup_vprintf(const char *msg, va_list args) - G_GNUC_PRINTF(1, 0); - -#if !GLIB_CHECK_VERSION(2, 64, 0) -# define g_strdup_printf vir_g_strdup_printf -# define g_strdup_vprintf vir_g_strdup_vprintf -#endif - -#undef g_fsync -#define g_fsync vir_g_fsync - -void vir_g_source_unref(GSource *src, GMainContext *ctx); - - /* Drop once we require glib-2.68 at minimum */ guint vir_g_string_replace(GString *string, diff --git a/src/util/vireventglib.c b/src/util/vireventglib.c index 023dc37445..6c54f62123 100644 --- a/src/util/vireventglib.c +++ b/src/util/vireventglib.c @@ -213,7 +213,7 @@ virEventGLibHandleUpdate(int watch, if (data->source != NULL) { VIR_DEBUG("Removed old handle source=%p", data->source); g_source_destroy(data->source); - vir_g_source_unref(data->source, NULL); + g_source_unref(data->source); } data->source = virEventGLibAddSocketWatch( @@ -227,7 +227,7 @@ virEventGLibHandleUpdate(int watch, VIR_DEBUG("Removed old handle source=%p", data->source); g_source_destroy(data->source); - vir_g_source_unref(data->source, NULL); + g_source_unref(data->source); data->source = NULL; data->events = 0; } @@ -276,7 +276,7 @@ virEventGLibHandleRemove(int watch) if (data->source != NULL) { g_source_destroy(data->source); - vir_g_source_unref(data->source, NULL); + g_source_unref(data->source); data->source = NULL; data->events = 0; } @@ -409,7 +409,7 @@ virEventGLibTimeoutUpdate(int timer, if (interval >= 0) { if (data->source != NULL) { g_source_destroy(data->source); - vir_g_source_unref(data->source, NULL); + g_source_unref(data->source); } data->interval = interval; @@ -419,7 +419,7 @@ virEventGLibTimeoutUpdate(int timer, goto cleanup; g_source_destroy(data->source); - vir_g_source_unref(data->source, NULL); + g_source_unref(data->source); data->source = NULL; } @@ -468,7 +468,7 @@ virEventGLibTimeoutRemove(int timer) if (data->source != NULL) { g_source_destroy(data->source); - vir_g_source_unref(data->source, NULL); + g_source_unref(data->source); data->source = NULL; } -- 2.48.1

2 2

[PATCH] secret: Check length of value in secret object
by Adam Julis 28 Jan '25

28 Jan '25

Ensure that the value in the secret object is validated not only for NULL but also for its size. An empty value may not always be NULL, if it has been manually deleted from the .base64 file. Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- src/conf/virsecretobj.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index 455798d414..3cb1ec2b4b 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -719,7 +719,7 @@ virSecretObjGetValue(virSecretObj *obj) virSecretDef *def = obj->def; unsigned char *ret = NULL; - if (!obj->value) { + if (!obj->value || (obj->value_size < 1 )) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(def->uuid, uuidstr); virReportError(VIR_ERR_NO_SECRET, -- 2.47.1

2 3

[PATCH] ch: Include unistd.h in ch_events.c
by Michal Privoznik 28 Jan '25

28 Jan '25

There's a call to read() in the file but corresponding include of unistd.h is missing causing a build failure. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Green pipeline: https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/1642845347 src/ch/ch_events.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c index ba5b247c83..1cce30836a 100644 --- a/src/ch/ch_events.c +++ b/src/ch/ch_events.c @@ -20,6 +20,8 @@ #include <config.h> +#include <unistd.h> + #include "ch_domain.h" #include "ch_events.h" #include "ch_process.h" -- 2.45.2

2 1

[PATCH 0/8] Enable virtio-mem-ccw
by Michal Privoznik 28 Jan '25

28 Jan '25

To test these you'll need: QEMU: v9.2.0-492-gaa910c20ec Kernel: v6.13-rc1~206^2~46^2~3 or newer. Michal Prívozník (8): qemu: Do NOT autoadd NUMA node for s390 qemu_command: Use qemuBuildVirtioDevProps() to build cmd line for virtio-mem and virtio-pmem qemuxmlconftest: Introduce memory-hotplug-virtio-mem-s390x.xml qemu_caps: Introduce QEMU_CAPS_DEVICE_VIRTIO_MEM_CCW qemu: Validate virtio-mem-ccw qemu: Allow virtio-mem on CCW qemuxmlconftest: Update memory-hotplug-virtio-mem-s390x.xml NEWS: Document virtio-mem-ccw NEWS.rst | 5 ++ src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 28 +++++++-- src/qemu/qemu_domain.c | 6 +- src/qemu/qemu_postparse.c | 1 + src/qemu/qemu_validate.c | 35 ++++++++++- .../caps_10.0.0_s390x.xml | 1 + ...hotplug-virtio-mem-s390x.s390x-latest.args | 39 ++++++++++++ ...-hotplug-virtio-mem-s390x.s390x-latest.xml | 60 +++++++++++++++++++ .../memory-hotplug-virtio-mem-s390x.xml | 57 ++++++++++++++++++ tests/qemuxmlconftest.c | 1 + 12 files changed, 227 insertions(+), 9 deletions(-) create mode 100644 tests/qemuxmlconfdata/memory-hotplug-virtio-mem-s390x.s390x-latest.args create mode 100644 tests/qemuxmlconfdata/memory-hotplug-virtio-mem-s390x.s390x-latest.xml create mode 100644 tests/qemuxmlconfdata/memory-hotplug-virtio-mem-s390x.xml -- 2.45.2

4 26

[PATCH 0/5] qemu: IO error reporting fixes and improvements (part 1?)
by Peter Krempa 27 Jan '25

27 Jan '25

This series: - fixed bug in lookup of devices for the IO error which made them useless (2/5) - adds framework to return error messages (3/5) - implements the passthrough of errors (4/5) - logs the I/O errors in the VM log (5/5) Peter Krempa (5): qemuProcessHandleIOError: Refactor to extract 'priv' instead of 'driver' qemuProcessHandleIOError: Fix logic for lookup of disk API: Document more possibilities for @reason field of VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON event qemu: monitor: Plumb in disk IO error message passthrough to VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON qemuProcessHandleIOError: Log IO errors in the VM log file include/libvirt/libvirt-domain.h | 8 +++-- src/qemu/qemu_monitor_json.c | 19 ++++++++++-- src/qemu/qemu_process.c | 51 ++++++++++++++++++-------------- 3 files changed, 51 insertions(+), 27 deletions(-) -- 2.48.1

2 11

[PATCH V2 00/20] qemu: support mapped-ram+directio+mulitfd
by Jim Fehlig 25 Jan '25

25 Jan '25

I'm claiming this to be V2 of a series to support QEMU's mapped-ram stream format [1] and migration capability, even though the approach to control the feature is different than V1. To control mapped-ram in V1, I bumped the save format version and added a 'features' field to the save image header. In V2, I use the save_image_format setting in qemu.conf, extending it with a 'sparse' option to enable mapped-ram. IMO, the latter is nicer approach that fits better with existing save options. V2 leaves the "raw" save image format as default. save_image_format must be set to "sparse" to enable use of mapped-ram. Like V1, this version of the series also includes patches that leverage mapped-ram to add support for parallel save/restore. In one of the previous threads on this topic we discussed per-VM control of save image format by adding a 'format' parameter to virDomainSaveParams. I can do that as a followup series if folks agree its useful. Changes in V2 * Use save_image_format setting to control mapped-ram instead of introducing a new field to the header and bumping the save image version * Rename save parameter parallel-connections to parallel-channels * Address other minor comments from V1 [1] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/MNB… Claudio Fontana (3): include: Define constants for parallel save/restore tools: add parallel parameter to virsh save command tools: add parallel parameter to virsh restore command Jim Fehlig (17): lib: virDomain{Save,Restore}Params: Ensure absolute path qemu_fd: Add function to retrieve fdset ID qemu: Add function to check capability in migration params qemu: Add function to get bool value from migration params qemu: Add mapped-ram migration capability qemu: Add function to get migration params for save qemu_saveimage: add "sparse" to supported save image formats qemu: Add helper function for creating save image fd qemu: Move declaration of virQEMUSaveFormat to header file qemu: Add support for mapped-ram on save qemu: Decompose qemuSaveImageOpen qemu: Move creation of qemuProcessIncomingDef struct qemu: Apply migration parameters in qemuMigrationDstRun qemu: Add support for mapped-ram on restore qemu: Support O_DIRECT with mapped-ram on save qemu: Support O_DIRECT with mapped-ram on restore qemu: Add support for parallel save and restore docs/manpages/virsh.rst | 21 ++- include/libvirt/libvirt-domain.h | 11 ++ src/libvirt-domain.c | 95 +++++++++++--- src/qemu/qemu.conf.in | 9 +- src/qemu/qemu_driver.c | 108 +++++++++++----- src/qemu/qemu_fd.c | 18 +++ src/qemu/qemu_fd.h | 3 + src/qemu/qemu_migration.c | 194 +++++++++++++++++++--------- src/qemu/qemu_migration.h | 10 +- src/qemu/qemu_migration_params.c | 92 ++++++++++++++ src/qemu/qemu_migration_params.h | 17 +++ src/qemu/qemu_monitor.c | 40 ++++++ src/qemu/qemu_monitor.h | 5 + src/qemu/qemu_process.c | 117 ++++++++++++----- src/qemu/qemu_process.h | 19 ++- src/qemu/qemu_saveimage.c | 211 ++++++++++++++++++------------- src/qemu/qemu_saveimage.h | 39 +++++- src/qemu/qemu_snapshot.c | 29 +++-- tools/virsh-domain.c | 79 ++++++++++-- 19 files changed, 856 insertions(+), 261 deletions(-) -- 2.43.0

1 21

[PATCH v5 0/5] ch: handle events from cloud-hypervisor
by Purna Pavan Chandra Aekkaladevi 25 Jan '25

25 Jan '25

changes from v4->v5: * Incase of invalid events, exit VirCHEventHandler * Open eventmonitor fifo (Reader end) in virCHMonitorNew itself. This way, we can clean exit incase open fails for some reason. changes from v3->v4: * Don't abort; instead kill the VM and exit * use g_clear_pointer along with g_free * Fix possible memory leak changes from v2->v3: * Remove patch 'utils: Implement virFileIsNamedPipe' as it is no more needed. * Remove the eventmonitorpath only if it exists * Added domain name as a prefix to logs from ch_events.c. This will make debugging easier. * Simplified event parsing logic by reserving a byte for null char. changes from v1->v2: * Rebase on latest master * Use /* */ for comments * Remove fifo file if already exists * Address other comments from Praveen Paladugu cloud-hypervisor raises various events, including VM lifecylce operations such as boot, shutdown, pause, resume, etc. Libvirt will now read these events and take the necessary actions, such as correctly updating the domain state. A FIFO file is passed to `--event-monitor` option of cloud-hypervisor. Libvirt creates a new thread that acts as the reader of the fifo file and continuously monitors for new events. Currently, shutdown events are handled by updating the domain state appropriately. Purna Pavan Chandra Aekkaladevi (5): ch: pass --event-monitor option to cloud-hypervisor ch: start a new thread for handling ch events ch: events: Read and parse cloud-hypervisor events ch: events: facilitate lifecycle events handling NEWS: Mention event handling support in ch driver NEWS.rst | 7 + po/POTFILES | 1 + src/ch/ch_events.c | 312 ++++++++++++++++++++++++++++++++++++++++++++ src/ch/ch_events.h | 54 ++++++++ src/ch/ch_monitor.c | 76 ++++++++++- src/ch/ch_monitor.h | 12 ++ src/ch/meson.build | 2 + 7 files changed, 458 insertions(+), 6 deletions(-) create mode 100644 src/ch/ch_events.c create mode 100644 src/ch/ch_events.h -- 2.34.1

3 8

[PATCH] qemu: Allow virDomainBlockJobSetSpeed during migration
by Jiri Denemark 25 Jan '25

25 Jan '25

When a migration with non-shared storage is started with VIR_MIGRATE_PARAM_BANDWIDTH set, it will be applied to both memory migration and each block job started for storage migration. Once the migration is running virDomainMigrateSetMaxSpeed may be used to change the bandwidth used by memory migration, but there was no way of changing storage migration speed. Let's allow virDomainBlockJobSetSpeed during migration to enable the missing functionality. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/conf/virdomainjob.h | 11 +++++++---- src/qemu/qemu_driver.c | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/conf/virdomainjob.h b/src/conf/virdomainjob.h index 8b2dccd298..d8685cf236 100644 --- a/src/conf/virdomainjob.h +++ b/src/conf/virdomainjob.h @@ -37,10 +37,13 @@ typedef enum { VIR_JOB_MODIFY, /* May change state */ VIR_JOB_ABORT, /* Abort current async job */ VIR_JOB_MIGRATION_OP, /* Operation influencing outgoing migration */ - VIR_JOB_MODIFY_MIGRATION_SAFE, /* Internal only job for event handlers which - need to be processed even during migration. - The code may only change state in a way - that does not affect migration. */ + VIR_JOB_MODIFY_MIGRATION_SAFE, /* Modify job that is safe to be run during + migration. The code may only change state + in a way that does not affect migration. + Used in event handlers which need to be + processed even during migration or for + setting block job speed when migrating + with non-shared storage. */ /* The following two items must always be the last items before JOB_LAST */ VIR_JOB_ASYNC, /* Asynchronous job */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d2eddbd9ae..a1fc61bae2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13982,7 +13982,7 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, if (virDomainBlockJobSetSpeedEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY_MIGRATION_SAFE) < 0) goto cleanup; if (virDomainObjCheckActive(vm) < 0) -- 2.48.1

2 1

[PATCH] ch: implement domainInterfaceAddresses
by Praveen K Paladugu 25 Jan '25

25 Jan '25

From: Anirudh Rayabharam <anrayabh(a)linux.microsoft.com> Implement domainInterfaceAddresses for the Cloud Hypervisor driver. Support VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE and VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP sources. Implementation is similar to other drivers. Signed-off-by: Anirudh Rayabharam <anrayabh(a)linux.microsoft.com> Signed-off-by: Praveen K Paladugu <praveenkpaladugu(a)gmail.com> --- src/ch/ch_driver.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c index 6a8da5f356..d284524337 100644 --- a/src/ch/ch_driver.c +++ b/src/ch/ch_driver.c @@ -2298,6 +2298,47 @@ chConnectDomainEventDeregisterAny(virConnectPtr conn, return 0; } +static int +chDomainInterfaceAddresses(virDomain *dom, + virDomainInterfacePtr **ifaces, + unsigned int source, + unsigned int flags) +{ + virDomainObj *vm = NULL; + int ret = -1; + + virCheckFlags(0, -1); + + if (!(vm = virCHDomainObjFromDomain(dom))) + goto cleanup; + + if (virDomainInterfaceAddressesEnsureACL(dom->conn, vm->def, source) < 0) + goto cleanup; + + if (virDomainObjCheckActive(vm) < 0) + goto cleanup; + + switch (source) { + case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE: + ret = virDomainNetDHCPInterfaces(vm->def, ifaces); + break; + + case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP: + ret = virDomainNetARPInterfaces(vm->def, ifaces); + break; + + default: + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _("Unsupported IP address data source %1$d"), + source); + break; + } + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + /* Function Tables */ static virHypervisorDriver chHypervisorDriver = { @@ -2316,6 +2357,7 @@ static virHypervisorDriver chHypervisorDriver = { .domainCreateXML = chDomainCreateXML, /* 7.5.0 */ .domainCreate = chDomainCreate, /* 7.5.0 */ .domainCreateWithFlags = chDomainCreateWithFlags, /* 7.5.0 */ + .domainInterfaceAddresses = chDomainInterfaceAddresses, /* 10.10.1 */ .domainShutdown = chDomainShutdown, /* 7.5.0 */ .domainShutdownFlags = chDomainShutdownFlags, /* 7.5.0 */ .domainReboot = chDomainReboot, /* 7.5.0 */ -- 2.47.0

2 3

[PATCH v2 0/3] qemu: Add support for avail-switchover-bandwidth migration parameter
by Jiri Denemark 25 Jan '25

25 Jan '25

See documentation in 1/3 for details. Jiri Denemark (3): Add a migration parameter for available bandwidth in switchover virsh migrate: Add --available-switchover-bandwidth option qemu: Add support for avail-switchover-bandwidth migration parameter docs/manpages/virsh.rst | 12 ++++++++++++ include/libvirt/libvirt-domain.h | 18 ++++++++++++++++++ src/qemu/qemu_migration.h | 1 + src/qemu/qemu_migration_params.c | 9 +++++++++ src/qemu/qemu_migration_params.h | 1 + tools/virsh-domain.c | 13 +++++++++++++ 6 files changed, 54 insertions(+) -- 2.48.1

2 4

[PATCH 0/3] Unify handling of directories used for loadable modules
by Andrea Bolognani 25 Jan '25

25 Jan '25

This solves the same problem as 7a8c2319acf2 but in a different, more generic fashion. Andrea Bolognani (3): meson: Unify handling of directories used for loadable modules rpm: Add missing Requires rpm: Unify handling of directories used for loadable modules libvirt.spec.in | 7 ++++--- meson.build | 5 +++++ src/locking/meson.build | 6 ++---- src/meson.build | 13 +++++++++++-- src/storage/meson.build | 2 -- src/storage_file/meson.build | 2 -- 6 files changed, 22 insertions(+), 13 deletions(-) -- 2.48.1

2 4