On Tue, Sep 13, 2011 at 10:07:37AM +0100, Daniel P. Berrange wrote:
> On Tue, Sep 13, 2011 at 01:08:00PM +0800, Daniel Veillard wrote:
> > 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.
>
> Err, my example *was* still requiring that apps use 'import libvirt_qemu'
> in order to get access to the QEMU specific APIs, and use that separate
> objects to get their connection handle.
yes but from that point on if you use inheritance, the fact of using
the qemu tainted objects instead of the normal ones disapear completely.
The fact of using those temporary APIs get hidden in an import and a
new() somewhere. I would really prefer to see something explicit at the
place where it is used, something that people can't miss where reading
the code using it.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list