Re: [libvirt] [PATCH 3/5] qemu: domain: Store and restore autoCpuset to status XML
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(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization