6:38 a.m.
*** BLURB HERE ***
Michal Prívozník (2):
virhostcpu: Introduce virHostCPUGetIsolated()
qemu: Substract isolcpus from all online affinity
src/libvirt_private.syms | 1 +
src/qemu/qemu_process.c | 7 +++++++
src/util/virhostcpu.c | 21 +++++++++++++++++++++
src/util/virhostcpu.h | 1 +
4 files changed, 30 insertions(+)
--
2.43.2
6:38 a.m.
New subject: [PATCH 1/2] virhostcpu: Introduce virHostCPUGetIsolated()
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>
---
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)
+{
+#ifdef __linux__
+ virBitmap *ret = NULL;
+
+ virFileReadValueBitmap(&ret, "%s/cpu/isolated", SYSFS_SYSTEM_PATH);
+
+ return ret;
+#else
+ virReportError(VIR_ERR_NO_SUPPORT, "%s",
+ _("map of isolated CPUs not implemented on this
platform"));
+ return NULL;
+#endif
+}
#if WITH_LINUX_KVM_H && defined(KVM_CAP_PPC_SMT)
diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h
index d7e09bff22..e7b15a2649 100644
--- a/src/util/virhostcpu.h
+++ b/src/util/virhostcpu.h
@@ -43,6 +43,7 @@ bool virHostCPUHasBitmap(void);
virBitmap *virHostCPUGetPresentBitmap(void);
virBitmap *virHostCPUGetOnlineBitmap(void);
virBitmap *virHostCPUGetAvailableCPUsBitmap(void);
+virBitmap *virHostCPUGetIsolated(void);
int virHostCPUGetCount(void);
int virHostCPUGetThreadsPerSubcore(virArch arch) G_NO_INLINE;
--
2.43.2
7:29 a.m.
New subject: [PATCH 1/2] virhostcpu: Introduce virHostCPUGetIsolated()
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>
---
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.
You can also use the return value of virFileReadValueBitmap() to see if
the file exists, but I'd even go as far as just using the bitmap if it
returned non-NULL, otherwise carrying on.
+ return ret;
+#else
+ virReportError(VIR_ERR_NO_SUPPORT, "%s",
+ _("map of isolated CPUs not implemented on this
platform"));
+ return NULL;
+#endif
+}
#if WITH_LINUX_KVM_H && defined(KVM_CAP_PPC_SMT)
diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h
index d7e09bff22..e7b15a2649 100644
--- a/src/util/virhostcpu.h
+++ b/src/util/virhostcpu.h
@@ -43,6 +43,7 @@ bool virHostCPUHasBitmap(void);
virBitmap *virHostCPUGetPresentBitmap(void);
virBitmap *virHostCPUGetOnlineBitmap(void);
virBitmap *virHostCPUGetAvailableCPUsBitmap(void);
+virBitmap *virHostCPUGetIsolated(void);
int virHostCPUGetCount(void);
int virHostCPUGetThreadsPerSubcore(virArch arch) G_NO_INLINE;
--
2.43.2
_______________________________________________
Devel mailing list -- devel(a)lists.libvirt.org
To unsubscribe send an email to devel-leave(a)lists.libvirt.org
8:02 a.m.
New subject: [PATCH 1/2] virhostcpu: Introduce virHostCPUGetIsolated()
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>
> ---
> 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 ?
You can also use the return value of virFileReadValueBitmap() to see if
the file exists, but I'd even go as far as just using the bitmap if it
returned non-NULL, otherwise carrying on.
With 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 :|
8:11 a.m.
New subject: [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>
> > ---
> > 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.
>
> You can also use the return value of virFileReadValueBitmap() to see if
> the file exists, but I'd even go as far as just using the bitmap if it
> returned non-NULL, otherwise carrying on.
With 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 :|
8:46 a.m.
New subject: [PATCH 1/2] virhostcpu: Introduce virHostCPUGetIsolated()
On 4/22/24 15:11, Martin Kletzander wrote:
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>
>> > ---
>> > 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.
Oh, I didn't realize an empty file (well, "\n\0") makes this function
fail. That complicates things a bit. Let me see what can be done about it.
Michal
7:59 a.m.
New subject: [PATCH 1/2] virhostcpu: Introduce virHostCPUGetIsolated()
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>
---
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)
+{
+#ifdef __linux__
+ virBitmap *ret = NULL;
+
+ virFileReadValueBitmap(&ret, "%s/cpu/isolated", SYSFS_SYSTEM_PATH);
This can return '-2' if the file does not exist, or '-1' with error
reported if there's a problem opennig/parsing it.
In both cases 'ret' should end up NULL
The next patch treats NULL as an error scenario, but this
means in the '-2' case we'll be returning an error status
up the call stack, but without having actually reported an
error message.
+
+ return ret;
+#else
+ virReportError(VIR_ERR_NO_SUPPORT, "%s",
+ _("map of isolated CPUs not implemented on this
platform"));
+ return NULL;
+#endif
+}
#if WITH_LINUX_KVM_H && defined(KVM_CAP_PPC_SMT)
diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h
index d7e09bff22..e7b15a2649 100644
--- a/src/util/virhostcpu.h
+++ b/src/util/virhostcpu.h
@@ -43,6 +43,7 @@ bool virHostCPUHasBitmap(void);
virBitmap *virHostCPUGetPresentBitmap(void);
virBitmap *virHostCPUGetOnlineBitmap(void);
virBitmap *virHostCPUGetAvailableCPUsBitmap(void);
+virBitmap *virHostCPUGetIsolated(void);
int virHostCPUGetCount(void);
int virHostCPUGetThreadsPerSubcore(virArch arch) G_NO_INLINE;
--
2.43.2
_______________________________________________
Devel mailing list -- devel(a)lists.libvirt.org
To unsubscribe send an email to devel-leave(a)lists.libvirt.org
With 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 :|
6:38 a.m.
New subject: [PATCH 2/2] qemu: Substract isolcpus from all online affinity
When starting a domain and there's no vCPU/emulator pinning set,
we query the list of all online physical CPUs and set affinity of
the child process (which eventually becomes QEMU) to that list.
We can't assume libvirtd itself had affinity to all online CPUs
and since affinity of the child process is inherited, we should
fix it afterwards. But that's not necessarily correct. Users
might isolate some physical CPUs and we should avoid touching
them unless explicitly told so (i.e. vCPU/emulator pinning told
us so).
Therefore, when attempting to set affinity to all online CPUs
subtract the isolated ones.
Before this commit:
root@localhost:~# cat /sys/devices/system/cpu/isolated
19,21,23
root@virtlab414:~# taskset -cp $(pgrep qemu)
pid 14835's current affinity list: 0-23
After:
root@virtlab414:~# taskset -cp $(pgrep qemu)
pid 17153's current affinity list: 0-18,20,22
Resolves: https://issues.redhat.com/browse/RHEL-33082
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_process.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index da2b024f92..ef5338eda3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2334,6 +2334,8 @@ qemuProcessDetectIOThreadPIDs(virDomainObj *vm,
static int
qemuProcessGetAllCpuAffinity(virBitmap **cpumapRet)
{
+ g_autoptr(virBitmap) isolCpus = NULL;
+
*cpumapRet = NULL;
if (!virHostCPUHasBitmap())
@@ -2342,6 +2344,11 @@ qemuProcessGetAllCpuAffinity(virBitmap **cpumapRet)
if (!(*cpumapRet = virHostCPUGetOnlineBitmap()))
return -1;
+ if (!(isolCpus = virHostCPUGetIsolated()))
+ return -1;
+
+ virBitmapSubtract(*cpumapRet, isolCpus);
+
return 0;
}
--
2.43.2
214
days inactive
214
days old
7 comments
4 participants
participants (4)
-
Daniel P. Berrangé -
Martin Kletzander -
Michal Privoznik -
Michal Prívozník