Saggi Mizrahi has posted comments on this change.
Change subject: ISCSI Subsystem refactoring
......................................................................
Patch Set 6: (12 inline comments)
Since all the comments that I actually need to address are text changes. Please
have someone else look at it so I can just do the text changes and repost for
fast approval.
....................................................
File vdsm/storage/hsm.py
Line 2304: # I unparse the response. Why you ask? Backward
compatibility! At
If parsing is changing human readable text to machine readable text. Unparsing
is doing the reverse, changing machine readable data is the reverse.
What I'm doing here is not parseing.
You are, however, correct that unparsing is not a word.
I should have written:
"I format the data to it's original textual representation."
....................................................
File vdsm/storage/iscsiadm.py
Line 49: def iface_exists(interfaceName):
I think it's clearer this way.
it's object_action. It's no the classic naming scheme.
Think c namespaces namespace_clearSomething().
....................................................
File vdsm/storage/iscsi.py
Line 68: IscsiTarget(IscsiPortal("", 0), 0, ""), None)
in multipath, to preserve the interface. I don't want to poke things I don't
have to. This is preserves the original interface of returning an empty object.
It has a FIXME around it and will hopefully be fixed in another patch.
Line 72: sessionDir = glob.glob("/sys/devices/platform/host*/%s/" %
sessionName)
If you look at the original code you'll see I just moved small bits around. I
already did enough changes and didn't want to test these things as well.
Line 89: conn_pattern = os.path.join(sessionDir, "connection*")
original implementation, some else is welcome to fix it.
Line 91: connectiondir = glob.glob(conn_pattern)[0]
like the original
Line 176: key = "discovery.sendtargets." + key
why? it's faster and simpler this way.
and you actually mean '.'.join(["discovery.sendtargets", key])
Line 195: addIscsiPortal(iface, portal, credentials)
I clear it, this was the original behavior.
Line 199: deleteIscsiPortal(iface, portal)
This can't logically happen
Line 204: # returns here has it's IP resolved!
Discovery with a hostname will actually give results for all IPs covered by the
hostname
Line 210: for sessionDir in
glob.iglob("/sys/devices/platform/host*/session*/iscsi_session/session?*"):
Yes, constants are villainous.
Line 329: # Suggestions are welcome.
I doubt it. Anyway, out of the scope of this patch.
--
To view, visit http://gerrit.ovirt.org/1607
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b5f99598a559731951c271e97f04f03a2d2173a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: shu ming <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/vdsm-patches
