- Devel - libvirt List Archives

[PATCH 0/4] qemu: Fix broken check rejecting virtiofs hotplug with 'bootindex' set
by Peter Krempa 31 May '24

31 May '24

See 3/4 Peter Krempa (4): syms: Properly export 'virDomainDeviceDefValidate' qemu: hotplug: Validate definition of 'FS' device after address allocation qemu: validate: Fix check for unsupported FS-device bootindex use on un-assigned addresses NEWS: Mention fix for broken 'fs' device bootindex support check NEWS.rst | 6 ++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_hotplug.c | 4 ++++ src/qemu/qemu_validate.c | 5 ++++- 4 files changed, 15 insertions(+), 1 deletion(-) -- 2.45.1

2 5

Entering freeze for libvirt-10.4.0
by Jiri Denemark 30 May '24

30 May '24

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

1 1

[PATCH] log_cleaner: Detect rotated filenames properly
by Michal Privoznik 30 May '24

30 May '24

When removing rotated log files, their name is matched against a regex (@log_regex) and if they contain '.N' suffix the 'N' is then parsed into an integer. Well, due to a bug in virLogCleanerParseFilename() this is not how the code works. If the suffix isn't found then g_match_info_fetch() returns an empty string instead of NULL which then makes str2int parsing fail. Just check for this case before parsing the string. Based on the original patch sent by David. Reported-by: David Negreira <david.negreira(a)canonical.com> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- The original patch was posted here: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/ENXJ… In the thread you can see me suggesting this alternative approach and David confirming it works. Therefore, I'd like to get this in before the release. src/logging/log_cleaner.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/logging/log_cleaner.c b/src/logging/log_cleaner.c index 4ee91843aa..4efcbc18e4 100644 --- a/src/logging/log_cleaner.c +++ b/src/logging/log_cleaner.c @@ -82,10 +82,8 @@ virLogCleanerParseFilename(const char *path, *rotated_index = 0; rotated_index_str = g_match_info_fetch(matchInfo, 3); - if (!rotated_index_str) - return chain_prefix; - - if (virStrToLong_i(rotated_index_str, NULL, 10, rotated_index) < 0) { + if (rotated_index_str && STRNEQ(rotated_index_str, "") && + virStrToLong_i(rotated_index_str, NULL, 10, rotated_index) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to parse rotated index from '%1$s'"), rotated_index_str); -- 2.44.1

2 1

[PATCH] NEWS: Document my contributions for upcoming release
by Michal Privoznik 30 May '24

30 May '24

These are either features/bugfixes I've worked on or participated in. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- NEWS.rst | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 14505116b1..57e8f0d0c3 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -45,6 +45,16 @@ v10.4.0 (unreleased) * **Improvements** + * qemu: add zstd to supported compression formats + + Extend the list of supported formats of QEMU save image by adding zstd + compression. + + * qemu: Implement support for hotplugging evdev input devices + + As of this release, hotplug and hotunplug of evdev ``<input/>`` devices is + supported. + * **Bug fixes** * virsh/virt-admin: Fix ``--help`` option for all commands @@ -70,6 +80,19 @@ v10.4.0 (unreleased) The bug was introduced in `v10.1.0 (2024-03-01)`_. + * qemu: Don't set affinity for isolcpus unless explicitly requested + + When starting a domain, by default libvirt sets affinity of QEMU process to + all online CPUs. This also included isolated CPUs (``isolcpus=``) which is + wrong. As of this release, isolated CPUs are left untouched, unless + explicitly configured in domain XML. + + * qemu_hotplug: Properly assign USB address to hotplugged usb-net device + + Previously, the network device hotplug logic would try to ensure only CCW + or PCI addresses. With recent support for the usb-net model, USB addresses + for usb-net network devices are assigned automatically. + v10.3.0 (2024-05-02) ==================== -- 2.44.1

2 1

[PATCH] qemu_hotplug: Clear QoS if required in qemuDomainChangeNet()
by Michal Privoznik 30 May '24

30 May '24

In one of my recent commits, I've introduced virDomainInterfaceClearQoS() which is a helper that either calls virNetDevBandwidthClear() ('tc' implementation) or virNetDevOpenvswitchInterfaceClearQos() (for ovs ifaces). But I made a micro optimization which leads to a bug: the function checks whether passed iface has any QoS set and returns early if it has none. In majority of cases this is right thing to do, but when removing QoS on virDomainUpdateDeviceFlags() this is problematic. The new definition (passed as argument to virDomainInterfaceClearQoS()) contains no QoS (because user requested its removal) and thus instead of removing the old QoS setting nothing is done. Fortunately, the fix is simple - pass olddev which contains the old QoS setting. Fixes: 812a146dfe784315edece43d09f8d9e432f8230e Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4739beead8..c98b0b5d52 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4071,7 +4071,7 @@ qemuDomainChangeNet(virQEMUDriver *driver, goto cleanup; } } else { - if (virDomainInterfaceClearQoS(vm->def, newdev) < 0) + if (virDomainInterfaceClearQoS(vm->def, olddev) < 0) goto cleanup; } -- 2.44.1

2 1

[PATCH v3 0/5] qemu: Introduce shared_filesystems configuration option
by Andrea Bolognani 29 May '24

29 May '24

The need to have something like this in the first place is driven by KubeVirt (see [1] and [2]). A draft version of this series has been integrated into KubeVirt and it has been confirmed that it was effective in removing the need to use LD_PRELOAD hacks in the storage provider. Changes from [v2]: * added canonicalization for user-provided paths; * fixed compilation issues when AppArmor support is enabled. Changes from [v1]: * documented more explicitly that the newly introduced option is intended for very specific scenarios and not general usage; as part of this, the NEWS update has been dropped too; * made a few tweaks and addressed a few oversight based on review feedback; * several preparatory cleanup patches have been pushed. Changes from [v0]: * reworked approach. [v2] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/XPCP… [v1] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/XEIS… [v0] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/MMKV… [1] https://issues.redhat.com/browse/CNV-34322 [2] https://issues.redhat.com/browse/CNV-39370 Andrea Bolognani (5): security: Fix alignment qemu: Introduce shared_filesystems configuration option qemu: Propagate shared_filesystems utils: Use overrides in virFileIsSharedFS() qemu: Always set labels for TPM state src/lxc/lxc_controller.c | 3 +- src/lxc/lxc_driver.c | 2 +- src/lxc/lxc_process.c | 4 +- src/qemu/libvirtd_qemu.aug | 3 ++ src/qemu/qemu.conf.in | 23 ++++++++ src/qemu/qemu_conf.c | 31 +++++++++++ src/qemu/qemu_conf.h | 2 + src/qemu/qemu_domain.c | 7 ++- src/qemu/qemu_extdevice.c | 2 +- src/qemu/qemu_migration.c | 23 ++++---- src/qemu/qemu_security.c | 85 +++++++++++++++++++++++------- src/qemu/qemu_tpm.c | 38 +++++++------ src/qemu/qemu_tpm.h | 10 ++-- src/qemu/test_libvirtd_qemu.aug.in | 5 ++ src/security/security_apparmor.c | 8 ++- src/security/security_dac.c | 47 +++++++++++++---- src/security/security_driver.h | 8 ++- src/security/security_manager.c | 33 +++++++++--- src/security/security_manager.h | 9 +++- src/security/security_nop.c | 5 ++ src/security/security_selinux.c | 56 +++++++++++++++----- src/security/security_stack.c | 32 ++++++++--- src/util/virfile.c | 53 +++++++++++++++++-- src/util/virfile.h | 3 +- tests/securityselinuxlabeltest.c | 2 +- tests/virfiletest.c | 2 +- 26 files changed, 389 insertions(+), 107 deletions(-) -- 2.44.0

3 20

[libvirt PATCH] qemu_snapshot: fix memory leak when reverting external snapshot
by Pavel Hrdina 29 May '24

29 May '24

The code cleaning up virStorageSource doesn't free data allocated by virStorageSourceInit() so we need to call virStorageSourceDeinit() explicitly. Fixes: 8e664737813378d2a1bdeacc2ca8e942327e2cab Resolves: https://issues.redhat.com/browse/RHEL-33044 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/qemu/qemu_snapshot.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 09ec959f10..f5260c4a22 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2260,6 +2260,8 @@ qemuSnapshotRevertExternalFinish(virDomainObj *vm, snapdisk->src->path); } + virStorageSourceDeinit(snapdisk->src); + virDomainSnapshotDiskDefClear(snapdisk); } @@ -2277,6 +2279,8 @@ qemuSnapshotRevertExternalFinish(virDomainObj *vm, VIR_WARN("Failed to remove snapshot image '%s'", snapdisk->src->path); } + + virStorageSourceDeinit(snapdisk->src); } } -- 2.45.1

2 1

[PATCH v2] run.in: Detect binaries in builddir properly
by Michal Privoznik 29 May '24

29 May '24

When attempting to run: libvirt.git/_build # ./run --selinux ./src/libvirtd the following error is thrown: Refusing to change selinux context of file './src/libvirtd' outside build directory which is obviously wrong. The problem is 'being inside of build directory' is detected by simple progpath.startswith(builddir). While builddir is an absolute path, progpath isn't necessarily. And while looking into the code, I've noticed chcon() function accessing variable outside its scope when printing out the path it's working on. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- v2 of: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/UZ6F… diff to v1: - error out if binary to run can't be identified (i.e. 'which' returns None). run.in | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/run.in b/run.in index 5b89b3dcd5..cada74dfcd 100644 --- a/run.in +++ b/run.in @@ -138,7 +138,7 @@ def change_unit(name, action): def chcon(path, user, role, type): - print("Setting file context of {} to u={}, r={}, t={}...".format(progpath, + print("Setting file context of {} to u={}, r={}, t={}...".format(path, user, role, type)) @@ -187,6 +187,10 @@ else: try: dorestorecon = False progpath = shutil.which(prog) + if not progpath: + raise Exception("Can't find executable {}" + .format(prog)) + progpath = os.path.abspath(progpath) if len(try_stop_units): print("Temporarily stopping systemd units...") -- 2.44.1

2 1

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

29 May '24

Provides completers for auth-type and source-format commands for virsh pool-create-as and pool-define-as commands. Use Empty completers for options where completions are not required. I left the ones where I was not sure if they need completers. Related Issue: https://gitlab.com/libvirt/libvirt/-/issues/9 Signed-off-by: Abhiram Tilak <atp.exp(a)gmail.com> --- src/libvirt_private.syms | 2 ++ tools/virsh-completer-pool.c | 48 +++++++++++++++++++++++++++++++++++- tools/virsh-completer-pool.h | 10 ++++++++ tools/virsh-pool.c | 8 ++++++ 4 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6b6bcc368a..5a413ca832 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1117,6 +1117,8 @@ virStorageAuthDefCopy; virStorageAuthDefFormat; virStorageAuthDefFree; virStorageAuthDefParse; +virStorageAuthTypeFromString; +virStorageAuthTypeToString; virStorageFileFeatureTypeFromString; virStorageFileFeatureTypeToString; virStorageFileFormatTypeFromString; diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c index 3568bb985b..4a771d894e 100644 --- a/tools/virsh-completer-pool.c +++ b/tools/virsh-completer-pool.c @@ -84,7 +84,6 @@ virshPoolEventNameCompleter(vshControl *ctl G_GNUC_UNUSED, return g_steal_pointer(&tmp); } - char ** virshPoolTypeCompleter(vshControl *ctl, const vshCmd *cmd, @@ -106,3 +105,50 @@ virshPoolTypeCompleter(vshControl *ctl, return virshCommaStringListComplete(type_str, (const char **)tmp); } + +char ** +virshPoolFormatCompleter(vshControl *ctl G_GNUC_UNUSED, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int flags) +{ + size_t i = 0, j = 0; + g_auto(GStrv) tmp = NULL; + size_t nformats = VIR_STORAGE_POOL_FS_LAST + VIR_STORAGE_POOL_NETFS_LAST + + VIR_STORAGE_POOL_DISK_LAST + VIR_STORAGE_POOL_LOGICAL_LAST; + + virCheckFlags(0, NULL); + + tmp = g_new0(char *, nformats + 1); + + /* Club all PoolFormats for completion */ + for (i = 0; i < VIR_STORAGE_POOL_FS_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatFileSystemTypeToString(i)); + + for (i = 0; i < VIR_STORAGE_POOL_NETFS_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatFileSystemNetTypeToString(i)); + + for (i = 0; i < VIR_STORAGE_POOL_DISK_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatDiskTypeToString(i)); + + for (i = 0; i < VIR_STORAGE_POOL_LOGICAL_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatLogicalTypeToString(i)); + + return g_steal_pointer(&tmp); +} + +char ** virshPoolAuthTypeCompleter(vshControl *ctl G_GNUC_UNUSED, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int flags) +{ + size_t i = 0; + g_auto(GStrv) tmp = NULL; + + virCheckFlags(0, NULL); + + tmp = g_new0(char *, VIR_STORAGE_AUTH_TYPE_LAST + 1); + + for (i = 0; i < VIR_STORAGE_AUTH_TYPE_LAST; i++) + tmp[i] = g_strdup(virStorageAuthTypeToString(i)); + + return g_steal_pointer(&tmp); +} diff --git a/tools/virsh-completer-pool.h b/tools/virsh-completer-pool.h index bff3e5742b..4a53f99577 100644 --- a/tools/virsh-completer-pool.h +++ b/tools/virsh-completer-pool.h @@ -40,3 +40,13 @@ char ** virshPoolTypeCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); + +char ** +virshPoolFormatCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + +char ** +virshPoolAuthTypeCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 67cc1b94cf..1a294bb0f6 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -81,26 +81,32 @@ }, \ {.name = "source-path", \ .type = VSH_OT_STRING, \ + .completer = virshCompletePathLocalExisting, \ .help = N_("source path for underlying storage") \ }, \ {.name = "source-dev", \ .type = VSH_OT_STRING, \ + .completer = virshCompleteEmpty, \ .help = N_("source device for underlying storage") \ }, \ {.name = "source-name", \ .type = VSH_OT_STRING, \ + .completer = virshCompleteEmpty, \ .help = N_("source name for underlying storage") \ }, \ {.name = "target", \ .type = VSH_OT_STRING, \ + .completer = virshCompleteEmpty, \ .help = N_("target for underlying storage") \ }, \ {.name = "source-format", \ .type = VSH_OT_STRING, \ + .completer = virshPoolFormatCompleter, \ .help = N_("format for underlying storage") \ }, \ {.name = "auth-type", \ .type = VSH_OT_STRING, \ + .completer = virshPoolAuthTypeCompleter, \ .help = N_("auth type to be used for underlying storage") \ }, \ {.name = "auth-username", \ @@ -118,6 +124,7 @@ }, \ {.name = "adapter-name", \ .type = VSH_OT_STRING, \ + .completer = virshCompleteEmpty, \ .help = N_("adapter name to be used for underlying storage") \ }, \ {.name = "adapter-wwnn", \ @@ -146,6 +153,7 @@ }, \ {.name = "source-protocol-ver", \ .type = VSH_OT_STRING, \ + .completer = virshCompleteEmpty, \ .help = N_("nfsvers value for NFS pool mount option") \ }, \ {.name = "source-initiator", \ -- 2.34.1

3 3

[PATCH] fix virLogCleanerParseFilename logic
by David Negreira 29 May '24

29 May '24

We should return the full filename path when we don't have a match on the third group of the regex. Signed-off-by: David Negreira <david.negreira(a)canonical.com> --- src/logging/log_cleaner.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/logging/log_cleaner.c b/src/logging/log_cleaner.c index 4ee91843aa..d8e6ce9cdd 100644 --- a/src/logging/log_cleaner.c +++ b/src/logging/log_cleaner.c @@ -82,7 +82,7 @@ virLogCleanerParseFilename(const char *path, *rotated_index = 0; rotated_index_str = g_match_info_fetch(matchInfo, 3); - if (!rotated_index_str) + if (rotated_index_str) return chain_prefix; if (virStrToLong_i(rotated_index_str, NULL, 10, rotated_index) < 0) { -- 2.40.1

2 8