January 2025 - Devel - libvirt List Archives

[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é 29 Jan '25

29 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 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

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

29 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 28 Jan '25

28 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