[libvirt PATCH] conf: skip resource cache init if sysfs files are missing

[Ján Tomko]

On a Monday in 2022, Daniel P. Berrangé wrote:
>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.

*requested

>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 at redhat.com>
>---
> src/conf/capabilities.c | 57 ++++++++++++++++++++++++++++++-----------
> 1 file changed, 42 insertions(+), 15 deletions(-)
>
>diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
>index 1a7ebf4a13..ae63515fd3 100644
>--- a/src/conf/capabilities.c
>+++ b/src/conf/capabilities.c
>@@ -2184,6 +2184,7 @@ virCapabilitiesInitCaches(virCaps *caps)
>             g_autofree char *type = NULL;
>             int kernel_type;
>             unsigned int level;
>+            int err;
>
>             if (!STRPREFIX(ent->d_name, "index"))
>                 continue;
>@@ -2199,29 +2200,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)
>+            err = virFileReadValueUint(&bank->id,
>+                                       "%s/cpu/cpu%zd/cache/%s/id",
>+                                       SYSFS_SYSTEM_PATH, pos, ent->d_name);
>+            if (err == -2) {
>+                VIR_DEBUG("CPU %zd cache %s 'id' missing", pos, ent->d_name);
>+                goto skip_cache;
>+            }
>+            if (err < 0)
>                 goto cleanup;
>
>-            if (virFileReadValueUint(&bank->level,
>-                                     "%s/cpu/cpu%zd/cache/%s/level",
>-                                     SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0)
>+            err = virFileReadValueUint(&bank->level,
>+                                       "%s/cpu/cpu%zd/cache/%s/level",
>+                                       SYSFS_SYSTEM_PATH, pos, ent->d_name);
>+            if (err == -2) {
>+                VIR_DEBUG("CPU %zd cache %s 'level' missing", pos, ent->d_name);
>+                goto skip_cache;
>+            }
>+            if (err < 0)
>                 goto cleanup;
>
>-            if (virFileReadValueString(&type,
>-                                       "%s/cpu/cpu%zd/cache/%s/type",
>-                                       SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0)
>+            err = virFileReadValueString(&type,
>+                                         "%s/cpu/cpu%zd/cache/%s/type",
>+                                         SYSFS_SYSTEM_PATH, pos, ent->d_name);
>+            if (err == -2) {
>+                VIR_DEBUG("CPU %zd cache %s 'type' missing", pos, ent->d_name);
>+                goto skip_cache;
>+            }
>+            if (err < 0)
>                 goto cleanup;
>
>-            if (virFileReadValueScaledInt(&bank->size,
>-                                          "%s/cpu/cpu%zd/cache/%s/size",
>-                                          SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0)
>+            err = virFileReadValueScaledInt(&bank->size,
>+                                            "%s/cpu/cpu%zd/cache/%s/size",
>+                                            SYSFS_SYSTEM_PATH, pos, ent->d_name);
>+            if (err == -2) {
>+                VIR_DEBUG("CPU %zd cache %s 'size' missing", pos, ent->d_name);
>+                goto skip_cache;
>+            }
>+            if (err < 0)
>                 goto cleanup;
>
>-            if (virFileReadValueBitmap(&bank->cpus,
>-                                       "%s/cpu/cpu%zd/cache/%s/shared_cpu_list",
>-                                       SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0)
>+            err = virFileReadValueBitmap(&bank->cpus,
>+                                         "%s/cpu/cpu%zd/cache/%s/shared_cpu_list",
>+                                         SYSFS_SYSTEM_PATH, pos, ent->d_name);
>+            if (err == -2) {
>+                VIR_DEBUG("CPU %zd cache %s 'shared_cpu_list' missing", pos, ent->d_name);
>+                goto skip_cache;
>+            }
>+            if (err < 0)
>                 goto cleanup;
>
>             kernel_type = virCacheKernelTypeFromString(type);
>@@ -2249,6 +2275,7 @@ virCapabilitiesInitCaches(virCaps *caps)
>                 VIR_APPEND_ELEMENT(caps->host.cache.banks, caps->host.cache.nbanks, bank);
>             }
>
>+        skip_cache:
>             g_clear_pointer(&bank, virCapsHostCacheBankFree);

If you define a cleanup function for virCapsHostCacheBank, there is no
need to explicitly clear the pointer and 'goto skip_cache' becomes just
'continue'.

Jano