January 2025 - Devel - libvirt List Archives

[PATCH] qemu: introduce load qemu.conf for "virt-qemu-run"
by Adam Julis 31 Jan '25

31 Jan '25

Adding a new option --config (or -c) for specifying a custom qemu.conf file. Previously, virt-qemu-run loaded default configuration values for QEMU via qemuStateInitialize(). The configuration was loaded from a temporary ../etc/ directory using virQEMUDriverConfigLoadFile(), and any qemu.conf file present in that directory was also loaded automatically. This patch allows users to specify a custom configuration file, which is copied into the temporary directory (or a permanent folder if the -r option is used) before loading the configuration. If an existing qemu.conf is present, it is properly backed up and restored in case of a permanent folder. The custom qemu.conf is always removed when the program exits. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/723 Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- docs/manpages/virt-qemu-run.rst | 9 +++ src/qemu/qemu_shim.c | 121 +++++++++++++++++++++++++++++++- 2 files changed, 129 insertions(+), 1 deletion(-) diff --git a/docs/manpages/virt-qemu-run.rst b/docs/manpages/virt-qemu-run.rst index 4d546ff8cc..ba1c90b52a 100644 --- a/docs/manpages/virt-qemu-run.rst +++ b/docs/manpages/virt-qemu-run.rst @@ -72,6 +72,15 @@ whose UUID should match a secret referenced in the guest domain XML. Display verbose information about startup. +``-c`` *QEMU-CONF-FILE*, +``--config``\ =\ *QEMU-CONF-FILE* + +Specify the QEMU configuration file to be used for starting the VM. +*QEMU-CONF-FILE* is the full path to the QEMU configuration file. + +If this parameter is omitted, the default configuration values will +be used. + ``-h``, ``--help`` Display the command line help. diff --git a/src/qemu/qemu_shim.c b/src/qemu/qemu_shim.c index 7fdd69b538..64c87ab264 100644 --- a/src/qemu/qemu_shim.c +++ b/src/qemu/qemu_shim.c @@ -23,6 +23,7 @@ #include <stdio.h> #include <stdbool.h> #include <unistd.h> +#include <fcntl.h> #include "virfile.h" #include "virgettext.h" @@ -132,6 +133,100 @@ qemuShimQuench(void *userData G_GNUC_UNUSED, { } +/* Load specific QEMU config file, is the -c option is used */ +static int +qemuAddConfigFile(const char *source_file, const char *root, bool *config_to_delete, long long deltams) +{ + int ret = -1; + struct stat st; + VIR_AUTOCLOSE srcFD = -1; + VIR_AUTOCLOSE dstFD = -1; + g_autofree char *config_dir = NULL; + g_autofree char *source_file_real = NULL; + g_autofree char *config_path_file = NULL; + g_autofree char *config_path_file_real = NULL; + + if ((srcFD = open(source_file, O_RDONLY)) < 0) { + g_printerr("Couldn't open specific config file\n"); + goto cleanup; + } + + if (fstat(srcFD, &st) != 0) { + g_printerr("Specific config file does not exist\n"); + goto cleanup; + } + if (!S_ISREG(st.st_mode)) { + g_printerr("Specific config is not a regular file\n"); + goto cleanup; + } + + /* Since source file exists, make the destination path or + * validate that it already exists */ + config_dir = g_strdup_printf("%s/etc/", root); + + if (g_mkdir_with_parents(config_dir, 0777) < 0) { + g_printerr("Couldn't make the directory for specific config file\n"); + goto cleanup; + } + + config_path_file = g_strdup_printf("%sqemu.conf", config_dir); + + /* If the source file is same as the destination file, no action needed */ + if ((source_file_real = realpath(source_file, NULL)) && + (config_path_file_real = realpath(config_path_file, NULL))) { + if (STREQ(source_file_real, config_path_file_real)) { + ret = 0; + goto cleanup; + } + } + + /* Check already existing qemu.conf in the subfolder, if so, renamed + * (appended via deltams constant - should be unique). Final cleanup + * at main() will revert this change */ + if (access(config_path_file, R_OK) == 0) { + if (rename(config_path_file, g_strdup_printf("%sqemu_old_%lld.conf", + config_dir, deltams)) != 0) { + g_printerr("Couldn't rename old config file, try delete it\n"); + goto cleanup; + } + } + + if ((dstFD = open(config_path_file, O_WRONLY | O_CREAT | O_TRUNC, 0644)) < 0) { + g_printerr("Couldn't open file for define specific config\n"); + goto cleanup; + } + + /* Set the flag for deleting the new/modified config in main() cleanup, + * for ensure that behaviour without the user config will be always + * consistent and take default value */ + *config_to_delete = true; + + do { + char buffer[1024]; + ssize_t readed = 0; + + readed = saferead(srcFD, buffer, 1024); + + if (readed < 0) { + g_printerr("Couldn't read from specific config\n"); + goto cleanup; + } else if (readed == 0) { + break; + } else { + ssize_t writed = safewrite(dstFD, buffer, readed); + if (writed != readed) { + g_printerr("Couldn't write to file for define specific config\n"); + goto cleanup; + } + } + } while (1); + + ret = 0; + + cleanup: + return ret; +} + int main(int argc, char **argv) { g_autoptr(virIdentity) sysident = NULL; @@ -142,8 +237,11 @@ int main(int argc, char **argv) g_autofree char *uri = NULL; g_autofree char *suri = NULL; const char *root = NULL; + const char *config = NULL; + long long delta_ms = 0; g_autofree char *escaped = NULL; bool tmproot = false; + bool config_to_delete = false; int ret = 1; g_autoptr(GError) error = NULL; g_auto(GStrv) secrets = NULL; @@ -156,6 +254,7 @@ int main(int argc, char **argv) { "root", 'r', 0, G_OPTION_ARG_STRING, &root, "Root directory", "DIR" }, { "debug", 'd', 0, G_OPTION_ARG_NONE, &debug, "Debug output", NULL }, { "verbose", 'v', 0, G_OPTION_ARG_NONE, &verbose, "Verbose output", NULL }, + { "config", 'c', 0, G_OPTION_ARG_STRING, &config, "Load specific QEMU configuration file", "QEMU-CONF-FILE"}, { 0 } }; int quitfd[2] = {-1, -1}; @@ -172,7 +271,7 @@ int main(int argc, char **argv) return 1; } - if (argc != 2) { + if (argc != 2 && argc != 3) { g_autofree char *help = g_option_context_get_help(ctx, TRUE, NULL); g_printerr("%s", help); return 1; @@ -234,6 +333,10 @@ int main(int argc, char **argv) goto cleanup; } + delta_ms = deltams(); + if (config && (qemuAddConfigFile(config, root, &config_to_delete, delta_ms) < 0)) + g_printerr("Specific config file was not loaded, default was used\n"); + escaped = g_uri_escape_string(root, NULL, true); virFileActivateDirOverrideForProg(argv[0]); @@ -402,6 +505,22 @@ int main(int argc, char **argv) VIR_FORCE_CLOSE(quitfd[0]); VIR_FORCE_CLOSE(quitfd[1]); + if (config_to_delete) { + g_autofree char *possible_old_file = g_strdup_printf("%s/etc/qemu_old_%lld.conf", + root, delta_ms); + g_autofree char *to_delete = g_strdup_printf("%s/etc/qemu.conf", root); + + if (remove(to_delete) != 0) + g_printerr("Deleting specific config failed, located in: %s\n", + to_delete); + + if (access(possible_old_file, R_OK) == 0) { + if (rename(possible_old_file, to_delete) != 0) + g_printerr("Renaming your old qemu.conf failed, ups, located in %s\n", + possible_old_file); + } + } + if (dom != NULL) virDomainFree(dom); if (sconn != NULL) -- 2.47.1

3 3

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

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

30 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é 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