12:31 p.m.
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 <mpoledni(a)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 <mpoledni(a)redhat.com>
6:23 p.m.
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 <mpoledni(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/19331
Reviewed-by: Dan Kenigsberg <danken(a)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 <mpoledni(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpoledni(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
6:19 p.m.
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 <mpoledni(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpoledni(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
3:16 a.m.
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 <mpoledni(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpoledni(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
7:43 a.m.
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 <mpoledni(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpoledni(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
7:27 a.m.
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 <mpoledni(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpoledni(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:59 a.m.
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 <mpoledni(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpoledni(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
7:53 a.m.
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 <mpoledni(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpoledni(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
8:39 a.m.
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 <mpoledni(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpoledni(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
8:43 a.m.
Michal Skrivanek has posted comments on this change.
Change subject: vdsm: move watchdog default params to WatchdogDevice
......................................................................
Patch Set 1: Code-Review+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 <mpoledni(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpoledni(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
12:35 p.m.
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 <mpoledni(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:51 p.m.
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 <mpoledni(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpoledni(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
3619
days inactive
3667
days old
vdsm-patches@lists.fedorahosted.org
11 comments
6 participants
participants (6)
-
abaron＠redhat.com -
Dan Kenigsberg -
Federico Simoncelli -
Jenkins CI RO -
michal.skrivanek＠redhat.com -
mpoledni＠redhat.com