Change in vdsm[master]: vdsm: move watchdog default params to WatchdogDevice

2013-11-03 Thread danken 
Dan Kenigsberg has posted comments on this change.

Change subject: vdsm: move watchdog default params to WatchdogDevice
..


Patch Set 2: Code-Review+2

(1 comment)


File vdsm/vm.py
Line 1631: 
Line 1632: 
Line 1633: class WatchdogDevice(VmDevice):
Line 1634: def __init__(self, *args, **kwargs):
Line 1635: super(WatchdogDevice, self).__init__(*args, **kwargs)
Ayal is right. VmDevice's conversion of a dict into object attribute is a piece 
of bad design. I wish we never had it that way.

However, this patch is not about big refactoring of VmDevice, but about 
splitting a huge buildConfDevices functioninto smaller pieces. In this task, I 
think this patch succeeds.
Line 1636: 
Line 1637: if not hasattr(self, 'specParams'):
Line 1638: self.specParams = {}
Line 1639: 


-- 
To view, visit http://gerrit.ovirt.org/19331
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Peter V. Saveliev p...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vdsm: move watchdog default params to WatchdogDevice

2013-11-03 Thread danken 
Dan Kenigsberg has submitted this change and it was merged.

Change subject: vdsm: move watchdog default params to WatchdogDevice
..


vdsm: move watchdog default params to WatchdogDevice

Watchdog device creation is currently handled in
buildConfDevices, creating unnecessary code pollution.
This patch moves the creation to WatchdogDevice class,
cleaning up buildConfDevices code. Side effect is that
we lose access to specParams if they are not passed from
engine.

Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Signed-off-by: Martin Polednik mpole...@redhat.com
Reviewed-on: http://gerrit.ovirt.org/19331
Reviewed-by: Dan Kenigsberg dan...@redhat.com
---
M vdsm/vm.py
1 file changed, 8 insertions(+), 10 deletions(-)

Approvals:
  Dan Kenigsberg: Looks good to me, approved
  Martin Polednik: Verified



-- 
To view, visit http://gerrit.ovirt.org/19331
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Peter V. Saveliev p...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vdsm: move watchdog default params to WatchdogDevice

2013-10-15 Thread mpoledni 
Martin Polednik has posted comments on this change.

Change subject: vdsm: move watchdog default params to WatchdogDevice
..


Patch Set 2:

(1 comment)


File vdsm/vm.py
Line 1631: 
Line 1632: 
Line 1633: class WatchdogDevice(VmDevice):
Line 1634: def __init__(self, *args, **kwargs):
Line 1635: super(WatchdogDevice, self).__init__(*args, **kwargs)
VmDevice is giving us means of generating the XML
Line 1636: 
Line 1637: if not hasattr(self, 'specParams'):
Line 1638: self.specParams = {}
Line 1639: 


-- 
To view, visit http://gerrit.ovirt.org/19331
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Peter V. Saveliev p...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vdsm: move watchdog default params to WatchdogDevice

2013-10-14 Thread abaron 
Ayal Baron has posted comments on this change.

Change subject: vdsm: move watchdog default params to WatchdogDevice
..


Patch Set 2:

(1 comment)


File vdsm/vm.py
Line 1631: 
Line 1632: 
Line 1633: class WatchdogDevice(VmDevice):
Line 1634: def __init__(self, *args, **kwargs):
Line 1635: super(WatchdogDevice, self).__init__(*args, **kwargs)
It would be a lot clearer if the properties of each device were explicit.
I don't see the point of inheriting from VmDevice as it isn't really giving us 
anything.
You can instead specify here the two params that a WatchDogDevice should have 
and set the default values if not passed.
Line 1636: 
Line 1637: if not hasattr(self, 'specParams'):
Line 1638: self.specParams = {}
Line 1639: 


-- 
To view, visit http://gerrit.ovirt.org/19331
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Peter V. Saveliev p...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vdsm: move watchdog default params to WatchdogDevice

2013-10-09 Thread oVirt Jenkins CI Server 
oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdsm: move watchdog default params to WatchdogDevice
..


Patch Set 2:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4783/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4859/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3974/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/19331
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Peter V. Saveliev p...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vdsm: move watchdog default params to WatchdogDevice

2013-10-09 Thread mpoledni 
Martin Polednik has posted comments on this change.

Change subject: vdsm: move watchdog default params to WatchdogDevice
..


Patch Set 2: Verified+1

-- 
To view, visit http://gerrit.ovirt.org/19331
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Peter V. Saveliev p...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vdsm: move watchdog default params to WatchdogDevice

2013-10-08 Thread fsimonce 
Federico Simoncelli has posted comments on this change.

Change subject: vdsm: move watchdog default params to WatchdogDevice
..


Patch Set 1:

(1 comment)


File vdsm/vm.py
Line 1616: /watchdog
Line 1617: 
Line 1618: m = self.createXmlElem(self.type, None, ['address'])
Line 1619: m.setAttrs(model=self.specParams.get('model', 'i6300esb'),
Line 1620:action=self.specParams.get('action', 'none'))
Fine by me but we have to keep in mind that now we won't have any info about 
the model and action in the VDSM recovery file. (Please double check if we had 
it before, it might be that we weren't saving the recovery file anyway).
Line 1621: return m
Line 1622: 
Line 1623: 
Line 1624: class SmartCardDevice(VmDevice):


-- 
To view, visit http://gerrit.ovirt.org/19331
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Peter V. Saveliev p...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vdsm: move watchdog default params to WatchdogDevice

2013-09-30 Thread mpoledni 
Martin Polednik has posted comments on this change.

Change subject: vdsm: move watchdog default params to WatchdogDevice
..


Patch Set 1: Verified-1

needs a bigger change, currently breaks things

-- 
To view, visit http://gerrit.ovirt.org/19331
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Peter V. Saveliev p...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vdsm: move watchdog default params to WatchdogDevice

2013-09-19 Thread mpoledni 
Martin Polednik has posted comments on this change.

Change subject: vdsm: move watchdog default params to WatchdogDevice
..


Patch Set 1: Verified+1

-- 
To view, visit http://gerrit.ovirt.org/19331
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vdsm: move watchdog default params to WatchdogDevice

2013-09-17 Thread oVirt Jenkins CI Server 
oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdsm: move watchdog default params to WatchdogDevice
..


Patch Set 1:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4496/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4415/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3599/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/19331
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vdsm: move watchdog default params to WatchdogDevice

2013-09-17 Thread mpoledni 
Martin Polednik has uploaded a new change for review.

Change subject: vdsm: move watchdog default params to WatchdogDevice
..

vdsm: move watchdog default params to WatchdogDevice

Watchdog device creation is currently for no reason in buildConfDevices
while it belongs to WatchdogDevice getXML method

Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Signed-off-by: Martin Polednik mpole...@redhat.com
---
M vdsm/vm.py
1 file changed, 2 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/31/19331/1

diff --git a/vdsm/vm.py b/vdsm/vm.py
index 92d274e..ba268f6 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -1616,8 +1616,8 @@
 /watchdog
 
 m = self.createXmlElem(self.type, None, ['address'])
-m.setAttrs(model=self.specParams['model'],
-   action=self.specParams['action'])
+m.setAttrs(model=self.specParams.get('model', 'i6300esb'),
+   action=self.specParams.get('action', 'none'))
 return m
 
 
@@ -1908,14 +1908,6 @@
 # libvirt only support one watchdog device
 if len(devices[WATCHDOG_DEVICES])  1:
 raise ValueError(only a single watchdog device is supported)
-if len(devices[WATCHDOG_DEVICES]) == 1:
-if not 'specParams' in devices[WATCHDOG_DEVICES][0]:
-devices[WATCHDOG_DEVICES][0]['specParams'] = {}
-if not 'model' in devices[WATCHDOG_DEVICES][0]['specParams']:
-devices[WATCHDOG_DEVICES][0]['specParams']['model'] = \
-'i6300esb'
-if not 'action' in devices[WATCHDOG_DEVICES][0]['specParams']:
-devices[WATCHDOG_DEVICES][0]['specParams']['action'] = 'none'
 
 if len(devices[CONSOLE_DEVICES])  1:
 raise ValueError(Only a single console device is supported)


-- 
To view, visit http://gerrit.ovirt.org/19331
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik mpole...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches