[libvirt] [PATCH] qemu: don't error out when cgroups don't exist

[Martin Kletzander]

On Wed, Jul 09, 2014 at 01:07:47PM +0200, Ján Tomko wrote:
>On 07/09/2014 10:15 AM, Martin Kletzander wrote:
>> When creating cgroups for vcpu and emulator threads whilst starting a
>> domain, we explicitly skip creating those cgroups in case priv->cgroup
>> is NULL (cgroups not supported) because SetAffinity() serves the same
>> purpose.  If the host supports only some cgroups (the ones we need are
>> either unmounted or disabled in qemu.conf), we error out with weird
>> message even though we could continue starting the domain.
>
>Yet this patch does not change the error message.
>

Yes, because there should be no error.

>>
>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1097028
>>
>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>> ---
>>  src/qemu/qemu_cgroup.c | 12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
>> index 3394c68..0af6ac5 100644
>> --- a/src/qemu/qemu_cgroup.c
>> +++ b/src/qemu/qemu_cgroup.c
>> @@ -949,7 +949,11 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
>>          virCgroupFree(&cgroup_vcpu);
>>      }
>>
>> -    return -1;
>> +    if (period || quota)
>> +        return -1;
>> +
>> +    virResetLastError();
>> +    return 0;
>>  }
>>
>
>This also resets OOM errors and errors that happen when these controllers are
>mounted.
>
>Can't we just check upfront if the needed controllers are available?
>

There might be more problems than the controllers not being mounted,
but OK, the others are corner cases anyway.  Would the following diff
be more suitable?

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 3394c68..d4c969b 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -893,6 +893,15 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
         return -1;
     }

+    /*
+     * If CPU cgroup controller is not initialized here, than we need
+     * neither period nor quota settings.  And if CPUSET controller is
+     * not initialized either, than there's nothing to do anyway.
+     */
+    if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) &&
+        !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
+        return 0;
+
     /* We are trying to setup cgroups for CPU pinning, which can also
     be done
      * with virProcessSetAffinity, thus the lack of cgroups is not
      fatal here.
      */
@@ -972,6 +981,15 @@ qemuSetupCgroupForEmulator(virQEMUDriverPtr driver,
         return -1;
     }

+    /*
+     * If CPU cgroup controller is not initialized here, than we need
+     * neither period nor quota settings.  And if CPUSET controller is
+     * not initialized either, than there's nothing to do anyway.
+     */
+    if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) &&
+        !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
+        return 0;
+
     if (priv->cgroup == NULL)
         return 0; /* Not supported, so claim success */

--

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140709/c9bc91cd/attachment-0001.sig>