4:06 a.m.
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
4:06 a.m.
New subject: [libvirt PATCH v2 1/2] conf: define autoptr func for virCapsHostCacheBankFree
This lets us simplify the cleanup paths when populating the host cache
bank information in capabilities XML.
Signed-off-by: Daniel P. Berrangé <berrange(a)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
4:06 a.m.
New subject: [libvirt PATCH v2 2/2] conf: skip resource cache init if sysfs files are missing
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(a)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
6:17 a.m.
Daniel P. Berrangé <berrange(a)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(a)linaro.org>
--
Alex Bennée
6:48 a.m.
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(a)redhat.com>
Jano
807
days inactive
807
days old
4 comments
3 participants
participants (3)
-
Alex Bennée -
Daniel P. Berrangé -
Ján Tomko