[libvirt PATCH v2 0/2] conf: fix handling of missing CPU cache info in sysfs
Changed in v2: - Introduce g_autoptr support for virCapsHostCacheBank struct Daniel P. Berrangé (2): conf: define autoptr func for virCapsHostCacheBankFree conf: skip resource cache init if sysfs files are missing src/conf/capabilities.c | 90 +++++++++++++++++++++++++---------------- src/conf/capabilities.h | 3 ++ 2 files changed, 58 insertions(+), 35 deletions(-) -- 2.37.3
This lets us simplify the cleanup paths when populating the host cache bank information in capabilities XML. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/capabilities.c | 34 ++++++++++++++-------------------- src/conf/capabilities.h | 3 +++ 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 1a7ebf4a13..d693d38f3c 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -2150,9 +2150,7 @@ virCapabilitiesInitCaches(virCaps *caps) size_t i = 0; g_autoptr(virBitmap) cpus = NULL; ssize_t pos = -1; - int ret = -1; struct dirent *ent = NULL; - virCapsHostCacheBank *bank = NULL; const virResctrlMonitorType montype = VIR_RESCTRL_MONITOR_TYPE_CACHE; const char *prefix = virResctrlMonitorPrefixTypeToString(montype); @@ -2172,10 +2170,11 @@ 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) - goto cleanup; + return -1; if (!dirp) continue; @@ -2191,7 +2190,7 @@ virCapabilitiesInitCaches(virCaps *caps) if (virFileReadValueUint(&level, "%s/cpu/cpu%zd/cache/%s/level", SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) - goto cleanup; + return -1; if (level < cache_min_level) continue; @@ -2202,33 +2201,33 @@ virCapabilitiesInitCaches(virCaps *caps) if (virFileReadValueUint(&bank->id, "%s/cpu/cpu%zd/cache/%s/id", SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) - goto cleanup; + return -1; if (virFileReadValueUint(&bank->level, "%s/cpu/cpu%zd/cache/%s/level", SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) - goto cleanup; + return -1; if (virFileReadValueString(&type, "%s/cpu/cpu%zd/cache/%s/type", SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) - goto cleanup; + return -1; if (virFileReadValueScaledInt(&bank->size, "%s/cpu/cpu%zd/cache/%s/size", SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) - goto cleanup; + return -1; if (virFileReadValueBitmap(&bank->cpus, "%s/cpu/cpu%zd/cache/%s/shared_cpu_list", SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) - goto cleanup; + return -1; kernel_type = virCacheKernelTypeFromString(type); if (kernel_type < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown cache type '%s'"), type); - goto cleanup; + return -1; } bank->type = kernel_type; @@ -2244,15 +2243,13 @@ virCapabilitiesInitCaches(virCaps *caps) bank->size, &bank->ncontrols, &bank->controls) < 0) - goto cleanup; + return -1; VIR_APPEND_ELEMENT(caps->host.cache.banks, caps->host.cache.nbanks, bank); } - - g_clear_pointer(&bank, virCapsHostCacheBankFree); } if (rv < 0) - goto cleanup; + return -1; } /* Sort the array in order for the tests to be predictable. This way we can @@ -2264,16 +2261,13 @@ virCapabilitiesInitCaches(virCaps *caps) } if (virCapabilitiesInitResctrlMemory(caps) < 0) - goto cleanup; + return -1; if (virResctrlInfoGetMonitorPrefix(caps->host.resctrl, prefix, &caps->host.cache.monitor) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - virCapsHostCacheBankFree(bank); - return ret; + return 0; } diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 6fe7818b2e..ef6e8ab685 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -137,6 +137,9 @@ struct _virCapsHostCacheBank { virResctrlInfoPerCache **controls; }; +void virCapsHostCacheBankFree(virCapsHostCacheBank *ptr); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCapsHostCacheBank, virCapsHostCacheBankFree); + struct _virCapsHostCache { size_t nbanks; virCapsHostCacheBank **banks; -- 2.37.3
On aarch64 the 'id' file is not present for CPU cache information in sysfs. This causes the local stateful hypervisor drivers to fail to initialize capabilities: virStateInitialize:657 : Initialisation of cloud-hypervisor state driver failed: no error The 'no error' is because the 'virFileReadValueNNN' methods return ret==-2, with no error raised, when the requeted file does not exist. None of the callers were checking for this scenario when populating capabilities. The most graceful way to handle this is to skip the cache bank in question. This fixes failure to launch libvirt drivers on certain aarch64 hardware. Fixes: https://gitlab.com/libvirt/libvirt/-/issues/389 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/capabilities.c | 56 ++++++++++++++++++++++++++++++----------- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index d693d38f3c..a79547f6ca 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -2183,6 +2183,7 @@ virCapabilitiesInitCaches(virCaps *caps) g_autofree char *type = NULL; int kernel_type; unsigned int level; + int ret; if (!STRPREFIX(ent->d_name, "index")) continue; @@ -2198,29 +2199,54 @@ virCapabilitiesInitCaches(virCaps *caps) bank = g_new0(virCapsHostCacheBank, 1); bank->level = level; - if (virFileReadValueUint(&bank->id, - "%s/cpu/cpu%zd/cache/%s/id", - SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) + ret = virFileReadValueUint(&bank->id, + "%s/cpu/cpu%zd/cache/%s/id", + SYSFS_SYSTEM_PATH, pos, ent->d_name); + if (ret == -2) { + VIR_DEBUG("CPU %zd cache %s 'id' missing", pos, ent->d_name); + continue; + } + if (ret < 0) return -1; - if (virFileReadValueUint(&bank->level, - "%s/cpu/cpu%zd/cache/%s/level", - SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) + ret = virFileReadValueUint(&bank->level, + "%s/cpu/cpu%zd/cache/%s/level", + SYSFS_SYSTEM_PATH, pos, ent->d_name); + if (ret == -2) { + VIR_DEBUG("CPU %zd cache %s 'level' missing", pos, ent->d_name); + continue; + } + if (ret < 0) return -1; - if (virFileReadValueString(&type, - "%s/cpu/cpu%zd/cache/%s/type", - SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) + ret = virFileReadValueString(&type, + "%s/cpu/cpu%zd/cache/%s/type", + SYSFS_SYSTEM_PATH, pos, ent->d_name); + if (ret == -2) { + VIR_DEBUG("CPU %zd cache %s 'type' missing", pos, ent->d_name); + continue; + } + if (ret < 0) return -1; - if (virFileReadValueScaledInt(&bank->size, - "%s/cpu/cpu%zd/cache/%s/size", - SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) + ret = virFileReadValueScaledInt(&bank->size, + "%s/cpu/cpu%zd/cache/%s/size", + SYSFS_SYSTEM_PATH, pos, ent->d_name); + if (ret == -2) { + VIR_DEBUG("CPU %zd cache %s 'size' missing", pos, ent->d_name); + continue; + } + if (ret < 0) return -1; - if (virFileReadValueBitmap(&bank->cpus, - "%s/cpu/cpu%zd/cache/%s/shared_cpu_list", - SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) + ret = virFileReadValueBitmap(&bank->cpus, + "%s/cpu/cpu%zd/cache/%s/shared_cpu_list", + SYSFS_SYSTEM_PATH, pos, ent->d_name); + if (ret == -2) { + VIR_DEBUG("CPU %zd cache %s 'shared_cpu_list' missing", pos, ent->d_name); + continue; + } + if (ret < 0) return -1; kernel_type = virCacheKernelTypeFromString(type); -- 2.37.3
Daniel P. Berrangé <berrange@redhat.com> writes:
Changed in v2:
- Introduce g_autoptr support for virCapsHostCacheBank struct
Daniel P. Berrangé (2): conf: define autoptr func for virCapsHostCacheBankFree conf: skip resource cache init if sysfs files are missing
src/conf/capabilities.c | 90 +++++++++++++++++++++++++---------------- src/conf/capabilities.h | 3 ++ 2 files changed, 58 insertions(+), 35 deletions(-)
Tested-by: Alex Bennée <alex.bennee@linaro.org> -- Alex Bennée
On a Tuesday in 2022, Daniel P. Berrangé wrote:
Changed in v2:
- Introduce g_autoptr support for virCapsHostCacheBank struct
Daniel P. Berrangé (2): conf: define autoptr func for virCapsHostCacheBankFree conf: skip resource cache init if sysfs files are missing
src/conf/capabilities.c | 90 +++++++++++++++++++++++++---------------- src/conf/capabilities.h | 3 ++ 2 files changed, 58 insertions(+), 35 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (3)
-
Alex Bennée -
Daniel P. Berrangé -
Ján Tomko