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