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.