<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<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 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>
<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>
<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>
<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>
</body>
</html>