Re: [libvirt] [PATCH 10/13] conf: Don't always recalculate initial memory size from NUMA size totals

On 21.09.2015 19:21, Peter Krempa wrote: > When implementing memory hotplug I've opted to recalculate the initial > memory size (contents of the <memory> element) as a sum of the sizes of > NUMA nodes when NUMA was enabled. This was based on an assumption that > qemu did not allow starting when the NUMA node size total didn't equal > to the initial memory size. Unfortunately the check was introduced to > qemu just lately. > > This patch uses the new XML parser flag to decide whether it's safe to > update the memory size total from the NUMA cell sizes or not. > > As an additional improvement we now report an error in case when the > size of hotplug memory would exceed the total memory size. > > The rest of the changes assures that the function is called with correct > flags. > --- > src/conf/domain_conf.c | 37 ++++++++++++++++++++++++++++++------- > src/conf/domain_conf.h | 1 + > src/qemu/qemu_command.c | 3 ++- > 3 files changed, 33 insertions(+), 8 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index d2a13ca..64cfd8c 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -3726,15 +3726,36 @@ virDomainDefRemoveDuplicateMetadata(virDomainDefPtr def) > > > static int > -virDomainDefPostParseMemory(virDomainDefPtr def) > +virDomainDefPostParseMemory(virDomainDefPtr def, > + unsigned int parseFlags) > { > size_t i; > + unsigned long long numaMemory = 0; > + unsigned long long hotplugMemory = 0; > > - if ((def->mem.initial_memory = virDomainNumaGetMemorySize(def->numa)) == 0) { > + /* Attempt to infer the initial memory size from the sum NUMA memory sizes > + * in case ABI updates are allowed or the <memory> element wasn't specified */ > + if (def->mem.total_memory == 0 || > + parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE) > + numaMemory = virDomainNumaGetMemorySize(def->numa); > + > + if (numaMemory) { > + def->mem.initial_memory = numaMemory; Is there a reason to not use the setter function you've introduced in previous patch?