Re: [libvirt] [PATCHv3 11/12] qemu: Implement memory device hotplug
On Thu, Mar 19, 2015 at 18:40:36 -0400, John Ferlan wrote:
>
>
> 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(-)
> >
> > @@ -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.
I'll fix qemuBuildMemoryBackendStr separately rather than adding a
pseudo-hack that would violate the style we are using for functions that
return via argument.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 11/12] qemu: Implement memory device hotplug
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,
> +
[libvirt] [PATCHv3 11/12] qemu: Implement memory device hotplug
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;
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)
+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, obj
