Nir Soffer 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: } This is very smart shell script, but I'm not sure that it is equivalent to the python code or correct, and it is not less complex. Since we don't know yet how to clone you, it will be easier to maintain the Python version.
Can you point to the complex or unclear code in the Python version so I can fix them? Line 185: #### pre-start tasks end #### Line 186: Line 187: Line 188: #### post-stop tasks ####
Line 180: } Line 181: Line 182: task_lvm_deactivate_lvs(){ Line 183: "$VDSM_TOOL" lvm-deactivate-lvs Line 184: } The tests check that lvm is called with the correct parameters, and lvm output is handled correctly.
The only thing that we don't have to test there is parameter order; [--noheadings, --separator, "|"] is the same as [--separator, "|", --noheadings]. To support that we need a much smarter mock or stub which knows lvm semantics. I don't think it worth the effort to go in this direction, as parameter order is unlikely to change. Line 185: #### pre-start tasks end #### Line 186: Line 187: Line 188: #### post-stop tasks ####