<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 19, 2016 at 11:42 PM, Laine Stump <span dir="ltr"><<a href="mailto:laine@laine.org" target="_blank">laine@laine.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Since you have to unplug all the functions in a slot at the same time anyway, I don't see the point in reverting this commit - you'll just end up needing to call it multiple times - once for each function that was in the slot.<br></blockquote><div><br></div><div>I thought of going this way because, incomplete hotplugs can be attempted towards completion if the hotplug reverts failed. Otherwise, we will have</div><div>the devices left in the xml with the slot marked free if the hotplug abort failed by any means. This being for a negative test case, I am not sure if recovery</div><div>of such sorts is possible.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
(just guessing without looking at the code - perhaps it's already being called from within lower level functions for each device, and each device gets its own notification from qemu that it's been detached? Or to ask a more specific question - exactly what happens with device detach? Do you send qemu a single detach command and it detaches all the functions as a single unit? Or do you send it multiple detach commands, with function 0 being the last?)<div class="HOEnZb"><div class="h5"><br></div></div></blockquote><div> </div><div>Yes. There is an event for each function. On x86, any function device_del would detach all functions of the card and events are sent for all functions. On PPC64, each function should be device_del'ed and function 0 at last and on function 0 deletion alone all the events will come for each device.</div><div><br></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">
<br>
<br>
On 05/18/2016 05:30 PM, Shivaprasad G Bhat wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The commit 6fe678c is reverted. The code is moved around and cant revert<br>
staright.<br>
<br>
Signed-off-by: Shivaprasad G Bhat <<a href="mailto:sbhat@linux.vnet.ibm.com" target="_blank">sbhat@linux.vnet.ibm.com</a>><br>
---<br>
src/conf/domain_addr.c | 22 +++++++++-------------<br>
src/libvirt_private.syms | 1 +<br>
src/qemu/qemu_domain_address.c | 2 +-<br>
3 files changed, 11 insertions(+), 14 deletions(-)<br>
<br>
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c<br>
index acd8ce6..35c7cd4 100644<br>
--- a/src/conf/domain_addr.c<br>
+++ b/src/conf/domain_addr.c<br>
@@ -472,21 +472,17 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs,<br>
goto cleanup;<br>
if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {<br>
- /* We do not support hotplug multi-function PCI device now, so we should<br>
- * reserve the whole slot. The function of the PCI device must be 0.<br>
- */<br>
- if (dev->addr.pci.function != 0) {<br>
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",<br>
- _("Only PCI device addresses with function=0"<br>
- " are supported"));<br>
- goto cleanup;<br>
- }<br>
+ if (((dev->addr.pci.function == 0) && (dev->addr.pci.multi == VIR_TRISTATE_SWITCH_ON)) ||<br>
+ dev->addr.pci.function != 0) {<br>
- if (!virDomainPCIAddressValidate(addrs, &dev->addr.pci,<br>
- addrStr, flags, true))<br>
- goto cleanup;<br>
+ if (!virDomainPCIAddressValidate(addrs, &dev->addr.pci,<br>
+ addrStr, flags, true))<br>
+ goto cleanup;<br>
- ret = virDomainPCIAddressReserveSlot(addrs, &dev->addr.pci, flags);<br>
+ ret = virDomainPCIAddressReserveAddr(addrs, &dev->addr.pci, flags, false, true);<br>
+ } else {<br>
+ ret = virDomainPCIAddressReserveSlot(addrs, &dev->addr.pci, flags);<br>
+ }<br>
} else {<br>
ret = virDomainPCIAddressReserveNextSlot(addrs, dev, flags);<br>
}<br>
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms<br>
index fb24808..e4953b7 100644<br>
--- a/src/libvirt_private.syms<br>
+++ b/src/libvirt_private.syms<br>
@@ -98,6 +98,7 @@ virDomainPCIAddressBusSetModel;<br>
virDomainPCIAddressEnsureAddr;<br>
virDomainPCIAddressFlagsCompatible;<br>
virDomainPCIAddressGetNextSlot;<br>
+virDomainPCIAddressReleaseAddr;<br>
virDomainPCIAddressReleaseSlot;<br>
virDomainPCIAddressReserveAddr;<br>
virDomainPCIAddressReserveNextSlot;<br>
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c<br>
index 9c8c262..1e7d98c 100644<br>
--- a/src/qemu/qemu_domain_address.c<br>
+++ b/src/qemu/qemu_domain_address.c<br>
@@ -1682,7 +1682,7 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,<br>
NULLSTR(devstr));<br>
else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&<br>
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) &&<br>
- virDomainPCIAddressReleaseSlot(priv->pciaddrs,<br>
+ virDomainPCIAddressReleaseAddr(priv->pciaddrs,<br>
&info->addr.pci) < 0)<br>
VIR_WARN("Unable to release PCI address on %s",<br>
NULLSTR(devstr));<br>
<br>
--<br>
libvir-list mailing list<br>
<a href="mailto:libvir-list@redhat.com" target="_blank">libvir-list@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/libvir-list" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/libvir-list</a><br>
<br>
</blockquote>
<br>
--<br>
libvir-list mailing list<br>
<a href="mailto:libvir-list@redhat.com" target="_blank">libvir-list@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/libvir-list" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/libvir-list</a><br>
</div></div></blockquote></div><br></div></div>