Sergey Gotliv has posted comments on this change. Change subject: deactivateSD - perform nothing if the domain is already deactivated ......................................................................
Patch Set 1: Code-Review-1 (2 comments) .................................................... Commit Message Line 7: deactivateSD - perform nothing if the domain is already deactivated Line 8: Line 9: When calling deactivateSD with an already deactivated domain, there's no need Line 10: to perform all the given operations (including mount) - if it's already Line 11: deactivated, we can silently ignore like done in activateSD. Maybe it can ignore. Please, don't make it silently! Line 12: Line 13: Change-Id: I927557637cc6588dae832427018f6ac304f0a562 .................................................... File vdsm/storage/sp.py Line 1143: Line 1144: if sdUUID not in domList: Line 1145: raise se.StorageDomainNotInPool(self.spUUID, sdUUID) Line 1146: Line 1147: if domList[sdUUID] == sd.DOM_ATTACHED_STATUS: 1. I agree with Nir about the log. 2. I afraid from that "if". Because activateSD is already wrong! it implicitly assumes that if domain is active the host has a link to that domain what is not true (I can reffer to the bug). if domainStatuses[sdUUID] == sd.DOM_ACTIVE_STATUS: return True self._refreshDomainLinks(dom) Now you are adding the symmetric if, right? Please, check that the code after that "if" (line 1190) is not needed in case of "statis == attached"? Line 1148: return True Line 1149: Line 1150: try: Line 1151: dom = sdCache.produce(sdUUID) -- To view, visit http://gerrit.ovirt.org/22343 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I927557637cc6588dae832427018f6ac304f0a562 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Eduardo <ewars...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: Yeela Kaplan <ykap...@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
