Re: [libvirt] [PATCH 1/2] conf: Remove pre-calculation of initial memory size

While we need to know the difference between the total memory stored in <memory> and the actual size not included in the possible memory modules we can't pre-calculate it reliably. This is due to the fact that libvirt's XML is copied via formatting and parsing the XML and the initial memory size can be reliably calculated only when certain conditions are met due to backwards compatibility. This patch removes the storage of 'initial_memory' and fixes the helpers to recalculate the initial memory size all the time from the total memory size. This conversion is possible when we also make sure that memory hotplug accounts properly for the update of the total memory size and thus the helpers for inserting and removing memory devices need to be tweaked too. This fixes a bug where a cold-plug and cold-remove of a memory device would increase the size reported in <memory> in the XML by the size of the memory device. This would happen as the persistent definition is copied before attaching the device and this would lead to the loss of data in 'initial_memory'. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1344892 --- src/conf/domain_conf.c | 85 +++++++++++++++++++++++++----------------------- src/conf/domain_conf.h | 3 -- src/libvirt_private.syms | 1 - src/qemu/qemu_domain.c | 6 ++-- 4 files changed, 49 insertions(+), 46 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9504e5f..10e9e8b 100644

+/** + * virDomainMemoryInsert: + * + * Inserts a memory device definition into the domain definition. This helper + * should be used only in hotplug cases as it's blindly modifying the total + * memory size. + */

int virDomainMemoryInsert(virDomainDefPtr def, virDomainMemoryDefPtr mem) { + unsigned long long memory = virDomainDefGetMemoryActual(def); int id = def->nmems; if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && @@ -14565,24 +14556,38 @@ virDomainMemoryInsert(virDomainDefPtr def, return -1; } - if (VIR_APPEND_ELEMENT(def->mems, def->nmems, mem) < 0) + if (VIR_APPEND_ELEMENT_COPY(def->mems, def->nmems, mem) < 0) return -1; + virDomainDefSetMemoryTotal(def, memory + mem->size); + return id; } +/** + * virDomainMemoryInsert:

+ * + * Removes a memory device definition into the domain definition. This helper

+ * should be used only in hotplug cases as it's blindly modifying the total + * memory size. + */ virDomainMemoryDefPtr virDomainMemoryRemove(virDomainDefPtr def, int idx) { + unsigned long long memory = virDomainDefGetMemoryActual(def); virDomainMemoryDefPtr ret = def->mems[idx]; + VIR_DELETE_ELEMENT(def->mems, idx, def->nmems); /* fix up balloon size */ if (def->mem.cur_balloon > virDomainDefGetMemoryActual(def)) def->mem.cur_balloon = virDomainDefGetMemoryActual(def);

+ /* fix total memory size of the domain */ + virDomainDefSetMemoryTotal(def, memory - ret->size); + return ret; }