[libvirt] [PATCH v2 09/16] numatune: add support for per-node memory bindings in private APIs

[Martin Kletzander]

On Fri, Jul 11, 2014 at 05:11:02PM +0200, Michal Privoznik wrote:
>On 08.07.2014 13:50, Martin Kletzander wrote:
>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>> ---
>>   src/conf/numatune_conf.c | 111 ++++++++++++++++++++++++++++++++++++++++-------
>>   src/conf/numatune_conf.h |  14 ++++--
>>   src/libvirt_private.syms |   1 +
>>   src/lxc/lxc_cgroup.c     |   3 +-
>>   src/qemu/qemu_cgroup.c   |   2 +-
>>   src/qemu/qemu_driver.c   |  10 ++---
>>   src/util/virnuma.c       |   6 +--
>>   7 files changed, 117 insertions(+), 30 deletions(-)
>>
>> diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
>> index 67fc799..60b6867 100644
>> --- a/src/conf/numatune_conf.c
>> +++ b/src/conf/numatune_conf.c
>> @@ -63,6 +63,18 @@ struct _virDomainNumatune {
>>   };
>>
>>
>> +static inline bool
>> +numa_cell_specified(virDomainNumatunePtr numatune,
>
>Whoa, when I met this function call I thought to myself that this is
>some libnuma function. Please name it differently to match our
>virSomethingShiny pattern.
>

I wanted this function to stand out as it is a macro (or static
inline) and I've seen it somewhere else in the code.  I changed it to
virDomainNumatuneNodeSpecified(ewww, gross) and all the calls too
(with proper wrapping as well).

>> +                    int cellid)
>> +{
>> +    if (numatune &&
>> +        cellid >= 0 &&
>> +        cellid < numatune->nmem_nodes)
>> +        return numatune->mem_nodes[cellid].nodeset;
>> +
>> +    return false;
>> +}
>> +
>>   static int
>>   virDomainNumatuneNodeParseXML(virDomainDefPtr def,
>>                                 xmlXPathContextPtr ctxt)
>> @@ -312,6 +324,7 @@ void
>>   virDomainNumatuneFree(virDomainNumatunePtr numatune)
>>   {
>>       size_t i = 0;
>> +
>
>This change is spurious. Either move it to 8/16 or drop this one.
>

Done.

[...]
>> @@ -469,6 +500,35 @@ virDomainNumatuneSet(virDomainDefPtr def,
>>       return ret;
>>   }
>>
>> +static bool
>> +virDomainNumatuneNodesEqual(virDomainNumatunePtr n1,
>> +                            virDomainNumatunePtr n2)
>> +{
>> +    size_t i = 0;
>> +
>> +    if (n1->nmem_nodes != n2->nmem_nodes)
>> +        return false;
>> +
>> +    for (i = 0; i < n1->nmem_nodes; i++) {
>> +        virDomainNumatuneNodePtr nd1 = &n1->mem_nodes[i];
>> +        virDomainNumatuneNodePtr nd2 = &n2->mem_nodes[i];
>> +
>> +        if (!nd1->nodeset && !nd2->nodeset)
>> +            continue;
>
>So if both are missing nodeset, they are considered equal? What if they
>differ in mode?
>

Yes, because !nd->nodeset means there was no <memnode/> with that
cellid.  Therefore mode doesn't make sense (and is not set anyway).

Martin
-------------- 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/20140715/6c598616/attachment-0001.sig>