Federico Simoncelli has posted comments on this change.
Change subject: fcp: Deactivate vdsm logical volumes ......................................................................
Patch Set 17:
(4 comments)
Nice. Few minor questions.
.................................................... File lib/vdsm/tool/lvm_deactivate_lvs.py Line 36: """ Line 37: if _was_run(): Line 38: _log("already run") Line 39: return 0 Line 40: _set_was_run() What was the conclusion about this? It shouldn't be moved after deactivation? Line 41: for vg in _iter_vdsm_vgs(): Line 42: _deactivate_lvs(vg) Line 43: _log("deactivated lvs") Line 44: return 0
Line 56: def _log(msg): Line 57: sys.stderr.write("%s\n" % (msg,)) Line 58: Line 59: Line 60: def _iter_vdsm_vgs(): I have the feeling that you can get rid of this entirely, check comment at line ~74. Line 61: rc, out, err = _run_lvm(["vgs", "--noheading", "--separator", Line 62: _SEPARATOR, "-o", "name,tags"]) Line 63: if rc != 0: Line 64: raise RuntimeError("Error finding vgs: %s" % err)
Line 70: Line 71: Line 72: def _iter_active_lvs(vg): Line 73: rc, out, err = _run_lvm(["lvs", "--noheading", "--separator", Line 74: _SEPARATOR, "-o", "name,attr", vg]) I think that using:
lvs @RHAT_storage_domain
(plus the other required options) would do. I'm not sure if you can specify somehow also the MDT_TYPE=FCP. But we probably don't care so much about that one. Line 75: if rc != 0: Line 76: raise RuntimeError("Error finding lvs: %s" % err) Line 77: for line in out.splitlines(): Line 78: line = line.strip()
Line 103: def _deactivate_lvs(vg): Line 104: cmd = ["vgchange", "--available", "n", vg] Line 105: rc, out, err = _run_lvm(cmd) Line 106: if rc != 0: Line 107: # Probbaly there are open lvs. lvm does not gives us any way to detect Typo Line 108: # error type. Line 109: cmd = ["lvchange", "--available", "n"] Line 110: cmd.extend("%s/%s" % (vg, lv) for lv in _iter_active_lvs(vg)) Line 111: rc, out, err = _run_lvm(cmd)