Nir Soffer has posted comments on this change. Change subject: Changed InvalidPhysDev to InaccessiblePhysDev InvalidPhysDev gave a cryptic error message - Invalid physical device. The message was amended into "Multipath cannot access device" along with device name. The error key was changed according to the new messa ......................................................................
Patch Set 1: (4 comments) The message change looks good, I'm not sure about renaming the exception. These are two unrelated changes and the later is not required for fixing the bug. I suggest to keep this change minimal and only fix the error message for now. .................................................... 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 07:47:43 +0200 Line 6: Line 7: Changed InvalidPhysDev to InaccessiblePhysDev The title does not describe the intent of this change. How about: exception: Improve error message when multipath cannot access a pv Line 8: InvalidPhysDev gave a cryptic error message - Invalid physical device. Line 9: The message was amended into "Multipath cannot access device" along with Line 10: device name. The error key was changed according to the new message content. Line 11: The error code remains the same. Line 6: Line 7: Changed InvalidPhysDev to InaccessiblePhysDev Line 8: InvalidPhysDev gave a cryptic error message - Invalid physical device. Line 9: The message was amended into "Multipath cannot access device" along with Line 10: device name. The error key was changed according to the new message content. Error key? I think you mean something like: The exception was renamed to match the semantics of this error. Please verify that the exception name is not bubbled up to engine and the different name can cause trouble. Line 11: The error code remains the same. Line 12: Line 13: Change-Id: I648ee519873c51573e6e6306b79380f54bb25d2e Line 14: Bug-url: https://bugzilla.redhat.com/852003 Line 7: Changed InvalidPhysDev to InaccessiblePhysDev Line 8: InvalidPhysDev gave a cryptic error message - Invalid physical device. Line 9: The message was amended into "Multipath cannot access device" along with Line 10: device name. The error key was changed according to the new message content. Line 11: The error code remains the same. If it did not change, why mention it? Line 12: Line 13: Change-Id: I648ee519873c51573e6e6306b79380f54bb25d2e Line 14: Bug-url: https://bugzilla.redhat.com/852003 .................................................... File vdsm/storage/storage_exception.py Line 1454: Line 1455: Line 1456: class InaccessiblePhysDev(StorageException): Line 1457: def __init__(self, pvname): Line 1458: self.value = "pvname=%s" % (pvname) I would not :-) Look at other exceptions around this. Line 1459: code = 606 Line 1460: message = "Multipath cannot access storage device" Line 1461: Line 1462: -- 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: 1 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: 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