[Kimchi-devel] [PATCH 00/13][WIP] Refactor model

Tue Jan 21 17:11:46 UTC 2014

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 at 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...
>