Re: [Xen-devel] [PATCH 2/4] sysctl/libxl: Add interface for returning IO topology data

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.12.14 at 18:10, <boris.ostrovsky@xxxxxxxxxx> wrote:
> On 12/05/2014 11:03 AM, Jan Beulich wrote:
>>>>> On 05.12.14 at 16:55, <JBeulich@xxxxxxxx> wrote:
>>>>>> On 02.12.14 at 22:34, <boris.ostrovsky@xxxxxxxxxx> wrote:
>>>> +struct xen_sysctl_iotopo {
>>>> +    uint16_t seg;
>>>> +    uint8_t bus;
>>>> +    uint8_t devfn;
>>>> +    uint32_t node;
>>>> +};
>>> This is PCI-centric without expressing in the name or layout.
> 
> xen_sysctl_pcitopo would be a better name.
> 
>>>   Perhaps
>>> the first part should be a union from the very beginning?
>> And I wonder whether that supposed union part wouldn't be nicely
>> done using struct physdev_pci_device.
> 
> The do look strikingly similar ;-)
> 
> How would a union be useful here?

If this was left as iotopo, PCI would be one of potentially many I/O
variants, and hence a union would seem warranted. If you want to
rename it to pcitopo that of course would be pointless.

>> Additionally please add IN and OUT annotations. When I first saw
>> this I assumed they would all be OUT (in which case the long running
>> loop problem mentioned in the reply to one of the other patches
>> wouldn't have been there), matching their CPU counterpart...
> 
> I don't follow this. Are you saying that if ti->max_devs in patch 3/4 is 
> an IN (which it is) then we don't have to guard for long-running loops?

If they were all OUT then there wouldn't be a way for the entire
operation to be fooled into going over more devices than there are
in the system.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel