Nir Soffer has posted comments on this change. Change subject: blockSD: Avoid stale lvs ......................................................................
Patch Set 3: (3 comments) https://gerrit.ovirt.org/#/c/56876/3/vdsm/storage/lvm.py File vdsm/storage/lvm.py: Line 653: for vg in _lvminfo.getAllVgs(): Line 654: deactivateUnusedLVs(vg.name, refreshlvs=refreshlvs) Line 655: Line 656: Line 657: def deactivateUnusedLVs(vgname, refreshlvs=()): > refreshlvs is a confusing parameter name. To me it suggests that these lvs This is the same name used in bootrap(), I'm reusing both the code and the interface. I agree that the name does not explain that these lvs are not deactivated, but excludelvs or keeplvs does not explain that the lvs will be refreshed, so we better keep the current interface for now. If we find a better name, we should change both functions. I will document this function (same as bootstrap) to make this clear: Ensure that all unused lvs are deactivated, except lvs matching refreshlvs, which are refreshed instead. Line 658: deactivate = [] Line 659: refresh = [] Line 660: Line 661: for lv in _lvminfo.getLv(vgname): Line 672: try: Line 673: _setLVAvailability(vgname, deactivate, "n") Line 674: except se.CannotDeactivateLogicalVolume: Line 675: log.error("Error deactivating lvs: vg=%s lvs=%s", vgname, Line 676: deactivate) > Since stale lvs could lead to data corruption, should this cause a failure stale lvs can lead to data corruption if lv was removed on storage, and we start a vm or do write into it on the host where it was active. Since we do not do that, I don't think we should go that far and disable a domain because of temporary lvm failure. You can use the same argument for bootstrap - when we added it we intentionally logged error and continue if some lvs could not be deactivated or refreshed. Line 677: # Some lvs are inactive now Line 678: _lvminfo._invalidatelvs(vgname, deactivate) Line 679: Line 680: if refresh: https://gerrit.ovirt.org/#/c/56876/3/vdsm/storage/sd.py File vdsm/storage/sd.py: Line 503: # Life cycle Line 504: Line 505: def setup(self): Line 506: """ Line 507: Called when after storage domain is produced in the storage domain > Called after Thanks, will fix. Line 508: monitor. Line 509: """ Line 510: Line 511: def teardown(self): -- To view, visit https://gerrit.ovirt.org/56876 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7227bb43c2e1ee67a6239956aae48173a27f566e Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Ala Hino <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Freddy Rolland <[email protected]> Gerrit-Reviewer: Idan Shaby <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Simone Tiraboschi <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
