On Fri, Apr 23, 2021 at 15:24:29 +0200, Michal Privoznik wrote:
As advertised in one of previous commits, we want to be able to
change 'requested-size' attribute of virtio-mem on the fly. This
commit does exactly that. Changing anything else is checked for
and forbidden.
Once guest has changed the allocation, QEMU emits an event which
we will use to track the allocation. In the next commit.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 36 ++++++++
src/conf/domain_conf.h | 4 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 172 ++++++++++++++++++++++++++++++++++-
src/qemu/qemu_hotplug.c | 18 ++++
src/qemu/qemu_hotplug.h | 5 +
src/qemu/qemu_monitor.c | 13 +++
src/qemu/qemu_monitor.h | 5 +
src/qemu/qemu_monitor_json.c | 15 +++
src/qemu/qemu_monitor_json.h | 5 +
10 files changed, 273 insertions(+), 1 deletion(-)
[...]
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d908e95ba7..9a241ad551 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7130,6 +7130,165 @@ qemuDomainChangeDiskLive(virDomainObj *vm,
return 0;
}
+
+static bool
+qemuDomainChangeMemoryLiveValidateChange(const virDomainMemoryDef *oldDef,
+ const virDomainMemoryDef *newDef)
+{
I must say this function feels almost as the ABI stability checker.
+ /* The only thing that is allowed to change is
'requestedsize' for virtio
+ * model. */
Also this is a bit weird. You mention that we are dealing with
virtio-mem ...
+ if (oldDef->model != newDef->model) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot modify memory model from '%s' to
'%s'"),
+ virDomainMemoryModelTypeToString(oldDef->model),
+ virDomainMemoryModelTypeToString(newDef->model));
+ return false;
+ }
+
+ if (oldDef->access != newDef->access) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot modify memory access from '%s' to
'%s'"),
+ virDomainMemoryAccessTypeToString(oldDef->access),
+ virDomainMemoryAccessTypeToString(newDef->access));
+ return false;
+ }
+
+ if (oldDef->discard != newDef->discard) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot modify memory discard from '%s' to
'%s'"),
+ virTristateBoolTypeToString(oldDef->discard),
+ virTristateBoolTypeToString(newDef->discard));
+ return false;
+ }
+
+ if (!virBitmapEqual(oldDef->sourceNodes,
+ newDef->sourceNodes)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("cannot modify memory source nodes"));
+ return false;
+ }
+
+ if (oldDef->pagesize != newDef->pagesize) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot modify memory pagesize from '%llu' to
'%llu'"),
+ oldDef->pagesize,
+ newDef->pagesize);
+ return false;
+ }
+
+ if (STRNEQ_NULLABLE(oldDef->nvdimmPath, newDef->nvdimmPath)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot modify memory path from '%s' to
'%s'"),
+ NULLSTR(oldDef->nvdimmPath),
+ NULLSTR(newDef->nvdimmPath));
+ return false;
+ }
... but also check stuff not relevant to virtio-mem ...
+
+ if (oldDef->alignsize != newDef->alignsize) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot modify memory align size from '%llu' to
'%llu'"),
+ oldDef->alignsize, newDef->alignsize);
+ return false;
+ }
+
+ if (oldDef->nvdimmPmem != newDef->nvdimmPmem) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot modify memory pmem from '%d' to
'%d'"),
+ oldDef->nvdimmPmem, newDef->nvdimmPmem);
+ return false;
+ }
+
+ if (oldDef->targetNode != newDef->targetNode) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot modify memory targetNode from '%d' to
'%d'"),
+ oldDef->targetNode, newDef->targetNode);
+ return false;
+ }
+
+ if (oldDef->size != newDef->size) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot modify memory size from '%llu' to
'%llu'"),
+ oldDef->size, newDef->size);
+ return false;
+ }
+
+ if (oldDef->labelsize != newDef->labelsize) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot modify memory label size from '%llu' to
'%llu'"),
+ oldDef->labelsize, newDef->labelsize);
+ return false;
+ }
+ if (oldDef->blocksize != newDef->blocksize) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot modify memory block size from '%llu' to
'%llu'"),
+ oldDef->blocksize, newDef->blocksize);
+ return false;
+ }
+
+ /* requestedsize can change */
+
+ if (oldDef->readonly != newDef->readonly) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("cannot modify memory pmem flag"));
+ return false;
+ }
+
+ if ((oldDef->uuid || newDef->uuid) &&
+ !(oldDef->uuid && newDef->uuid &&
+ memcmp(oldDef->uuid, newDef->uuid, VIR_UUID_BUFLEN) == 0)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("cannot modify memory UUID"));
+ return false;
+ }
+
+ switch (oldDef->model) {
+ case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
+ break;
+
+ case VIR_DOMAIN_MEMORY_MODEL_NONE:
+ case VIR_DOMAIN_MEMORY_MODEL_DIMM:
+ case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
+ case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM:
+ case VIR_DOMAIN_MEMORY_MODEL_LAST:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot modify memory of model '%s'"),
+ virDomainMemoryModelTypeToString(oldDef->model));
+ return false;
... but at the end only allow virtio-mem, so the irrelevant checks are
actually not even necessary.
+ break;
+ }
+
+ return true;
+}
+
+
+static int
+qemuDomainChangeMemoryLive(virQEMUDriver *driver G_GNUC_UNUSED,
+ virDomainObj *vm,
+ virDomainDeviceDef *dev)
+{
+ virDomainMemoryDef *newDef = dev->data.memory;
+ virDomainMemoryDef *oldDef = NULL;
+
+ oldDef = virDomainMemoryFindByDeviceInfo(vm->def,
&dev->data.memory->info);
+ if (!oldDef) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("memory '%s' not found"),
dev->data.memory->info.alias);
[1]
+ return -1;
+ }
+
+ if (!qemuDomainChangeMemoryLiveValidateChange(oldDef, newDef))
+ return -1;
+
+ if (qemuDomainChangeMemoryRequestedSize(driver, vm,
+ newDef->info.alias,
+ newDef->requestedsize) < 0)
+ return -1;
+
+ oldDef->requestedsize = newDef->requestedsize;
+ return 0;
+}
+
+
static int
qemuDomainUpdateDeviceLive(virDomainObj *vm,
virDomainDeviceDef *dev,
@@ -7171,6 +7330,18 @@ qemuDomainUpdateDeviceLive(virDomainObj *vm,
ret = qemuDomainChangeNet(driver, vm, dev);
break;
+ case VIR_DOMAIN_DEVICE_MEMORY:
+ oldDev.data.memory = virDomainMemoryFindByDeviceInfo(vm->def,
&dev->data.memory->info);
+ if (oldDev.data.memory) {
Also this is a bit weird too. Here you do this optionally if
virDomainMemoryFindByDeviceInfo finds the device ...
+ if (virDomainDefCompatibleDevice(vm->def, dev,
&oldDev,
+ VIR_DOMAIN_DEVICE_ACTION_UPDATE,
+ true) < 0)
+ return -1;
+ }
+
+ ret = qemuDomainChangeMemoryLive(driver, vm, dev);
... but here the first thing that is done is that virDomainMemoryFindByDeviceInfo
is called again and a NULL return is fatal.
+ break;
+
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_INPUT:
case VIR_DOMAIN_DEVICE_SOUND:
With the wierdness reduced:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>