<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Fri, May 20, 2016 at 12:05 AM, Laine Stump <<a href="mailto:laine@laine.org" target="_blank">laine@laine.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 05/18/2016 05:35 PM, Shivaprasad G Bhat wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> The hostdevices are the only devices which have dependencies outside of themselves such that, other functions of the PCI card should also have been detached from host driver before attempting the hotplug. </blockquote> Are you saying that all the functions on a host device must be detached from their host driver, even if you're only assigning one or another of them to the guest? </blockquote><div>Yes. All devices from same iommu group should be detached from the host. Here, I hope the Card is independent. Otherwise, many cards can also</div><div>belong to same iommu group. In such case, manual the nodedev-detach for other card functions is necessary.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> This patch moves the detach to the beginning of the hotplug so that the following patch can detach all funtions first before attempting to hotplug any. We need not move the detach for net devices using SRIOV as all SRIOV devices are single function devices. </blockquote> I'm not sure why that makes any difference. In any case, you should detach all the devices that are going to be assigned, then assign them all, with the one going to function 0 being last.</blockquote><div> </div><div>There will be only function zero. So I felt its not necessary. Are you saying different SRIOV functions(all with function zero) be hotplugged as</div><div>different functions of single card in guest ? In that case, we would need to do that.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Signed-off-by: Shivaprasad G Bhat <<a href="mailto:sbhat@linux.vnet.ibm.com" target="_blank">sbhat@linux.vnet.ibm.com</a>> --- src/qemu/qemu_driver.c | 13 ++++++++++++- src/qemu/qemu_hotplug.c | 18 +++++++++--------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 37d970e..9cff397 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8273,8 +8273,13 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) goto endjob; - if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom)) < 0) + if (dev_copy->type == VIR_DOMAIN_DEVICE_HOSTDEV && + dev_copy->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && + qemuDomainAttachPCIHostDevicePrepare(driver,vm->def, dev_copy->data.hostdev, qemuCaps) < 0) goto endjob; + + if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom)) < 0) + goto undoprepare; /* * update domain status forcibly because the domain status may be * changed even if we failed to attach the device. For example, @@ -8309,6 +8314,12 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, virObjectUnref(cfg); virNWFilterUnlockFilterUpdates(); return ret; + + undoprepare: + if (dev_copy->type == VIR_DOMAIN_DEVICE_HOSTDEV && + dev_copy->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + qemuHostdevReAttachPCIDevices(driver, vm->def->name, &dev_copy->data.hostdev, 1); + goto endjob; } static int qemuDomainAttachDevice(virDomainPtr dom, const char *xml) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a2bcd89..bdfbafe 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -899,6 +899,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, actualType = virDomainNetGetActualType(net); if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net); /* This is really a "smart hostdev", so it should be attached * as a hostdev (the hostdev code will reach over into the * netdev-specific code as appropriate), then also added to @@ -907,8 +908,14 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, * qemuDomainAttachHostDevice uses a connection to resolve * a SCSI hostdev secret, which is not this case, so pass NULL. */ - ret = qemuDomainAttachHostDevice(NULL, driver, vm, - virDomainNetGetActualHostdev(net)); + if (qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, + hostdev, priv->qemuCaps) < 0) + goto cleanup; + + ret = qemuDomainAttachHostDevice(NULL, driver, vm, hostdev); + if (!ret) + qemuHostdevReAttachPCIDevices(driver, vm->def->name, &hostdev, 1); + goto cleanup; } @@ -1248,10 +1255,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0) return -1; - if (qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, - hostdev, priv->qemuCaps) < 0) - return -1; - backend = hostdev->source.subsys.u.pci.backend; /* Temporarily add the hostdev to the domain definition. This is needed @@ -1330,13 +1333,10 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, if (releaseaddr) qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL); - qemuHostdevReAttachPCIDevices(driver, vm->def->name, &hostdev, 1); - VIR_FREE(devstr); VIR_FREE(configfd_name); VIR_FORCE_CLOSE(configfd); - cleanup: return -1; } -- libvir-list mailing list <a href="mailto:libvir-list@redhat.com" target="_blank">libvir-list@redhat.com</a> <a href="https://www.redhat.com/mailman/listinfo/libvir-list" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/libvir-list</a> </blockquote> </div></div></blockquote></div> </div></div>