Re: [libvirt] [PATCHv2 3/4] qemu: Implement share memory device hot-plug

2015-12-15 Thread lhuang 



On 12/10/2015 09:23 AM, John Ferlan wrote:

$SUBJ

s/share/shared


thanks for pointing out this



On 11/26/2015 04:06 AM, Luyao Huang wrote:

Signed-off-by: Luyao Huang 
---
  src/qemu/qemu_driver.c  | 10 -
  src/qemu/qemu_hotplug.c | 58 +
  src/qemu/qemu_hotplug.h |  3 +++
  3 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5ded9ef..3c25c07 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7784,6 +7784,15 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
  dev->data.memory = NULL;
  break;
  
+case VIR_DOMAIN_DEVICE_SHMEM:

+ret = qemuDomainAttachShmemDevice(driver, vm,
+  dev->data.shmem);
+if (!ret) {
+alias = dev->data.shmem->info.alias;
+dev->data.shmem = NULL;
+}
+break;
+
  case VIR_DOMAIN_DEVICE_NONE:
  case VIR_DOMAIN_DEVICE_FS:
  case VIR_DOMAIN_DEVICE_INPUT:
@@ -7795,7 +7804,6 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
  case VIR_DOMAIN_DEVICE_SMARTCARD:
  case VIR_DOMAIN_DEVICE_MEMBALLOON:
  case VIR_DOMAIN_DEVICE_NVRAM:
-case VIR_DOMAIN_DEVICE_SHMEM:
  case VIR_DOMAIN_DEVICE_TPM:
  case VIR_DOMAIN_DEVICE_PANIC:
  case VIR_DOMAIN_DEVICE_LAST:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8804d3d..c5e544d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1882,6 +1882,64 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
  }
  
  

It seems this is modeled after qemuDomainAttachRNGDevice, right?


Right :)


+int
+qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+virDomainShmemDefPtr shmem)
+{
+int ret = -1;
+qemuDomainObjPrivatePtr priv = vm->privateData;
+char *devstr = NULL;
+char *charAlias = NULL;
+
+if (virAsprintf(>info.alias, "shmem%zu", vm->def->nshmems) < 0)
+return -1;
+
+if (VIR_REALLOC_N(vm->def->shmems, vm->def->nshmems + 1) < 0)
+return -1;
+
+if ((shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
+ shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
+ (virDomainPCIAddressEnsureAddr(priv->pciaddrs, >info) < 0))
+return -1;
+
+if (!(devstr = qemuBuildShmemDevStr(vm->def, shmem, priv->qemuCaps)))
+goto cleanup;
+



+if (virAsprintf(, "char%s", shmem->info.alias) < 0)
+goto cleanup;
+

This seems to be up to a 2 stage process "if" server.enabled" is true"


Indeed, i will fix it in next version




+qemuDomainObjEnterMonitor(driver, vm);
+
+if (shmem->server.enabled &&
+qemuMonitorAttachCharDev(priv->mon, charAlias,
+ >server.chr) < 0) {

Instead of the following change to:

goto failchardev;

and the {} won't be necessary


...

Instead of the following change to

goto failadddevice;

and the {} won't be necessary

...

Again following RNG model - this should have a

 if (*Exit*() < 0) {
 vm = NULL;
 goto cleanup
 }

...

And of course the auditing change as well.

...
Following RNG the && vm would be used here... See your patch commits 
'0ed3b3353' and '980b265d0'

...

  failadddevice:
 if (shmem->server.enabled)
 ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));

  failchardev:
 ignore_value(qemuDomainObjExitMonitor(driver, vm));
 goto cleanup;



Nice improve, i must forgot a old problem that the  will be changed 
after qemu unexpected exit during enter the monitor.



Hope this all makes sense (it's been a long day ;-))


Thanks a lot for your review and suggestion, and i am sorry that i 
didn't reply your mails when i saw them (I'm kind of overwhelmed lately :) )


Have a nice day !

Luyao



John

+}
+
+
  static int
  qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 4140da3..60137a6 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -109,6 +109,9 @@ int qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
  int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainRNGDefPtr rng);
+int qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+virDomainShmemDefPtr shmem);
  
  int

  qemuDomainChrInsert(virDomainDefPtr vmdef,



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv2 3/4] qemu: Implement share memory device hot-plug

2015-12-09 Thread John Ferlan 

[...]

> +int
> +qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
> +virDomainObjPtr vm,
> +virDomainShmemDefPtr shmem)
> +{
> +int ret = -1;
> +qemuDomainObjPrivatePtr priv = vm->privateData;
> +char *devstr = NULL;
> +char *charAlias = NULL;

Dang... one more thing... why no?

   if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
   _("qemu does not support -device"));
return -1;
}

I see it's not in AttachRNG (so why it was missed), *BUT* it is in
DetachRNG... So it seems a separate patch for AttachRNG should add that?

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv2 3/4] qemu: Implement share memory device hot-plug

2015-12-09 Thread John Ferlan 

$SUBJ

s/share/shared

On 11/26/2015 04:06 AM, Luyao Huang wrote:
> Signed-off-by: Luyao Huang 
> ---
>  src/qemu/qemu_driver.c  | 10 -
>  src/qemu/qemu_hotplug.c | 58 
> +
>  src/qemu/qemu_hotplug.h |  3 +++
>  3 files changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5ded9ef..3c25c07 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7784,6 +7784,15 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
>  dev->data.memory = NULL;
>  break;
>  
> +case VIR_DOMAIN_DEVICE_SHMEM:
> +ret = qemuDomainAttachShmemDevice(driver, vm,
> +  dev->data.shmem);
> +if (!ret) {
> +alias = dev->data.shmem->info.alias;
> +dev->data.shmem = NULL;
> +}
> +break;
> +
>  case VIR_DOMAIN_DEVICE_NONE:
>  case VIR_DOMAIN_DEVICE_FS:
>  case VIR_DOMAIN_DEVICE_INPUT:
> @@ -7795,7 +7804,6 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
>  case VIR_DOMAIN_DEVICE_SMARTCARD:
>  case VIR_DOMAIN_DEVICE_MEMBALLOON:
>  case VIR_DOMAIN_DEVICE_NVRAM:
> -case VIR_DOMAIN_DEVICE_SHMEM:
>  case VIR_DOMAIN_DEVICE_TPM:
>  case VIR_DOMAIN_DEVICE_PANIC:
>  case VIR_DOMAIN_DEVICE_LAST:
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 8804d3d..c5e544d 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1882,6 +1882,64 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
>  }
>  
>  

It seems this is modeled after qemuDomainAttachRNGDevice, right?

> +int
> +qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
> +virDomainObjPtr vm,
> +virDomainShmemDefPtr shmem)
> +{
> +int ret = -1;
> +qemuDomainObjPrivatePtr priv = vm->privateData;
> +char *devstr = NULL;
> +char *charAlias = NULL;
> +
> +if (virAsprintf(>info.alias, "shmem%zu", vm->def->nshmems) < 0)
> +return -1;
> +
> +if (VIR_REALLOC_N(vm->def->shmems, vm->def->nshmems + 1) < 0)
> +return -1;
> +
> +if ((shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
> + shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
> + (virDomainPCIAddressEnsureAddr(priv->pciaddrs, >info) < 0))
> +return -1;
> +
> +if (!(devstr = qemuBuildShmemDevStr(vm->def, shmem, priv->qemuCaps)))
> +goto cleanup;
> +


> +if (virAsprintf(, "char%s", shmem->info.alias) < 0)
> +goto cleanup;
> +

This seems to be up to a 2 stage process "if" server.enabled" is true"...

> +qemuDomainObjEnterMonitor(driver, vm);
> +
> +if (shmem->server.enabled &&
> +qemuMonitorAttachCharDev(priv->mon, charAlias,
> + >server.chr) < 0) {

Instead of the following change to:

   goto failchardev;

and the {} won't be necessary

> +ignore_value(qemuDomainObjExitMonitor(driver, vm));
> +goto cleanup;
> +}
> +
> +if (qemuMonitorAddDevice(priv->mon, devstr) < 0) {

Instead of the following change to

   goto failadddevice;

and the {} won't be necessary

> +if (shmem->server.enabled)
> +ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
> +ignore_value(qemuDomainObjExitMonitor(driver, vm));
> +goto cleanup;
> +}
> +if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +goto cleanup;

Again following RNG model - this should have a

if (*Exit*() < 0) {
vm = NULL;
goto cleanup
}
> +
> +if (virDomainShmemInsert(vm->def, shmem) < 0)
> +goto cleanup;

And of course the auditing change as well.
> +
> +ret = 0;
> + cleanup:
> +if (ret < 0)

Following RNG the && vm would be used here... See your patch commits
'0ed3b3353' and '980b265d0'

> +qemuDomainReleaseDeviceAddress(vm, >info, NULL);
> +VIR_FREE(charAlias);
> +VIR_FREE(devstr);
> +return ret;

 failadddevice:
if (shmem->server.enabled)
ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));

 failchardev:
ignore_value(qemuDomainObjExitMonitor(driver, vm));
goto cleanup;


Hope this all makes sense (it's been a long day ;-))

John
> +}
> +
> +
>  static int
>  qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
>virDomainObjPtr vm,
> diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
> index 4140da3..60137a6 100644
> --- a/src/qemu/qemu_hotplug.h
> +++ b/src/qemu/qemu_hotplug.h
> @@ -109,6 +109,9 @@ int qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
>  int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
>virDomainObjPtr vm,
>virDomainRNGDefPtr rng);
> +int qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
> +

[libvirt] [PATCHv2 3/4] qemu: Implement share memory device hot-plug

2015-11-26 Thread Luyao Huang 
Signed-off-by: Luyao Huang 
---
 src/qemu/qemu_driver.c  | 10 -
 src/qemu/qemu_hotplug.c | 58 +
 src/qemu/qemu_hotplug.h |  3 +++
 3 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5ded9ef..3c25c07 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7784,6 +7784,15 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
 dev->data.memory = NULL;
 break;
 
+case VIR_DOMAIN_DEVICE_SHMEM:
+ret = qemuDomainAttachShmemDevice(driver, vm,
+  dev->data.shmem);
+if (!ret) {
+alias = dev->data.shmem->info.alias;
+dev->data.shmem = NULL;
+}
+break;
+
 case VIR_DOMAIN_DEVICE_NONE:
 case VIR_DOMAIN_DEVICE_FS:
 case VIR_DOMAIN_DEVICE_INPUT:
@@ -7795,7 +7804,6 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
 case VIR_DOMAIN_DEVICE_SMARTCARD:
 case VIR_DOMAIN_DEVICE_MEMBALLOON:
 case VIR_DOMAIN_DEVICE_NVRAM:
-case VIR_DOMAIN_DEVICE_SHMEM:
 case VIR_DOMAIN_DEVICE_TPM:
 case VIR_DOMAIN_DEVICE_PANIC:
 case VIR_DOMAIN_DEVICE_LAST:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8804d3d..c5e544d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1882,6 +1882,64 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
 }
 
 
+int
+qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+virDomainShmemDefPtr shmem)
+{
+int ret = -1;
+qemuDomainObjPrivatePtr priv = vm->privateData;
+char *devstr = NULL;
+char *charAlias = NULL;
+
+if (virAsprintf(>info.alias, "shmem%zu", vm->def->nshmems) < 0)
+return -1;
+
+if (VIR_REALLOC_N(vm->def->shmems, vm->def->nshmems + 1) < 0)
+return -1;
+
+if ((shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
+ shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
+ (virDomainPCIAddressEnsureAddr(priv->pciaddrs, >info) < 0))
+return -1;
+
+if (!(devstr = qemuBuildShmemDevStr(vm->def, shmem, priv->qemuCaps)))
+goto cleanup;
+
+if (virAsprintf(, "char%s", shmem->info.alias) < 0)
+goto cleanup;
+
+qemuDomainObjEnterMonitor(driver, vm);
+
+if (shmem->server.enabled &&
+qemuMonitorAttachCharDev(priv->mon, charAlias,
+ >server.chr) < 0) {
+ignore_value(qemuDomainObjExitMonitor(driver, vm));
+goto cleanup;
+}
+
+if (qemuMonitorAddDevice(priv->mon, devstr) < 0) {
+if (shmem->server.enabled)
+ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
+ignore_value(qemuDomainObjExitMonitor(driver, vm));
+goto cleanup;
+}
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+goto cleanup;
+
+if (virDomainShmemInsert(vm->def, shmem) < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+if (ret < 0)
+qemuDomainReleaseDeviceAddress(vm, >info, NULL);
+VIR_FREE(charAlias);
+VIR_FREE(devstr);
+return ret;
+}
+
+
 static int
 qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 4140da3..60137a6 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -109,6 +109,9 @@ int qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
 int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
   virDomainRNGDefPtr rng);
+int qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+virDomainShmemDefPtr shmem);
 
 int
 qemuDomainChrInsert(virDomainDefPtr vmdef,
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list