[node-patches] Change in ovirt-node[master]: make sure the repo is file before mount it.

dougsland at redhat.com dougsland at redhat.com
Mon Sep 15 13:16:30 UTC 2014


Douglas Schilling Landgraf has posted comments on this change.

Change subject: make sure the repo is file before mount it.
......................................................................


Patch Set 2: Code-Review-1

(2 comments)

-1 as I have comment/question.

http://gerrit.ovirt.org/#/c/32916/2/tools/edit-node
File tools/edit-node:

Line 1036
Line 1037
Line 1038
Line 1039
Line 1040
I do believe this above if is duplicate of 1039 line. There is no 'else' statement for this if and also the already checked if options.repo.lower() ends with .iso.


Line 1037:                 return True
Line 1038:             elif os.path.isdir(options.repo) or \
Line 1039:                     options.repo.lower().endswith(".iso"):
Line 1040:                 if options.repo.lower().endswith(".iso") and \
Line 1041:                     os.path.isfile(options.repo):
I would move the and to above line and remove the duplicate code. What you say hadong?
Line 1042:                     cmd = "mount -o loop %s %s" % (options.repo, self.dd_dir)
Line 1043:                     f = subprocess.Popen(cmd, shell=True, stdout=PIPE,
Line 1044:                                          stderr=STDOUT)
Line 1045:                     output, err = f.communicate()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id033c87eb2d8723c43d06e74fdcf5e1f18d2d52f
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: hadong <hadong0720 at gmail.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at redhat.com>
Gerrit-Reviewer: Ryan Barry <rbarry at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes



More information about the node-patches mailing list