[libvirt] [PATCH v2 2/8] Release address in function granularity than slot

Shivaprasad bhat shivaprasadbhat at gmail.com
Thu May 19 18:29:07 UTC 2016


On Thu, May 19, 2016 at 11:42 PM, Laine Stump <laine at laine.org> wrote:

> 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.
>

I thought of going this way because, incomplete hotplugs can be attempted
towards completion if the hotplug reverts failed. Otherwise, we will have
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
of such sorts is possible.


>
> (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?)
>
>
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.

.

>
>
> On 05/18/2016 05:30 PM, Shivaprasad G Bhat wrote:
>
>> The commit 6fe678c is reverted. The code is moved around and cant revert
>> staright.
>>
>> Signed-off-by: Shivaprasad G Bhat <sbhat at linux.vnet.ibm.com>
>> ---
>>   src/conf/domain_addr.c         |   22 +++++++++-------------
>>   src/libvirt_private.syms       |    1 +
>>   src/qemu/qemu_domain_address.c |    2 +-
>>   3 files changed, 11 insertions(+), 14 deletions(-)
>>
>> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
>> index acd8ce6..35c7cd4 100644
>> --- a/src/conf/domain_addr.c
>> +++ b/src/conf/domain_addr.c
>> @@ -472,21 +472,17 @@
>> virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs,
>>           goto cleanup;
>>         if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
>> -        /* We do not support hotplug multi-function PCI device now, so
>> we should
>> -         * reserve the whole slot. The function of the PCI device must
>> be 0.
>> -         */
>> -        if (dev->addr.pci.function != 0) {
>> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> -                           _("Only PCI device addresses with function=0"
>> -                             " are supported"));
>> -            goto cleanup;
>> -        }
>> +        if (((dev->addr.pci.function == 0) && (dev->addr.pci.multi ==
>> VIR_TRISTATE_SWITCH_ON)) ||
>> +            dev->addr.pci.function != 0) {
>>   -        if (!virDomainPCIAddressValidate(addrs, &dev->addr.pci,
>> -                                         addrStr, flags, true))
>> -            goto cleanup;
>> +            if (!virDomainPCIAddressValidate(addrs, &dev->addr.pci,
>> +                                             addrStr, flags, true))
>> +                goto cleanup;
>>   -        ret = virDomainPCIAddressReserveSlot(addrs, &dev->addr.pci,
>> flags);
>> +            ret = virDomainPCIAddressReserveAddr(addrs, &dev->addr.pci,
>> flags, false, true);
>> +        } else {
>> +            ret = virDomainPCIAddressReserveSlot(addrs, &dev->addr.pci,
>> flags);
>> +        }
>>       } else {
>>           ret = virDomainPCIAddressReserveNextSlot(addrs, dev, flags);
>>       }
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index fb24808..e4953b7 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -98,6 +98,7 @@ virDomainPCIAddressBusSetModel;
>>   virDomainPCIAddressEnsureAddr;
>>   virDomainPCIAddressFlagsCompatible;
>>   virDomainPCIAddressGetNextSlot;
>> +virDomainPCIAddressReleaseAddr;
>>   virDomainPCIAddressReleaseSlot;
>>   virDomainPCIAddressReserveAddr;
>>   virDomainPCIAddressReserveNextSlot;
>> diff --git a/src/qemu/qemu_domain_address.c
>> b/src/qemu/qemu_domain_address.c
>> index 9c8c262..1e7d98c 100644
>> --- a/src/qemu/qemu_domain_address.c
>> +++ b/src/qemu/qemu_domain_address.c
>> @@ -1682,7 +1682,7 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
>>                    NULLSTR(devstr));
>>       else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
>>                virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) &&
>> -             virDomainPCIAddressReleaseSlot(priv->pciaddrs,
>> +             virDomainPCIAddressReleaseAddr(priv->pciaddrs,
>>                                               &info->addr.pci) < 0)
>>           VIR_WARN("Unable to release PCI address on %s",
>>                    NULLSTR(devstr));
>>
>> --
>> libvir-list mailing list
>> libvir-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>>
>>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160519/86ce02b1/attachment-0001.htm>


More information about the libvir-list mailing list