2014-03-04 20:17 GMT+08:00 Daniel P. Berrange <berrange@redhat.com>:
On Sat, Mar 01, 2014 at 02:28:57PM +0800, Chunyan Liu wrote:
Same logic of preparing/reattaching hostdevs could be used in attach/detach hotplug places, so reuse hostdev interfaces to avoid duplicate, also for later extracting general code to common library.
Signed-off-by: Chunyan Liu <cyliu@suse.com> --- src/qemu/qemu_hostdev.c | 8 +++--- src/qemu/qemu_hostdev.h | 11 +++++++- src/qemu/qemu_hotplug.c | 61 +++------------------------------------------- 3 files changed, 18 insertions(+), 62 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6703c92..5546693 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1454,28 +1454,16 @@ qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver, virDomainHostdevDefPtr hostdev) { qemuDomainObjPrivatePtr priv = vm->privateData; - virUSBDeviceList *list = NULL; - virUSBDevicePtr usb = NULL; char *devstr = NULL; bool added = false; bool teardowncgroup = false; bool teardownlabel = false; int ret = -1;
- if (qemuFindHostdevUSBDevice(hostdev, true, &usb) < 0) - return -1; - - if (!(list = virUSBDeviceListNew())) - goto cleanup; - - if (virUSBDeviceListAdd(list, usb) < 0) - goto cleanup; - - if (qemuPrepareHostdevUSBDevices(driver, vm->def->name, list) < 0)
This code you're deleting took the 'hostdev' parameter that was passed into this method, and runs the 'qemuPrepareHostdevUSBDevices' method on it.
+ if (qemuPrepareHostUSBDevices(driver, vm->def, 0) < 0) goto cleanup;
This code you're adding takes the list of existing hostdevs in the virDomainDefPtr and runs the 'qemuPrepareHostdevUSBDevices' method on it.
I don't see how this can possibly be correct, since the before and after code processes totally different hostdev device instances.
My mistake. It should be: qemuPrepareHostUSBDevices(driver, vm->def->name,driver, &hostdev, 1, 0); qemuPrepareHostUSBDevices interface shoule be updated a bit, just to be consistency with *PreparePCIDevices and *PrepareSCSIDevices. Will update.
@@ -2531,29 +2514,7 @@ qemuDomainRemovePCIHostDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm, virDomainHostdevDefPtr hostdev) { - virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; - virPCIDevicePtr pci; - virPCIDevicePtr activePci; - - virObjectLock(driver->activePciHostdevs); - virObjectLock(driver->inactivePciHostdevs); - pci = virPCIDeviceNew(subsys->u.pci.addr.domain,
- subsys->u.pci.addr.slot, subsys->u.pci.addr.function); - if (pci) { - activePci = virPCIDeviceListSteal(driver->activePciHostdevs,
subsys->u.pci.addr.bus, pci);
- if (activePci && - virPCIDeviceReset(activePci, driver->activePciHostdevs, - driver->inactivePciHostdevs) == 0) { - qemuReattachPciDevice(activePci, driver); - } else { - /* reset of the device failed, treat it as if it was returned */ - virPCIDeviceFree(activePci); - } - virPCIDeviceFree(pci); - } - virObjectUnlock(driver->activePciHostdevs); - virObjectUnlock(driver->inactivePciHostdevs); - + qemuDomainReAttachHostdevDevices(driver, vm->def->name, &hostdev, 1); qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL); }
@@ -2562,19 +2523,7 @@ qemuDomainRemoveUSBHostDevice(virQEMUDriverPtr driver, virDomainObjPtr vm ATTRIBUTE_UNUSED, virDomainHostdevDefPtr hostdev) { - virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; - virUSBDevicePtr usb; - - usb = virUSBDeviceNew(subsys->u.usb.bus, subsys->u.usb.device, NULL); - if (usb) { - virObjectLock(driver->activeUsbHostdevs); - virUSBDeviceListDel(driver->activeUsbHostdevs, usb); - virObjectUnlock(driver->activeUsbHostdevs); - virUSBDeviceFree(usb); - } else { - VIR_WARN("Unable to find device %03d.%03d in list of used USB devices", - subsys->u.usb.bus, subsys->u.usb.device); - } + qemuDomainReAttachHostUsbDevices(driver, vm->def->name, &hostdev, 1); }
static void @@ -2622,8 +2571,6 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
virDomainAuditHostdev(vm, hostdev, "detach", true);
- qemuDomainHostdevNetConfigRestore(hostdev, cfg->stateDir); -
This doesn't look valid to remove.
qemuDomainHostdevNetConfigRestore will be done in qemuDomainReAttachHostdevDevices after using the latter in qemuDomainRemovePCIHostDevice. So, it's not needed here.
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/:| |: http://libvirt.org -o- http://virt-manager.org:| |: http://autobuild.org -o- http://search.cpan.org/~danberr/:| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc:|