[libvirt] [PATCH 3/5] qemu: domain: Store and restore autoCpuset to status XML

[Andrea Bolognani]

On Thu, 2017-07-20 at 15:46 +0200, Peter Krempa wrote:
> > > -    virBufferAsprintf(buf, "<numad nodeset='%s'/>\n", nodeset);
> > > +    if (priv->autoCpuset &&
> > > +        !((cpuset = virBitmapFormat(priv->autoCpuset))))
> > > +        goto cleanup;
> > 
> > The previous implementation didn't format the <numad>
> > element at all if there was nodeset, whereas the new one
> > will always format it. You could add
> > 
> >     if (!nodeset && !cpuset)
> >         goto cleanup;
> 
> If you call virBitmapFormat on an empty or NULL bitmap you still get a
> (empty) string allocated so that condition is basically identical to the
> one that's already there due to how the bitmaps are formatted:
> 
>     if (!priv->autoNodeset && !priv->autoCpuset)
>         return 0;
> 
>     if (priv->autoNodeset &&
>         !((nodeset = virBitmapFormat(priv->autoNodeset))))
>         goto cleanup;
> 
>     if (priv->autoCpuset &&
>         !((cpuset = virBitmapFormat(priv->autoCpuset))))
>         goto cleanup;

Oh, you're right. Nevermind then.

> > > -    if (!nodeset)
> > > +    if (!nodeset && !cpuset)
> > >           return 0;
> > 
> > I don't think you want this hunk.
> > 
> > With the new condition, you will perform an early exit only
> > if both nodeset and cpuset are NULL; if nodeset is NULL but
> > cpuset isn't, the first call to virBitmapParse() a few lines
> > below will error out.
> 
> That shouldn't ever happen (tm) :D
> 
> I'll can add a condition that if nodeset is not in the XML the parsing
> will be skipped. So in that case only cpuset can be present (for future
> use).
> 
> I'll also add a note that accessing autoNodeset in the else branch of if
> (cpuset) is safe.

Works for me :)

-- 
Andrea Bolognani / Red Hat / Virtualization