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

Re: [libvirt] [PATCH 0/7] Expose QEMU APIs to Python binding



On Mon, Sep 12, 2011 at 03:29:32PM +0100, Daniel P. Berrange wrote:
> On Fri, Sep 09, 2011 at 07:24:40PM +0800, Osier Yang wrote:
> > This patchset is to expose QEMU APIs to Python binding, as we
> > don't intend to support the QEMU APIs officially, it's exposed
> > seperately with general libvirt APIs with standalone
> > libvirt_qemu.py and libvirtmod_qemu.so. And there is no class
> > for QEMU APIs, there are written directly as function in
> > libvirt_qemu.py.
> > 
> > How to use the APIs.
> > 
> > #! /usr/bin/python -u
> > import libvirt
> > import libvirt_qemu
> > 
> > conn = libvirt.open(None)
> > dom = conn.lookupByName('test')
> > 
> > print libvirt_qemu.qemuMonitorCommand(dom, 'info blockstats', 1)
> > libvirt_qemu.qemuAttach(conn, 2307, 0)
> 
> This feels like rather a strange way to expose this in Python.
> 
> 
> 
> We currently have 'libvirt.Connection' and 'libvirt.Domain'
> objects in the Python binding.
> 
> > conn = libvirt.open(None)
> 
> This is giving us a libvirt.Connection object.
> 
> > dom = conn.lookupByName('test')
> 
> This is giving us a libvirt.Domain object.
> 
> > print libvirt_qemu.qemuMonitorCommand(dom, 'info blockstats', 1)
> 
> And this is just wierd.
> 

  Yes, but bypassing libvirt API by using the monitor direct is weird.
Let's repeat that the goal is to provide the functionality until a
correct long term supported API for the feature is provided in libvirt
itself.

> What I think is that we should have a 'libvirt_qemu.QemuConnection' object
> which is a subclass of 'libvirt.Connection', and 'libvirt_qemu.QemuDomain'
> object which is a subclass of libvirt.Domain' which adds the new QEMU
> specific method 'qemuMonitorCommand'.

  And then you completely blur the lines of what is supported normal
libvirt APIs and what is bypassing them. qemuMonitorCommand has no
structure, not the same kind of long term garantees like libvirt has
and IMHO if it is to be used in client code it should be done in a
separate module to well identify the separation. Blurring the
fundamental difference by using a subclass at creation is the best way
to garantee that code bypassing libvirt APIs will be intermixed with
normal API code.
  And that's precisely why I suggested osier to not use inheritance
here. The goal is not to make nice code, the goal is to allow temporary
code but make sure people switch to the libvirt APIs once they are
implemented. That's also the reason why we want a separate
  import libvirt_qemu
instead of just making it available from
  import libvirt
which from a coding perspective would also be nicer.

  So I really prefer version 2, even if it looks less nice from a coding
perspective.

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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