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

Re: [Libguestfs] [PATCH 1/2] availability: Add guestfs_available.



On Mon, Nov 23, 2009 at 04:54:55PM +0000, Matthew Booth wrote:
> To avoid me having to expound 2 independent arguments, if you can  
> clarify which group management policy you favour, I will gladly expand  
> further on why it is a bad idea.

Let's have some definitions first:

  function:  An individual guestfs function, eg. guestfs_pread

  group:     Some broad feature of libguestfs, at the moment limited
             to defining just a set of functions.  Groups of functions
	     can overlap.

You can already test whether libguestfs.so contains individual
function calls, using dlopen.  In the example below I linked a program
dynamically against a recent version of libguestfs.so which has the
symbol guestfs_dd.  I then ran the program against the recent
libguestfs.so and against an older libguestfs.so which lacked this
symbol.  As you can see, the presence or absence of the function was
detected.

  $ gcc -I ~/d/libguestfs/src -L ~/d/libguestfs/src/.libs  -Wall test-dd.c -lguestfs -ldl -o test-dd
  $ ./test-dd 
  this libguestfs.so does NOT have guestfs_dd function
  $ LD_LIBRARY_PATH=~/d/libguestfs/src/.libs ./test-dd 
  this libguestfs.so has guestfs_dd function

(Program attached).

However the fact that a function is available in libguestfs.so doesn't
mean that it is available in the appliance (for two reasons: you might
be using an older appliance, or the appliance itself might not have
implemented the function).

Therefore the only way to truly know if a function is available and
will work is to try to run the function and test if it returns an
error.  This advice hasn't changed, and the guestfs_available call
documentation clearly states that you have to do this.

So you may be wondering what guestfs_available is for if it can't test
for availability of function(s).

guestfs_available is designed for two scenarios (outlined in [1]).
Either you can use it to fail early when a group of functions that you
need is definitely not available in the appliance at runtime.  You can
fail early with a clear error message.  Or you can use it to disable
part of a tool's functionality.

[1] http://git.annexia.org/?p=libguestfs.git;a=commit;h=8fd7f255d611d2092a244c4a48c6b7b4529e98b1

There are two drivers for doing this at all: Firstly a Windows-based
appliance would never be able to implement certain features
(eg. LVM2).  Secondly some distros right now ship a version of
libguestfs that lacks working features (eg. guestfs_zerofree is not
available in RHEL because zerofree is not packaged, and there are many
more examples).

> 1. Groups remain static
> -----------------------
>
> In the static group management policy an API group remains constant  
> forever. New [functions] are either omitted or added to a new group.
>
> This policy has the advantage of having a well-defined meaning. i.e. you  
> can look at the documentation and be sure that if  
> guestfs_available("augeas") returns true then a certain set of augeas  
> APIs are definitely implemented.
>
> The problem with this policy is that the groups become unmanageable as  
> new APIs are added. So in a year's time I want to required the full  
> range of augeas apis. I have to remember to check 'augeas',  
> 'augeas_new', 'augeas_newer' and 'augeas_newer_still'. What's more, the  
> mapping to these things is arbitrary because the base has a huge wadge  
> of stuff in it, whereas the incrementals are very small and I can't  
> remember what falls in which bucket. Unless I use this API day in, day  
> out, I will need to head over to the documentation every time to look up  
> the group mappings I need. We end up with a proliferation of unmemorable  
> groups.

There is documentation and I think it's pretty clear, but the larger
incorrect assumptions here are: (a) that we'll incrementally add new
groups as you've described, and (b) that you have to use
guestfs_available to check for all these new groups.

On point (a): Let's say that Augeas adds a compelling new function
called augeas_new, which we wish to add to the libguestfs API
(guestfs_augeas_new).  If this function becomes quickly available in
the base appliances that we care about [currently: RHEL/CentOS 5.4+,
Fedora 11+, Debian; in future: Windows] then we don't need to add
another group at all.  The only purpose of the new group would be to
distinguish between platforms that have augeas_new and those that
don't.

If this function becomes available in the Linux platforms that we care
about, but Augeas itself is still completely unavailable in Windows,
then we'll add guestfs_augeas_new to the existing "augeas" group.

The only case where we have to add a new group is if (eg) RHEL 5.4
sticks with the old Augeas and everyone else upgrades to the new
Augeas, and therefore we'd want to distinguish between 3 cases (no
Augeas at all for Windows, original Augeas for RHEL 5.4, new Augeas
for everyone else).  To distinguish 3 cases you need two groups.

On point (b): You won't be using guestfs_available as the final test
anyway.  Because the advice remains: the only way to truly know if a
function is available and will work is to try to run the function and
test if it returns an error.

guestfs_available gives you the ability to fail early with a nice
error message.  But it doesn't test whether a function finally works.
So your existing code still works:

  r = guestfs_available (g, ["augeas"]);
  if (r == -1) {
     fail "sorry folks, this program is never gonna work";
  }
  //...
  r = guestfs_augeas_new (g);
  if (r == -1) {
    fail "augeas_new: failed";
  }

OK, so the error message is ever so slightly less useful because you
didn't fail early.  But because you're still checking the error
returns for each function *which you have to do, always* you still
fail with an error and Bad Stuff doesn't happen.

We're not doing always '1. Groups remain static' I guess we must be
doing ...

> 2. Groups are updated to include new APIs
> -----------------------------------------
>
> In this model, a new augeas call would be added to the augeas group.
>
> This policy can no longer be called 'availability' because it does not  
> determine the availability of an API. For example, augeas_new is  
> introduced in libguestfs version X, and the augeas group is updated. The  
> problem now is that if the code is run agains libguestfs version X-1,  
> the test for 'augeas' returns true, despite the fact that my API is not  
> available.

You should avoid the linking error using the attached code.  That's
more of a problem with C itself, and it affects every C program that
calls a new API and then is linked against an old *.so file.

Once you've avoided the C error, by checking error codes you're in the
situation outlined immediately above.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <dlfcn.h>
#include <guestfs.h>

int
main ()
{
  void *dl = dlopen (NULL, RTLD_LAZY);
  if (!dl) {
    fprintf (stderr, "dlopen: %s\n", dlerror ());
    exit (1);
  }
  int has_dd = dlsym (dl, "guestfs_dd") != NULL;
  dlclose (dl);

  if (!has_dd)
    printf ("this libguestfs.so does NOT have guestfs_dd function\n");
  else {
    printf ("this libguestfs.so has guestfs_dd function\n");
    // Now it's safe to call guestfs_dd (g, "foo", "bar");
  }

  exit (0);
}

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