[Kimchi-devel] [PATCH 00/13][WIP] Refactor model
Aline Manera
alinefm at linux.vnet.ibm.com
Tue Jan 21 17:15:21 UTC 2014
On 01/21/2014 06:44 AM, Royce Lv wrote:
>
> On 2014年01月21日 13:09, 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 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.
> I think common logic abstraction is good.
> While Zhengsheng's point here is to maintain the consistency between
> old model api and new model api(e.g. vms_get_list),
> So that testcases in test_model.py can switch to newest seamless. What
> about combine 1st and 2nd proposal?
Yeap! I think that way we will have a more modularized and reusable code
>
> Our aim is:
> 1. reuse common logic between mockmodel implementation and model
> implementation.
> 2. make libvirt or mock implementation confined to there own class and
> transparent to caller.
> 3. keep consistency between old model api and refactored api to make
> test_model.py testcases reusable.
>
> To address 1 and 2 we can adopt Zhengsheng's proposal of factory pattern.
> 1. Abstract common logic in a parent class, like we did in
> VMScreenshot(we need to limit the lookup inside)
> 2. libvirt and mock reload their own logic as we do in
> LibvirtVMScreenshot and MockVMScreenshot.
>
> To address 3 we can use a synthesize single module together with
> Sheldon's module function probe:
> (REF: [Kimchi-devel] [PATCH 1/4] improve controller: add a method to
> load root sub collections/resouces automatically)
>
> Under model directory: there are vm.py, storagepool.py, etc
>
> in __init__.py:
In fact, in __init__.py I was thinking to add the code made by
Zhengsheng (in plugins/__init__.py)
and libvirtmodel.py will have
class LibvirtModel(Model):
self.vms = VMsModel()
self.templates = TemplatesModel()
....
The same way, Zhengsheng did for RootModel() in "[PATCH] Break the
'sample' plugin's monolithic model into several smaller ones"
>
> class LibvirtModel(object):
> model_type = 'real'
>
> def __init__(self):
>
> modules = listPathModules('./')
> for module in modules:
> # probe libvirt vm modules and classes, see sheldon's
> patch for module probe implementation
> module = imp.load_module()
> cls_names = self._get_classes(module)
>
> # assgin these class methods to Model class, see
> zhengsheng's patch for synthesize:
> ....
> m =getattr(model_instance,member_name,None)
> setattr(self,'%s_%s'%(method_prefix,member_name),m)
>
>
> def _get_classes(module):
> klasses = []
> for name, obj in inspect.getmembers(module):
> if inspect.isclass(obj):
> klasses.append(name)
>
> The whole refactor thing is huge, I suggest we do it one by one,
> starting from parts which is easy to decouple, like distro, host
> and things depend less on each other, not swallow it all at once.
> Also Mark has proposal to make the db and libvirt connection to be a
> singleton so that class does not need to keep instance of it.
> He will follow up with this thread.|
>
> >
> >
> > 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...
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140121/eddf4541/attachment.html>
More information about the Kimchi-devel
mailing list