[libvirt] [REPOST 2/4] qemu: Add check for NULL cgroup return from virCgroupNewMachine

[Martin Kletzander]

On Wed, Jan 13, 2016 at 07:29:48AM -0500, John Ferlan wrote:
>Commit id '71ce4759' altered the cgroup processing with respect to the
>call to virCgroupAddTask being moved out from lower layers into the calling
>layers especially for qemu processing of emulator and vcpu threads.
>
>What was missed in the processing was that it is possible for a code path to
>return a NULL cgroup *and* a 0 return status via virCgroupNewPartition
>failure when virCgroupNewIgnoreError succeeded when virCgroupNewMachineManual
>returns. This was initially seen for lxc cgroup processing and patched
>by comit id 'ae09988eb'. We'll make the same check here as a subsquent
>patch will revert commit id 'a41c00b47' which would leave the possibility
>that priv->cgroup is NULL.
>
>Signed-off-by: John Ferlan <jferlan at redhat.com>
>---
> src/qemu/qemu_cgroup.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
>index 91b3328..515d76a 100644
>--- a/src/qemu/qemu_cgroup.c
>+++ b/src/qemu/qemu_cgroup.c
>@@ -782,7 +782,7 @@ qemuInitCgroup(virQEMUDriverPtr driver,
>                             nnicindexes, nicindexes,
>                             vm->def->resource->partition,
>                             cfg->cgroupControllers,
>-                            &priv->cgroup) < 0) {
>+                            &priv->cgroup) < 0 || !priv->cgroup) {
>         if (virCgroupNewIgnoreError())

Why don't we check for virCgroupNewIgnoreError() in LXC?  If cgroups are
required there, then why *does* the vircgroup code check for that?  I
think we should not call that function here, mainly because
virCgroupNewMachine() calls that every time cgroup creation fails.
Calling this second time probably doesn't hurt now, but could be
potentially problematic in hypothetical future change.

Anyway, I think the proper call codeflow should be:

if (virCgroupNewMachine() < 0)
    goto cleanup;
if (!priv->cgroup)
    goto done;

It's more readable, more error-prone and so on.  Same with LXC.

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