Re: [libvirt] [PATCH] conf: Format numatune XML correctly while placement is none

setNumaParameters tunes the numa setting using cgroup, it's another entry except libnuma/numad for numa tuning. And it doesn't set the placement, and further more, the formating codes doesn't take this into consideration. How to reproduce: conn = libvirt.open(None) dom = conn.lookupByName('linux') param = {'numa_nodeset': '0', 'numa_mode': 1} dom.setNumaParameters(param, 2) % virsh start linux error: Failed to start domain rhel6.3rc error: (domain_definition):8: error parsing attribute name <memory mode='preferred' </numatune> -------------------------------^ --- By the way, I see problems of setNumaParameters too. conn = libvirt.open(None) dom = conn.lookupByName('linux') param = {'numa_mode': 1} dom.setNumaParameters(param, 2) The numa 'mode' will be just ignored, and no 'numatune' XML is formated, as neither 'nodeset' nor 'placement' is specified. I'd think it's right to ignore it when formating, it's meaningless to only specify the 'mode'. However, we might have to fix setNumaParameters to prevent setting the numa mode without nodeset, and error out, as it's really a bad user experience to see the API call succeeded, but the expected XML doesn't show up in the end. --- src/conf/domain_conf.c | 17 ++++++++++------- 1 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 81c6308..c44d89d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12795,23 +12795,26 @@ virDomainDefFormatInternal(virDomainDefPtr def, const char *placement; mode = virDomainNumatuneMemModeTypeToString(def->numatune.memory.mode); - virBufferAsprintf(buf, " <memory mode='%s' ", mode); + virBufferAsprintf(buf, " <memory mode='%s'", mode); - if (def->numatune.memory.placement_mode == - VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC) { + if (def->numatune.memory.nodemask) { nodemask = virDomainCpuSetFormat(def->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + VIR_DOMAIN_CPUMASK_LEN); if (nodemask == NULL) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to format nodeset for " "NUMA memory tuning")); goto cleanup; } - virBufferAsprintf(buf, "nodeset='%s'/>\n", nodemask); + virBufferAsprintf(buf, " nodeset='%s'/>\n", nodemask); VIR_FREE(nodemask); - } else if (def->numatune.memory.placement_mode) { + } else if (def->numatune.memory.placement_mode == + VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO) { placement = virDomainNumatuneMemPlacementModeTypeToString(def->numatune.memory.placement_mode); - virBufferAsprintf(buf, "placement='%s'/>\n", placement); + virBufferAsprintf(buf, " placement='%s'/>\n", placement); + } else { + /* Should not hit here. */ + virBufferAddLit(buf, "/>\n"); } virBufferAddLit(buf, " </numatune>\n"); }