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

>> +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.