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

[John Ferlan]

$SUBJ

s/share/shared

On 11/26/2015 04:06 AM, Luyao Huang wrote:
> Signed-off-by: Luyao Huang <lhuang at redhat.com>
> ---
>  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(&shmem->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, &shmem->info) < 0))
> +        return -1;
> +
> +    if (!(devstr = qemuBuildShmemDevStr(vm->def, shmem, priv->qemuCaps)))
> +        goto cleanup;
> +


> +    if (virAsprintf(&charAlias, "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,
> +                                 &shmem->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, &shmem->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,
> +                                virDomainObjPtr vm,
> +                                virDomainShmemDefPtr shmem);
>  
>  int
>  qemuDomainChrInsert(virDomainDefPtr vmdef,
>