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

[Andrea Bolognani]

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 at redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization