On Fri, Jul 11, 2014 at 05:11:00PM +0200, Michal Privoznik wrote:
On 08.07.2014 13:50, Martin Kletzander wrote:There were numerous places where numatune configuration (and thus domain config as well) was changed in different ways. On some places this even resulted in persistent domain definition not to be stable (it would change with daemon's restart). In order to uniformly change how numatune config is dealt with, all the internals are now accessible directly only in numatune_conf.c and outside this file accessors must be used. Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> --- po/POTFILES.in | 1 + src/conf/domain_conf.c | 159 ++--------- src/conf/domain_conf.h | 8 +- src/conf/numatune_conf.c | 316 +++++++++++++++++++++ src/conf/numatune_conf.h | 72 ++++- src/libvirt_private.syms | 11 + src/lxc/lxc_cgroup.c | 19 +- src/lxc/lxc_controller.c | 5 +- src/lxc/lxc_native.c | 15 +- src/parallels/parallels_driver.c | 7 +- src/qemu/qemu_cgroup.c | 23 +- src/qemu/qemu_driver.c | 84 +++--- src/qemu/qemu_process.c | 8 +- src/util/virnuma.c | 48 ++-- src/util/virnuma.h | 2 +- .../qemuxml2argv-numatune-auto-prefer.xml | 29 ++ .../qemuxml2xmlout-numatune-auto-prefer.xml | 29 ++ tests/qemuxml2xmltest.c | 2 + 18 files changed, 553 insertions(+), 285 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-auto-prefer.xmlNice.
Thanks :) [...]
+ tmp = virXMLPropString(node, "nodeset"); + if (tmp && virBitmapParse(tmp, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) + goto cleanup; + VIR_FREE(tmp); + + if (virDomainNumatuneSet(def, placement, mode, nodeset) < 0)The virDomainNumatuneSet() takes a copy of @nodeset, so you need to call virBitmaskFree(nodeset); at the cleanup label.
Yep, that happens when you change the behaviour of a function that used to steal a pointer, in a rebase. Thanks!
+ goto cleanup; + + if (!n) { + ret = 0; + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(tmp); + return ret; +} + +int +virDomainNumatuneFormatXML(virBufferPtr buf, + virDomainNumatunePtr numatune) +{ + const char *tmp = NULL;s /const// ..+ + if (!numatune) + return 0; + + virBufferAddLit(buf, "<numatune>\n"); + virBufferAdjustIndent(buf, 2); + + tmp = virDomainNumatuneMemModeTypeToString(numatune->memory.mode); + virBufferAsprintf(buf, "<memory mode='%s' ", tmp); + + if (numatune->memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC) { + if (!(tmp = virBitmapFormat(numatune->memory.nodeset))) + return -1; + virBufferAsprintf(buf, "nodeset='%s'/>\n", tmp); + VIR_FREE(tmp);.. because free()-ing a const char * is not nice. If you, however, do this I bet you'll get error in TypeToString(). So just leave tmp as const char * and introduce char *nodeset;
I take the 'const' as a sign of the fact that I won't be modifying
any part of the string. Just adding 'const' to a pointer should be
perfectly OK, but I have not objections to your idea, so I squashed
this in:
diff --git i/src/conf/numatune_conf.c w/src/conf/numatune_conf.c
index 8b66558..375428c 100644
--- i/src/conf/numatune_conf.c
+++ w/src/conf/numatune_conf.c
@@ -141,6 +142,7 @@ virDomainNumatuneFormatXML(virBufferPtr buf,
virDomainNumatunePtr numatune)
{
const char *tmp = NULL;
+ char *nodeset = NULL;
if (!numatune)
return 0;
@@ -152,10 +154,10 @@ virDomainNumatuneFormatXML(virBufferPtr buf,
virBufferAsprintf(buf, "<memory mode='%s' ", tmp);
if (numatune->memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC) {
- if (!(tmp = virBitmapFormat(numatune->memory.nodeset)))
+ if (!(nodeset = virBitmapFormat(numatune->memory.nodeset)))
return -1;
virBufferAsprintf(buf, "nodeset='%s'/>\n", tmp);
- VIR_FREE(tmp);
+ VIR_FREE(nodeset);
} else if (numatune->memory.placement) {
tmp = virDomainNumatunePlacementTypeToString(numatune->memory.placement);
virBufferAsprintf(buf, "placement='%s'/>\n", tmp);
--
Martin
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list