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

[Matthew Booth]

Arjun,

I really like this. A general point first, then other comments inline.

'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 ;)

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 library would be implemented in C, and define a few major types:
>
> osi_lib_t : an opaque handle to a library instance.
> osi_distro_t : represents a distro object.
> osi_distro_id_t : represents the unique ID for a distro.
> osi_distro_list_t : a list of osi_distro_t objects.
> osi_filter_t: filter for searching through distros.
> osi_hypervisor_t : represents a hypervisor
>
> And a bunch of methods:
>
> osi_lib_t osi_get_lib_handle();
>
> /* 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?

> /* 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)?

> /* Querying for distros and iterating over them */
> osi_distro_list_t osi_get_distros_list(osi_lib_t lib, osi_distro_filter_t filter);
> int osi_put_distros_list(osi_distro_list_t distros);
> int osi_distro_list_length(osi_distro_list_t distros);

Again, what's put() doing here?

> /* Methods for filtering results when querying distros */
> osi_filter_t osi_get_filter(osi_lib_t lib);
> int osi_put_filter(osi_filter_t filter);
> int osi_add_constraint(osi_filter_t filter, char* propname, char* propval);
> int osi_add_relation_constraint(osi_filter_t filter, enum_t relationship, os_distro_t distro);
> int osi_clear_constraint(osi_filter_t filter, char* propname);
> int osi_clear_relation_constraint(osi_filter_t filter, enum_t relationship);
> int osi_clear_all_constraints(osi_filter_t filter);
>
> /* Get a single distro, either from a list or by ID */
> osi_distro_t osi_get_distro_by_index(osi_distro_list_t distros, int index);
> osi_distro_t osi_get_distro_by_id(osi_lib_t lib, osi_distro_id_t id);
>
> /* Query Properties for a given distro */
> osi_distro_id_t osi_get_distro_id(osi_distro_t distro);
> osi_distro_t osi_get_related_distro(osi_distro_t distro, enum_t relationship);
> char* osi_get_property_pref_value(osi_distro_t distro, char* propname);
> char** osi_get_property_all_values(osi_distro_t distro, char* propname, int* num);
> char** osi_get_all_property_keys(osi_distro_t distro, int* num);
>
> /* Query unique values for a given property */
> char** osi_unique_property_values(osi_lib_t lib, char* propname, int* num);
> os_distro_list_t osi_unique_relationship_values(osi_lib_t lib, enum_t relationship);
>
>
> Here is a sample program that sets libvirt version, hypervisor type and version,
> opens the library, gets a handle to RHEL5.4's representation, finds all the
> distros deriving from it, and lists the audio drivers available for each.
>
> #include<stdio.h>
> #include "libosinfo.h"
>
> int main(void)
> {
>    int i, ret, count;
>    osi_lib_t lib = osi_get_lib_handle();
>
>    ret = osi_set_lib_param(lib, "libvirt-version", "3.4");
>    if (ret != 0) {
>      printf("Error: Could not set libvirt version!\n");
>      exit(1);
>    }
>
>    ret = osi_set_hypervisor(lib, "kvm", "1.2");
>    if (ret != 0) {
>      printf("Error: Could not set hypervisor!\n");
>      exit(1);
>    }

Mentioned above. It's not clear to me what these set calls do.

>    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?

>
>    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?

>      printf("Bad result list!\n");
>      exit(1);
>    }
>
>    if (osi_distro_list_length(results) == 0) {
>      printf("No results. Quitting...\n");
>      exit(0);
>    }
>    else if (osi_distro_list_length(results)>  1) {
>      printf("Failed sanity check. 'short-id' should be unique...\n");
>      exit(1);
>    }
>
>    osi_distro_t rhel = osi_get_distro_by_index(results, 0);
>
>    // 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_...

>    // 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.

>    osi_distro_list_t more_results = osi_get_distros_list(lib, filter);
>    if (osi_bad_object(more_results))
>      printf("Bad result list!\n");
>      exit(1);
>    }
>
>    // For each distro:
>    count = osi_distro_list_length(more_results);
>    for (i = 0; i<  count; i++) {
>      int j, num;
>      osi_distro_t distro = osi_distro_by_index(more_results, i);
>      char* distroname = osi_get_property_pref_value(distro, "name");
>      char** audiodrivers = osi_get_property_values(distro, "audio-driver",&num);
>
>      // For each audio driver:
>      for (j = 0; j<  num; j++) {
>        printf("Audio driver for %s is: %s\n", distroname, audiodrivers[j]);
>        free(audiodrivers[j]);
>      }
>      free(audiodrivers);
>      free(distroname);
>    }
>
>    ret = osi_put_distros_list(more_results); // Done with that list
>    if (ret != 0) {
>      printf("Error freeing distro list!\n");
>      exit(1);
>    }
>
>    ret = osi_put_filter(filter); // Done with the filter
>    if (ret != 0) {
>      printf("Error freeing filter!\n"):
>      exit(1);
>    }

Again, let's call these _free_.

>
>    ret = osi_close_lib(lib);
>    if (ret != 0) {
>      printf("Error cleaning up library handle!\n");
>      exit(1);
>    }
>
>    printf("Done.\n");
>    return 0;
> }
>

Matt
-- 
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

M:       +44 (0)7977 267231
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490