4:14 a.m.
From: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
When using get_list function of vms model,
Background Task will run multiple times because of started
in vm init function.
Fix this by making vms model singleton.
Royce Lv (2):
Make vm model to be a singleton
Hack test_model to support vm model singleton
src/kimchi/model/vms.py | 3 +++
tests/test_model.py | 2 ++
2 files changed, 5 insertions(+)
--
1.8.1.2
4:14 a.m.
New subject: [PATCH 1/2] Make vm model to be a singleton
From: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
When refer vm model get_list method, the stats collection task
will run multiple times. Make vms model class a singleton so that
this task just run once.
Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
---
src/kimchi/model/vms.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
index 3ae2048..449acdb 100644
--- a/src/kimchi/model/vms.py
+++ b/src/kimchi/model/vms.py
@@ -27,6 +27,7 @@ from cherrypy.process.plugins import BackgroundTask
from kimchi import vnc
from kimchi import xmlutils
+from kimchi.basemodel import Singleton
from kimchi.exception import InvalidOperation, InvalidParameter
from kimchi.exception import MissingParameter, NotFoundError, OperationFailed
from kimchi.model.config import CapabilitiesModel
@@ -52,6 +53,8 @@ stats = {}
class VMsModel(object):
+ __metaclass__ = Singleton
+
def __init__(self, **kargs):
self.conn = kargs['conn']
self.objstore = kargs['objstore']
--
1.8.1.2
6:23 a.m.
New subject: [PATCH 1/2] Make vm model to be a singleton
Reviewed-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
do you means this BackgroundTask?
self.guests_stats_thread = BackgroundTask(GUESTS_STATS_INTERVAL,
self._update_guests_stats)
we should avoid multiple times to run this BackgroundTask.
when refer get_list method?
I'm afraid the there are server instances of VMsModel, but need to check
where are
these instances from the code.
On 03/04/2014 06:14 PM, lvroyce(a)linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
When refer vm model get_list method, the stats collection task
will run multiple times. Make vms model class a singleton so that
this task just run once.
Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
---
src/kimchi/model/vms.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
index 3ae2048..449acdb 100644
--- a/src/kimchi/model/vms.py
+++ b/src/kimchi/model/vms.py
@@ -27,6 +27,7 @@ from cherrypy.process.plugins import BackgroundTask
from kimchi import vnc
from kimchi import xmlutils
+from kimchi.basemodel import Singleton
from kimchi.exception import InvalidOperation, InvalidParameter
from kimchi.exception import MissingParameter, NotFoundError, OperationFailed
from kimchi.model.config import CapabilitiesModel
@@ -52,6 +53,8 @@ stats = {}
class VMsModel(object):
+ __metaclass__ = Singleton
+
def __init__(self, **kargs):
self.conn = kargs['conn']
self.objstore = kargs['objstore']
--
Thanks and best regards!
Sheldon Feng(冯少合)<shaohef(a)linux.vnet.ibm.com>
IBM Linux Technology Center
10:28 a.m.
New subject: [PATCH 1/2] Make vm model to be a singleton
2014/3/4 20:23, Sheldon:
Reviewed-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
do you means this BackgroundTask?
self.guests_stats_thread = BackgroundTask(GUESTS_STATS_INTERVAL,
self._update_guests_stats)
we should avoid multiple times to run this BackgroundTask.
when refer get_list method?
I get the same question here. Does that mean class
VMsModel will be
instantiated multiples times and VMsModel.__init__() will be called
multiple times?
I'm afraid the there are server instances of VMsModel, but need
to
check where are
these instances from the code.
Agree. Not sure where are the multiple instances of class VMsModel
coming from.
On 03/04/2014 06:14 PM, lvroyce(a)linux.vnet.ibm.com wrote:
> From: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
>
> When refer vm model get_list method, the stats collection task
> will run multiple times. Make vms model class a singleton so that
> this task just run once.
>
> Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
> ---
> src/kimchi/model/vms.py | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
> index 3ae2048..449acdb 100644
> --- a/src/kimchi/model/vms.py
> +++ b/src/kimchi/model/vms.py
> @@ -27,6 +27,7 @@ from cherrypy.process.plugins import BackgroundTask
>
> from kimchi import vnc
> from kimchi import xmlutils
> +from kimchi.basemodel import Singleton
> from kimchi.exception import InvalidOperation, InvalidParameter
> from kimchi.exception import MissingParameter, NotFoundError,
> OperationFailed
> from kimchi.model.config import CapabilitiesModel
> @@ -52,6 +53,8 @@ stats = {}
>
>
> class VMsModel(object):
> + __metaclass__ = Singleton
> +
> def __init__(self, **kargs):
> self.conn = kargs['conn']
> self.objstore = kargs['objstore']
8:04 p.m.
New subject: [PATCH 1/2] Make vm model to be a singleton
On 2014年03月04日 20:23, Sheldon wrote:
Reviewed-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
do you means this BackgroundTask?
self.guests_stats_thread = BackgroundTask(GUESTS_STATS_INTERVAL,
self._update_guests_stats)
we should avoid multiple times to run this BackgroundTask.
when refer get_list method?
I'm afraid the there are server instances of VMsModel, but need to
check where are
these instances from the code.
Thanks for your review, Sheldon, I will modify my
commit msg on next
version.
On 03/04/2014 06:14 PM, lvroyce(a)linux.vnet.ibm.com wrote:
> From: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
>
> When refer vm model get_list method, the stats collection task
> will run multiple times. Make vms model class a singleton so that
> this task just run once.
>
> Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
> ---
> src/kimchi/model/vms.py | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
> index 3ae2048..449acdb 100644
> --- a/src/kimchi/model/vms.py
> +++ b/src/kimchi/model/vms.py
> @@ -27,6 +27,7 @@ from cherrypy.process.plugins import BackgroundTask
>
> from kimchi import vnc
> from kimchi import xmlutils
> +from kimchi.basemodel import Singleton
> from kimchi.exception import InvalidOperation, InvalidParameter
> from kimchi.exception import MissingParameter, NotFoundError,
> OperationFailed
> from kimchi.model.config import CapabilitiesModel
> @@ -52,6 +53,8 @@ stats = {}
>
>
> class VMsModel(object):
> + __metaclass__ = Singleton
> +
> def __init__(self, **kargs):
> self.conn = kargs['conn']
> self.objstore = kargs['objstore']
4:14 a.m.
New subject: [PATCH 2/2] Hack test_model to support vm model singleton
From: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Because test_model have singleton import for its whole life cycle,
the dict of singleton class not cleared even though instance deleted.
Hack it so that model instance can be generated again.
Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
---
tests/test_model.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/test_model.py b/tests/test_model.py
index a08b3d9..f5989a9 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -33,6 +33,7 @@ import iso_gen
import kimchi.objectstore
import utils
from kimchi import netinfo
+from kimchi.basemodel import Singleton
from kimchi.exception import InvalidOperation, InvalidParameter
from kimchi.exception import NotFoundError, OperationFailed
from kimchi.iscsi import TargetClient
@@ -46,6 +47,7 @@ class ModelTests(unittest.TestCase):
self.tmp_store = '/tmp/kimchi-store-test'
def tearDown(self):
+ Singleton._instances = {}
os.unlink(self.tmp_store)
def test_vm_info(self):
--
1.8.1.2
12:48 p.m.
On 03/04/2014 07:14 AM, lvroyce(a)linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
When using get_list function of vms model,
I could not find an occurrence when we use VMsModel.get_list() outside
its class.
Background Task will run multiple times because of started
in vm init function.
Fix this by making vms model singleton.
Royce Lv (2):
Make vm model to be a singleton
Hack test_model to support vm model singleton
src/kimchi/model/vms.py | 3 +++
tests/test_model.py | 2 ++
2 files changed, 5 insertions(+)
7:56 p.m.
On 2014年03月05日 02:48, Aline Manera wrote:
On 03/04/2014 07:14 AM, lvroyce(a)linux.vnet.ibm.com wrote:
> From: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
>
> When using get_list function of vms model,
I could not find an occurrence when we use VMsModel.get_list() outside
its class.
Aline,
I used this when counting volume reference--query all vm disks and see
if there is path match between volume and vmstorage. And for
functionality like restart host,
host.py:
def _get_vms_list_by_state(self, state):
conn = self.conn.get()
names = [dom.name().decode('utf-8') for dom in conn.listAllDomains(0)]
ret_list = []
for name in names:
dom = conn.lookupByName(name.encode("utf-8"))
info = dom.info()
if (DOM_STATE_MAP[info[0]]) == state:
ret_list.append(name)
return ret_list
This is duplicate code for vms.get_list().
I mean, in future extension we may also introduce vms.get_list() and we
don't want multiple background task running right? So based on this
making vms model singleton make sense to me.
And also, for me there is no need to have multiple instance of every
model because in run time we just need one.
What do you think?
> Background Task will run multiple times because of started
> in vm init function.
> Fix this by making vms model singleton.
>
> Royce Lv (2):
> Make vm model to be a singleton
> Hack test_model to support vm model singleton
>
> src/kimchi/model/vms.py | 3 +++
> tests/test_model.py | 2 ++
> 2 files changed, 5 insertions(+)
>
12:08 p.m.
On 03/04/2014 10:56 PM, Royce Lv wrote:
On 2014年03月05日 02:48, Aline Manera wrote:
> On 03/04/2014 07:14 AM, lvroyce(a)linux.vnet.ibm.com wrote:
>> From: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
>>
>> When using get_list function of vms model,
>
> I could not find an occurrence when we use VMsModel.get_list()
> outside its class.
Aline,
I used this when counting volume reference--query all vm disks and see
if there is path match between volume and vmstorage. And for
functionality like restart host,
host.py:
def _get_vms_list_by_state(self, state):
conn = self.conn.get()
names = [dom.name().decode('utf-8') for dom in conn.listAllDomains(0)]
ret_list = []
for name in names:
dom = conn.lookupByName(name.encode("utf-8"))
info = dom.info()
if (DOM_STATE_MAP[info[0]]) == state:
ret_list.append(name)
return ret_list
This is duplicate code for vms.get_list().
I mean, in future extension we may also introduce vms.get_list() and
we don't want multiple background task running right? So based on this
making vms model singleton make sense to me.
And also, for me there is no need to have multiple instance of every
model because in run time we just need one.
What do you think?
I am OK with it.
I was just trying to understand if it is a bug fix patch or a preventive
one.
As you commented you will need it on future patches.
>
>> Background Task will run multiple times because of started
>> in vm init function.
>> Fix this by making vms model singleton.
>>
>> Royce Lv (2):
>> Make vm model to be a singleton
>> Hack test_model to support vm model singleton
>>
>> src/kimchi/model/vms.py | 3 +++
>> tests/test_model.py | 2 ++
>> 2 files changed, 5 insertions(+)
>>
>
3945
days inactive
3946
days old
8 comments
5 participants
participants (5)
-
Aline Manera -
lvroyce＠linux.vnet.ibm.com -
Royce Lv
-
Sheldon -
Shu Ming