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