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

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



On 26/10/09 17:59, Arjun Roy wrote:
/* 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.

I see where you're coming from here. However, this looks like a second filter mechanism. Why can't you use the explicit filter mechanism for this?


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.

Ok. I think, however, there are 2 different 'updates' relationships. We need to differentiate major version upgrades from minor version upgrades. E.g.:

RHEL 4.6 -> RHEL 4.7		vs RHEL 4.6 -> RHEL 5.4
Vista SP1 -> Vista SP2		vs Vista SP1 -> Windows 7
SLES 9 SP2 -> SLES 9 SP3	vs SLES 9 SP2 -> SLES 10

The former is a relatively minor update. Reconfiguration of workload is unlikely to be necessary. Can probably be done with an update followed by a short outage for a reboot.

The latter is a major update. Workload will almost certainly require reconfiguration. Some workloads may not work at all on upgraded system. Requires extensive testing. Possibly manually intensive.

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]