9:56 p.m.
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
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3714,21 +3714,24 @@ virDomainDefPostParseMemory(virDomainDefPtr def,
parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE)
numaMemory = virDomainNumaGetMemorySize(def->numa);
+ /* calculate the sizes of hotplug memory */
+ for (i = 0; i < def->nmems; i++)
+ hotplugMemory += def->mems[i]->size;
+
if (numaMemory) {
- virDomainDefSetMemoryInitial(def, numaMemory);
+ /* update the sizes in XML if nothing was set in the XML or ABI update
+ * is supported*/
+ virDomainDefSetMemoryTotal(def, numaMemory + hotplugMemory);
} else {
- /* calculate the sizes of hotplug memory */
- for (i = 0; i < def->nmems; i++)
- hotplugMemory += def->mems[i]->size;
-
+ /* verify that the sum of memory modules doesn't exceed the total
+ * memory. This is necessary for virDomainDefGetMemoryInitial to work
+ * properly. */
if (hotplugMemory > def->mem.total_memory) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("Total size of memory devices exceeds the total "
"memory size"));
return -1;
}
-
- virDomainDefSetMemoryInitial(def, def->mem.total_memory - hotplugMemory);
}
if (virDomainDefGetMemoryInitial(def) == 0) {
@@ -8041,13 +8044,18 @@ virDomainDefHasMemoryHotplug(const virDomainDef *def)
* @def: domain definition
*
* Returns the size of the initial amount of guest memory. The initial amount
- * is the memory size is either the configured amount in the <memory> element
- * or the sum of memory sizes of NUMA nodes in case NUMA is enabled in @def.
+ * is the memory size excluding possible memory modules.
*/
unsigned long long
virDomainDefGetMemoryInitial(const virDomainDef *def)
{
- return def->mem.initial_memory;
+ size_t i;
+ unsigned long long ret = def->mem.total_memory;
+
+ for (i = 0; i < def->nmems; i++)
+ ret -= def->mems[i]->size;
+
+ return ret;
}
@@ -8056,30 +8064,14 @@ virDomainDefGetMemoryInitial(const virDomainDef *def)
* @def: domain definition
* @size: size to set
*
- * Sets the total memory size in @def. This function should be used only by
- * hypervisors that don't support memory hotplug.
+ * Sets the total memory size in @def. This value needs to include possible
+ * additional memory modules.
*/
void
virDomainDefSetMemoryTotal(virDomainDefPtr def,
unsigned long long size)
{
def->mem.total_memory = size;
- def->mem.initial_memory = size;
-}
-
-
-/**
- * virDomainDefSetMemoryInitial:
- * @def: domain definition
- * @size: size to set
- *
- * Sets the initial memory size (without memory modules) in @def.
- */
-void
-virDomainDefSetMemoryInitial(virDomainDefPtr def,
- unsigned long long size)
-{
- def->mem.initial_memory = size;
}
@@ -8088,21 +8080,12 @@ virDomainDefSetMemoryInitial(virDomainDefPtr def,
* @def: domain definition
*
* Returns the current maximum memory size usable by the domain described by
- * @def. This size is a sum of size returned by virDomainDefGetMemoryInitial
- * and possible additional memory devices.
+ * @def. This size includes possible additional memory devices.
*/
unsigned long long
virDomainDefGetMemoryActual(virDomainDefPtr def)
{
- unsigned long long ret;
- size_t i;
-
- ret = def->mem.initial_memory;
-
- for (i = 0; i < def->nmems; i++)
- ret += def->mems[i]->size;
-
- return ret;
+ return def->mem.total_memory;
}
@@ -14551,10 +14534,18 @@ virDomainMemoryFindInactiveByDef(virDomainDefPtr def,
}
+/**
+ * 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;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 15f9c80..96ac510 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2070,8 +2070,6 @@ struct _virDomainMemtune {
/* total memory size including memory modules in kibibytes, this field
* should be accessed only via accessors */
unsigned long long total_memory;
- /* initial memory size in kibibytes = total_memory excluding memory modules*/
- unsigned long long initial_memory;
unsigned long long cur_balloon; /* in kibibytes, capped at ulong thanks
to virDomainGetInfo */
@@ -2272,7 +2270,6 @@ virDomainVcpuInfoPtr virDomainDefGetVcpu(virDomainDefPtr def,
unsigned int vcpu)
unsigned long long virDomainDefGetMemoryInitial(const virDomainDef *def);
void virDomainDefSetMemoryTotal(virDomainDefPtr def, unsigned long long size);
-void virDomainDefSetMemoryInitial(virDomainDefPtr def, unsigned long long size);
unsigned long long virDomainDefGetMemoryActual(virDomainDefPtr def);
bool virDomainDefHasMemoryHotplug(const virDomainDef *def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e939de3..3d1716c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -240,7 +240,6 @@ virDomainDefParseFile;
virDomainDefParseNode;
virDomainDefParseString;
virDomainDefPostParse;
-virDomainDefSetMemoryInitial;
virDomainDefSetMemoryTotal;
virDomainDefSetVcpus;
virDomainDefSetVcpusMax;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d1f8175..8df3996 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4718,6 +4718,7 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def)
unsigned long long maxmemkb = virMemoryMaxValue(false) >> 10;
unsigned long long maxmemcapped = virMemoryMaxValue(true) >> 10;
unsigned long long initialmem = 0;
+ unsigned long long hotplugmem = 0;
unsigned long long mem;
unsigned long long align = qemuDomainGetMemorySizeAlignment(def);
size_t ncells = virDomainNumaGetNodeCount(def->numa);
@@ -4748,8 +4749,6 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def)
return -1;
}
- virDomainDefSetMemoryInitial(def, initialmem);
-
def->mem.max_memory = VIR_ROUND_UP(def->mem.max_memory, align);
if (def->mem.max_memory > maxmemkb) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -4761,6 +4760,7 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def)
for (i = 0; i < def->nmems; i++) {
align = qemuDomainGetMemoryModuleSizeAlignment(def, def->mems[i]);
def->mems[i]->size = VIR_ROUND_UP(def->mems[i]->size, align);
+ hotplugmem += def->mems[i]->size;
if (def->mems[i]->size > maxmemkb) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -4770,6 +4770,8 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def)
}
}
+ virDomainDefSetMemoryTotal(def, initialmem + hotplugmem);
+
return 0;
}
--
2.8.3