Re: [PATCH 1/2] virhostcpu: Introduce virHostCPUGetIsolated()

On Mon, Apr 22, 2024 at 02:02:28PM +0100, Daniel P. Berrangé wrote: > On Mon, Apr 22, 2024 at 02:29:32PM +0200, Martin Kletzander wrote: >> On Mon, Apr 22, 2024 at 01:38:38PM +0200, Michal Privoznik wrote: >> > This is a helper that parses /sys/devices/system/cpu/isolated >> > into a virBitmap. It's going to be needed soon. >> > >> > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >> > --- >> > src/libvirt_private.syms |  1 + >> > src/util/virhostcpu.c    | 21 +++++++++++++++++++++ >> > src/util/virhostcpu.h    |  1 + >> > 3 files changed, 23 insertions(+) >> > >> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms >> > index 839fe4f545..ed85f022e4 100644 >> > --- a/src/libvirt_private.syms >> > +++ b/src/libvirt_private.syms >> > @@ -2502,6 +2502,7 @@ virHostCPUGetCount; >> > virHostCPUGetCPUID; >> > virHostCPUGetHaltPollTime; >> > virHostCPUGetInfo; >> > +virHostCPUGetIsolated; >> > virHostCPUGetKVMMaxVCPUs; >> > virHostCPUGetMap; >> > virHostCPUGetMicrocodeVersion; >> > diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c >> > index 01de69c0d1..e61eb12414 100644 >> > --- a/src/util/virhostcpu.c >> > +++ b/src/util/virhostcpu.c >> > @@ -1151,6 +1151,27 @@ virHostCPUGetAvailableCPUsBitmap(void) >> >     return g_steal_pointer(&bitmap); >> > } >> > >> > +/** >> > + * virHostCPUGetIsolated: >> > + * >> > + * Returns a bitmap of isolated CPUs (e.g. those passed to >> isolcpus= kernel >> > + * cmdline). >> > + */ >> > +virBitmap * >> > +virHostCPUGetIsolated(void) >> > +{ >> >> So this function will fail on anything non-Linux and we probably do not >> want that.  If the platform does not support isolcpus, then it is fine >> to return an empty bitmap. >> >> > +#ifdef __linux__ >> > +    virBitmap *ret = NULL; >> > + >> > +    virFileReadValueBitmap(&ret, "%s/cpu/isolated", >> SYSFS_SYSTEM_PATH); >> > + >> >> And this will fail (return NULL) on any system without isolcpus=, which >> is also probably something we do not want. > > The 'isolated' file exists, but is empty on such systems. So IIUC you're > saying virFileReadValueBitmap will fail to parse the empty string and > report it as an error ? > Yes, virBitmapParseUnlimited() will call virBitmapParseInternal() which checks: [...] const char *cur = str; [...] if (!str)     goto error; virSkipSpaces(&cur); if (*cur == '\0')     goto error; and to make sure I tested it and the virBitmap * is NULL and the function returns -1.