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); > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list