[libvirt] [PATCH v3 01/12] conf: Add definitions for 'uid' and 'fid' PCI address attributes

[Yi Min Zhao]

在 2018/8/20 下午6:35, Andrea Bolognani 写道:
> On Mon, 2018-08-20 at 16:19 +0800, Yi Min Zhao wrote:
>> 在 2018/8/16 下午10:38, Andrea Bolognani 写道:
>>> On Tue, 2018-08-07 at 17:10 +0800, Yi Min Zhao wrote:
>>>> +struct _virZPCIDeviceAddress {
>>>> +    unsigned int zpci_fid;
>>>> +    unsigned int zpci_uid;
>>>> +    bool fid_assigned;
>>>> +    bool uid_assigned;
>>>> +};
>>> A couple of questions about the approach here, one of which I have
>>> mentioned already and one of which I probably haven't (my bad):
>>>
>>>     * do you really need to have separate booleans tracking whether
>>>       or not either id has been assigned? Wouldn't the same approach
>>>       as virPCIDeviceAddressIsEmpty() work, eg. consider the address
>>>       absent if both are zero and present otherwise?
>> It's OK for uid. But for fid, zero is a valid value, so we need a bool
>> to track its assignment.
> See virPCIDeviceAddressIsEmpty() and virPCIDeviceAddressIsValid(),
> which have very similar requirement but don't use extra booleans
> to keep track of state.
>
> You could do the same thing those functions do:
>
>    * the zPCI address is empty if both uid and fid are zero;
uid=0 and fid=0 can't mean zPCI address is empty, because the user might
only define fid with 0. If fid=0 has been assigned, we should report 
error. If
it is not defined by user, fid is also 0, then we should allocate a 
valid value
for fid.
>    * the zPCI address is invalid if it's empty or uid is too large.
>
> You already have the latter covered, so it's only a matter of
> implementing the former.
>
>> If we add a boolean for fid, why not also add another one for uid to
>> keep consistency?
>> This also makes code easy to read and obvious. It doesn't waste much
>> memory space.
> I think it actually makes more complicated, which is why I'd rather
> get rid of it :)
>
>>>     * especially if you don't need the additional booleans, would it
>>>       be preferable to just add the two ids to the existing struct
>>>       instead of declaring a new one that you'll have to allocate
>>>       and make sure it's not NULL before accessing it? Again, I seem
>>>       to remember Laine feeling somewhat strongly about the topic.
>> For other platforms, is it OK to waste this unused memory (of course,
>> it's little) ?
> I believe adding a couple of unsigned ints is worth it if we can
> get rid of all the checks on addr->zpci because of it.
>