On 08.07.2014 13:50, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> --- 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.
+ 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.
if (!numatune) return; @@ -324,26 +337,37 @@ virDomainNumatuneFree(virDomainNumatunePtr numatune) } virDomainNumatuneMemMode -virDomainNumatuneGetMode(virDomainNumatunePtr numatune) +virDomainNumatuneGetMode(virDomainNumatunePtr numatune, + int cellid) { - return (numatune && numatune->memory.specified) ? numatune->memory.mode : 0; + if (!numatune) + return 0; + + if (numa_cell_specified(numatune, cellid)) + return numatune->mem_nodes[cellid].mode; + + if (numatune->memory.specified) + return numatune->memory.mode; + + return 0; } virBitmapPtr virDomainNumatuneGetNodeset(virDomainNumatunePtr numatune, - virBitmapPtr auto_nodeset) + virBitmapPtr auto_nodeset, + int cellid) { if (!numatune) return NULL; - if (numatune->memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO) + if (numatune->memory.specified && + numatune->memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO) return auto_nodeset; - /* - * This weird logic has the same meaning as switch with - * auto/static/default, but can be more readably changed later. - */ - if (numatune->memory.placement != VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC) + if (numa_cell_specified(numatune, cellid)) + return numatune->mem_nodes[cellid].nodeset; + + if (!numatune->memory.specified) return NULL; return numatune->memory.nodeset; @@ -351,23 +375,30 @@ virDomainNumatuneGetNodeset(virDomainNumatunePtr numatune, char * virDomainNumatuneFormatNodeset(virDomainNumatunePtr numatune, - virBitmapPtr auto_nodeset) + virBitmapPtr auto_nodeset, + int cellid) { return virBitmapFormat(virDomainNumatuneGetNodeset(numatune, - auto_nodeset)); + auto_nodeset, + cellid)); } int virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr numatune, virBitmapPtr auto_nodeset, - char **mask) + char **mask, + int cellid) { *mask = NULL; if (!numatune) return 0; - if (numatune->memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO && + if (!numa_cell_specified(numatune, cellid) && !numatune->memory.specified) + return 0; + + if (numatune->memory.specified && + numatune->memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO && !auto_nodeset) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Advice from numad is needed in case of " @@ -375,7 +406,7 @@ virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr numatune, return -1; } - *mask = virDomainNumatuneFormatNodeset(numatune, auto_nodeset); + *mask = virDomainNumatuneFormatNodeset(numatune, auto_nodeset, cellid); if (!*mask) return -1; @@ -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?
+ + if (!nd1->nodeset || !nd2->nodeset) + return false; + + if (nd1->mode != nd2->mode) + return false; + + if (!virBitmapEqual(nd1->nodeset, nd2->nodeset)) + return false; + } + + return true; +} +
Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list