[libvirt] [PATCHv2 4/4] qemu: Implement shared memory device hot-unplug

[John Ferlan]

On 11/26/2015 04:07 AM, Luyao Huang wrote:
> Signed-off-by: Luyao Huang <lhuang at redhat.com>
> ---
>  src/qemu/qemu_driver.c  |  4 ++-
>  src/qemu/qemu_hotplug.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  src/qemu/qemu_hotplug.h |  3 ++
>  3 files changed, 99 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 3c25c07..9e7429a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7881,6 +7881,9 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
>      case VIR_DOMAIN_DEVICE_MEMORY:
>          ret = qemuDomainDetachMemoryDevice(driver, vm, dev->data.memory);
>          break;
> +    case VIR_DOMAIN_DEVICE_SHMEM:
> +        ret = qemuDomainDetachShmemDevice(driver, vm, dev->data.shmem);
> +        break;
>  
>      case VIR_DOMAIN_DEVICE_FS:
>      case VIR_DOMAIN_DEVICE_INPUT:
> @@ -7892,7 +7895,6 @@ qemuDomainDetachDeviceLive(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_REDIRDEV:
>      case VIR_DOMAIN_DEVICE_NONE:
>      case VIR_DOMAIN_DEVICE_TPM:
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index c5e544d..4ab05f3 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3347,6 +3347,45 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
>  }
>  
>  
> +static int
> +qemuDomainRemoveShmemDevice(virQEMUDriverPtr driver,
> +                            virDomainObjPtr vm,
> +                            virDomainShmemDefPtr shmem)
> +{
> +    virObjectEventPtr event;
> +    char *charAlias = NULL;
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    ssize_t idx;
> +    int rc = 0;
> +
> +    VIR_DEBUG("Removing shared memory device %s from domain %p %s",
> +              shmem->info.alias, vm, vm->def->name);
> +
> +    if (shmem->server.enabled) {
> +        if (virAsprintf(&charAlias, "char%s", shmem->info.alias) < 0)
> +            return -1;
> +
> +        qemuDomainObjEnterMonitor(driver, vm);
> +        rc = qemuMonitorDetachCharDev(priv->mon, charAlias);
> +        VIR_FREE(charAlias);
> +        if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +            return -1;
> +    }
> +

Auditing code?

> +    if (rc < 0)
> +        return -1;
> +

I know this is a copy of the RemoveRNGDevice; however, this code doesn't
remove an 'obj'. In fact, if !shmem->server.enabled, then we don't enter
the monitor at all.

Thus the following event probably won't happen...

> +    if ((event = virDomainEventDeviceRemovedNewFromObj(vm, shmem->info.alias)))
> +        qemuDomainEventQueue(driver, event);
> +
> +    if ((idx = virDomainShmemFind(vm->def, shmem, true)) >= 0)
> +        virDomainShmemRemove(vm->def, idx);
> +    qemuDomainReleaseDeviceAddress(vm, &shmem->info, NULL);
> +    virDomainShmemDefFree(shmem);
> +    return 0;
> +}
> +
> +
>  int
>  qemuDomainRemoveDevice(virQEMUDriverPtr driver,
>                         virDomainObjPtr vm,
> @@ -3378,6 +3417,10 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
>          ret = qemuDomainRemoveMemoryDevice(driver, vm, dev->data.memory);
>          break;
>  
> +    case VIR_DOMAIN_DEVICE_SHMEM:
> +        ret = qemuDomainRemoveShmemDevice(driver, vm, dev->data.shmem);
> +        break;
> +
>      case VIR_DOMAIN_DEVICE_NONE:
>      case VIR_DOMAIN_DEVICE_LEASE:
>      case VIR_DOMAIN_DEVICE_FS:
> @@ -3391,7 +3434,6 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
>      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:
> @@ -4378,3 +4420,53 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver,
>      qemuDomainResetDeviceRemoval(vm);
>      return ret;
>  }
> +
> +
> +int
> +qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
> +                            virDomainObjPtr vm,
> +                            virDomainShmemDefPtr shmem)
> +{
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    ssize_t idx;
> +    virDomainShmemDefPtr tmpshmem;
> +    int rc;
> +    int ret = -1;
> +
> +    if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("qemu does not support -device"));
> +        return -1;
> +    }

Well it's here, but not in AttachShmemDevice...

> +
> +    if ((idx = virDomainShmemFind(vm->def, shmem, true)) < 0) {

Again we could use a 'flags' argument instead...  Of course there isn't
a "false" argument to any of the added callers to date, so not sure of
the need for it...  Could still use/add flags and keep it as "unused"
for now.

> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("device not present in domain configuration"));

s/(("/(("shared memory /


John
> +        return -1;
> +    }
> +
> +    tmpshmem = vm->def->shmems[idx];
> +
> +    if (!tmpshmem->info.alias) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("alias not set for shared memory device"));
> +        return -1;
> +    }
> +
> +    qemuDomainMarkDeviceForRemoval(vm, &tmpshmem->info);
> +
> +    qemuDomainObjEnterMonitor(driver, vm);
> +    rc = qemuMonitorDelDevice(priv->mon, tmpshmem->info.alias);
> +    if (qemuDomainObjExitMonitor(driver, vm) || rc < 0)
> +        goto cleanup;
> +
> +    rc = qemuDomainWaitForDeviceRemoval(vm);
> +    if (rc == 0 || rc == 1)
> +        ret = qemuDomainRemoveShmemDevice(driver, vm, tmpshmem);
> +    else
> +        ret = 0;
> +
> + cleanup:
> +    qemuDomainResetDeviceRemoval(vm);
> +    return ret;
> +}
> diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
> index 60137a6..1b18e8a 100644
> --- a/src/qemu/qemu_hotplug.h
> +++ b/src/qemu/qemu_hotplug.h
> @@ -112,6 +112,9 @@ int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
>  int qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
>                                  virDomainObjPtr vm,
>                                  virDomainShmemDefPtr shmem);
> +int qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
> +                                virDomainObjPtr vm,
> +                                virDomainShmemDefPtr shmem);
>  
>  int
>  qemuDomainChrInsert(virDomainDefPtr vmdef,
>