在 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.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
