On Wed, 2017-07-12 at 15:44 +0200, Peter Krempa wrote:
@@ -1765,20 +1765,30 @@ qemuDomainObjPtrivateXMLFormatAutomaticPlacement(virBufferPtr buf, qemuDomainObjPrivatePtr priv) { [...] - 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; here to restore that behavior, but maybe you just decided it's not worth it. Just felt like I should point that out.
+ virBufferAddLit(buf, "<numad"); + virBufferEscapeString(buf, " nodeset='%s'", nodeset); + virBufferEscapeString(buf, " cpuset='%s'", cpuset);
I had to look up the implementation of virBufferEscapeString() to figure out that nodeset or cpuset possibly being NULL is handled automatically by that function, which I found to be a pretty surprising behavior. Not a problem with your patch though :)
@@ -1958,11 +1968,13 @@ qemuDomainObjPrivateXMLParseAutomaticPlacement(virQEMUDriverPtr driver, { virCapsPtr caps = NULL; char *nodeset; + char *cpuset; int ret = -1; nodeset = virXPathString("string(./numad/@nodeset)", ctxt); + cpuset = virXPathString("string(./numad/@cpuset)", ctxt); - 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. But leaving the condition alone makes sure all scenarios are handled successfully. Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization