[libvirt] [PATCH] pci: properly handle out-of-order SRIOV virtual functions

[Laine Stump]

On 11/06/2013 02:33 PM, Ján Tomko wrote:
> On 11/05/2013 11:43 AM, Laine Stump wrote:
>> +            goto error;
>> +        }
>>  
>> -            (*virtual_functions)[*num_virtual_functions] = config_addr;
>> -            (*num_virtual_functions)++;
>> +        if ((*virtual_functions)[virtfnIndex]) {
>> +            virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                           _("Virtual function link name '%s' "
>> +                             "generates duplicate index %zu "
>> +                             "in physical function directory '%s'"),
>> +                           virtfnNames[i], nVirtfnNames, sysfs_path);
>> +            goto error;
>> +        }
>> +
>> +        if (virPCIGetDeviceAddressFromSysfsLink(device_link, &config_addr) !=
>> +            SRIOV_FOUND) {
>> +            VIR_WARN("Failed to get SRIOV function from device link '%s'",
>> +                     device_link);
>>              VIR_FREE(device_link);
>> +            continue;
> (*virtual_functions)[virtfnIndex] will be left NULL here.
> virNetDevGetVirtualFunctions looks like it would segfault on such array.

Ugh. Yes, previously this failure would lead to no entry added to the
array, but now the array is pre-allocated, and the position of each
element within the array is important so we can't do that.

However, the only reason that a problem would be encountered getting a
device address from a virtfn* link should not be considered an error is
the case where some driver has decided to have a filename in the device
directory that starts with "virtfn", but is something other than a link
to a virtual function. If that were to happen, it would still be the
case that all the virtual functions would be at the beginning of the
array, and there should be no holes in the middle. For example, if we had:

    virtfn0
    virtfn2
    virtfn-poor-name-choice
    virtfn1

then the result would be an array of 4 items, with 0-2 filled in, and 3
left NULL.
   

So, I believe a solution to this problem is to scan the array when were
finished and do two things: 1) truncate any NULLs at the end by
decreasing num_virtual_functions, and 2) fail and log an error if there
are any NULLs earlier in the array than the highest non-NULL.

I'll revise the patch to do that and resubmit.