[libvirt] [PATCHv2 2/4] qemu: Implement shared memory device cold (un)plug

[John Ferlan]

On 11/26/2015 04:06 AM, Luyao Huang wrote:
> Add support for using the attach/detach device APIs on the inactive
> configuration to add/del shared memory devices.
> 
> Signed-off-by: Luyao Huang <lhuang at redhat.com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/qemu/qemu_driver.c   | 21 +++++++++++++++++++--
>  2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 88c2c53..c89d27a 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -451,6 +451,7 @@ virDomainSaveStatus;
>  virDomainSaveXML;
>  virDomainSeclabelTypeFromString;
>  virDomainSeclabelTypeToString;
> +virDomainShmemDefFree;
>  virDomainShmemFind;
>  virDomainShmemInsert;
>  virDomainShmemRemove;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 65ccf99..5ded9ef 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8174,6 +8174,15 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
>          dev->data.memory = NULL;
>          break;
>  
> +    case VIR_DOMAIN_DEVICE_SHMEM:
> +        if (virDomainShmemInsert(vmdef, dev->data.shmem) < 0)
> +            return -1;
> +        dev->data.shmem = NULL;
> +
> +        if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
> +            return -1;
> +        break;
> +
>      case VIR_DOMAIN_DEVICE_INPUT:
>      case VIR_DOMAIN_DEVICE_SOUND:
>      case VIR_DOMAIN_DEVICE_VIDEO:
> @@ -8183,7 +8192,6 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
>      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:
> @@ -8310,6 +8318,16 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
>          virDomainMemoryDefFree(virDomainMemoryRemove(vmdef, idx));
>          break;
>  
> +    case VIR_DOMAIN_DEVICE_SHMEM:
> +        if ((idx = virDomainShmemFind(vmdef, dev->data.shmem, true)) < 0) {

Here rather than 'true' which to me only has meaning if I go read the
code... Of course same thing holds true when passing a 0 (zero) flags value.

BTW: My idea around flags matches what I recently added for
VolReadErrorMode in storage_backend.h... But for this it'd be something
like the following in (I assume) domain_conf.h near _virDomainShmemDef.

    /* DomainShmemMatchFlags
     * Flags to dictate the level of matching when searching for a
     * shmem object in the domain 'nshmems' list.
     */
    enum {
        VIR_DOMAIN_SHMEM_MATCH_NAME_ONLY = 1 << 0,
    }


In general though it seems fine.

John

> +            virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +                           _("no matching shared memory device was found"));
> +            return -1;
> +        }
> +
> +        virDomainShmemDefFree(virDomainShmemRemove(vmdef, idx));
> +        break;
> +
>      case VIR_DOMAIN_DEVICE_INPUT:
>      case VIR_DOMAIN_DEVICE_SOUND:
>      case VIR_DOMAIN_DEVICE_VIDEO:
> @@ -8319,7 +8337,6 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
>      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:
>