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@xxxxxxxx>
> ---
> 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.
> @@ -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.bus,
> - subsys->u.pci.addr.slot, subsys->u.pci.addr.function);
> - if (pci) {
> - activePci = virPCIDeviceListSteal(driver->activePciHostdevs, 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.
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list