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

Re: [virt-tools-list] Introducing: virtlib library



On 04/15/2011 09:16 AM, Darryl L. Pierce wrote:
> On Thu, Apr 14, 2011 at 04:38:33PM -0400, Cole Robinson wrote:
>> Please excuse the double response.
>>
>> So I've scanned through the code briefly, but I'm a bit confused. Where
>> is this heading? Can you give a provide an explicit example of:
>>
>> 1) Functionality that is duplicated between vmm and vmmtui
> 
> vmmtui seeks to be a text-based version of vmm and provide all of the
> same features. That would make all of the functionality that's not
> directly related to the UI widgets duplicated. 
> 
>> 2) Problems that prevent vmmtui from just reusing the relevant vmm code
> 
> In previous discussions with you my understanding was that there was no
> clean separate of the view code in vmm from the business logic. 
> 

Well, overall we basically have MVC:

Model: connection, domain, network, interface, storage*, mediadev, netdev.py
View: connect.py, create*, addhardware, details, host, delete, clone, migrate
Controller: engine.py

There are definitely places where we violate this, and useful functionality
that can be shared is stuck in the controller and view files, but AFAIK those
are all fixable.

>> 3) Proposed solution
> 
> This patch set is intended to kick off an effort to first pull the
> business logic (the M and C from the MVC design pattern) out of vmmtui,
> leaving that application as purely the UI code.
> 
> Then, working from that foundation, refactor vmm to use the new shared
> libraries. Anything that vmmtui was lacking would be added into the
> shared bits and move vmmtui to be more closely identical to vmm.
> 

IMO I don't think starting fresh is the way to go, I think we should start
with the working virt-manager code (which has been around for years and gets a
lot of usage) and work backwards to make it shareable.

Specifically, this new code doesn't cache anything so will hit libvirt really
hard, which doesn't sit well with remote management, and probably won't even
be able to handle a dozen VMs on RHEL5 xen which is notoriously slow.

There's also back compat with older libvirt that's important: I usually test
virt-manager against RHEL5.2 vintage libvirt (0.3.3 or something like that)
before doing a release, just to make sure things don't blow up. The code you
posted uses APIs like isActive, which have only been around for the past year
and a half (and in some cases only working for a year): that means the app
will error if we try to connect to say an older rhel5/centos or ubuntu lts
machine, even if connecting remotely from a machine with the latest and greatest.

You also use python2.6 features like ABCMeta, which means this code won't run
on RHEL5. virt-manager makes an effort to stay compatible with python2.4.


Let's take a specific example of some code we probably want to share:
src/virtManager/network.py contains a class vmmNetwork which wraps a libvirt
virtual network, providing convenience functions for various bits of info.
This is one class that doesn't do a whole lot, and this file itself contains
nothing that would prevent it from being used by the TUI.

But vmmNetwork is a subclass of vmmLibvirtObject in
src/virtManager/libvirtobject.py. Now we find somethings that won't just work
for the TUI, since they are gobject/gtk specific. The main piece here is we
define and emit gobject signals. vmmLibvirtObject defines a "config-changed"
signal: this allows virt-manager to be notified when the XML has changed, so
we know to update the UI if required. For example, details.py hooks to this
signal for its associated VM. If the details window is visible when it
receives the signal, we refresh the current page. Granted this signal is
driven by our controller polling in the background so it isn't truly async,
but it will be when we support libvirt domain events.

Making these signals a noop for the TUI would actually be fairly straight
forward I think. We would have to add a common base class at the top, which
would overwrite a few gobject functions (connect, disconnect, emit). We'd also
need to find a way other than __gobject_signals__ to register signals, but I
know there are different methods that we could probably make work. In fact,
implementing these signals in python would be pretty trivial come to think of
it, it's really just registering a synchronous callback. That way TUI could
benefit from the signals as well.

vmmLibvirtObject also has another baseclass vmmGObject. This is just more of
the same, but since it's the class at the top, we might have to do some small
ugly hacks to avoid import gobject/gtk in the TUI, but what I'm imagining
would be small and safe.

The only other primary piece that I think would be an issue is virt-manager's
use of gconf, which is sprinkled in various places, but all accessed through
vmmConfig in src/virtManager/config.py. It would be easy enough in the short
term for the TUI to stub out that class and provide it's own instance, so all
those actions are noops. Then we can discuss a plan for moving the gconf
pieces out of common code.

Thanks,
Cole


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