Change in vdsm[master]: Related to BZ#854151 - Fix disconnect iscsi connections.

abaron at redhat.com abaron at redhat.com
Sun Feb 3 15:01:58 UTC 2013


Ayal Baron has posted comments on this change.

Change subject: Related to BZ#854151 - Fix disconnect iscsi connections.
......................................................................


Patch Set 1: I would prefer that you didn't submit this

(3 inline comments)

It's impossible to understand what was actually fixed here.
Need to split this into general cleanup and actual fix.
Also, Igor's comments are correct but have gone unanswered for months...

....................................................
File vdsm/storage/hsm.py
Line 2030:         for conDef in conList:
Line 2031:             conInfo = _connectionDict2ConnectionInfo(domType, conDef)
Line 2032:             conObj = storageServer.ConnectionFactory.createConnection(conInfo)
Line 2033:             try:
Line 2034:                 conObj.disconnect()
else is redundant here.  If anything, instead of setting: status = 0
just change disconnect to return 0 (or True and return result of == True) in case of success so you'd have:
status = conObj.disconnect()
Line 2035:             except Exception as err:
Line 2036:                 self.log.error("Could not disconnect from storageServer", exc_info=True)
Line 2037:                 status, _ = self._translateConnectionError(err)
Line 2038:             else:


....................................................
File vdsm/storage/storageServer.py
Line 335: 
Line 336:         elif self._cred != cred:
Line 337:             return False
Line 338: 
Line 339:         else:
all of these changes are noise and have nothing to do with the real problem. please split into 2 patches.
Line 340:             return True
Line 341: 
Line 342:     def getSessionInfo(self):
Line 343:         sessions = iscsi.iterateIscsiSessions()


Line 372:         try:
Line 373:             sid = self.getSessionInfo().id
Line 374:         except OSError, e:
Line 375:             if e.errno == errno.ENOENT:
Line 376:                 self.log.warning("Can't get iscsi session id, ENOENT: %s", e.errno)
also, error should be "cannot disconnect session, session %s not found", sid
Line 377:                 return
Line 378:             self.log.warning("Cant get iscsi session id, errno: %s", e.errno)
Line 379:             raise
Line 380: 


--
To view, visit http://gerrit.ovirt.org/8305
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic390cf1a63594a87ee03c010df31f68a432ebff4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>


More information about the vdsm-patches mailing list