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

Re: [fedora-virt] ANNOUNCE: Augeas support added to libguestfs



On Thu, Apr 16, 2009 at 10:09:53AM +0200, Ján ONDREJ (SAL) wrote:
> hosts = g.cat('/etc/hosts')
> hosts += '127.0.0.9 testing\n'
> g.write_file('/tmp/hosts', hosts, len(hosts))

Note also if you want to do this in a more structured way, use Augeas.
It's similar to example #3 here:

http://rwmj.wordpress.com/2009/04/12/libguestfs-08-now-with-fewer-bugs/

> I have some suggestions for python interface. It's working, but it's not
> what an python programmer can expect. You can apply/ignore any of them.
> 
> - documentation is missing
>   Using "import guestfs;help(guestfs.GuestFS)" does not return documentation
>   for functions, only list of functions. I know, there is documentation
>   on you page or in man page, but an python programmer is accustomed to
>   use this documentation.

I didn't know this.  I'll add the documentation.

> - only one object GuestFS
>   python is an object oriented language. Almost anything in python is an
>   object. There is only one object in guestfs.py. I see at least aug_*
>   functions, which can be another object.

I mentioned in a reply to someone else that the bindings won't be
completely natural, because we providing bindings equally in all
languages.  Moreover although it looks like there is an "augeas
object" (and similarly a "volgroup object" etc), these objects don't
really exist anywhere.  It's perfectly possible to do:

  vgcreate Foo /dev/sda1     # we've created Foo "volgroup object"
     # now we go behind the scenes and remove that volgroup
     # by issuing a direct command to the guest
  command "vgremove Foo"
  vgs                        # Foo has gone

So if you attempt to emulate these objects in a higher layer, you're
bound to come unstuck at some point.

> - __init__ does not have parameters
>   guestfish has many parameters, which can be run at start time.
>   I think this should be in python functions too. For example at least
>   add_drive and mount options can be very useful. Also starting launch()
>   when drives are defined will be a good idea.
>   May be testing if drives parameter is an tuple/list or a normal string
>   can be used to set one or more drives at startup.
> 
> - missing default parameters
>   Similary as for __init__, other functions should have their default
>   parameters. Examples:
>     def set_path (self, path='/'):
>     def set_autosync (self, autosync=True):
>     def mount (self, device, mountpoint='/'):
>     def ls (self, directory='/'):
>   Also for aug_ functions, I don't know what to defined as defaults.
>
> - size can be counted from python object:
>   def write_file (self, path, content, size=None):
>       if size is None:
>         size = len(content)
>       ...

Yes, we can implement some of this.

> - non python function names
>   In python all objects uses readlines() and not read_lines() function.
>   readlines() is a standard in python.
>     def read_lines (self, path):
>   Similar situation for mkdir_p have to be makedirs, as in python's os object:
>     def mkdir_p (self, path):

Need to provide parity between bindings.  I think actually it's
confusing to have special names for commands in one language and not
in another.

> - parameter position
>   Some python functions have different order of parameters like your
>   functions. For a python programmer it can be diffucult to know, where are
>   which orders of paramters. I see these functions:
>     libguestfs				python
>     chmod (self, mode, path):		os.chmod(path, mode)
>     chown (self, owner, group, path):	os.chown(path, uid, gid
> 
> - unmount_all in __del__
>   I don't know, if it's neccessary, but calling umount_all before removing
>   this object may be useful. May be it's called from libguestfsmod.close,
>   then it's not a problem.

This is something to consider in the base library.  We already do a
lot of cleanup (eg. killing the qemu process).

> - 2 functions which do 1 thing
>   After calling launch() it's always needed to call wait_ready().
>   Why it's neccessary? May be creating an:

See the documentation on asynch calls.

Thanks for the suggestions, I'll see what can be done.

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/


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