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@xxxxxxxxxx>
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
