Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
Idan Shaby has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/38421/1/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 3003: try: Line 3004: pvInfo = lvm.getPV(pv) Line 3005: except se.InaccessiblePhysDev: Line 3006: self.log.error(PV %s no longer exists, pv) Line 3007: continue What if you catch InaccessiblePhysDev and you take the entire vg out of the I ran a few tests. Here are the results when trying to add an iscsi domain: 1. A test that makes getPV in lvm.py always fail (raise an InaccessiblePhysDev), which is the scenario that can most probably happen today - fails with the error Error while executing action New SAN Storage Domain: Multipath cannot access physical device. There was no db pollution. 2. A test that makes getPV in lvm.py fail only for a specific pv - same error is shown, and no db pollution. 3. A test that removes the vg out of the vg list (what Fede suggested) - the storage domain is created, a new record is added to the storage_domain_static table but the lun (the only one) is not added to the luns table. That means that we saved a partial information in the db (db pollution). If you create another storage domain, you will see the luns that you used earlier as free luns (not greyed out), and if you try to edit the storage domain that we created, you will get a bunch of java.lang.NullPointerException-s. I am abandoning the patch. Line 3008: vgInfo['pvlist'].append(self.__fillPVDict(dev, pvInfo, vgType)) Line 3009: Line 3010: if vgType == multipath.DEV_FCP: Line 3011: vgType = sd.FCP_DOMAIN -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Ala Hino ah...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Candace Sheremeta csher...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Freddy Rolland froll...@redhat.com Gerrit-Reviewer: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Tal Nisan tni...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
Idan Shaby has abandoned this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Abandoned This patch will cause problems in the engine side. For further explanations, please see the discussion in hsm.py. -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Ala Hino ah...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Candace Sheremeta csher...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Freddy Rolland froll...@redhat.com Gerrit-Reviewer: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Tal Nisan tni...@redhat.com Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
automat...@ovirt.org has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: * Update tracker::#1048696::OK -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Ala Hino ah...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Candace Sheremeta csher...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Freddy Rolland froll...@redhat.com Gerrit-Reviewer: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Tal Nisan tni...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
Tal Nisan has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: ping? -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Ala Hino ah...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Candace Sheremeta csher...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Freddy Rolland froll...@redhat.com Gerrit-Reviewer: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Tal Nisan tni...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
Federico Simoncelli has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/38421/1/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 3003: try: Line 3004: pvInfo = lvm.getPV(pv) Line 3005: except se.InaccessiblePhysDev: Line 3006: self.log.error(PV %s no longer exists, pv) Line 3007: continue After talking about this with Nir, we realized two things: What if you catch InaccessiblePhysDev and you take the entire vg out of the list? (so you don't report that specific vg at all because you don't have the visibility of one of its pv) So you don't feed wrong information, you don't have the entire visibility of that VG so you don't report it at all. Line 3008: vgInfo['pvlist'].append(self.__fillPVDict(dev, pvInfo, vgType)) Line 3009: Line 3010: if vgType == multipath.DEV_FCP: Line 3011: vgType = sd.FCP_DOMAIN -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Ala Hino ah...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Candace Sheremeta csher...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Freddy Rolland froll...@redhat.com Gerrit-Reviewer: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Tal Nisan tni...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
Federico Simoncelli has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/38421/1/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 3003: try: Line 3004: pvInfo = lvm.getPV(pv) Line 3005: except se.InaccessiblePhysDev: Line 3006: self.log.error(PV %s no longer exists, pv) Line 3007: continue Previously the entire request was failing with AttributeError, now the requ The problem here is not about swallowing the ugly exception, it's that we don't want to feed partial/incomplete information to engine. So, in a sense failing hard was better than letting believe the engine that the VG was fine with just those luns. Nir, can we easily add a patch on top of this that gives feedback to the engine about the messing pv? Line 3008: vgInfo['pvlist'].append(self.__fillPVDict(dev, pvInfo, vgType)) Line 3009: Line 3010: if vgType == multipath.DEV_FCP: Line 3011: vgType = sd.FCP_DOMAIN -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Ala Hino ah...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Candace Sheremeta csher...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Freddy Rolland froll...@redhat.com Gerrit-Reviewer: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Tal Nisan tni...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
Nir Soffer has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/38421/1/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 3003: try: Line 3004: pvInfo = lvm.getPV(pv) Line 3005: except se.InaccessiblePhysDev: Line 3006: self.log.error(PV %s no longer exists, pv) Line 3007: continue The problem here is not about swallowing the ugly exception, it's that we d I'll check this. Line 3008: vgInfo['pvlist'].append(self.__fillPVDict(dev, pvInfo, vgType)) Line 3009: Line 3010: if vgType == multipath.DEV_FCP: Line 3011: vgType = sd.FCP_DOMAIN -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Ala Hino ah...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Candace Sheremeta csher...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Freddy Rolland froll...@redhat.com Gerrit-Reviewer: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Tal Nisan tni...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
Idan Shaby has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: Verified-1 (1 comment) https://gerrit.ovirt.org/#/c/38421/1/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 3003: try: Line 3004: pvInfo = lvm.getPV(pv) Line 3005: except se.InaccessiblePhysDev: Line 3006: self.log.error(PV %s no longer exists, pv) Line 3007: continue I'll check this. After talking about this with Nir, we realized two things: 1. We can't send the engine a fake PV (which is an implementation of your idea), i.e a PV with a GUID and with no other info, since we don't have any other info for the lost PV. The reason is that we can't prove there is no flow in the engine that relies on the existence of these fields. That is, we can break the engine. 2. We can't send the engine a partial info about the PVs (which is what this patch aims to do), i.e send the engine one or more PVs less than the real number of PVs. The reason is that the engine will store the partial info in the DB, and we don't know which flow will break because of that. Fede, if you don't have any other idea, I think that I will abandon this patch. What do you say? Line 3008: vgInfo['pvlist'].append(self.__fillPVDict(dev, pvInfo, vgType)) Line 3009: Line 3010: if vgType == multipath.DEV_FCP: Line 3011: vgType = sd.FCP_DOMAIN -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Ala Hino ah...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Candace Sheremeta csher...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Freddy Rolland froll...@redhat.com Gerrit-Reviewer: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Tal Nisan tni...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
Nir Soffer has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: Code-Review-1 I agree with Idan that this change or the change suggested by Federico will be too risky and may break older engine versions. For 4.0 we will be able to return info about missing pvs instead of failing the entire request. -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Ala Hino ah...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Candace Sheremeta csher...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Freddy Rolland froll...@redhat.com Gerrit-Reviewer: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Tal Nisan tni...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
Nir Soffer has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/38421/1/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 3003: try: Line 3004: pvInfo = lvm.getPV(pv) Line 3005: except se.InaccessiblePhysDev: Line 3006: self.log.error(PV %s no longer exists, pv) Line 3007: continue Does this mean that now getVGInfo and getVGList may silently return incompl Previously the entire request was failing with AttributeError, now the request will return the available pvs. It would be nice if we can return info about missing pvs, but it is not the scope of this patch. lvm.getPV() could return a missing pv in this case, or we can add MissingPV namedduple for feeding __fillPVDict() Line 3008: vgInfo['pvlist'].append(self.__fillPVDict(dev, pvInfo, vgType)) Line 3009: Line 3010: if vgType == multipath.DEV_FCP: Line 3011: vgType = sd.FCP_DOMAIN -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Ala Hino ah...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Candace Sheremeta csher...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Freddy Rolland froll...@redhat.com Gerrit-Reviewer: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Tal Nisan tni...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
Federico Simoncelli has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/38421/1/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 3003: try: Line 3004: pvInfo = lvm.getPV(pv) Line 3005: except se.InaccessiblePhysDev: Line 3006: self.log.error(PV %s no longer exists, pv) Line 3007: continue Does this mean that now getVGInfo and getVGList may silently return incomplete information? (e.g. a missing pv). For example when a single lun (of a VG) is unreachable we don't have information about it in the cache but still that PV is part of the VG. Previously we were returning None (which is wrong from the xml-rpc point of view) but at least there was a feedback on the unreachable PV. Line 3008: vgInfo['pvlist'].append(self.__fillPVDict(dev, pvInfo, vgType)) Line 3009: Line 3010: if vgType == multipath.DEV_FCP: Line 3011: vgType = sd.FCP_DOMAIN -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Ala Hino ah...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Candace Sheremeta csher...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Freddy Rolland froll...@redhat.com Gerrit-Reviewer: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Tal Nisan tni...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
Idan Shaby has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/38421/1/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 3003: try: Line 3004: pvInfo = lvm.getPV(pv) Line 3005: except se.InaccessiblePhysDev: Line 3006: self.log.error(PV %s no longer exists, pv) Line 3007: continue Does this mean that now getVGInfo and getVGList may silently return incompl Previously we got None in pvInfo and when we tried to access its attributes we got an AttributeError. Now all we do is skip this pv and log the error. I think it's a better feedback than an exception, isn't it? Line 3008: vgInfo['pvlist'].append(self.__fillPVDict(dev, pvInfo, vgType)) Line 3009: Line 3010: if vgType == multipath.DEV_FCP: Line 3011: vgType = sd.FCP_DOMAIN -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Ala Hino ah...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Candace Sheremeta csher...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Freddy Rolland froll...@redhat.com Gerrit-Reviewer: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Tal Nisan tni...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
Tal Nisan has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: Guys, any reason this patch doesn't have +2? Quite simple and straight forward... Dan, can you upgrade to +2? -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Ala Hino ah...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Candace Sheremeta csher...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Freddy Rolland froll...@redhat.com Gerrit-Reviewer: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Tal Nisan tni...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
automat...@ovirt.org has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: * Update tracker::#1048696::OK * Check Bug-Url::OK * Check Public Bug::#1048696::OK, public bug * Check Product::#1048696::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
Dan Kenigsberg has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Ala Hino ah...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Candace Sheremeta csher...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Freddy Rolland froll...@redhat.com Gerrit-Reviewer: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Tal Nisan tni...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
Idan Shaby has uploaded a new change for review. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. lvm: Update hsm.__processVGInfos() with lvm.getPV() new API In change I9932b044c (lvm: Raise an exception if missing physical volume), we changed lvm.getPV() to raise an InaccessiblePhysDev exception when the device is no longer in lvm's cache. This patch updates hsm.__processVGInfos() to log a warning in such a case since if we called lvm.getPV() with a device that is no longer in lvm's cache, we still want to get the information for the other PVs. That is, we don't want to stop hsm.__processVGInfos() operation. Note that blovkSD.getMetaDataMapping() also calls lvm.getPV(), but in contrast to hsm.__processVGInfos(), we don't want blovkSD.getMetaDataMapping() to handle InaccessiblePhysDev because it's a part of a scenario that creates a storage domain, and we want to fail the whole operation in case that the PV is no longer in lvm's cache. Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Bug-Url: https://bugzilla.redhat.com/1048696 Signed-off-by: Idan Shaby ish...@redhat.com --- M vdsm/storage/hsm.py 1 file changed, 5 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/21/38421/1 diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index b92349c..bbc0f84 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -3000,7 +3000,11 @@ vgType != dev[devtype]): vgType = multipath.DEV_MIXED -pvInfo = lvm.getPV(pv) +try: +pvInfo = lvm.getPV(pv) +except se.InaccessiblePhysDev: +self.log.error(PV %s no longer exists, pv) +continue vgInfo['pvlist'].append(self.__fillPVDict(dev, pvInfo, vgType)) if vgType == multipath.DEV_FCP: -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
Idan Shaby has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: Verified+1 -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: Idan Shaby ish...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
oVirt Jenkins CI Server has posted comments on this change. Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API .. Patch Set 1: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/16300/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/15500/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/16470/ : SUCCESS -- To view, visit https://gerrit.ovirt.org/38421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby ish...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
