[libvirt PATCH v2 0/2] Use the default capabilities cache lookup rather than an arch based lookup

https://bugzilla.redhat.com/show_bug.cgi?id=1852311 Since v1: - dropped virQEMUCapsCacheLookupByArch since after patch 1 there was no further usage - dropped virQEMUCapsCompareArch since virQEMUCapsCacheLookupByArch was the only caller pipeline: https://gitlab.com/eskultety/libvirt/-/pipelines/164239584 Erik Skultety (2): qemu: Use virQEMUCapsCacheLookupDefault instead of lookup by arch qemu: capabilities: Drop the virQEMUCapsCacheLookupByArch function src/qemu/qemu_capabilities.c | 63 ------------------------------------ src/qemu/qemu_capabilities.h | 2 -- src/qemu/qemu_driver.c | 6 ++-- 3 files changed, 4 insertions(+), 67 deletions(-) -- 2.26.2

Firstly, SEV is present only on AMD, so we can safely assume x86. Secondly, the problem with looking up capabilities in the cache by arch is that it's using virHashSearch with a callback to find the right capabilities and get the binary name from it as well, but since the cache is empty, it will return NULL and we won't get the corresponding binary name out of the lookup either. Then, during the cache validation we try to create a new cache entry for the emulator, but since we don't have the binary name, nothing gets created. Therefore, virQEMUCapsCacheLookupDefault is used to fix this issue, because it doesn't rely on the capabilities cache to construct the emulator binary name. https://bugzilla.redhat.com/show_bug.cgi?id=1852311 Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/qemu/qemu_driver.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a5b38b3d24..cd8d7ffb56 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -22823,8 +22823,10 @@ qemuNodeGetSEVInfo(virConnectPtr conn, if (virNodeGetSevInfoEnsureACL(conn) < 0) return -1; - qemucaps = virQEMUCapsCacheLookupByArch(driver->qemuCapsCache, - virArchFromHost()); + qemucaps = virQEMUCapsCacheLookupDefault(driver->qemuCapsCache, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL); + if (!qemucaps) return -1; -- 2.26.2

Previous commit removed the last usage of the function. Drop virQEMUCapsCompareArch as well since virQEMUCapsCacheLookupByArch was its only caller. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/qemu/qemu_capabilities.c | 63 ------------------------------------ src/qemu/qemu_capabilities.h | 2 -- 2 files changed, 65 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index efc42aac17..25a0b36940 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5668,69 +5668,6 @@ virQEMUCapsCacheLookupCopy(virFileCachePtr cache, } -static int -virQEMUCapsCompareArch(const void *payload, - const void *name G_GNUC_UNUSED, - const void *opaque) -{ - struct virQEMUCapsSearchData *data = (struct virQEMUCapsSearchData *)opaque; - const virQEMUCaps *qemuCaps = payload; - - if (qemuCaps->arch != data->arch) - return false; - - if (data->binaryFilter && - !strstr(qemuCaps->binary, data->binaryFilter)) { - return false; - } - - return true; -} - - -virQEMUCapsPtr -virQEMUCapsCacheLookupByArch(virFileCachePtr cache, - virArch arch) -{ - virQEMUCapsCachePrivPtr priv = virFileCacheGetPriv(cache); - virQEMUCapsPtr ret = NULL; - const char *binaryFilters[] = { - "qemu-system-", - NULL, - }; - virArch archs[] = { - arch, - virQEMUCapsFindTarget(virArchFromHost(), arch), - }; - size_t i; - size_t j; - - priv->microcodeVersion = virHostCPUGetMicrocodeVersion(); - - for (i = 0; i < G_N_ELEMENTS(binaryFilters); i++) { - for (j = 0; j < G_N_ELEMENTS(archs); j++) { - struct virQEMUCapsSearchData data = { - .arch = archs[j], - .binaryFilter = binaryFilters[i], - }; - - ret = virFileCacheLookupByFunc(cache, virQEMUCapsCompareArch, &data); - if (ret) - goto done; - } - } - - virReportError(VIR_ERR_INVALID_ARG, - _("unable to find any emulator to serve '%s' " - "architecture"), virArchToString(arch)); - - done: - VIR_DEBUG("Returning caps %p for arch %s", ret, virArchToString(arch)); - - return ret; -} - - /** * virQEMUCapsCacheLookupDefault: * @cache: QEMU capabilities cache diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 3e585d1229..9834fc2a52 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -703,8 +703,6 @@ virQEMUCapsPtr virQEMUCapsCacheLookupCopy(virFileCachePtr cache, virDomainVirtType virtType, const char *binary, const char *machineType); -virQEMUCapsPtr virQEMUCapsCacheLookupByArch(virFileCachePtr cache, - virArch arch); virQEMUCapsPtr virQEMUCapsCacheLookupDefault(virFileCachePtr cache, const char *binary, const char *archStr, -- 2.26.2

On Wed, Jul 08, 2020 at 10:49:29AM +0200, Erik Skultety wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1852311
Since v1: - dropped virQEMUCapsCacheLookupByArch since after patch 1 there was no further usage - dropped virQEMUCapsCompareArch since virQEMUCapsCacheLookupByArch was the only caller
pipeline: https://gitlab.com/eskultety/libvirt/-/pipelines/164239584
Erik Skultety (2): qemu: Use virQEMUCapsCacheLookupDefault instead of lookup by arch qemu: capabilities: Drop the virQEMUCapsCacheLookupByArch function
src/qemu/qemu_capabilities.c | 63 ------------------------------------ src/qemu/qemu_capabilities.h | 2 -- src/qemu/qemu_driver.c | 6 ++-- 3 files changed, 4 insertions(+), 67 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé
-
Erik Skultety