Change in vdsm[master]: Removing update of 'node.startup' setting in 'addIscsiPortal...

fsimonce at redhat.com fsimonce at redhat.com
Wed Jul 24 16:13:41 UTC 2013


Federico Simoncelli has posted comments on this change.

Change subject: Removing update of 'node.startup' setting in 'addIscsiPortal'
......................................................................


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

(1 inline comment)

Downvoting for visibility to my comment.

....................................................
File vdsm/storage/iscsi.py
Line 181:     portalStr = "%s:%d" % (portal.hostname, portal.port)
Line 182: 
Line 183:     with _iscsiadmTransactionLock:
Line 184:         iscsiadm.discoverydb_new(discoverType, iface.name, portalStr)
Line 185: 
Isn't this a problem if you move an host from a data center to another? (leftovers)
If the host can still reach them it'll be seeing luns that is not supposed to see, if it  can't, they might timeout (slowing down boot?).

The fact that this wasn't working for 16 months is not an excuse (see lv teardown).

Let's put things in prospective, http://gerrit.ovirt.org/#/c/305 was introduced to remove the entire code that was overwriting the iscsid.conf file.
Dan nacked the patch because he wanted to maintain node.startup=manual (comment 4 on the above gerrit link), and that made sense.

So generally I'm fine removing this code, but I think we should make sure that we really don't care about leftovers (and Dan really changed his mind).
Line 186:         try:
Line 187:             # Push credentials
Line 188:             if credentials is not None:
Line 189:                 for key, value in credentials.getIscsiadmOptions():


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib34952047367db196733e942416a232d12a4e491
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list