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. > { > int i, ret = -1; > virDomainDiskDefPtr detach = NULL; > @@ -7884,7 +7884,8 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, > goto cleanup; > } > > - if (!virDomainDeviceAddressIsValid(&detach->info, > + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && > + !virDomainDeviceAddressIsValid(&detach->info, > VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { > qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", > _("device cannot be detached without a PCI address")); > @@ -8373,8 +8374,9 @@ static int qemudDomainDetachDevice(virDomainPtr dom, > > if (dev->type == VIR_DOMAIN_DEVICE_DISK && > dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK && > - dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { > - ret = qemudDomainDetachPciDiskDevice(driver, vm, dev, qemuCmdFlags); > + (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO || > + dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI)) { > + ret = qemudDomainDetachDiskDevice(driver, vm, dev, qemuCmdFlags); > } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { > ret = qemudDomainDetachNetDevice(driver, vm, dev, qemuCmdFlags); > } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list