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

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.