[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[virt-tools-list] Re: libosinfo - another try



Some responses inline below.

On 10/22/2009 08:47 AM, Cole Robinson wrote:
> If we are going to use 'ID' here as the unique identifier, I think it
> should have some human readable value. Just using numbers will quickly
> become confusing if manually navigating the XML.
>
> In the previous discussion, someone mentioned just using <name> as the
> unique identifier, however I don't think that is sufficient either,
> since it is not very machine friendly. Something like
>
> "Fedora 12" -> "fedora12"
> "Red Hat Enterprise Linux 5.4" -> "rhel5.4" or
>                                   "red_hat_enterprise_linux_5.4"
>
> Think of using this value from the command line (like virt-install
> --os-variant), no one is going to want to have to use spaces and proper
> capitalization, or a plain digit.

I agree. Since this is going to be a heavily user-facing library in that sense,
there needs to be some way to make this nicer. I just put in integer IDs as a
placeholder, but am open to suggestions.

>> This would effectively force our view of the data as a graph, where the set of
>> nodes are the set of distros and the edges are described by the structure
>> attribute tags. (With a different graph resulting depending on attribute)
>>
>> 2. Informative Attributes
>> This could refer to things such as name, kernel-type, architecture, or anything
>> else that would be useful to track. I suspect that it would be alright to limit
>> the kind of data to strings (kernel : linux) and version numbers 
>> (kernel-version: 2.6.30).
>>
>
> I kind of wonder if the 'version' distinction is even necessary, but I
> don't think I could say for certain until we start trying to use the code.

The idea here was that you could search for distros with a kernel type exactly
equaling linux (a string attribute) with kernel version *less than* 2.6.30,
which implies special handling. But the more I think about it, I realize
projects will have versioning methods that are wildly different enough that a
one size fits all solution would be hard to implement. I'm looking for a good
solution on this one.

I mean, we need to be able to express that we are interested in a distro with
a property foo that has version number bigger/smaller than a given version,
right?

>> /* Methods for filtering results when querying distros */
>> osi_filter_t osi_getFilter(osi_lib_t lib);
>> int osi_putFilter(osi_filter_t filter);
>> int osi_addStringPropertyConstraint(osi_filter_t filter, cstring propName, cstring propVal);
>> int osi_addVersionPropertyConstraint(osi_filter_t filter, cstring propName, cstring propVal, enum_t ordering);
>> int osi_addLinkedDistroConstraint(osi_filter_t filter, enum_t relationship, os_distro_t distro);
>> int osi_clearStringPropertyConstraint(osi_filter_t filter, cstring propName);
>> int osi_clearVersionPropertyConstraint(osi_filter_t filter, cstring propName);
>> int osi_clearLinkedDistroConstraint(osi_filter_t filter, enum_t relationship);
>
> I think all these calls mean we should break out an 'osi_prop_t'. This
> way, we only have one call for addProp, clearProp, getProp, and then a
> set of methods to manipulate properties.
>
> Not sure how the implementation details would work out.
>
There are two kinds of properties here. The first are those that affect the 
osi_lib_t handle, which are for setting environment parameters. The second are 
for specifying constraints when querying for distros. They'd have to be kept 
separate since they mean different things.

I suppose for the latter, one could define an osi_prop_t. Then it would be 
defined with the propName and a propType (either one of the string properties or
one of the link based properties, which is a distinction that I do believe we 
need) and an appropriate propValue. My only issue is that such an implementation
would rapidly pollute the code with a *lot* of objects from this library that
need to be accounted for, which would lead to bugs in usage. On the other hand,
if we only have a filter object and pass in the properties 'anonymously' as
defined above, it's much easier on the user of the API.

> Overall it looks good, there seems to be enough here to handle even the
> difficult cases
>
> I think some (psuedo)code examples using the API would also go a long
> way in helping reviewers wrap their head around the code: it could also
> expose some awkwardness in the above methods.
I'll see if I can write some up shortly.

-Arjun


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]