subject:"\[libvirt\] \[PATCHv3 11\/12\] qemu\: Implement memory device hotplug"

Re: [libvirt] [PATCHv3 11/12] qemu: Implement memory device hotplug

2015-03-23 Thread Peter Krempa

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

2015-03-19 Thread John Ferlan



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

[libvirt] [PATCHv3 11/12] qemu: Implement memory device hotplug

2015-03-17 Thread Peter Krempa

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, objalias));
+

Re: [libvirt] [PATCHv3 11/12] qemu: Implement memory device hotplug

Re: [libvirt] [PATCHv3 11/12] qemu: Implement memory device hotplug

[libvirt] [PATCHv3 11/12] qemu: Implement memory device hotplug

3 matches

Site Navigation

Mail list logo

Footer information