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