[PATCH 3/4] qemu: Start emulator thread with more generous mbind()

Michal Privoznik posted 4 patches 1 year ago
[PATCH 3/4] qemu: Start emulator thread with more generous mbind()
Posted by Michal Privoznik 1 year ago
This is very similar to the previous commit, but fixes the case
where there are no CGroups (or are disabled by admin) and thus
libvirt calls mbind() before exec()-ing QEMU.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_process.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6b85b7cee7..0fafda0beb 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3106,10 +3106,8 @@ struct qemuProcessHookData {
 static int qemuProcessHook(void *data)
 {
     struct qemuProcessHookData *h = data;
-    qemuDomainObjPrivate *priv = h->vm->privateData;
     int ret = -1;
     int fd;
-    virBitmap *nodeset = NULL;
     virDomainNumatuneMemMode mode;
 
     /* This method cannot use any mutexes, which are not
@@ -3154,8 +3152,9 @@ static int qemuProcessHook(void *data)
              * and enabled in the config) rather than virNuma*. */
             VIR_DEBUG("Relying on CGroups for memory binding");
         } else {
-            nodeset = virDomainNumatuneGetNodeset(h->vm->def->numa,
-                                                  priv->autoNodeset, -1);
+            g_autoptr(virBitmap) nodeset = NULL;
+
+            qemuDomainNumatuneMaybeFormatNodesetUnion(h->vm, &nodeset, NULL);
 
             if (virNumaSetupMemoryPolicy(mode, nodeset) < 0)
                 goto cleanup;
-- 
2.39.3
Re: [PATCH 3/4] qemu: Start emulator thread with more generous mbind()
Posted by Martin Kletzander 1 year ago
On Tue, May 23, 2023 at 12:06:20PM +0200, Michal Privoznik wrote:
>This is very similar to the previous commit, but fixes the case
>where there are no CGroups (or are disabled by admin) and thus
>libvirt calls mbind() before exec()-ing QEMU.
>

nitpick: we don't call mbind, but numa_set_membind

but anyway, I'm not sure this is the right way to do it.  I don't know
if set_membind and mbind would fight the same way it does with cgroups.
Did you try it out before this patch?  Looking at the code there are
more problems, but unrelated to this change, so I'll send a different
patch once I get to it.

>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/qemu/qemu_process.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
>diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>index 6b85b7cee7..0fafda0beb 100644
>--- a/src/qemu/qemu_process.c
>+++ b/src/qemu/qemu_process.c
>@@ -3106,10 +3106,8 @@ struct qemuProcessHookData {
> static int qemuProcessHook(void *data)
> {
>     struct qemuProcessHookData *h = data;
>-    qemuDomainObjPrivate *priv = h->vm->privateData;
>     int ret = -1;
>     int fd;
>-    virBitmap *nodeset = NULL;
>     virDomainNumatuneMemMode mode;
>
>     /* This method cannot use any mutexes, which are not
>@@ -3154,8 +3152,9 @@ static int qemuProcessHook(void *data)
>              * and enabled in the config) rather than virNuma*. */
>             VIR_DEBUG("Relying on CGroups for memory binding");
>         } else {
>-            nodeset = virDomainNumatuneGetNodeset(h->vm->def->numa,
>-                                                  priv->autoNodeset, -1);
>+            g_autoptr(virBitmap) nodeset = NULL;
>+
>+            qemuDomainNumatuneMaybeFormatNodesetUnion(h->vm, &nodeset, NULL);
>
>             if (virNumaSetupMemoryPolicy(mode, nodeset) < 0)
>                 goto cleanup;
>-- 
>2.39.3
>