On 03/17/2015 10:20 AM, Peter Krempa wrote:
Add code to hot-add memory devices to running qemu instances.
---
Notes:
Version 3:
- added comment to clarify that @mem is always consumed by qemuDomainAttachMemory
Version 2:
- no change
Version 2:
- no change
src/qemu/qemu_command.c | 4 +--
src/qemu/qemu_command.h | 15 ++++++++
src/qemu/qemu_driver.c | 5 ++-
src/qemu/qemu_hotplug.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_hotplug.h | 3 ++
5 files changed, 119 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2d85567..1f72437 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4599,7 +4599,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
* other configuration was used (to detect legacy configurations). Returns
* -1 in case of an error.
*/
-static int
+int
qemuBuildMemoryBackendStr(unsigned long long size,
unsigned long long pagesize,
int guestNode,
@@ -4872,7 +4872,7 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem,
}
-static char *
+char *
qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
virQEMUCapsPtr qemuCaps)
{
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index ee81f92..a29db41 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -162,6 +162,21 @@ char *qemuBuildSoundDevStr(virDomainDefPtr domainDef,
virDomainSoundDefPtr sound,
virQEMUCapsPtr qemuCaps);
+int qemuBuildMemoryBackendStr(unsigned long long size,
+ unsigned long long pagesize,
+ int guestNode,
+ virBitmapPtr userNodeset,
+ virBitmapPtr autoNodeset,
+ virDomainDefPtr def,
+ virQEMUCapsPtr qemuCaps,
+ virQEMUDriverConfigPtr cfg,
+ const char **backendType,
+ virJSONValuePtr *backendProps,
+ bool force);
+
+char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
+ virQEMUCapsPtr qemuCaps);
+
/* Legacy, pre device support */
char *qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev,
virQEMUCapsPtr qemuCaps);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e948cca..cbdf279 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7649,8 +7649,11 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
dev->data.rng = NULL;
break;
- /*TODO: implement later */
case VIR_DOMAIN_DEVICE_MEMORY:
+ ret = qemuDomainAttachMemory(driver, vm,
+ dev->data.memory);
+ dev->data.memory = NULL;
+ break;
FWIW: Although there is a note in the AttachMemory code about
consumption it may be worth noting here too just so no one in the future
"sees" it missing and adds it thinking it's necessary
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_FS:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5c5ad0e..88c5e3c 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1672,6 +1672,101 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
}
+/**
+ * qemuDomainAttachMemory:
+ * @driver: qemu driver data
+ * @vm: VM object
+ * @mem: Definition of the memory device to be attached. @mem is always consumed
+ *
+ * Attaches memory device described by @mem to domain @vm.
+ *
+ * Returns 0 on success -1 on error.
+ */
+int
+qemuDomainAttachMemory(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainMemoryDefPtr mem)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ char *devstr = NULL;
+ char *objalias = NULL;
+ const char *backendType;
+ virJSONValuePtr props = NULL;
+ int id;
+ int ret = -1;
+
+ if (virAsprintf(&mem->info.alias, "dimm%zu", vm->def->nmems)
< 0)
+ goto cleanup;
+
+ if (virAsprintf(&objalias, "mem%s", mem->info.alias) < 0)
+ goto cleanup;
+
+ if (!(devstr = qemuBuildMemoryDeviceStr(mem, priv->qemuCaps)))
+ goto cleanup;
+
+ qemuDomainMemoryDeviceAlignSize(mem);
+
+ if (qemuBuildMemoryBackendStr(mem->size, mem->pagesize,
+ mem->targetNode, mem->sourceNodes, NULL,
+ vm->def, priv->qemuCaps, cfg,
+ &backendType, &props, true) < 0)
Coverity determines that qemuBuildMemoryBackendStr can return props here
with a -1 return and thus leak props
That's because qemuBuildMemoryBackendStr sets the returned *backendProps
and sets the local props to NULL before the (!hugepages) code which if
it fails won't cause 'props' to be free'd properly
Adding the virJSONValueFree(props); makes Coverity happy again.
John
+ goto cleanup;
+
+ if (virDomainMemoryInsert(vm->def, mem) < 0) {
+ virJSONValueFree(props);
+ goto cleanup;
+ }
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ if (qemuMonitorAddObject(priv->mon, backendType, objalias, props) < 0)
+ goto removedef;
+
+ if (qemuMonitorAddDevice(priv->mon, devstr) < 0) {
+ virErrorPtr err = virSaveLastError();
+ ignore_value(qemuMonitorDelObject(priv->mon, objalias));
+ virSetError(err);
+ virFreeError(err);
+ goto removedef;
+ }
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0) {
+ /* we shouldn't touch mem now, as the def might be freed */
+ mem = NULL;
+ goto cleanup;
+ }
+
+ /* mem is consumed by vm->def */
+ mem = NULL;
+
+ /* this step is best effort, removing the device would be so much trouble */
+ ignore_value(qemuDomainUpdateMemoryDeviceInfo(driver, vm,
+ QEMU_ASYNC_JOB_NONE));
+
+ ret = 0;
+
+ cleanup:
+ virObjectUnref(cfg);
+ VIR_FREE(devstr);
+ VIR_FREE(objalias);
+ virDomainMemoryDefFree(mem);
+ return ret;
+
+ removedef:
+ if (qemuDomainObjExitMonitor(driver, vm) < 0) {
+ mem = NULL;
+ goto cleanup;
+ }
+
+ if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0)
+ mem = virDomainMemoryRemove(vm->def, id);
+ else
+ mem = NULL;
+
+ goto cleanup;
+}
+
+
static int
qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 8a0b313..ad4ff38 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -57,6 +57,9 @@ int qemuDomainAttachHostDevice(virConnectPtr conn,
virDomainHostdevDefPtr hostdev);
int qemuDomainFindGraphicsIndex(virDomainDefPtr def,
virDomainGraphicsDefPtr dev);
+int qemuDomainAttachMemory(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainMemoryDefPtr mem);
int qemuDomainChangeGraphics(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainGraphicsDefPtr dev);