- Devel - libvirt List Archives

[PATCH] viridentity: Fix ref/unref imbalance in VIR_IDENTITY_AUTORESTORE
by Michal Privoznik 17 May '21

17 May '21

The basic use case of VIR_IDENTITY_AUTORESTORE() is in conjunction with virIdentityElevateCurrent(). What happens is that virIdentityElevateCurrent() gets current identity (which increases the refcounter of thread local virIdentity object) and returns a pointer to it. Later, when the variable goes out of scope the virIdentityRestoreHelper() is called which calls virIdentitySetCurrent() over the old identity. But this means that the refcounter is increased again. Therefore, we have to explicitly decrease the refcounter by calling g_object_unref(). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- I've observed this imbalance whilst running qemuxml2argvtest under valgrind: ==10412== 949 (40 direct, 909 indirect) bytes in 1 blocks are definitely lost in loss record 524 of 539 ==10412== at 0x48397EF: malloc (vg_replace_malloc.c:307) ==10412== by 0x50806F8: g_malloc (in /usr/lib64/libglib-2.0.so.0.6600.7) ==10412== by 0x50992FD: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.6600.7) ==10412== by 0x50999B9: g_slice_alloc0 (in /usr/lib64/libglib-2.0.so.0.6600.7) ==10412== by 0x518D9AB: g_type_create_instance (in /usr/lib64/libgobject-2.0.so.0.6600.7) ==10412== by 0x51739DC: g_object_new_internal (in /usr/lib64/libgobject-2.0.so.0.6600.7) ==10412== by 0x5174F5C: g_object_new_with_properties (in /usr/lib64/libgobject-2.0.so.0.6600.7) ==10412== by 0x5175978: g_object_new (in /usr/lib64/libgobject-2.0.so.0.6600.7) ==10412== by 0x496C3C6: virIdentityNew (viridentity.c:407) ==10412== by 0x496BF1A: virIdentityGetSystem (viridentity.c:318) ==10412== by 0x117CEF: testCompareXMLToArgv (qemuxml2argvtest.c:653) ==10412== by 0x148505: virTestRun (testutils.c:142) Test #315 doesn't tickle the bug, while test #316 does. src/util/viridentity.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util/viridentity.c b/src/util/viridentity.c index e7e5c31241..eb77f69e2e 100644 --- a/src/util/viridentity.c +++ b/src/util/viridentity.c @@ -197,8 +197,12 @@ void virIdentityRestoreHelper(virIdentity **identptr) { virIdentity *ident = *identptr; - if (ident != NULL) + if (ident != NULL) { virIdentitySetCurrent(ident); + /* virIdentitySetCurrent() grabs its own reference. + * We don't need ours anymore. */ + g_object_unref(ident); + } } #define TOKEN_BYTES 16 -- 2.26.3

2 1

[PATCH 0/4] Couple of virnuma cleanups
by Michal Privoznik 17 May '21

17 May '21

I'm looking into NUMA code in capabilities and came up with a couple of cleanups. Technically, 3/4 is not needed yet, but I'll be introducing new data to vircaps2xmltest where a NUMA node has no CPUs and that's why the patch is needed. Michal Prívozník (4): numa_conf: Use virXMLFormatElement() in virDomainNumaDefFormatXML virnuma: Export virNumaGetMaxCPUs properly virnumamock: Allow CPU-less NUMA nodes virCapabilitiesHostNUMAFormat: Bring variables into loops src/conf/capabilities.c | 9 ++-- src/conf/numa_conf.c | 107 ++++++++++++++++++--------------------- src/libvirt_private.syms | 1 + src/util/virnuma.h | 2 +- tests/virnumamock.c | 7 ++- 5 files changed, 61 insertions(+), 65 deletions(-) -- 2.26.3

3 7

[PATCH v1 5/7] docs: mark intention to deprecate TCG tracing functionality
by Alex Bennée 17 May '21

17 May '21

Currently attempts to add a new TCG trace events results in failures to build. Previous discussions have suggested maybe it's time to mark the feature as deprecated and push people towards using plugins. Signed-off-by: Alex Bennée <alex.bennee(a)linaro.org> Cc: Luis Vilanova <vilanova(a)imperial.ac.uk> Cc: Stefan Hajnoczi <stefanha(a)redhat.com> --- docs/devel/tcg-plugins.rst | 2 ++ docs/devel/tracing.rst | 7 +++++++ docs/system/deprecated.rst | 13 +++++++++++++ 3 files changed, 22 insertions(+) diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst index 18c6581d85..edf04e3091 100644 --- a/docs/devel/tcg-plugins.rst +++ b/docs/devel/tcg-plugins.rst @@ -3,6 +3,8 @@ Copyright (c) 2019, Linaro Limited Written by Emilio Cota and Alex Bennée +.. _tcgplugin-ref: + ================ QEMU TCG Plugins ================ diff --git a/docs/devel/tracing.rst b/docs/devel/tracing.rst index ba83954899..6b0f46cd54 100644 --- a/docs/devel/tracing.rst +++ b/docs/devel/tracing.rst @@ -414,6 +414,13 @@ disabled, this check will have no performance impact. "tcg" ----- +.. warning:: + The ability to add new TCG trace points relies on a having a good + understanding of the TCG internals. In the meantime TCG plugins + have been introduced which solve many of the same problems with + more of a focus on analysing guest code. See :ref:`tcgplugin-ref` + for more details. + Guest code generated by TCG can be traced by defining an event with the "tcg" event property. Internally, this property generates two events: "<eventname>_trans" to trace the event at translation time, and diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 80cae86252..0c9d3c1e1e 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -312,6 +312,19 @@ The ``I7200`` guest CPU relies on the nanoMIPS ISA, which is deprecated (the ISA has never been upstreamed to a compiler toolchain). Therefore this CPU is also deprecated. +TCG introspection features +-------------------------- + +TCG trace-events (since 6.1) +'''''''''''''''''''''''''''' + +The ability to add new TCG trace points has bit rotted and as the +feature can be replicated with TCG plugins it will be deprecated. If +any user is currently using this feature and needs help with +converting to using TCG plugins they should contact the qemu-devel +mailing list. + + Related binaries ---------------- -- 2.20.1

3 6

[PATCH] driver: Don't leak saved error in virGetConnectGeneric()
by Michal Privoznik 17 May '21

17 May '21

Recently, a new code was added to virGetConnectGeneric() that saves the original error into a variable so that it's not lost in virConnectClose() called under the 'error' label. However, the error saving code uses virSaveLastError() + virSetError() combo which leaks the memory allocated for the error copy. Using virErrorPreserveLast() + virErrorRestore() does the same job without the memleak. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/driver.c b/src/driver.c index 227bb56e48..329d493a50 100644 --- a/src/driver.c +++ b/src/driver.c @@ -138,7 +138,7 @@ static virConnectPtr virGetConnectGeneric(virThreadLocal *threadPtr, const char *name) { virConnectPtr conn; - virErrorPtr saved; + virErrorPtr orig_err; if (virConnectCacheInitialize() < 0) return NULL; @@ -178,9 +178,9 @@ virGetConnectGeneric(virThreadLocal *threadPtr, const char *name) return conn; error: - saved = virSaveLastError(); + virErrorPreserveLast(&orig_err); virConnectClose(conn); - virSetError(saved); + virErrorRestore(&orig_err); return NULL; } -- 2.26.3

2 1

[PATCH 0/4] tests: qemucapabilities: Add data fro the 6.1 cycle
by Peter Krempa 17 May '21

17 May '21

QEMU already committed some significant changes since the tree opened. The most notable which has fallout in libvirt is the dropping of the 'sheepdog' driver. This series adapts to that and then adds the qemu capabilities data for this cycle based on the most recent qemu upstream commit. Patch 4 is heavily truncated. To fetch the full version please: git fetch https://gitlab.com/pipo.sk/libvirt.git qemu-caps-6.1 Peter Krempa (4): testQemuInfoSetArgs: Strip default machine alias only for 'latest' test cases qemublocktest: Drop 'network-sheepdog-qcow2' image creation test case qemuxml2argvtest: Limit 'disk-network-sheepdog' testcase to qemu-6.0.0 tests: qemucapabilities: Add test-data for the qemu-6.1 cycle .../domaincapsdata/qemu_6.1.0-q35.x86_64.xml | 208 + .../domaincapsdata/qemu_6.1.0-tcg.x86_64.xml | 211 + tests/domaincapsdata/qemu_6.1.0.x86_64.xml | 208 + tests/qemublocktest.c | 1 - .../imagecreate/network-sheepdog-qcow2.json | 20 - .../imagecreate/network-sheepdog-qcow2.xml | 12 - .../caps_6.1.0.x86_64.replies | 32734 ++++++++++++++++ .../caps_6.1.0.x86_64.xml | 3339 ++ .../cpu-tsc-high-frequency.x86_64-latest.args | 2 +- ...> disk-network-sheepdog.x86_64-6.0.0.args} | 2 +- .../hugepages-memaccess3.x86_64-latest.args | 2 +- tests/qemuxml2argvtest.c | 2 +- tests/testutilsqemu.c | 6 +- 13 files changed, 36707 insertions(+), 40 deletions(-) create mode 100644 tests/domaincapsdata/qemu_6.1.0-q35.x86_64.xml create mode 100644 tests/domaincapsdata/qemu_6.1.0-tcg.x86_64.xml create mode 100644 tests/domaincapsdata/qemu_6.1.0.x86_64.xml delete mode 100644 tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json delete mode 100644 tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.xml create mode 100644 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.replies create mode 100644 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml rename tests/qemuxml2argvdata/{disk-network-sheepdog.x86_64-latest.args => disk-network-sheepdog.x86_64-6.0.0.args} (95%) -- 2.30.2

3 6

[PATCH] Delete forward declaration in storage_backend_sheepdog.c.
by peili 17 May '21

17 May '21

--- src/storage/storage_backend_sheepdog.c | 99 +++++++++++++------------- 1 file changed, 48 insertions(+), 51 deletions(-) diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 6490dfae52..6b000ff9b8 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -35,11 +35,54 @@ #define VIR_FROM_THIS VIR_FROM_STORAGE -static int virStorageBackendSheepdogRefreshVol(virStoragePoolObj *pool, - virStorageVolDef *vol); +static void +virStorageBackendSheepdogAddHostArg(virCommand *cmd, + virStoragePoolObj *pool) +{ + virStoragePoolDef *def = virStoragePoolObjGetDef(pool); + const char *address = "localhost"; + int port = 7000; + if (def->source.nhost > 0) { + if (def->source.hosts[0].name != NULL) + address = def->source.hosts[0].name; + if (def->source.hosts[0].port) + port = def->source.hosts[0].port; + } + virCommandAddArg(cmd, "-a"); + virCommandAddArgFormat(cmd, "%s", address); + virCommandAddArg(cmd, "-p"); + virCommandAddArgFormat(cmd, "%d", port); +} + + +static int +virStorageBackendSheepdogRefreshVol(virStoragePoolObj *pool, + virStorageVolDef *vol) +{ + char *output = NULL; + virStoragePoolDef *def = virStoragePoolObjGetDef(pool); + g_autoptr(virCommand) cmd = NULL; + + cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "list", vol->name, "-r", NULL); + virStorageBackendSheepdogAddHostArg(cmd, pool); + virCommandSetOutputBuffer(cmd, &output); + if (virCommandRun(cmd, NULL) < 0) + return -1; + + if (virStorageBackendSheepdogParseVdiList(vol, output) < 0) + return -1; + + vol->type = VIR_STORAGE_VOL_NETWORK; + + VIR_FREE(vol->key); + vol->key = g_strdup_printf("%s/%s", def->source.name, vol->name); + + VIR_FREE(vol->target.path); + vol->target.path = g_strdup(vol->name); + + return 0; +} -void virStorageBackendSheepdogAddHostArg(virCommand *cmd, - virStoragePoolObj *pool); int virStorageBackendSheepdogParseNodeInfo(virStoragePoolDef *pool, @@ -88,24 +131,6 @@ virStorageBackendSheepdogParseNodeInfo(virStoragePoolDef *pool, return -1; } -void -virStorageBackendSheepdogAddHostArg(virCommand *cmd, - virStoragePoolObj *pool) -{ - virStoragePoolDef *def = virStoragePoolObjGetDef(pool); - const char *address = "localhost"; - int port = 7000; - if (def->source.nhost > 0) { - if (def->source.hosts[0].name != NULL) - address = def->source.hosts[0].name; - if (def->source.hosts[0].port) - port = def->source.hosts[0].port; - } - virCommandAddArg(cmd, "-a"); - virCommandAddArgFormat(cmd, "%s", address); - virCommandAddArg(cmd, "-p"); - virCommandAddArgFormat(cmd, "%d", port); -} static int virStorageBackendSheepdogAddVolume(virStoragePoolObj *pool, const char *diskInfo) @@ -134,6 +159,7 @@ virStorageBackendSheepdogAddVolume(virStoragePoolObj *pool, const char *diskInfo return 0; } + static int virStorageBackendSheepdogRefreshAllVol(virStoragePoolObj *pool) { @@ -310,34 +336,6 @@ virStorageBackendSheepdogParseVdiList(virStorageVolDef *vol, return -1; } -static int -virStorageBackendSheepdogRefreshVol(virStoragePoolObj *pool, - virStorageVolDef *vol) -{ - char *output = NULL; - virStoragePoolDef *def = virStoragePoolObjGetDef(pool); - g_autoptr(virCommand) cmd = NULL; - - cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "list", vol->name, "-r", NULL); - virStorageBackendSheepdogAddHostArg(cmd, pool); - virCommandSetOutputBuffer(cmd, &output); - if (virCommandRun(cmd, NULL) < 0) - return -1; - - if (virStorageBackendSheepdogParseVdiList(vol, output) < 0) - return -1; - - vol->type = VIR_STORAGE_VOL_NETWORK; - - VIR_FREE(vol->key); - vol->key = g_strdup_printf("%s/%s", def->source.name, vol->name); - - VIR_FREE(vol->target.path); - vol->target.path = g_strdup(vol->name); - - return 0; -} - static int virStorageBackendSheepdogResizeVol(virStoragePoolObj *pool, @@ -356,7 +354,6 @@ virStorageBackendSheepdogResizeVol(virStoragePoolObj *pool, } - virStorageBackend virStorageBackendSheepdog = { .type = VIR_STORAGE_POOL_SHEEPDOG, -- 2.20.1

2 1

[PATCH 0/3] tests: Return EXIT_* from main instead of 0/-1
by Michal Privoznik 17 May '21

17 May '21

See 3/3 for rationale. Michal Prívozník (3): testutils: Drop libtool binary name handling tests: Return EXIT_FAILURE/EXIT_SUCCESS instead of -1/0 testutils: Document and enforce @func callback retvals for virTestMain() tests/fchosttest.c | 2 +- tests/qemusecuritytest.c | 2 +- tests/scsihosttest.c | 2 +- tests/seclabeltest.c | 2 +- tests/storagepoolcapstest.c | 2 +- tests/testutils.c | 20 +++++++++++++++----- tests/testutils.h | 4 ++++ tests/virbitmaptest.c | 2 +- tests/vircaps2xmltest.c | 2 +- tests/vircapstest.c | 2 +- tests/virconftest.c | 2 +- tests/virendiantest.c | 2 +- tests/virlogtest.c | 2 +- tests/virresctrltest.c | 2 +- tests/virscsitest.c | 2 +- 15 files changed, 32 insertions(+), 18 deletions(-) -- 2.26.3

2 5

[libvirt PATCH v3 00/10] make internal only secrets work with split daemons
by Daniel P. Berrangé 15 May '21

15 May '21

If you define a secret with private="yes", then libvirt won't let any client query the secret value after it is set. Only other libvirt drivers inside the daemon can query it by passing a special internal only flag to the virSecretGetValue API. The remote driver/daemon refuses to let this internal flag go over the wire preventing normal clients from using it This doesn't work with the split daemons because the virSecretGetValue API done by virqemud / virtstoraged has to go over the wire to reach the virsecretd. We need to come up with an alternative way to "prove" that the caller of virSecretGetValue is a libvirt daemon, as opposed to a general libvirt client. Note with if only traditional POSIX DAC permissions are in effect then we could consider it pointless trying to restrict access to clients running the same user/group as the libvirt daemon. We ought to take into account that the client might be confined by SELinux though, so the "private secret" concept isn't entirely pointless. Thus doing a simple uid of client == uid of daemon check is a bit too weak. The UID check might also not fly if the modular daemons are run inside containers with user namespaces, as the container for virtsecretd and virtqemud might have different user mappings in theory. This series adds a concept of a "token" which is known only to the libvirt daemons. The first daemon to use it writes a random hex string to /var/run/libvirt/common/system.token. Other daemons can read and compare this. Unless a MAC system is present this is still largely security theatre, but that's not really worse than the historical behaviour. When an API call is made the virIdentity by default reflects the identity of the UNIX process that initiated it. When connecting to virtproxyd, the client apps' identity is forwarded to the next virtNNNNd daemon. When libvirt drivers, however, initiate an API call we never set any identity. With monolithic libvirtd, they'd inherit the current client identity automagically since it was all in the same thread local. With modular daemons the othe driver would see the identity of the other libvirt daemon which is bad as this gives elevated privileges in the ACL check. Thus we fix the code which drivers use to open a connection to other daemons, such that it applies the current caller's identity. It does this using an "elevated" identity though, which means, we have added in the system token. Thus the virtsecretd daemon getting the call virSecretGetValue sees the virIdentity reflecting the client application which originally called the virDomainCreate() API, but with the system token set. Thus virsecretd can see that the virSecretGetValue was invoked by another daemon, not a libvirt client app. Changed in v3... Properly mock the new APIs in test suite Changed in v2... We can't set the elevated identity only when opening the virConnect for the secret driver. This works for modular daemons, as the identity is passed to the virsecretd at time of opening and thus applies to the later virSecretGetValue call on that connection. For monolithic daemon, the identity present at virConnectOpen is irrelevant. The virSecretGetValue call will just directly query the current thread's identity. IOW, to work in both deployment scenarios we need to have the elevated identity set across both virConnectOpen and virSecretGetValue Daniel P. Berrangé (10): util: add virRandomToken API util: introduce concept of a system token into identities util: generate a persistent system token util: set system token for system identity util: add API for copying identity objects util: helper to temporary elevate privileges of the current identity src: add API to determine if current identity is a system identity src: set identity when opening secondary drivers src: elevate current identity privilege when fetching secret secret: rework handling of private secrets src/driver-secret.h | 9 +- src/driver.c | 27 +++ src/libvirt-secret.c | 2 +- src/libvirt_private.syms | 8 + src/libxl/libxl_conf.c | 5 + src/qemu/qemu_domain.c | 11 +- src/qemu/qemu_tpm.c | 5 + src/remote/remote_driver.c | 8 +- src/secret/secret_driver.c | 34 ++- src/storage/storage_backend_iscsi.c | 5 + src/storage/storage_backend_iscsi_direct.c | 5 + src/storage/storage_backend_rbd.c | 5 + src/storage/storage_util.c | 5 + src/util/viridentity.c | 244 ++++++++++++++++++++- src/util/viridentity.h | 11 + src/util/viridentitypriv.h | 30 +++ src/util/virrandom.c | 18 ++ src/util/virrandom.h | 1 + src/util/virsecret.c | 3 +- tests/qemuxml2argvmock.c | 9 + tests/qemuxml2argvtest.c | 9 +- tests/viridentitytest.c | 11 +- 22 files changed, 435 insertions(+), 30 deletions(-) create mode 100644 src/util/viridentitypriv.h -- 2.31.1

3 12

[PATCH] rpm: re-enable ppc64 on RHEL-8
by Daniel P. Berrangé 15 May '21

15 May '21

Historically PowerPC 64 was always supported with qemu-kvm in RHEL. In future RHEL-9 it is being discontinued and this was addressed in commit 03cc3c9064322ac4028a2213105cd230fe28c013 Author: Jiri Denemark <jdenemar(a)redhat.com> Date: Wed Apr 21 14:55:03 2021 +0200 spec: Do not build qemu driver for Power on RHEL-9 when the specfile was cleaned up to remove RHEL-7 support: commit 0f601d2f868f2017cdd16e0a7ca90a59e7d5e120 Author: Andrea Bolognani <abologna(a)redhat.com> Date: Wed May 5 19:30:46 2021 +0200 spec: Bump min_fedora and min_rhel it also removed the logic that applied to RHEL-8 wrt arch list and lost PowerPC 64 support on 8. This reverts that part of the change but with the condition reversed to prioritize the future state. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- libvirt.spec.in | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index f421828d16..8ac324be0a 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -8,7 +8,11 @@ %define arches_qemu_kvm %{ix86} x86_64 %{power64} %{arm} aarch64 s390x %if 0%{?rhel} - %define arches_qemu_kvm x86_64 aarch64 s390x + %if 0%{?rhel} > 8 + %define arches_qemu_kvm x86_64 aarch64 s390x + %else + %define arches_qemu_kvm x86_64 %{power64} aarch64 s390x + %endif %endif %define arches_64bit x86_64 %{power64} aarch64 s390x riscv64 -- 2.31.1

3 2

[PATCH] rpm: disable glusterfs on RHEL-9
by Daniel P. Berrangé 15 May '21

15 May '21

Support for glusterfs with KVM is being dropped in RHEL-9 in the virtualization stack. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- libvirt.spec.in | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 8ac324be0a..c52f607bd1 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -65,10 +65,15 @@ %endif %define with_storage_gluster 0%{!?_without_storage_gluster:1} -%ifnarch %{arches_qemu_kvm} - # gluster is only built where qemu driver is enabled on RHEL - %if 0%{?rhel} +%if 0%{?rhel} + # Glusterfs dropped in RHEL-9, and before that only + # enabled on arches where KVM exists + %if 0%{?rhel} > 8 %define with_storage_gluster 0 + %else + %ifnarch %{arches_qemu_kvm} + %define with_storage_gluster 0 + %endif %endif %endif -- 2.31.1

3 2