Change in vdsm[master]: vm: janitorial: simplify the recovering flow

nsoffer at redhat.com nsoffer at redhat.com
Thu Jan 30 17:47:46 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: vm: janitorial: simplify the recovering flow
......................................................................


Patch Set 2:

(1 comment)

Using if self.recovering is little better then if not self.recovering, but the second change does not improve anything if not little worse.

The real problem in this method is trying to do 3 things: run new vm, revover running vm, and recoverState(?). The real fix would be to split the big method into 2 or 3 smaller methods that do one thing.

This may require extracting some helper methods to avoid code duplication and make the code more clear.

It is not easy as the code have no tests and is messy.

http://gerrit.ovirt.org/#/c/23915/2/vdsm/vm.py
File vdsm/vm.py:

Line 3127
Line 3128
Line 3129
Line 3130
Line 3131
This was better here without duplicating the code.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2c1bc48b9f4df7cb236f27c44755d9778e7e6c9
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list