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

> > -    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;

> > -    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.