7:35 a.m.
Yeela Kaplan has uploaded a new change for review.
Change subject: clientIF: Fix use of getConfDevices
......................................................................
clientIF: Fix use of getConfDevices
getConfDevices was merged into buildConfDevices
Change-Id: Iaf9776e1237f6799dfccd5ff3edf197c5bf32f5e
Signed-off-by: Yeela Kaplan <ykaplan(a)redhat.com>
---
M vdsm/clientIF.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/32/21832/1
diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py
index 0b67e45..dd939f8 100644
--- a/vdsm/clientIF.py
+++ b/vdsm/clientIF.py
@@ -455,7 +455,7 @@
# Do not prepare volumes when system goes down
if self._enabled:
vmObj.preparePaths(
- vmObj.getConfDevices()[vm.DISK_DEVICES])
+ vmObj.buildConfDevices()[vm.DISK_DEVICES])
except:
self.log.error("Vm %s recovery failed",
vmId, exc_info=True)
--
To view, visit http://gerrit.ovirt.org/21832
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaf9776e1237f6799dfccd5ff3edf197c5bf32f5e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com>
5:06 a.m.
Martin Polednik has posted comments on this change.
Change subject: clientIF: Fix use of getConfDevices
......................................................................
Patch Set 2: Verified+1
manually verified that the function is passed the same parameters as before using log
messages inside the functions
automated functional test is currently not possible due to expcetions in
recoverExistingVms being caught and logged, making the test pass even when failure occurs
--
To view, visit http://gerrit.ovirt.org/21832
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9776e1237f6799dfccd5ff3edf197c5bf32f5e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(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: oVirt Jenkins CI Server
Gerrit-HasComments: No
3:16 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: clientIF: Fix use of getConfDevices
......................................................................
Patch Set 2:
Martin, please verify this fix by adding a functional test that starts a VM, kills vdsm,
and verifies recovery.
--
To view, visit http://gerrit.ovirt.org/21832
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9776e1237f6799dfccd5ff3edf197c5bf32f5e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(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: oVirt Jenkins CI Server
Gerrit-HasComments: No
11:26 a.m.
Martin Polednik has posted comments on this change.
Change subject: clientIF: Fix use of getConfDevices
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/21832
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9776e1237f6799dfccd5ff3edf197c5bf32f5e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(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: oVirt Jenkins CI Server
Gerrit-HasComments: No
10:47 a.m.
Federico Simoncelli has posted comments on this change.
Change subject: clientIF: Fix use of getConfDevices
......................................................................
Patch Set 2:
(1 comment)
....................................................
File vdsm/clientIF.py
Line 454: try:
Line 455: # Do not prepare volumes when system goes down
Line 456: if self._enabled:
Line 457: vmObj.preparePaths(
Line 458: vmObj.buildConfDevices()[vm.DISK_DEVICES])
(Not related to Yeela's fix) Is there anything better than using two internal (imo)
calls from here? I am sure we're already doing something similar inside the Vm object.
Can we refactor/group and reuse the same method?
Line 459: except:
Line 460: self.log.error("Vm %s recovery failed",
Line 461: vmId, exc_info=True)
Line 462: except:
--
To view, visit http://gerrit.ovirt.org/21832
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9776e1237f6799dfccd5ff3edf197c5bf32f5e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(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: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9:23 a.m.
Michal Skrivanek has posted comments on this change.
Change subject: clientIF: Fix use of getConfDevices
......................................................................
Patch Set 2: Code-Review+1
ugh.
Yes, the tests should cover recovery. Well, The recovery should actually be fixed to
handle migrating VMs in the first place, hm, …
--
To view, visit http://gerrit.ovirt.org/21832
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9776e1237f6799dfccd5ff3edf197c5bf32f5e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(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: oVirt Jenkins CI Server
Gerrit-HasComments: No
9:03 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: clientIF: Fix use of getConfDevices
......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4970/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5770/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5860/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/21832
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9776e1237f6799dfccd5ff3edf197c5bf32f5e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
7:39 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: clientIF: Fix use of getConfDevices
......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4963/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5763/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5853/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/21832
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9776e1237f6799dfccd5ff3edf197c5bf32f5e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
8:09 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: clientIF: Fix use of getConfDevices
......................................................................
Patch Set 1:
(1 comment)
....................................................
Commit Message
Line 5: CommitDate: 2013-11-28 15:30:29 +0200
Line 6:
Line 7: clientIF: Fix use of getConfDevices
Line 8:
Line 9: getConfDevices was merged into buildConfDevices
who's to blame (except the reviewer)? in which commit? When is this visible? it is
important to learn a lesson here.
Line 10:
Line 11: Change-Id: Iaf9776e1237f6799dfccd5ff3edf197c5bf32f5e
--
To view, visit http://gerrit.ovirt.org/21832
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9776e1237f6799dfccd5ff3edf197c5bf32f5e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
7:28 a.m.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: clientIF: Fix use of getConfDevices
......................................................................
clientIF: Fix use of getConfDevices
getConfDevices was merged into buildConfDevices in
commit Ifd3a209967f97a55fe861c4446e8f93e1a1da08e
Change-Id: Iaf9776e1237f6799dfccd5ff3edf197c5bf32f5e
Signed-off-by: Yeela Kaplan <ykaplan(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/21832
Reviewed-by: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Reviewed-by: Martin Polednik <mpoledni(a)redhat.com>
Tested-by: Martin Polednik <mpoledni(a)redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com>
---
M vdsm/clientIF.py
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Federico Simoncelli: Looks good to me, approved
Michal Skrivanek: Looks good to me, but someone else must approve
Martin Polednik: Verified; Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/21832
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iaf9776e1237f6799dfccd5ff3edf197c5bf32f5e
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(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: oVirt Jenkins CI Server
7:10 a.m.
Federico Simoncelli has posted comments on this change.
Change subject: clientIF: Fix use of getConfDevices
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.ovirt.org/21832
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9776e1237f6799dfccd5ff3edf197c5bf32f5e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(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: oVirt Jenkins CI Server
Gerrit-HasComments: No
3806
days inactive
3810
days old
vdsm-patches@lists.fedorahosted.org
10 comments
6 participants
participants (6)
-
Dan Kenigsberg -
Federico Simoncelli -
Jenkins CI RO -
michal.skrivanek＠redhat.com -
mpoledni＠redhat.com -
ykaplan＠redhat.com