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 _______________________________________________ virt-tools-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/virt-tools-list
