March 2023 - Devel - libvirt List Archives

[PATCH 0/2] Fix clang build without numactl and without any code/optimization changes
by Martin Kletzander 20 Mar '23

20 Mar '23

See 1/2 for details, pipeline for this is here: https://gitlab.com/nertpinx/libvirt/-/pipelines/811802916 Martin Kletzander (2): util: Separate numactl and stubs into different source files Revert "meson: stop CLang doing inter-procedural analysis" meson.build | 20 -- po/POTFILES | 2 + src/util/meson.build | 10 + src/util/virnuma.c | 427 +------------------------------------ src/util/virnuma_common.h | 57 +++++ src/util/virnuma_numactl.c | 357 +++++++++++++++++++++++++++++++ src/util/virnuma_stubs.c | 124 +++++++++++ 7 files changed, 551 insertions(+), 446 deletions(-) create mode 100644 src/util/virnuma_common.h create mode 100644 src/util/virnuma_numactl.c create mode 100644 src/util/virnuma_stubs.c -- 2.40.0

2 4

[PATCH 1/1] capabilities: reduce scope in virCapabilitiesInitCaches()
by Shaleen Bathla 20 Mar '23

20 Mar '23

over-writing a variable in inner while-loop without freeing previous memory leaks it over time. To fix this, we can just change scope of bank variable to the inner loop. Signed-off-by: Shaleen Bathla <shaleen.bathla(a)oracle.com> --- src/conf/capabilities.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 34770fc4165f..9144ced4984f 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -2178,7 +2178,6 @@ virCapabilitiesInitCaches(virCaps *caps) int rv = -1; g_autoptr(DIR) dirp = NULL; g_autofree char *path = g_strdup_printf("%s/cpu/cpu%zd/cache/", SYSFS_SYSTEM_PATH, pos); - g_autoptr(virCapsHostCacheBank) bank = NULL; rv = virDirOpenIfExists(&dirp, path); if (rv < 0) @@ -2189,6 +2188,7 @@ virCapabilitiesInitCaches(virCaps *caps) while ((rv = virDirRead(dirp, &ent, path)) > 0) { g_autofree char *type = NULL; + g_autoptr(virCapsHostCacheBank) bank = NULL; int kernel_type; unsigned int level; int ret; -- 2.31.1

2 1

[libvirt PATCH 0/8] Slightly reduce the number of translatable strings
by Ján Tomko 19 Mar '23

19 Mar '23

*** BLURB HERE *** Ján Tomko (8): bhyve: fix typo in error message src: unify symlink creation error message bhyve: unify error message for UUID generation vbox: remove unreachable error in virVBoxSnapshotConfLoadVboxFile vbox: remove unreachable error in virVBoxSnapshotConfSaveVboxFile util: xml: remove function names from error messages Unify error message when namespaces are unsupported vbox: unify "unable to get hard disk id" message src/bhyve/bhyve_parse_command.c | 2 +- src/bhyve/bhyve_process.c | 2 +- src/libxl/libxl_driver.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_namespace.c | 2 +- src/util/virprocess.c | 6 +++--- src/util/virxml.c | 6 +++--- src/vbox/vbox_common.c | 4 ++-- src/vbox/vbox_snapshot_conf.c | 11 ----------- 10 files changed, 15 insertions(+), 26 deletions(-) -- 2.39.2

2 10

[libvirt PATCH] meson: stop CLang doing inter-procedural analysis
by Daniel P. Berrangé 17 Mar '23

17 Mar '23

The virNumaNodeIsAvailable function is stubbed out when building without libnuma, such that it just returns a constant value. When CLang is optimizing, it does inter-procedural analysis across function calls. When it sees that the call to virNumaNodeIsAvailable returns a fixed constant, it elides the conditional check for errors in the callers such as virNumaNodesetIsAvailable. This is a valid optimization as the C standard declares that there must only be one implementation of each function in a binary. This is normally the case, but ELF allows for function overrides when linking or at runtime with LD_PRELOAD, which is technically outside the mandated C language behaviour. So while CLang's optimization works fine at runtime, it breaks in our test suite which aims to mock the virNumaNodeIsAvailable function so that it has specific semantics regardless of whether libnuma is built or not. The return value check optimization though means our mock override won't have the right effect. The mock will be invoked, but its return value is not used. Potentially the same problem could be exhibited with GCC if certain combinations of optimizations are enabled, though thus far we've not seen it. To be robust on both CLang and GCC we need to make it more explicit that we want to be able to replace functions and thus optimization of calls must be limited. Currently we rely on 'noinline' which does succesfully prevent inlining of the function, but it cannot stop the eliding of checks based on the constant return value. Thus we need a bigger hammer. There are a couple of options to disable this optimization: * Annotate a symbol as 'weak'. This is tells the compiler that the symbol is intended to be overridable at linktime or runtime, and thus it will avoid doing inter-procedural analysis for optimizations. This was tried previously but have to be reverted as it had unintended consequences when linking .a files into our final .so, resulting in all the weak symbol impls being lost. See commit 407a281a8e2b6c5078ba1148535663ea64fd9314 * Annotate a symbol with 'noipa'. This tells the compiler to avoid inter-procedural analysis for calls to just this function. This wold be ideal match for our scenario, but unfortunately it is only implemented for GCC currently: https://reviews.llvm.org/D101011 * The '-fsemantic-interposition' argument tells the optimizer that any functions may be replaced with alternative implementations that have different semantics. It thus blocks any optimizations across function calls. This is quite a harsh block on the optimizer, but it appears to be the only one that is viable with CLang. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- meson.build | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/meson.build b/meson.build index 319ed790f9..c35823a79a 100644 --- a/meson.build +++ b/meson.build @@ -404,6 +404,26 @@ cc_flags += [ '-Wwrite-strings', ] +if cc.get_id() == 'clang' + # Stop CLang from doing inter-procedural analysis of calls + # between functions in the same compilation unit. Such an + # optimization has been know to break the test suite by + # making assumptions that a return value is a constant. + # This makes it impossible to mock certain functions with + # replacement definitions via LD_PRELOAD that have different + # semantics. + # + # This is a bit of a big hammer, but alternatives don't work: + # + # - 'weak' attribute - weak symbols get dropped from + # when the .a libs are combined into the .so + # see commit 407a281a8e2b6c5078ba1148535663ea64fd9314 + # + # - 'noipa' attribute - only available with GCC currently + # https://reviews.llvm.org/D101011 + cc_flags += [ '-fsemantic-interposition' ] +endif + supported_cc_flags = [] if get_option('warning_level') == '2' supported_cc_flags = cc.get_supported_arguments(cc_flags) -- 2.39.2

3 2

[libvirt PATCH] manpages: fix secret injection example for SEV
by Daniel P. Berrangé 17 Mar '23

17 Mar '23

The --disk-password argument was present in early impls of the patch but replaced by the more generic --inject-secret argument. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- docs/manpages/virt-qemu-sev-validate.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/manpages/virt-qemu-sev-validate.rst b/docs/manpages/virt-qemu-sev-validate.rst index b1392e0a50..617ef0a57e 100644 --- a/docs/manpages/virt-qemu-sev-validate.rst +++ b/docs/manpages/virt-qemu-sev-validate.rst @@ -316,7 +316,7 @@ inject a disk password on success: --api-minor 24 \ --build-id 13 \ --policy 3 \ - --disk-password passwd.txt \ + --inject-secret 736869e5-84f0-4973-92ec-06879ce3da0b:passwd.txt \ --secret-header secret-header.b64 \ --secret-payload secret-payload.b64 @@ -393,7 +393,7 @@ inject a disk password on success: --firmware OVMF.sev.fd \ --tk this-guest-tk.bin \ --domain fedora34x86_64 \ - --disk-password passwd.txt + --inject-secret 736869e5-84f0-4973-92ec-06879ce3da0b:passwd.txt Fetch from local libvirt ------------------------ @@ -454,7 +454,7 @@ inject a disk password on success: --insecure \ --tk this-guest-tk.bin \ --domain fedora34x86_64 \ - --disk-password passwd.txt + --inject-secret 736869e5-84f0-4973-92ec-06879ce3da0b:passwd.txt COMMON MISTAKES CHECKLIST ========================= -- 2.39.2

3 2

[libvirt PATCH] rpc: fix typo in admin code generation
by Ján Tomko 17 Mar '23

17 Mar '23

An extra '&' introduced a crash. https://bugzilla.redhat.com/show_bug.cgi?id=2178866 Fixes: 778c3004609ede0a9df4cf3e01c031047530efb7 Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- src/rpc/gendispatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index b186849606..704c133215 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -1812,7 +1812,7 @@ elsif ($mode eq "client") { } if ($structprefix eq "admin") { - print " VIR_LOCK_GUARD lock = virObjectLockGuard(&priv);\n"; + print " VIR_LOCK_GUARD lock = virObjectLockGuard(priv);\n"; } else { print " VIR_LOCK_GUARD lock = remoteDriverLock(priv);\n"; } -- 2.39.2

2 1

[libvirt PATCH] tests: qemublocktest: fix memory leak
by Ján Tomko 17 Mar '23

17 Mar '23

Set enccount to 1, so the cleanup function knows there is something to be cleaned up. Fixes: 1992ae40fac90c315d0d8d1a9c6f880bd0a39b57 Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- Pushed as a trivial build fix. tests/qemublocktest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 2d790e2b2e..9a968477d7 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -242,6 +242,7 @@ testQemuDiskXMLToJSONFakeSecrets(virStorageSource *src) srcpriv->encinfo[0]->alias = g_strdup_printf("%s-encalias", NULLSTR(src->nodeformat)); + srcpriv->enccount = 1; } return 0; -- 2.39.2

1 0

[libvirt PATCH] qemu: use correct formatting string for size_t
by Ján Tomko 17 Mar '23

17 Mar '23

Otherwise the build on armv7l breaks: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘size_t’ {aka ‘unsigned int’} [-Werror=format=] Fixes: 1992ae40fac90c315d0d8d1a9c6f880bd0a39b57 Fixes: e239f7d0a86ebddf9aab3f8c8e6b6e66351485b2 Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- Pushed as a build breaker fix. src/qemu/qemu_alias.c | 4 ++-- src/qemu/qemu_domain.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index d7d9be1164..85db7fbfe3 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -824,8 +824,8 @@ qemuAliasForSecret(const char *parentalias, size_t secret_idx) { if (obj) - return g_strdup_printf("%s-%s-secret%lu", parentalias, obj, secret_idx); - return g_strdup_printf("%s-secret%lu", parentalias, secret_idx); + return g_strdup_printf("%s-%s-secret%zu", parentalias, obj, secret_idx); + return g_strdup_printf("%s-secret%zu", parentalias, secret_idx); } /* qemuAliasTLSObjFromSrcAlias diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 942e5843c2..0a7b30fa65 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2030,7 +2030,7 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, if (!(encalias = virXMLPropString(encnodes[i], "alias"))) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("missing alias on encryption secret #%lu"), i); + _("missing alias on encryption secret #%zu"), i); return -1; } -- 2.39.2

1 0

[libvirt PATCH 0/4] qemu: support host-phys-bits-limit
by Ján Tomko 16 Mar '23

16 Mar '23

Ján Tomko (4): conf: cpu: add limit for maxphysaddr qemu: add support for setting host-phys-bits-limit qemu: do not require bits for maxphysaddr mode emulate qemu: allow forcing emulated maxphysaddr docs/formatdomain.rst | 10 ++++-- src/conf/cpu_conf.c | 7 ++++ src/conf/cpu_conf.h | 1 + src/conf/schemas/cputypes.rng | 5 +++ src/qemu/qemu_command.c | 8 ++++- src/qemu/qemu_validate.c | 7 ---- .../cpu-phys-bits-emulate-bare.xml | 20 +++++++++++ .../cpu-phys-bits-emulate3.err | 1 - .../cpu-phys-bits-emulate3.xml | 20 ----------- .../cpu-phys-bits-limit.x86_64-latest.args | 34 +++++++++++++++++++ .../qemuxml2argvdata/cpu-phys-bits-limit.xml | 20 +++++++++++ tests/qemuxml2argvtest.c | 3 +- ...u-phys-bits-emulate-bare.x86_64-latest.xml | 31 +++++++++++++++++ .../cpu-phys-bits-limit.x86_64-latest.xml | 31 +++++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 15 files changed, 169 insertions(+), 32 deletions(-) create mode 100644 tests/qemuxml2argvdata/cpu-phys-bits-emulate-bare.xml delete mode 100644 tests/qemuxml2argvdata/cpu-phys-bits-emulate3.err delete mode 100644 tests/qemuxml2argvdata/cpu-phys-bits-emulate3.xml create mode 100644 tests/qemuxml2argvdata/cpu-phys-bits-limit.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/cpu-phys-bits-limit.xml create mode 100644 tests/qemuxml2xmloutdata/cpu-phys-bits-emulate-bare.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/cpu-phys-bits-limit.x86_64-latest.xml -- 2.39.2

2 9

[PATCH v2 0/7] qemu: add support for librbd layered encryption
by Or Ozeri 16 Mar '23

16 Mar '23

v2: - add luks-any commits (including nit fixes) - removed qemu 8.0.0 replies commit - remove tautological if condition in qemuBlockStorageSourceAttachData initialization - add comments on validation of a single secret in qemu encryption engine - fix leak of qemuDomainStorageSourcePrivate->encinfo - remove ctxt->node modification in privatedata xml parsing - add test to modern-in.xml - squash commit #6 - add validation for a single secret in sd card disk Or Ozeri (7): qemu: capabilities: Introduce QEMU_CAPS_RBD_ENCRYPTION_LAYERING capability qemu: add support for multiple secret aliases qemu: add multi-secret support in qemuBlockStorageSourceAttachData qemu: add multi-secret support in _qemuDomainStorageSourcePrivate qemu: add support for librbd layered encryption qemu: capabilities: Introduce QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY capability qemu: add luks-any encryption support for RBD images docs/formatstorageencryption.rst | 20 ++- src/conf/schemas/storagecommon.rng | 5 +- src/conf/storage_encryption_conf.c | 2 +- src/conf/storage_encryption_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_alias.c | 8 +- src/qemu/qemu_alias.h | 3 +- src/qemu/qemu_block.c | 77 ++++++++--- src/qemu/qemu_block.h | 5 +- src/qemu/qemu_blockjob.c | 6 + src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 29 +++- src/qemu/qemu_domain.c | 130 +++++++++++++++--- src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_migration_params.c | 2 +- src/qemu/qemu_validate.c | 8 ++ tests/qemublocktest.c | 7 +- .../caps_8.0.0.x86_64.xml | 2 + tests/qemustatusxml2xmldata/modern-in.xml | 14 ++ ...k-rbd-encryption-layering.x86_64-7.2.0.err | 1 + ...rbd-encryption-layering.x86_64-latest.args | 39 ++++++ .../disk-network-rbd-encryption-layering.xml | 41 ++++++ ...k-rbd-encryption-luks-any.x86_64-7.2.0.err | 1 + ...rbd-encryption-luks-any.x86_64-latest.args | 37 +++++ .../disk-network-rbd-encryption-luks-any.xml | 39 ++++++ tests/qemuxml2argvtest.c | 4 + ...-rbd-encryption-layering.x86_64-latest.xml | 46 +++++++ ...-rbd-encryption-luks-any.x86_64-latest.xml | 44 ++++++ tests/qemuxml2xmltest.c | 2 + 31 files changed, 517 insertions(+), 68 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption-layering.x86_64-7.2.0.err create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption-layering.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption-layering.xml create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption-luks-any.x86_64-7.2.0.err create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption-luks-any.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption-luks-any.xml create mode 100644 tests/qemuxml2xmloutdata/disk-network-rbd-encryption-layering.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/disk-network-rbd-encryption-luks-any.x86_64-latest.xml -- 2.25.1

2 8