Change in vdsm[master]: BZ#784931 - Fixing raise condition from deactivateSD().

abaron at redhat.com abaron at redhat.com
Tue Jun 26 09:18:58 UTC 2012


Ayal Baron has posted comments on this change.

Change subject: BZ#784931 - Fixing raise condition from deactivateSD().
......................................................................


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

(7 inline comments)

....................................................
Commit Message
Line 7: BZ#784931 - Fixing raise condition from deactivateSD().
s/raise/race/
s/from/in/

....................................................
File vdsm/storage/mount.py
Line 46: def _runcmd(self, cmd, timeout):
You changed this into a module function but did not remove self, yet invoking methods do not pass self  which leads me to think that this code was never tested as it couldn't not work.

Line 48:     p = misc.execCmd(cmd, sudo=not isRoot, sync=False)
if this requires sudo it probably means it should be run from superVdsm
I also don't understand why we need a new runcmd command (and yes, I know you just moved this function around)

Line 60:     raise MountError(rc, ";".join((out, err)))
this function gets a generic cmd so MountError has no business here (and yes, I know you didn't change this, but then again, you probably shouldn't have moved it either)

Line 149:         m = getMountFromTarget(target)
the assignment is redundant, just return getMountFromTarget

Line 152:             return None
should return False, not None.

Line 154:     else:
else is redundant

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If06f9cc8da81996590c366de9ad9ea2786a5d3ea
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw 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: Igor Lvovsky <ilvovsky at redhat.com>
Gerrit-Reviewer: Shu Ming <shuming at linux.vnet.ibm.com>


More information about the vdsm-patches mailing list