[libvirt] [PATCH v4 3/3] virnuma: use virNumaNodesetIsAvailable checking nodeset in virNumaSetupMemoryPolicy

[Martin Kletzander]

On Tue, Nov 04, 2014 at 02:05:16AM +0000, Chen, Fan wrote:
>On Mon, 2014-11-03 at 14:18 +0100, Martin Kletzander wrote:
>> On Thu, Oct 30, 2014 at 01:44:19PM +0800, Chen Fan wrote:
>> >Signed-off-by: Chen Fan <chen.fan.fnst at cn.fujitsu.com>
>> >---
>> > src/util/virnuma.c | 23 ++++-------------------
>> > 1 file changed, 4 insertions(+), 19 deletions(-)
>> >
>> >diff --git a/src/util/virnuma.c b/src/util/virnuma.c
>> >index 4188ef5..613a43c 100644
>> >--- a/src/util/virnuma.c
>> >+++ b/src/util/virnuma.c
>> >@@ -95,31 +95,19 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
>> >     int ret = -1;
>> >     int bit = 0;
>> >     size_t i;
>> >-    int maxnode = 0;
>> >     virBitmapPtr tmp_nodemask = NULL;
>> >
>> >+    if (!virNumaNodesetIsAvailable(numatune))

Here you call virNumaNodesetIsAvailable() with @numatune, but ...

>> >+        return -1;
>> >+
>> >     tmp_nodemask = virDomainNumatuneGetNodeset(numatune, nodemask, -1);

... here you can get the automatic one ...

>> I think this is safe, "numad" returning nodeset that's not on the host
>> would be weird error and it is easy to find in the logs.
>I think virNumaNodesetIsAvailable() has checked the case, but retain it
>here is ok.
>

... and that's what I meant here that it might be missed.  I would be
OK with the check removed though, since that should create no new
problems, but since you added it in the next version, I'll keep it
there ;)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141104/da5dcf75/attachment-0001.sig>