Daniel P. Berrange wrote: > On Tue, May 04, 2010 at 05:18:24PM +0200, Wolfgang Mauerer wrote: >> Detaching disk devices is not only possible for VIR_DOMAIN_DISK_BUS_VIRTIO, >> but also for VIR_DOMAIN_DISK_BUS_SCSI, so take care of this possibility. >> >> Additionally, when the new-style device syntax is used, we do not need to >> check if the PCI address is valid since we don't need it to do the >> hot-unplugging. And while we're at it, drop the "pci" part >> in qemudDomainDetachPciDiskDevice() -- it's misleading since we >> do not necessarily have to deal with PCI addresses. >> >> Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@xxxxxxxxxxx> >> --- >> src/qemu/qemu_driver.c | 16 +++++++++------- >> 1 files changed, 9 insertions(+), 7 deletions(-) >> >> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c >> index 704f824..f2b8517 100644 >> --- a/src/qemu/qemu_driver.c >> +++ b/src/qemu/qemu_driver.c >> @@ -7862,10 +7862,10 @@ cleanup: >> } >> >> >> -static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, >> - virDomainObjPtr vm, >> - virDomainDeviceDefPtr dev, >> - unsigned long long qemuCmdFlags) >> +static int qemudDomainDetachDiskDevice(struct qemud_driver *driver, >> + virDomainObjPtr vm, >> + virDomainDeviceDefPtr dev, >> + unsigned long long qemuCmdFlags) > > I'd rather we introduced a separate method > > qemudDomainDetachSCSIDiskDevice > > since logically these are different types of objects/operations, that just > happen to share a common monitor command with new QEMU. It also needs to > give a more explicit error in the case where QEMUD_CMD_FLAG_DEVICE is not > set for SCSI, since we can't support that for SCSI, but can for VirtIO. okay, I've created a separate function for SCSI disk unplug. Since the differences between VirtIO and SCSI disk unplug are tiny, I've tried to extract at least a few commonalities -- see the two follow-up patches. Albeit there's much more duplicated code lurking in qemu_driver.c, it might make sense to look at this some day. Best, Wolfgang -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list