[libvirt] [PATCH 8/8] qemu: Add support for hot/cold-(un)plug of shmem devices

[John Ferlan]

[...]


>>> +int
>>> +qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
>>> +                            virDomainObjPtr vm,
>>> +                            virDomainDeviceDefPtr dev)
>>> +{
>>> +    int ret = -1;
>>> +    ssize_t idx = -1;
>>> +    virErrorPtr orig_err = NULL;
>>> +    virDomainShmemDefPtr shmem = NULL;
>>> +    qemuDomainObjPrivatePtr priv = vm->privateData;
>>> +
>>> +    if ((idx = virDomainShmemDefFind(vm->def, dev->data.shmem) < 0)) {
>>> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>>> +                       _("device not present in domain
>>> configuration"));
>>> +        return -1;
>>> +    }
>>> +
>>> +    shmem = vm->def->shmems[idx];
>>> +
>>> +    switch ((virDomainShmemModel)shmem->model) {
>>> +    case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN:
>>> +    case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL:
>>> +        break;
>>> +
>>> +    case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM:
>>> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
>>> +                       _("live detach of shmem model '%s' is not
>>> supported"),
>>> +                       virDomainShmemModelTypeToString(shmem->model));
>>> +        /* fall-through */
>>> +    case VIR_DOMAIN_SHMEM_MODEL_LAST:
>>> +        return -1;
>>> +    }
>>> +
>>> +    qemuDomainMarkDeviceForRemoval(vm, &shmem->info);
>>> +    qemuDomainObjEnterMonitor(driver, vm);
>>> +
>>> +    ret = qemuMonitorDelDevice(priv->mon, shmem->info.alias);
>>> +
>>> +    if (ret < 0)
>>> +        orig_err = virSaveLastError();
>>> +
>>> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
>>> +        ret = -1;
>>> +
>>> +    if (ret == 0) {
>>> +        if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) {
>>> +            qemuDomainReleaseDeviceAddress(vm, &shmem->info, NULL);
>>> +            ret = qemuDomainRemoveDevice(driver, vm, dev);
>>
>> Why not a direct qemuDomainRemoveShmemDevice(driver, vm, shmem);
>>
>> It's the pattern other code uses - just concern over the difference - it
>> does result in the same call eventually.
>>
> 
> What other code?  It doesn't necessarily result in the same call every
> time.  That's what qemuDomainWaitForDeviceRemoval() is for.  We
> shouldn't remove it from the definition if QEMU didn't actually remove
> it.
> 

Most callers to qemuDomainWaitForDeviceRemoval except
qemuDomainDetachChrDevice which throws in release device address, but
still calls its RemoveChrDevice directly rather than the generic
RemoveDevice call.

John