Change in vdsm[master]: Move multipath configuration to vdsm-tool configurator

danken at redhat.com danken at redhat.com
Wed Mar 26 18:56:00 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: Move multipath configuration to vdsm-tool configurator
......................................................................


Patch Set 2: Code-Review-1

(3 comments)

Very very partial review.
Note the persistent pep8 errors

 lib/vdsm/tool/configurator.py:217:26: E128 continuation line under-indented for visual indent
lib/vdsm/tool/configurator.py:220:26: E124 closing bracket does not match visual indentation
lib/vdsm/tool/configurator.py:267:5: E303 too many blank lines (2)

http://gerrit.ovirt.org/#/c/26123/2/lib/vdsm/utils.py
File lib/vdsm/utils.py:

Line 154:         else:
Line 155:             raise
Line 156: 
Line 157: 
Line 158: def rotateFiles(directory, prefixName, gen, cp=False, persist=False):
this function is so old and dusty. would you break this big patch to smaller ones, and have a separate patch re-implementing this function?

I, for one, do not recall why we care about cp=True. I certainly resent the silent swallowing of exceptions. Luckily, when used by an external vdsm-tool, there is no excuse for doing that.
Line 159:     logging.debug("dir: %s, prefixName: %s, versions: %s" %
Line 160:                  (directory, prefixName, gen))
Line 161:     gen = int(gen)
Line 162:     files = os.listdir(directory)


Line 203:         except:
Line 204:             pass
Line 205:         if isOvirtNode() and persist and not cp:
Line 206:             try:
Line 207:                 execCmd([constants.EXT_PERSIST, newName], logErr=False,
we already have a function wrapper for this (persistFile). Let's use it.
Line 208:                         sudo=True)
Line 209:             except:
Line 210:                 pass
Line 211: 


Line 206:             try:
Line 207:                 execCmd([constants.EXT_PERSIST, newName], logErr=False,
Line 208:                         sudo=True)
Line 209:             except:
Line 210:                 pass
omg
Line 211: 
Line 212: 
Line 213: IPXMLRPCRequestHandler = SimpleXMLRPCRequestHandler
Line 214: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40f802477e39000c5cae01a496ac2d9f879ebfa8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list