<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 2014年01月22日 01:15, Aline Manera
wrote:<br>
</div>
<blockquote cite="mid:52DEAB29.7080106@linux.vnet.ibm.com"
type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<div class="moz-cite-prefix">On 01/21/2014 06:44 AM, Royce Lv
wrote:<br>
</div>
<blockquote cite="mid:52DE335E.9050200@linux.vnet.ibm.com"
type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<br>
On 2014年01月21日 13:09, Zhou Zheng Sheng wrote:<br>
<font color="#000000"><span style="white-space: pre;">> on
2014/01/21 11:31, Zhou Zheng Sheng wrote:<br>
>> on 2014/01/17 10:24, Aline Manera wrote:<br>
>>> From: Aline Manera <a moz-do-not-send="true"
class="moz-txt-link-rfc2396E"
href="mailto:alinefm@br.ibm.com"><alinefm@br.ibm.com></a><br>
>>> <br>
>>> I am sending the first patch set to refactor
model It is not<br>
>>> completed. I still need to do a lot of tests
and so on. But I<br>
>>> would like to share it with you to get your
suggestion.<br>
>>> <br>
>>> Basically, the common code between model and
mockmodel was placed<br>
>>> into model/<resource>.py and the specific
code into<br>
>>> model/libvirtbackend.py and mockbackend.py<br>
>>> <br>
>>> *** It still needs a lot of work *** - Tests
all functions -<br>
>>> Update Makefile and spec files - Update test
cases<br>
>>> <br>
>>> Aline Manera (13): refactor model: Create a
separated model for<br>
>>> task resource refactor model: Create a
separated model for<br>
>>> debugreport resource refactor model: Create a
separated model for<br>
>>> config resource refactor model: Create a
separated model for host<br>
>>> resource refactor model: Create a separated
model for plugin<br>
>>> resource refactor model: Create a separated
model for libvirt<br>
>>> connection refactor model: Move StoragePooldef
from model to <br>
>>> libvirtstoragepools.py refactor model: Create a
separated model<br>
>>> for storagepool resource refactor model: Create
a separated model<br>
>>> for storagevolume resource refactor model:
Create a separated<br>
>>> model for interface and network resources
refactor model: Create<br>
>>> a separated model for template resource
refactor model: Create a<br>
>>> separated model for vm resource refactor model:
Update server.py<br>
>>> and root.py to use new models<br>
>>> <br>
>>> src/kimchi/control/base.py | 32
+- <br>
>>> src/kimchi/control/config.py | 30
+- <br>
>>> src/kimchi/control/debugreports.py | 18
+- <br>
>>> src/kimchi/control/host.py | 24
+- <br>
>>> src/kimchi/control/interfaces.py | 11
+- <br>
>>> src/kimchi/control/networks.py | 11
+- <br>
>>> src/kimchi/control/plugins.py | 13
+- <br>
>>> src/kimchi/control/storagepools.py | 32
+- <br>
>>> src/kimchi/control/storagevolumes.py | 24
+- <br>
>>> src/kimchi/control/tasks.py | 11
+- <br>
>>> src/kimchi/control/templates.py | 11
+- <br>
>>> src/kimchi/control/utils.py | 2
+- <br>
>>> src/kimchi/control/vms.py | 17
+- <br>
>>> src/kimchi/mockmodel.py | 784
-------------- <br>
>>> src/kimchi/model.py | 1827<br>
>>> --------------------------------
src/kimchi/model/__init__.py<br>
>>> | 21 + src/kimchi/model/config.py
| 52 + <br>
>>> src/kimchi/model/debugreports.py | 86
++ <br>
>>> src/kimchi/model/host.py | 49 +
<br>
>>> src/kimchi/model/interfaces.py | 48 +
<br>
>>> src/kimchi/model/libvirtbackend.py | 955
+++++++++++++++++ <br>
>>> src/kimchi/model/libvirtconnection.py | 123
+++ <br>
>>> src/kimchi/model/libvirtstoragepool.py | 225
++++ <br>
>>> src/kimchi/model/mockbackend.py | 338
++++++ <br>
>>> src/kimchi/model/networks.py | 115
++ <br>
>>> src/kimchi/model/plugins.py | 29 +
<br>
>>> src/kimchi/model/storagepools.py | 86
++ <br>
>>> src/kimchi/model/storagevolumes.py | 95
++ <br>
>>> src/kimchi/model/tasks.py | 45 +
<br>
>>> src/kimchi/model/templates.py | 89
++ <br>
>>> src/kimchi/model/vms.py | 164
+++ <br>
>>> src/kimchi/networkxml.py | 6
+- <br>
>>> src/kimchi/root.py | 24
+- <br>
>>> src/kimchi/server.py | 16
+- <br>
>>> src/kimchi/vmtemplate.py | 18
+- 35 files<br>
>>> changed, 2674 insertions(+), 2757 deletions(-)
delete mode 100644<br>
>>> src/kimchi/mockmodel.py delete mode 100644
src/kimchi/model.py <br>
>>> create mode 100644 src/kimchi/model/__init__.py
create mode<br>
>>> 100644 src/kimchi/model/config.py create mode
100644<br>
>>> src/kimchi/model/debugreports.py create mode
100644<br>
>>> src/kimchi/model/host.py create mode 100644<br>
>>> src/kimchi/model/interfaces.py create mode
100644<br>
>>> src/kimchi/model/libvirtbackend.py create mode
100644<br>
>>> src/kimchi/model/libvirtconnection.py create
mode 100644<br>
>>> src/kimchi/model/libvirtstoragepool.py create
mode 100644<br>
>>> src/kimchi/model/mockbackend.py create mode
100644<br>
>>> src/kimchi/model/networks.py create mode 100644<br>
>>> src/kimchi/model/plugins.py create mode 100644<br>
>>> src/kimchi/model/storagepools.py create mode
100644<br>
>>> src/kimchi/model/storagevolumes.py create mode
100644<br>
>>> src/kimchi/model/tasks.py create mode 100644<br>
>>> src/kimchi/model/templates.py create mode
100644<br>
>>> src/kimchi/model/vms.py<br>
>>> <br>
>> <br>
>> I do not like this approach. It just put everything
into the <br>
>> libvirtbackend instead of in the current monolithic
model class.<br>
>> We still get monolithic libvirtbackend module.
While the change is<br>
>> big, but we do not benefit much in terms of logic
splitting and<br>
>> modulization.<br>
>> <br>
>> I'm proposing another mechanism. Maybe I'll send
the patches<br>
>> later.<br>
>> <br>
> <br>
> The libvirtbackend in "Refactor model" patches is still
a monolithic <br>
> class. This is due to the following reasons.<br>
> <br>
> 1. Before the split, all controllers use the same model
instance, and<br>
> we can feed the root controller a mock model instance
to have all <br>
> controllers work on the mock one.<br>
> <br>
> 2. After the controller split and model split, we want
concrete <br>
> controllers call concrete models, for example, VM
controller calls<br>
> VM model. This seems OK but there is no way to switch
all concrete <br>
> controllers to call mock models.<br>
> <br>
> 3. So comes a new layer of abstraction. A "backend"
concept was <br>
> introduced and the "backend" does the actual work. The
models share<br>
> the same "backend", and we just feed a real "backend"
or mock<br>
> "backend" to switch between production mode and test
mode. The key<br>
> problem of "share one thing" v.s. "modulization" is
still not<br>
> solved.<br>
> <br>
> I think we have many ways to solve this problem. The
key problem is <br>
> after split how we enable the concrete controllers call
concrete<br>
> models while we can easily switch between real models
and mock<br>
> models.<br>
> <br>
> The first way is that we can have it automatically
synthesize a<br>
> single module instance from all the concrete instances,
and all the<br>
> controllers still call this single model instance. This
is feasible<br>
> because the synthesis is automatic, the logic and
responsibilities<br>
> are still split to each concrete model.</span></font><br>
I think common logic abstraction is good.<br>
While Zhengsheng's point here is to maintain the consistency
between old model api and new model api(e.g. vms_get_list),<br>
So that testcases in test_model.py can switch to newest
seamless. What about combine 1st and 2nd proposal?<br>
</blockquote>
<br>
Yeap! I think that way we will have a more modularized and
reusable code<br>
<br>
<blockquote cite="mid:52DE335E.9050200@linux.vnet.ibm.com"
type="cite"> <br>
Our aim is:<br>
1. reuse common logic between mockmodel implementation and model
implementation.<br>
2. make libvirt or mock implementation confined to there own
class and transparent to caller.<br>
3. keep consistency between old model api and refactored api to
make test_model.py testcases reusable.<br>
<br>
To address 1 and 2 we can adopt Zhengsheng's proposal of factory
pattern.<br>
1. Abstract common logic in a parent class, like we did in
VMScreenshot(we need to limit the lookup inside)<br>
2. libvirt and mock reload their own logic as we do in
LibvirtVMScreenshot and MockVMScreenshot.<br>
<br>
To address 3 we can use a synthesize single module together with
Sheldon's module function probe:<br>
(REF: [Kimchi-devel] [PATCH 1/4] improve controller: add a
method to load root sub collections/resouces automatically)<br>
<br>
Under model directory: there are vm.py, storagepool.py, etc<br>
<br>
in __init__.py:<br>
</blockquote>
<br>
In fact, in __init__.py I was thinking to add the code made by
Zhengsheng (in plugins/__init__.py)<br>
and libvirtmodel.py will have<br>
<br>
<br>
class LibvirtModel(Model):<br>
self.vms = VMsModel()<br>
self.templates = TemplatesModel()<br>
....<br>
<br>
The same way, Zhengsheng did for RootModel() in "[PATCH] Break the
'sample' plugin's monolithic model into several smaller ones"<br>
</blockquote>
See, this is hard code "vms,templates,storagepools" by "registering"
the class to the model, actually we can "probe" what classes are
there and the root model does not need to know the details in each
model.<br>
So that once we add a new model, we don't need to worry to update
the root one.<br>
Idea's from Sheldon's controller patchset, I just steal it:P<br>
<blockquote cite="mid:52DEAB29.7080106@linux.vnet.ibm.com"
type="cite"> <br>
<blockquote cite="mid:52DE335E.9050200@linux.vnet.ibm.com"
type="cite"> <br>
class LibvirtModel(object):<br>
model_type = 'real'<br>
<br>
def __init__(self):<br>
<br>
modules = listPathModules('./')<br>
for module in modules:<br>
# probe libvirt vm modules and classes, see
sheldon's patch for module probe implementation<br>
module = imp.load_module()<br>
cls_names = self._get_classes(module)<br>
<br>
# assgin these class methods to Model class, see
zhengsheng's patch for synthesize:<br>
....<br>
m =getattr(model_instance,member_name,None)<br>
setattr(self,'%s_%s'%(method_prefix,member_name),m)<br>
<br>
<br>
def _get_classes(module):<br>
klasses = []<br>
for name, obj in inspect.getmembers(module):<br>
if inspect.isclass(obj):<br>
klasses.append(name)<br>
<br>
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<br>
and things depend less on each other, not swallow it all at
once.<br>
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.<br>
He will follow up with this thread.|<br>
</blockquote>
</blockquote>
<blockquote cite="mid:52DEAB29.7080106@linux.vnet.ibm.com"
type="cite">
<blockquote cite="mid:52DE335E.9050200@linux.vnet.ibm.com"
type="cite"> <br>
<span style="white-space: pre;">> <br>
> <br>
> The second way is to introduce an abstract model factory.
We can have<br>
> a "real" model factory instance and a "mock" one to
produce only<br>
> "mock" model instance. We let the concrete controller
request the<br>
> model factory instance to construct a concrete model
instance. Then<br>
> we can easily switch between the production mode and test
mode by<br>
> feeding the root controller with a real model factory
instance or a<br>
> mock one. This is feasible because the factory just cares
the<br>
> creation of a concrete model, the inner model
responsibilities are<br>
> still wrapped within the concrete model.<br>
> <br>
> I'm proposing the first way because the change will be
minimum and <br>
> non-intrusive. I'm trying to send the patches later...<br>
> </span><br>
<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>