On Thu, Aug 13, 2015 at 03:01:39PM +0200, Martin Kletzander wrote:
On Thu, Aug 13, 2015 at 02:49:34PM +0200, Ján Tomko wrote:
>s/reboot/daemon restart/ ?
>
Yes, exactly!
>On Thu, Aug 13, 2015 at 10:18:33AM +0200, Martin Kletzander wrote:
>> The numad hint stored in priv->autoNodeset is information that gets lost
>> during daemon restart. And because we would like to use that
>> information in the future, we also need to save it in the status XML.
>> For the sake of tests, we need to initialize nnumaCell_max to some
>> value, so that the restoration doesn't fail in our test suite. There is
>> no need to fill in the actual numa cell data since the recalculating
>> function virCapabilitiesGetCpusForNodemask() will not fail, it will just
>> skip filling the data in the bitmap which we don't use in tests anyway.
>>
>> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
>> ---
>> src/qemu/qemu_domain.c | 32 +++++++++++++++++++++++++++++++-
>> tests/qemuxml2xmltest.c | 3 ++-
>> tests/testutilsqemu.c | 2 ++
>> 3 files changed, 35 insertions(+), 2 deletions(-)
>
>> @@ -805,15 +817,33 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
>> }
>> VIR_FREE(nodes);
>>
>> + if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
>> + goto error;
>> +
>> + if ((tmp = virXPathString("string(./numad/@nodeset)", ctxt))) {
>> + if (virBitmapParse(tmp, 0, &priv->autoNodeset,
>> + caps->host.nnumaCell_max) < 0)
>> + goto error;
>> +
>> + if (!(priv->autoCpuset = virCapabilitiesGetCpusForNodemask(caps,
>> +
priv->autoNodeset)))
>> + goto error;
>> + }
>> + virObjectUnref(caps);
>> + caps = NULL;
>
>This assignment seems redundant.
>
I was thinking about the future when someone adds few lines after this
and will try to use 'caps'.
But what if someone tries to use it between the Unref and the NULL
assignment?
Feel free to remove it with follow-up
trivial patch if you don't like it.
I would not want to rob you of the chance to clean up after yourself.
Jan