[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 12:00:08AM +0100, Richard W.M. Jones wrote:
> On Sun, Apr 12, 2009 at 11:53:10AM +0100, Richard W.M. Jones wrote:
> > On Sun, Apr 12, 2009 at 11:35:49AM +0200, Ján ONDREJ (SAL) wrote:
> > > Btw, where there is no "edit filename" command in guestfish? Something
> > > similar to "virsh edit ...", invoke editor on file downloaded from image and
> > > then store changed filename back.
> [..]
> 
> This is now implemented in the git repo.

Works well. :)

An example for python may be useful too. This works for me (just is not
nice):

from guestfs import GuestFS

g = GuestFS()
g.add_drive('test.img')
g.launch()
g.wait_ready()
g.mount('/dev/sda1', '/')
hosts = g.cat('/etc/hosts')
hosts += '127.0.0.9 testing\n'
g.write_file('/tmp/hosts', hosts, len(hosts))
g.sync()
g.umount_all()

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.

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

- __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)
      ...

- 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):

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

- 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:
    def launch (self, wait=True):
  can do this always and if somebody needs, can set wait to False and
  call it manually.

			SAL


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