Change in vdsm[master]: BZ#726970 BZ#717868: iscsiadm better error handling
danken at redhat.com
danken at redhat.com
Mon Aug 1 06:42:38 UTC 2011
Dan Kenigsberg has posted comments on this change.
Change subject: BZ#726970 BZ#717868: iscsiadm better error handling
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(4 inline comments)
....................................................
Commit Message
Line 10: - Reflect the real error in case rc == ISCSI_ERR_LOGIN_AUTH_FAILED.
you have two bugs, and two lines of explanation. it feels like two separate patches would have been clearer.
....................................................
File vdsm/storage/iscsi.py
Line 476: if rc not in (0, ISCSI_ERR_SESS_EXISTS):
I think that a flat if..elif clause is clearer.
Line 484: if checkSession(ip, port, iqn, tpgt, username, password):
now that you check for ISCSI_ERR_SESS_EXISTS, there is no need for checkSession() here (or elsewhere).
....................................................
File vdsm/storage/storage_exception.py
Line 844: class iSCSILoginAuthError(StorageServeriSCSIError):
Why did you change the name of this exception? there may be other reasons for add iscsi node errors; why not accommodate them all?
--
To view, visit http://gerrit.usersys.redhat.com/770
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I67c8fc5430dd548877b26c802bcd3b3e775a20fe
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: David Naori <dnaori at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
More information about the vdsm-patches
mailing list