Change in vdsm[master]: clientIF: Fix use of getConfDevices

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 
---
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)

[Jenkins CI RO]

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

[Dan Kenigsberg]

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

[Jenkins CI RO]

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

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, …

[Federico Simoncelli]

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:

Martin Polednik has posted comments on this change.

Change subject: clientIF: Fix use of getConfDevices
......................................................................

Patch Set 2: Code-Review+1

[Dan Kenigsberg]

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.

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

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: clientIF: Fix use of getConfDevices
......................................................................

Patch Set 2: Code-Review+2

[Dan Kenigsberg]

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 
Reviewed-on: http://gerrit.ovirt.org/21832
Reviewed-by: Michal Skrivanek 
Reviewed-by: Martin Polednik 
Tested-by: Martin Polednik 
Reviewed-by: Federico Simoncelli 
---
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