Re: [libvirt] [PATCHv2] numatune: setting --mode does not work well

When trying to set numatune mode directly using virsh numatune command, correct error is raised, however numatune structure was not deallocated, thus resulting in creating an empty numatune element in the guest XML, if none was present before. Running the same command aftewards results in a successful change with broken XML structure. Patch fixes the deallocation problem as well as checking for invalid attribute combination VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO + a nonempty nodeset. Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1129998 --- src/conf/numatune_conf.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c index 48d1d04..3bfac56 100644 --- a/src/conf/numatune_conf.c +++ b/src/conf/numatune_conf.c @@ -439,7 +439,7 @@ virDomainNumatuneSet(virDomainNumatunePtr *numatunePtr, { bool create = !*numatunePtr; /* Whether we are creating new struct */ int ret = -1; - virDomainNumatunePtr numatune = NULL; + virDomainNumatunePtr numatune = *numatunePtr; /* No need to do anything in this case */ if (mode == -1 && placement == -1 && !nodeset) @@ -461,9 +461,15 @@ virDomainNumatuneSet(virDomainNumatunePtr *numatunePtr, goto cleanup; } - if (create && VIR_ALLOC(*numatunePtr) < 0) + if (placement_static && !nodeset) {

+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("nodeset for NUMA memory tuning must be set " + "if 'placement' is 'static'")); + goto cleanup; + } + + if (create && VIR_ALLOC(numatune) < 0) goto cleanup; - numatune = *numatunePtr; if (create) { /* Defaults for new struct */ @@ -492,12 +498,11 @@ virDomainNumatuneSet(virDomainNumatunePtr *numatunePtr, placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO; } - if (placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC && - !numatune->memory.nodeset) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("nodeset for NUMA memory tuning must be set " - "if 'placement' is 'static'")); - goto cleanup; + /* setting nodeset when placement auto is invalid */ + if (placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO && + numatune->memory.nodeset) { + virBitmapFree(numatune->memory.nodeset); + numatune->memory.nodeset = NULL; } if (placement != -1) @@ -505,8 +510,17 @@ virDomainNumatuneSet(virDomainNumatunePtr *numatunePtr, numatune->memory.specified = true; + if (create) { + *numatunePtr = numatune; + numatune = NULL; + } +

ret = 0; + return ret;

+ cleanup:

+ if (create) + virDomainNumatuneFree(numatune); return ret; }