On 01/21/2014 03:09 AM, Zhou Zheng Sheng wrote:
on 2014/01/21 11:31, Zhou Zheng Sheng wrote:
on 2014/01/17 10:24, Aline Manera wrote:
From: Aline Manera <alinefm@br.ibm.com>
I am sending the first patch set to refactor model It is not completed. I still need to do a lot of tests and so on. But I would like to share it with you to get your suggestion.
Basically, the common code between model and mockmodel was placed into model/<resource>.py and the specific code into model/libvirtbackend.py and mockbackend.py
*** It still needs a lot of work *** - Tests all functions - Update Makefile and spec files - Update test cases
Aline Manera (13): refactor model: Create a separated model for task resource refactor model: Create a separated model for debugreport resource refactor model: Create a separated model for config resource refactor model: Create a separated model for host resource refactor model: Create a separated model for plugin resource refactor model: Create a separated model for libvirt connection refactor model: Move StoragePooldef from model to libvirtstoragepools.py refactor model: Create a separated model for storagepool resource refactor model: Create a separated model for storagevolume resource refactor model: Create a separated model for interface and network resources refactor model: Create a separated model for template resource refactor model: Create a separated model for vm resource refactor model: Update server.py and root.py to use new models
src/kimchi/control/base.py | 32 +- src/kimchi/control/config.py | 30 +- src/kimchi/control/debugreports.py | 18 +- src/kimchi/control/host.py | 24 +- src/kimchi/control/interfaces.py | 11 +- src/kimchi/control/networks.py | 11 +- src/kimchi/control/plugins.py | 13 +- src/kimchi/control/storagepools.py | 32 +- src/kimchi/control/storagevolumes.py | 24 +- src/kimchi/control/tasks.py | 11 +- src/kimchi/control/templates.py | 11 +- src/kimchi/control/utils.py | 2 +- src/kimchi/control/vms.py | 17 +- src/kimchi/mockmodel.py | 784 -------------- src/kimchi/model.py | 1827 -------------------------------- src/kimchi/model/__init__.py | 21 + src/kimchi/model/config.py | 52 + src/kimchi/model/debugreports.py | 86 ++ src/kimchi/model/host.py | 49 + src/kimchi/model/interfaces.py | 48 + src/kimchi/model/libvirtbackend.py | 955 +++++++++++++++++ src/kimchi/model/libvirtconnection.py | 123 +++ src/kimchi/model/libvirtstoragepool.py | 225 ++++ src/kimchi/model/mockbackend.py | 338 ++++++ src/kimchi/model/networks.py | 115 ++ src/kimchi/model/plugins.py | 29 + src/kimchi/model/storagepools.py | 86 ++ src/kimchi/model/storagevolumes.py | 95 ++ src/kimchi/model/tasks.py | 45 + src/kimchi/model/templates.py | 89 ++ src/kimchi/model/vms.py | 164 +++ src/kimchi/networkxml.py | 6 +- src/kimchi/root.py | 24 +- src/kimchi/server.py | 16 +- src/kimchi/vmtemplate.py | 18 +- 35 files changed, 2674 insertions(+), 2757 deletions(-) delete mode 100644 src/kimchi/mockmodel.py delete mode 100644 src/kimchi/model.py create mode 100644 src/kimchi/model/__init__.py create mode 100644 src/kimchi/model/config.py create mode 100644 src/kimchi/model/debugreports.py create mode 100644 src/kimchi/model/host.py create mode 100644 src/kimchi/model/interfaces.py create mode 100644 src/kimchi/model/libvirtbackend.py create mode 100644 src/kimchi/model/libvirtconnection.py create mode 100644 src/kimchi/model/libvirtstoragepool.py create mode 100644 src/kimchi/model/mockbackend.py create mode 100644 src/kimchi/model/networks.py create mode 100644 src/kimchi/model/plugins.py create mode 100644 src/kimchi/model/storagepools.py create mode 100644 src/kimchi/model/storagevolumes.py create mode 100644 src/kimchi/model/tasks.py create mode 100644 src/kimchi/model/templates.py create mode 100644 src/kimchi/model/vms.py
I do not like this approach. It just put everything into the libvirtbackend instead of in the current monolithic model class. We still get monolithic libvirtbackend module. While the change is big, but we do not benefit much in terms of logic splitting and modulization.
I'm proposing another mechanism. Maybe I'll send the patches later.
The libvirtbackend in "Refactor model" patches is still a monolithic class. This is due to the following reasons.
1. Before the split, all controllers use the same model instance, and we can feed the root controller a mock model instance to have all controllers work on the mock one.
2. After the controller split and model split, we want concrete controllers call concrete models, for example, VM controller calls VM model. This seems OK but there is no way to switch all concrete controllers to call mock models.
3. So comes a new layer of abstraction. A "backend" concept was introduced and the "backend" does the actual work. The models share the same "backend", and we just feed a real "backend" or mock "backend" to switch between production mode and test mode. The key problem of "share one thing" v.s. "modulization" is still not solved.
I am not sure I completely understood this point. Do you mean I should also split libvirtbackend.py into libvirt/vms.py, libvirt/templates.py ? And the some for mockbackend.py? I've already thought about that and I don't have a solid conclusion. But I am OK in splitting it too. Could I do it after getting this patch set merged? So we will not block it for a long time.
I think we have many ways to solve this problem. The key problem is after split how we enable the concrete controllers call concrete models while we can easily switch between real models and mock models.
The first way is that we can have it automatically synthesize a single module instance from all the concrete instances, and all the controllers still call this single model instance. This is feasible because the synthesis is automatic, the logic and responsibilities are still split to each concrete model.
Yeap. Thanks! I liked the way you did and I will also add it to this patch set. So I don't need to change control and tests.
The second way is to introduce an abstract model factory. We can have a "real" model factory instance and a "mock" one to produce only "mock" model instance. We let the concrete controller request the model factory instance to construct a concrete model instance. Then we can easily switch between the production mode and test mode by feeding the root controller with a real model factory instance or a mock one. This is feasible because the factory just cares the creation of a concrete model, the inner model responsibilities are still wrapped within the concrete model.
I'm proposing the first way because the change will be minimum and non-intrusive. I'm trying to send the patches later...