Nir Soffer has posted comments on this change. Change subject: when boostraping lvm refresh/activate also LVs with special tags ......................................................................
Patch Set 5: Code-Review+1 (3 comments) Very nice, the commit message and bootstrap() doctring need some more love. http://gerrit.ovirt.org/#/c/27880/5//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-05-19 17:16:27 +0200 Line 4: Commit: Jiri Moskovcak <[email protected]> Line 5: CommitDate: 2014-05-23 13:43:38 +0200 Line 6: Line 7: when boostraping lvm refresh/activate also LVs with special tags I think the title should be something like: lvm: Support persistent lvs And description should explain additionally: - That vdsm does not activate hosted engine lvs when connecting to storage - How this is solved with the addition of the persistent tag - Why using tags is better then hard coding hosted engine lv names in vdsm Line 8: Line 9: This should solve the problem when the storage Line 10: is on block device and vdsm disables the LVs Line 11: created by hosted-engine and thus breaking it. http://gerrit.ovirt.org/#/c/27880/5/vdsm/storage/lvm.py File vdsm/storage/lvm.py: Line 128: VDSM_LVM_SYSTEM_DIR = os.path.join(VAR_RUN_VDSM, "lvm") Line 129: VDSM_LVM_CONF = os.path.join(VDSM_LVM_SYSTEM_DIR, "lvm.conf") Line 130: Line 131: USER_DEV_LIST = filter(None, config.get("irs", "lvm_dev_whitelist").split(",")) Line 132: VDSM_PERSISTENT = "VDSM_PERSISTENT" Consider adding back the _LV_TAG or _TAG suffix here, and remove the VDSM prefix: PERSISTNT_LV_TAG = "VDSM_PERSISTENT" Line 133: Line 134: Line 135: def getPersistenLVs(vgName): Line 136: return [lv.name for lv in _lvminfo.getLv(vgName) Line 640: Bootstrap lvm module Line 641: Line 642: This function builds the lvm cache and ensure that all unused lvs are Line 643: deactivated, expect lvs matching refreshlvs and LVs tagged with Line 644: VDSM_PERSISTENT, which are refreshed instead. The description is still not correct - see my comment in the previous patch. Line 645: """ Line 646: _lvminfo.bootstrap() Line 647: Line 648: refreshlvs = set(refreshlvs) -- To view, visit http://gerrit.ovirt.org/27880 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I850bf500e8eabfe414a6d6920155ac0697fe5604 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Jiří Moskovčák <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Jiří Moskovčák <[email protected]> Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Sandro Bonazzola <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
