Yaniv Bronhaim has posted comments on this change.

Change subject: fcp: Deactivate vdsm logical volumes
......................................................................


Patch Set 16:

(2 comments)

....................................................
File init/vdsmd_init_common.sh.in
Line 180: }
Line 181: 
Line 182: task_lvm_deactivate_lvs(){
Line 183:     "$VDSM_TOOL" lvm-deactivate-lvs
Line 184: }
you can put it in separate sh file and add tests in it that runs during make if 
you prefer. or add this logic to lvm.py with same tests and call it. I agree 
that tests are required here and seems like you add alot of thoughts into 
them...

but as we see, my first advice of putting the code in vdsm-tool was wrong.
Line 185: #### pre-start tasks end ####
Line 186: 
Line 187: 
Line 188: #### post-stop tasks ####


....................................................
File tests/toolLvmDeactivateLvsTests.py
Line 18: #
Line 19: # Refer to the README and COPYING files for full details of the license
Line 20: #
Line 21: 
Line 22: from vdsm.tool import lvm_deactivate_lvs as module
this is confusing .. leave it lvm_deactivate_lvs
Line 23: from vdsm import constants
Line 24: from vdsm import utils
Line 25: 
Line 26: from testrunner import VdsmTestCase as TestCaseBase


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f72a68ad09566ba222aa45448c78d1577c40d21
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@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: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@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

Reply via email to