7:48 p.m.
On 03/04/2015 11:25 AM, Peter Krempa wrote:
Add code to hot-add memory devices to running qemu instances.
---
src/qemu/qemu_command.c | 4 +--
src/qemu/qemu_command.h | 15 +++++++++
src/qemu/qemu_driver.c | 5 ++-
src/qemu/qemu_hotplug.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_hotplug.h | 3 ++
5 files changed, 109 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f1fca02..883c3d1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4546,7 +4546,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,
@@ -4819,7 +4819,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 6b10e96..b917d76 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7095,8 +7095,11 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
dev->data.rng = NULL;
break;
- /*TODO: implement later */
case VIR_DOMAIN_DEVICE_MEMORY:
+ ret = qemuDomainAttachMemory(driver, vm,
+ dev->data.memory);
Shouldn't there be a :
if (!ret)
prior to the following line (like the other cases in the switch)
+ dev->data.memory = NULL;
+ break;
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 cb2270e..967b678 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1672,6 +1672,91 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
}
+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)
+ goto cleanup;
+
+ if (virDomainMemoryInsert(vm->def, mem) < 0) {
+ virJSONValueFree(props);
+ goto cleanup;
+ }
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ if (qemuMonitorAddObject(priv->mon, backendType, objalias, props) < 0)
I see from the AddObject comments, props is consumed upon success, but
if we hit the else of mon->json, then we don't free it which we should -
not related to this particular code, but something that should be fixed...
+ 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;
Since both the Exit error path and this path set mem = NULL, why not
just set it before the Exit and comment that it'll either consumed by
vm->def on success or might be freed on failure...
+
+ /* this step is best effort, removing the device would be so much trouble */
+ ignore_value(qemuDomainUpdateMemoryDeviceInfo(driver, vm,
+ QEMU_ASYNC_JOB_NONE));
+
BTW: This is perhaps what I was thinking of in that migration path patch
(#6) where the code fails if not defined.
ACK - with the slight adjustment above for the first thing I noted.
John
+ 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);