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.