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

[virt-tools-list] Re: libosinfo - RFC v2



Responses inline.

On 10/26/2009 05:31 AM, Matthew Booth wrote:
> 'Distribution' and its contraction 'distro' are peculiar to Linux and *BSD 
> users. Everybody else calls it an Operating System, or OS for short. Could we
> replace all uses of 'distro' with 'os'. It will also save bits ;)

I have no objections to this.

> On 25/10/09 07:13, Arjun Roy wrote:
>> 1. Primary identifier will now use RDF. In addition, there will be a short name
>> parameter that is supposed to be unique as well, for use in console apps and
>> other uses where the user will want to refer to a distro.
>>
>>    <distro rdf:about="http://fedoraproject.org/fedora-10";>
>>      <name>Fedora 10</name>
>>      <kernel>linux</kernel>
>>      <kernel-version>2.6.30</kernel-version>
>>    </distro>
>>
>>    <distro rdf:about="http://fedoraproject.org/fedora-11";>
>>     <upgrades rdf:about="http://fedoraproject.org/fedora-10";>
>>     <short-id>fedora11</short-id>
>>     <name>Fedora 11</name>
>>    </distro>
>
> I don't think I'm going to win the CIM argument ;) However, it's still worth 
> defining a standard tag for TargetOSType, even if it isn't mandatory.

The way this library is set up, there are a very small number of 'official' tags
defined; just the minimum required for uniquely specifying a distro and for 
specifying the relationship between two distros, if one exists. If need be, a 
TargetOSType property with corresponding value could just be added to the backing
data for this library. It wouldn't change the API at all to have it included.

>> /* Setting parameters like libvirt version, etc. */
>> int osi_set_lib_param(osi_lib_t lib, char* key, char* val);
>> int osi_get_lib_param(osi_lib_t lib, char* key);
>> int osi_set_hypervisor(osi_lib_t lib, char* hvname, char* hvversion);
>> osi_hypervisor_t osi_get_hypervisor(osi_lib_t lib);
>
> What are the set methods for? Are you planning to allow updating of the XML 
> from the library?

The library will be read only. But, before initializing the library, the user 
will set the version of libvirt and the type and version of hypervisor he is 
using, which will affect the results.

For example, based on the version of libvirt, maybe a certain type of device 
driver will not be valid for a given OS.

So set param will set any 'environment parameters' like this, that affect results
when querying the library. Get param just shows for convenience what values have
been set on the library, given the lib handle. Set param will fail once the library
has been initialized, so we can only set parameters before the library starts.

Then if you want to find out what the value would have been if you had specified
another libvirt or another hypervisor, you get another handle to the library and
configure that one, in that fashion. You can have as many such handles as you want.

>> /* Initializing and closing down the library */
>> int osi_init_lib(osi_lib_t lib);
>> int osi_close_lib(osi_lib_t lib);
>>
>> /* Getting parameters for hypervisor */
>> char* osi_get_hv_property_pref_value(osi_hypervisor_t hv, char* propname);
>> char** osi_get_hv_property_all_values(osi_hypervisor_t hv, char* propname, int* num);
>> char** osi_get_all_property_keys(osi_hypervisor_t hv, int* num);
>
> How big are these lists going to get? Would an iterator type and API be more 
> appropriate here (and throughout)?

I think just returning a dynamically generated array of strings, and the size,
should be fine here. I don't know *exactly* what usage cases will result from 
this data, so it seems that sticking to simple return types is best.

>>    ret = osi_init_lib(lib);
>>    if (ret != 0) {
>>      printf("Error: Could not set initialize libosinfo!\n");
>>      exit(1);
>>    }
>
> Is anything more specific required here in terms of error reporting? Would you just
> try to make sure errno is set cleanly, or are there likely to be additional,
> non-system related failure conditions?

To be honest, I haven't thought of error reporting too much yet. For now it'd 
just be 0: it worked, otherwise failure. I suppose at some point during 
implementation I could figure out better error codes to return; it would tend to
be stuff like 'file doesn't exist' or ENOMEM, just using the errno codes. I don't
want to mess with actually setting errno though; seems like it would be cleaner
just to return the error condition.

I'll figure it out while doing implementation.

>>    osi_filter_t filter = osi_get_filter(lib);
>>    ret = osi_add_constraint(filter, "short-id", "rhel5.4");
>>    if (ret != 0) {
>>      printf("Error: Could not set constraint!\n");
>>      exit(1);
>>    }
>>
>>    osi_distro_list_t results = osi_get_distros_list(lib, filter);
>>    if (osi_bad_object(results))
>
> What does osi_bad_object() do?

The list type is opaque. If the operation for getting a list failed, then rather
than return a non-zero int to depict error, we return a singleton object that
corresponds to an error. Perhaps osi_operation_failed(results) would be more clear?

>>    // Now that we have a handle to rhel5.4, we can free the results list
>>    // that we used to get to it. The handle to the single distro is still
>>    // valid though, and we use it for the next step
>>    ret = osi_put_distros_list(results); // Done with that list so get rid of it
>>    if (ret != 0) {
>>      printf("Error freeing distro list!\n");
>>      exit(1);
>>    }
>
> Ah! Probably best to call that osi_free_...

Fine by me.

>>    // We shall reuse the filter
>>    ret = osi_clear_all_constraints(filter);
>>    if (ret != 0) {
>>      printf("Error clearing constraints!\n");
>>      exit(1);
>>    }
>>
>>    if (osi_add_constraint(filter, "kernel", "linux") != 0 ||
>>        osi_add_constraint(filter, "kernel-version", "2.6.30") != 0 ||
>>        osi_add_relation_constraint(filter, DERIVES_FROM, rhel) != 0)
>>    {
>>      printf("Error adding constraints!\n");
>>      exit(1);
>>    }
>
> Going back to data-modelling. Does RHEL 5.5 derive_from RHEL 5.4? How about 
> RHEL 6.0? If they both do, we're missing something here because these 
> relationships are quite different.

I believe Daniel Berrange specified these relationships in an earlier email; 
something along the lines of:

-rhel6.0 upgrades rhel5.4 which upgrades rhel5.3
-centos5.4 clones rhel5.4
-scientific linux release 53 derives from rhel (not sure which release exactly,
but it would derive from exactly one), but is different so it doesn't clone it
and it isn't an upgrade.

At most, a distro (DERIVES_FROM/UPDATES/CLONES) at most one other distro.

-Arjun


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