Vered Volansky has posted comments on this change. Change subject: Improve err msg when multipath cant access a pv ......................................................................
Patch Set 2: (5 comments) Accepted Dan's suggestion, will submit soon. .................................................... Commit Message Line 3: AuthorDate: 2013-11-05 17:33:36 +0200 Line 4: Commit: Vered Volansky <vvola...@redhat.com> Line 5: CommitDate: 2013-11-10 13:40:15 +0200 Line 6: Line 7: Improve err msg when multipath cant access a pv Because there's a limit to how long the error message can be, and I'm a character away from that limit. Line 8: Line 9: InvalidPhysDev gave a cryptic error message - Invalid physical device. Line 10: The message was amended to "Multipath cannot access storage device" along Line 11: with the device name. The error key was changed according to the new Line 3: AuthorDate: 2013-11-05 17:33:36 +0200 Line 4: Commit: Vered Volansky <vvola...@redhat.com> Line 5: CommitDate: 2013-11-10 13:40:15 +0200 Line 6: Line 7: Improve err msg when multipath cant access a pv Wasn't there before, I thought it might fail the script. If it won't, will add. Line 8: Line 9: InvalidPhysDev gave a cryptic error message - Invalid physical device. Line 10: The message was amended to "Multipath cannot access storage device" along Line 11: with the device name. The error key was changed according to the new Line 7: Improve err msg when multipath cant access a pv Line 8: Line 9: InvalidPhysDev gave a cryptic error message - Invalid physical device. Line 10: The message was amended to "Multipath cannot access storage device" along Line 11: with the device name. The error key was changed according to the new Done to both. Line 12: message content. The only thing needed here by the engine is the Line 13: error code, which was not changed in this patch. Line 14: Line 15: Change-Id: I648ee519873c51573e6e6306b79380f54bb25d2e .................................................... File vdsm/storage/hsm.py Line 2037: in multipath.getMPDevNamesIter()) Line 2038: size = 0 Line 2039: devices = [] Line 2040: Line 2041: for dev in devlist: More important does not mean the other issue is NOT important. Current situation is exception-name almost equals already-agreed-upon-bad-message. Why do you want to fix the whole thing and then have an exception name that turns not really related to it's error message? What if were totally different? would you still want the older exception name? I think you should look at this extremity to decide, and if you still think the naming should remain the same please explain why. Line 2042: if dev in knowndevs: Line 2043: devices.append(dev) Line 2044: size += multipath.getDeviceSize(devicemapper.getDmId(dev)) Line 2045: else: .................................................... File vdsm/storage/storage_exception.py Line 1456: class InaccessiblePhysDev(StorageException): Line 1457: def __init__(self, pvname): Line 1458: self.value = "pvname=%s" % (pvname) Line 1459: code = 606 Line 1460: message = "Multipath cannot access storage device" Ack. Note that when digging into the code in this exact flow the same devices are referred to as pvs. Maybe this should be changed as well (in a different patch, of course). Line 1461: Line 1462: Line 1463: class PartitionedPhysDev(StorageException): Line 1464: code = 607 -- To view, visit http://gerrit.ovirt.org/21089 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I648ee519873c51573e6e6306b79380f54bb25d2e Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vered Volansky <vvola...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Vered Volansky <vvola...@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