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

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



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


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