qemu: Allow setting affinity to fail and don't report error

[PATCH] qemu: Allow setting affinity to fail and don't report error

Posted by Martin Kletzander 5 years, 5 months ago

This is just a clean-up of commit 3791f29b085c using the new parameter of
virProcessSetAffinity() introduced in commit 9514e24984ee so that there is
no error reported in the logs.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/qemu/qemu_process.c | 65 +++++++++++++++++++----------------------
 1 file changed, 30 insertions(+), 35 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 04e5cbb65969..e29f35e10576 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2569,24 +2569,21 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm)
             return -1;
     }
 
+    /*
+     * We only want to error out if we failed to set the affinity to
+     * user-requested mapping.  If we are just trying to reset the affinity
+     * to all CPUs and this fails it can only be an issue if:
+     *  1) libvirtd does not have CAP_SYS_NICE
+     *  2) libvirtd does not run on all CPUs
+     *
+     * This scenario can easily occurr when libvirtd is run inside a
+     * container with restrictive permissions and CPU pinning.
+     *
+     * See also: https://bugzilla.redhat.com/1819801#c2
+     */
     if (cpumapToSet &&
-        virProcessSetAffinity(vm->pid, cpumapToSet, false) < 0) {
-        /*
-         * We only want to error out if we failed to set the affinity to
-         * user-requested mapping.  If we are just trying to reset the affinity
-         * to all CPUs and this fails it can only be an issue if:
-         *  1) libvirtd does not have CAP_SYS_NICE
-         *  2) libvirtd does not run on all CPUs
-         *
-         * This scenario can easily occurr when libvirtd is run inside a
-         * container with restrictive permissions and CPU pinning.
-         *
-         * See also: https://bugzilla.redhat.com/1819801#c2
-         */
-        if (settingAll)
-            virResetLastError();
-        else
-            return -1;
+        virProcessSetAffinity(vm->pid, cpumapToSet, settingAll) < 0) {
+        return -1;
     }
 
     return 0;
@@ -2739,25 +2736,23 @@ qemuProcessSetupPid(virDomainObjPtr vm,
     if (!affinity_cpumask)
         affinity_cpumask = use_cpumask;
 
-    /* Setup legacy affinity. */
+    /* Setup legacy affinity.
+     *
+     * We only want to error out if we failed to set the affinity to
+     * user-requested mapping.  If we are just trying to reset the affinity
+     * to all CPUs and this fails it can only be an issue if:
+     *  1) libvirtd does not have CAP_SYS_NICE
+     *  2) libvirtd does not run on all CPUs
+     *
+     * This scenario can easily occurr when libvirtd is run inside a
+     * container with restrictive permissions and CPU pinning.
+     *
+     * See also: https://bugzilla.redhat.com/1819801#c2
+     */
     if (affinity_cpumask &&
-        virProcessSetAffinity(pid, affinity_cpumask, false) < 0) {
-        /*
-         * We only want to error out if we failed to set the affinity to
-         * user-requested mapping.  If we are just trying to reset the affinity
-         * to all CPUs and this fails it can only be an issue if:
-         *  1) libvirtd does not have CAP_SYS_NICE
-         *  2) libvirtd does not run on all CPUs
-         *
-         * This scenario can easily occurr when libvirtd is run inside a
-         * container with restrictive permissions and CPU pinning.
-         *
-         * See also: https://bugzilla.redhat.com/1819801#c2
-         */
-        if (affinity_cpumask == hostcpumap)
-            virResetLastError();
-        else
-            goto cleanup;
+        virProcessSetAffinity(pid, affinity_cpumask,
+                              affinity_cpumask == hostcpumap) < 0) {
+        goto cleanup;
     }
 
     /* Set scheduler type and priority, but not for the main thread. */
-- 
2.28.0

Re: [PATCH] qemu: Allow setting affinity to fail and don't report error

Posted by Ján Tomko 5 years, 5 months ago

On a Monday in 2020, Martin Kletzander wrote:
>This is just a clean-up of commit 3791f29b085c using the new parameter of
>virProcessSetAffinity() introduced in commit 9514e24984ee so that there is
>no error reported in the logs.
>
>Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>---
> src/qemu/qemu_process.c | 65 +++++++++++++++++++----------------------
> 1 file changed, 30 insertions(+), 35 deletions(-)
>
>diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>index 04e5cbb65969..e29f35e10576 100644
>--- a/src/qemu/qemu_process.c
>+++ b/src/qemu/qemu_process.c
>@@ -2569,24 +2569,21 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm)
>             return -1;
>     }
>
>+    /*
>+     * We only want to error out if we failed to set the affinity to
>+     * user-requested mapping.  If we are just trying to reset the affinity
>+     * to all CPUs and this fails it can only be an issue if:
>+     *  1) libvirtd does not have CAP_SYS_NICE
>+     *  2) libvirtd does not run on all CPUs
>+     *
>+     * This scenario can easily occurr when libvirtd is run inside a

*occur

>+     * container with restrictive permissions and CPU pinning.
>+     *
>+     * See also: https://bugzilla.redhat.com/1819801#c2
>+     */
>     if (cpumapToSet &&
>-        virProcessSetAffinity(vm->pid, cpumapToSet, false) < 0) {
>-        /*
>-         * We only want to error out if we failed to set the affinity to
>-         * user-requested mapping.  If we are just trying to reset the affinity
>-         * to all CPUs and this fails it can only be an issue if:
>-         *  1) libvirtd does not have CAP_SYS_NICE
>-         *  2) libvirtd does not run on all CPUs
>-         *
>-         * This scenario can easily occurr when libvirtd is run inside a
>-         * container with restrictive permissions and CPU pinning.
>-         *
>-         * See also: https://bugzilla.redhat.com/1819801#c2
>-         */
>-        if (settingAll)
>-            virResetLastError();
>-        else
>-            return -1;
>+        virProcessSetAffinity(vm->pid, cpumapToSet, settingAll) < 0) {
>+        return -1;
>     }
>
>     return 0;
>@@ -2739,25 +2736,23 @@ qemuProcessSetupPid(virDomainObjPtr vm,
>     if (!affinity_cpumask)
>         affinity_cpumask = use_cpumask;
>
>-    /* Setup legacy affinity. */
>+    /* Setup legacy affinity.
>+     *
>+     * We only want to error out if we failed to set the affinity to
>+     * user-requested mapping.  If we are just trying to reset the affinity
>+     * to all CPUs and this fails it can only be an issue if:
>+     *  1) libvirtd does not have CAP_SYS_NICE
>+     *  2) libvirtd does not run on all CPUs
>+     *
>+     * This scenario can easily occurr when libvirtd is run inside a

*occur

>+     * container with restrictive permissions and CPU pinning.
>+     *
>+     * See also: https://bugzilla.redhat.com/1819801#c2
>+     */
>     if (affinity_cpumask &&
>-        virProcessSetAffinity(pid, affinity_cpumask, false) < 0) {
>-        /*
>-         * We only want to error out if we failed to set the affinity to
>-         * user-requested mapping.  If we are just trying to reset the affinity
>-         * to all CPUs and this fails it can only be an issue if:
>-         *  1) libvirtd does not have CAP_SYS_NICE
>-         *  2) libvirtd does not run on all CPUs
>-         *
>-         * This scenario can easily occurr when libvirtd is run inside a
>-         * container with restrictive permissions and CPU pinning.
>-         *
>-         * See also: https://bugzilla.redhat.com/1819801#c2
>-         */
>-        if (affinity_cpumask == hostcpumap)
>-            virResetLastError();
>-        else
>-            goto cleanup;
>+        virProcessSetAffinity(pid, affinity_cpumask,
>+                              affinity_cpumask == hostcpumap) < 0) {
>+        goto cleanup;
>     }
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano