Dan Kenigsberg has uploaded a new change for review.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
execCmd: no need to pass the default sudo=False value
These are remnants from the days where we had a crazier default. Cleaned with sed -i 's/execCmd(.*), sudo=False/execCmd\1/' and a couple of manual additions.
Change-Id: Id8c0e30756113ddb35d2e51984c3ea72bbe45a44 Signed-off-by: Dan Kenigsberg danken@redhat.com --- M lib/vdsm/utils.py M tests/functional/virtTests.py M tests/miscTests.py M tests/utilsTests.py M vdsm/hooks.py M vdsm/ksm.py M vdsm/storage/blockSD.py M vdsm/storage/clusterlock.py M vdsm/storage/devicemapper.py M vdsm/storage/fuser.py M vdsm/storage/misc.py M vdsm/storage/multipath.py M vdsm/storage/storage_mailbox.py M vdsm/storage/volume.py M vdsm/supervdsmServer M vdsm/tc.py M vdsm_hooks/checkimages/before_vm_start.py M vdsm_hooks/hostusb/after_vm_destroy.py M vdsm_hooks/hostusb/before_vm_start.py M vdsm_hooks/scratchpad/before_vm_start.py 20 files changed, 43 insertions(+), 44 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/45/24145/1
diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index 8a3bb64..3606464 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -556,7 +556,7 @@ self._poller.close()
-def execCmd(command, sudo=False, cwd=None, data=None, raw=False, logErr=True, +def execCmd(command, cwd=None, data=None, raw=False, logErr=True, printable=None, env=None, sync=True, nice=None, ioclass=None, ioclassdata=None, setsid=False, execCmdLogger=logging.root, deathSignal=0, childUmask=None): @@ -629,7 +629,7 @@ """ Executes an external command, optionally via sudo with stop abilities. """ - proc = execCmd(command, sudo=False, cwd=cwd, data=data, sync=False, + proc = execCmd(command, cwd=cwd, data=data, sync=False, nice=nice, ioclass=ioclass, execCmdLogger=execCmdLogger, deathSignal=deathSignal) if recoveryCallback: diff --git a/tests/functional/virtTests.py b/tests/functional/virtTests.py index 862cf45..cadcca9 100644 --- a/tests/functional/virtTests.py +++ b/tests/functional/virtTests.py @@ -86,7 +86,7 @@ logging.warning('Generating a temporary initramfs image') fd, path = tempfile.mkstemp() cmd = [_mkinitrd.cmd, "-f", path, _kernelVer] - rc, out, err = execCmd(cmd, sudo=False) + rc, out, err = execCmd(cmd) os.chmod(path, 0o644) return path
diff --git a/tests/miscTests.py b/tests/miscTests.py index 13eddbe..5858b05 100644 --- a/tests/miscTests.py +++ b/tests/miscTests.py @@ -342,7 +342,7 @@
def testWaitTimeout(self): ttl = 2 - p = utils.execCmd([EXT_SLEEP, str(ttl + 10)], sudo=False, sync=False) + p = utils.execCmd([EXT_SLEEP, str(ttl + 10)], sync=False) startTime = time.time() p.wait(ttl) duration = time.time() - startTime @@ -352,7 +352,7 @@
def testWaitCond(self): ttl = 2 - p = utils.execCmd([EXT_SLEEP, str(ttl + 10)], sudo=False, sync=False) + p = utils.execCmd([EXT_SLEEP, str(ttl + 10)], sync=False) startTime = time.time() p.wait(cond=lambda: time.time() - startTime > ttl) duration = time.time() - startTime @@ -363,7 +363,7 @@ def testCommunicate(self): data = ("The trouble with the world is that the stupid are cocksure " "and the intelligent are full of doubt") - p = utils.execCmd([EXT_DD], data=data, sudo=False, sync=False) + p = utils.execCmd([EXT_DD], data=data, sync=False) p.stdin.close() self.assertEquals(p.stdout.read(len(data)).strip(), data)
@@ -688,7 +688,7 @@ with tempfile.NamedTemporaryFile() as f: cmd = [EXT_DD, "bs=1", "if=/dev/urandom", 'of=%s' % f.name, 'count=%d' % count] - rc, out, err = utils.execCmd(cmd, sudo=False) + rc, out, err = utils.execCmd(cmd)
self.assertTrue(misc.validateDDBytes(err, count))
@@ -700,7 +700,7 @@ with tempfile.NamedTemporaryFile() as f: cmd = [EXT_DD, "bs=1", "if=/dev/urandom", 'of=%s' % f.name, 'count=%d' % count] - rc, out, err = utils.execCmd(cmd, sudo=False) + rc, out, err = utils.execCmd(cmd)
self.assertFalse(misc.validateDDBytes(err, count + 1))
@@ -993,12 +993,12 @@ """ Tests that execCmd execs and returns the correct ret code """ - ret, out, err = utils.execCmd([EXT_ECHO], sudo=False) + ret, out, err = utils.execCmd([EXT_ECHO])
self.assertEquals(ret, 0)
def testNoCommand(self): - self.assertRaises(OSError, utils.execCmd, ["I.DONT.EXIST"], sudo=False) + self.assertRaises(OSError, utils.execCmd, ["I.DONT.EXIST"])
def testStdOut(self): """ @@ -1008,7 +1008,7 @@ line = "All I wanted was to have some pizza, hang out with dad, " + \ "and not let your weirdness mess up my day" # (C) Nickolodeon - Invader Zim - ret, stdout, stderr = utils.execCmd((EXT_ECHO, line), sudo=False) + ret, stdout, stderr = utils.execCmd((EXT_ECHO, line)) self.assertEquals(stdout[0], line)
def testStdErr(self): @@ -1020,8 +1020,7 @@ "turning you on at all?" # (C) Fox - The X Files code = "import sys; sys.stderr.write('%s')" % line - ret, stdout, stderr = utils.execCmd([EXT_PYTHON, "-c", code], - sudo=False) + ret, stdout, stderr = utils.execCmd([EXT_PYTHON, "-c", code]) self.assertEquals(stderr[0], line)
def testSudo(self): @@ -1036,7 +1035,7 @@
def testNice(self): cmd = ["sleep", "10"] - proc = utils.execCmd(cmd, sudo=False, nice=10, sync=False) + proc = utils.execCmd(cmd, nice=10, sync=False)
def test(): nice = utils.pidStat(proc.pid).nice diff --git a/tests/utilsTests.py b/tests/utilsTests.py index b3b102d..784255c 100644 --- a/tests/utilsTests.py +++ b/tests/utilsTests.py @@ -57,7 +57,7 @@ class PidStatTests(TestCaseBase): def test(self): args = ["sleep", "3"] - sproc = utils.execCmd(args, sync=False, sudo=False) + sproc = utils.execCmd(args, sync=False) stats = utils.pidStat(sproc.pid) pid = int(stats.pid) # procName comes in the format of (procname) @@ -88,7 +88,7 @@ class GetCmdArgsTests(TestCaseBase): def test(self): args = [EXT_SLEEP, "4"] - sproc = utils.execCmd(args, sync=False, sudo=False) + sproc = utils.execCmd(args, sync=False) try: self.assertEquals(utils.getCmdArgs(sproc.pid), tuple(args)) finally: @@ -97,7 +97,7 @@
def testZombie(self): args = [EXT_SLEEP, "0"] - sproc = utils.execCmd(args, sync=False, sudo=False) + sproc = utils.execCmd(args, sync=False) sproc.kill() try: test = lambda: self.assertEquals(utils.getCmdArgs(sproc.pid), diff --git a/vdsm/hooks.py b/vdsm/hooks.py index d31096b..75f33f9 100644 --- a/vdsm/hooks.py +++ b/vdsm/hooks.py @@ -95,7 +95,7 @@
errorSeen = False for s in scripts: - rc, out, err = utils.execCmd([s], sudo=False, raw=True, + rc, out, err = utils.execCmd([s], raw=True, env=scriptenv) logging.info(err) if rc != 0: diff --git a/vdsm/ksm.py b/vdsm/ksm.py index 05e4ff9..7ace217 100644 --- a/vdsm/ksm.py +++ b/vdsm/ksm.py @@ -48,7 +48,7 @@ self._lock = threading.Lock() if config.getboolean('ksm', 'ksm_monitor_thread'): pids = utils.execCmd([constants.EXT_PGREP, '-xf', 'ksmd'], - raw=False, sudo=False)[1] + raw=False)[1] if pids: self._pid = pids[0].strip() self._cif.log.info( diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py index 5179c5a..7f608e1 100644 --- a/vdsm/storage/blockSD.py +++ b/vdsm/storage/blockSD.py @@ -213,7 +213,7 @@ cmd = [constants.EXT_DD, "oflag=%s" % misc.DIRECTFLAG, "if=/dev/zero", "of=%s" % lvm.lvPath(sdUUID, volUUID), "bs=%s" % BS, "count=%s" % count] - p = misc.execCmd(cmd, sudo=False, sync=False, + p = misc.execCmd(cmd, sync=False, nice=utils.NICENESS.HIGH, ioclass=utils.IOCLASS.IDLE, deathSignal=signal.SIGKILL) return p diff --git a/vdsm/storage/clusterlock.py b/vdsm/storage/clusterlock.py index f7e7fb6..3260664 100644 --- a/vdsm/storage/clusterlock.py +++ b/vdsm/storage/clusterlock.py @@ -65,7 +65,7 @@ def initLock(self): lockUtil = os.path.join(self.lockUtilPath, "safelease") initCommand = [lockUtil, "release", "-f", self._leasesPath, "0"] - rc, out, err = misc.execCmd(initCommand, sudo=False, + rc, out, err = misc.execCmd(initCommand, cwd=self.lockUtilPath) if rc != 0: self.log.warn("could not initialise spm lease (%s): %s", rc, out) @@ -128,7 +128,7 @@ releaseLockCommand = [freeLockUtil, self._sdUUID] self.log.info("Releasing cluster lock for domain %s" % self._sdUUID) - (rc, out, err) = misc.execCmd(releaseLockCommand, sudo=False, + (rc, out, err) = misc.execCmd(releaseLockCommand, cwd=self.lockUtilPath) if rc != 0: self.log.error("Could not release cluster lock " diff --git a/vdsm/storage/devicemapper.py b/vdsm/storage/devicemapper.py index 686b453..1a0d5cc 100644 --- a/vdsm/storage/devicemapper.py +++ b/vdsm/storage/devicemapper.py @@ -119,7 +119,7 @@
def _removeMapping(deviceName): cmd = [EXT_DMSETUP, "remove", deviceName] - rc = misc.execCmd(cmd, sudo=False)[0] + rc = misc.execCmd(cmd)[0] if rc != 0: raise Exception("Could not remove mapping `%s`" % deviceName)
@@ -150,7 +150,7 @@
def _getPathsStatus(): cmd = [EXT_DMSETUP, "status"] - rc, out, err = misc.execCmd(cmd, sudo=False) + rc, out, err = misc.execCmd(cmd) if rc != 0: raise Exception("Could not get device statuses")
diff --git a/vdsm/storage/fuser.py b/vdsm/storage/fuser.py index 0f235ee..3d5523c 100644 --- a/vdsm/storage/fuser.py +++ b/vdsm/storage/fuser.py @@ -27,7 +27,7 @@ cmd.append("-m")
cmd.append(path) - (rc, out, err) = misc.execCmd(cmd, raw=True, sudo=False) + (rc, out, err) = misc.execCmd(cmd, raw=True) if rc != 0: return []
diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py index 39eb88b..6c9dd15 100644 --- a/vdsm/storage/misc.py +++ b/vdsm/storage/misc.py @@ -199,7 +199,7 @@
if buffersize: cmd.extend(["bs=%d" % buffersize, "count=1"]) - (rc, out, err) = execCmd(cmd, sudo=False) + (rc, out, err) = execCmd(cmd) if rc: raise se.MiscFileReadException(name)
@@ -365,7 +365,7 @@ cmd.append("oflag=%s" % oflag)
if not stop: - (rc, out, err) = execCmd(cmd, sudo=False, nice=utils.NICENESS.HIGH, + (rc, out, err) = execCmd(cmd, nice=utils.NICENESS.HIGH, ioclass=utils.IOCLASS.IDLE, deathSignal=signal.SIGKILL) else: diff --git a/vdsm/storage/multipath.py b/vdsm/storage/multipath.py index 29851f3..82d435d 100644 --- a/vdsm/storage/multipath.py +++ b/vdsm/storage/multipath.py @@ -211,7 +211,7 @@ "--export", "--replace-whitespace", "--device=" + blkdev] - (rc, out, err) = misc.execCmd(cmd, sudo=False) + (rc, out, err) = misc.execCmd(cmd) if rc == 0: for line in out: if line.startswith("ID_SERIAL="): diff --git a/vdsm/storage/storage_mailbox.py b/vdsm/storage/storage_mailbox.py index d56a2d7..8ac56b9 100644 --- a/vdsm/storage/storage_mailbox.py +++ b/vdsm/storage/storage_mailbox.py @@ -275,7 +275,7 @@
def _initMailbox(self): # Sync initial incoming mail state with storage view - (rc, out, err) = _mboxExecCmd(self._inCmd, sudo=False, raw=True) + (rc, out, err) = _mboxExecCmd(self._inCmd, raw=True) if rc == 0: self._incomingMail = out self._init = True @@ -372,7 +372,7 @@ def _checkForMail(self): #self.log.debug("HSM_MailMonitor - checking for mail") #self.log.debug("Running command: " + str(self._inCmd)) - (rc, in_mail, err) = misc.execCmd(self._inCmd, sudo=False, raw=True) + (rc, in_mail, err) = misc.execCmd(self._inCmd, raw=True) if rc: raise RuntimeError("_handleResponses.Could not read mailbox - rc " "%s" % rc) @@ -391,7 +391,7 @@ pChk = struct.pack('<l', chk) # Assumes CHECKSUM_BYTES equals 4!!! self._outgoingMail = \ self._outgoingMail[0:MAILBOX_SIZE - CHECKSUM_BYTES] + pChk - _mboxExecCmd(self._outCmd, data=self._outgoingMail, sudo=False) + _mboxExecCmd(self._outCmd, data=self._outgoingMail)
def _handleMessage(self, message): # TODO: add support for multiple mailboxes @@ -575,7 +575,7 @@ self.log.debug("SPM_MailMonitor - clearing outgoing mail, command is: " "%s", self._outCmd) cmd = self._outCmd + ['bs=' + str(self._outMailLen)] - (rc, out, err) = _mboxExecCmd(cmd, sudo=False, data=self._outgoingMail) + (rc, out, err) = _mboxExecCmd(cmd, data=self._outgoingMail) if rc: self.log.warning("SPM_MailMonitor couldn't clear outgoing mail, " "dd failed") @@ -730,7 +730,7 @@ cmd = self._inCmd + ['bs=' + str(self._outMailLen)] #self.log.debug("SPM_MailMonitor - reading incoming mail, " # "command: " + str(cmd)) - (rc, in_mail, err) = misc.execCmd(cmd, sudo=False, raw=True) + (rc, in_mail, err) = misc.execCmd(cmd, raw=True) if rc: raise IOError(errno.EIO, "_handleRequests._checkForMail - " "Could not read mailbox: %s" % self._inbox) @@ -747,7 +747,7 @@ self._outLock.acquire() try: cmd = self._outCmd + ['bs=' + str(self._outMailLen)] - (rc, out, err) = _mboxExecCmd(cmd, sudo=False, + (rc, out, err) = _mboxExecCmd(cmd, data=self._outgoingMail) if rc: self.log.warning("SPM_MailMonitor couldn't write " @@ -773,7 +773,7 @@ 'seek=' + str(mailboxOffset / MAILBOX_SIZE)] #self.log.debug("Running command: %s, for message id: %s", # str(cmd), str(msgID)) - (rc, out, err) = _mboxExecCmd(cmd, sudo=False, data=mailbox) + (rc, out, err) = _mboxExecCmd(cmd, data=mailbox) if rc: self.log.error("SPM_MailMonitor: sendReply - couldn't send " "reply, dd failed") diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py index 341a54d..30de1bc 100644 --- a/vdsm/storage/volume.py +++ b/vdsm/storage/volume.py @@ -1033,7 +1033,7 @@ # +1 is so that odd numbers will round upwards. cmd += [volume, "%uK" % ((size + 1) / 2)]
- (rc, out, err) = misc.execCmd(cmd, sudo=False, cwd=cwd, + (rc, out, err) = misc.execCmd(cmd, cwd=cwd, deathSignal=signal.SIGKILL) if rc: raise se.VolumeCreationError(out) diff --git a/vdsm/supervdsmServer b/vdsm/supervdsmServer index 21351c3..54854dc 100755 --- a/vdsm/supervdsmServer +++ b/vdsm/supervdsmServer @@ -256,7 +256,7 @@ self.__udevReloadRules(guid) cmd = [EXT_UDEVADM, 'trigger', '--verbose', '--action', 'change', '--property-match=DM_NAME=%s' % guid] - rc, out, err = utils.execCmd(cmd, sudo=False) + rc, out, err = utils.execCmd(cmd) if rc: raise OSError(errno.EINVAL, "Could not trigger change for device \ %s, out %s\nerr %s" % (guid, out, err)) @@ -342,7 +342,7 @@ else: reload = "--reload-rules" cmd = [EXT_UDEVADM, 'control', reload] - rc, out, err = utils.execCmd(cmd, sudo=False) + rc, out, err = utils.execCmd(cmd) if rc: self.log.error("Udevadm reload-rules command failed rc=%s, " "out="%s", err="%s"", rc, out, err) @@ -352,7 +352,7 @@ @utils.memoized def __udevVersion(self): cmd = [EXT_UDEVADM, '--version'] - rc, out, err = utils.execCmd(cmd, sudo=False) + rc, out, err = utils.execCmd(cmd) if rc: self.log.error("Udevadm version command failed rc=%s, " " out="%s", err="%s"", rc, out, err) diff --git a/vdsm/tc.py b/vdsm/tc.py index 2b2dfa5..00dfc0b 100644 --- a/vdsm/tc.py +++ b/vdsm/tc.py @@ -99,7 +99,7 @@
def _process_request(command): - retcode, out, err = execCmd(command, raw=True, sudo=False) + retcode, out, err = execCmd(command, raw=True) if retcode != 0: raise TrafficControlException(retcode, err, command) return out diff --git a/vdsm_hooks/checkimages/before_vm_start.py b/vdsm_hooks/checkimages/before_vm_start.py index 0fb1d85..40a1326 100755 --- a/vdsm_hooks/checkimages/before_vm_start.py +++ b/vdsm_hooks/checkimages/before_vm_start.py @@ -75,7 +75,7 @@
# Check the image using qemu-img. Enforce check termination # on timeout expiration - p = hooking.execCmd(cmd, sudo=False, raw=True, sync=False) + p = hooking.execCmd(cmd, raw=True, sync=False)
if not p.wait(timeout): p.kill() diff --git a/vdsm_hooks/hostusb/after_vm_destroy.py b/vdsm_hooks/hostusb/after_vm_destroy.py index 8c18a53..ee3379f 100755 --- a/vdsm_hooks/hostusb/after_vm_destroy.py +++ b/vdsm_hooks/hostusb/after_vm_destroy.py @@ -48,7 +48,7 @@ # remove the 0x from the vendor and product id devid = vendorid[2:] + ':' + productid[2:] command = ['lsusb', '-d', devid] - retcode, out, err = hooking.execCmd(command, sudo=False, raw=True) + retcode, out, err = hooking.execCmd(command, raw=True) if retcode != 0: sys.stderr.write('hostusb: cannot find usb device: %s\n' % devid) sys.exit(2) diff --git a/vdsm_hooks/hostusb/before_vm_start.py b/vdsm_hooks/hostusb/before_vm_start.py index 266df60..8dabd8a 100755 --- a/vdsm_hooks/hostusb/before_vm_start.py +++ b/vdsm_hooks/hostusb/before_vm_start.py @@ -67,7 +67,7 @@ # remove the 0x from the vendor and product id devid = vendorid[2:] + ':' + productid[2:] command = ['lsusb', '-d', devid] - retcode, out, err = hooking.execCmd(command, sudo=False, raw=True) + retcode, out, err = hooking.execCmd(command, raw=True) if retcode != 0: sys.stderr.write('hostusb: cannot find usb device: %s\n' % devid) sys.exit(2) diff --git a/vdsm_hooks/scratchpad/before_vm_start.py b/vdsm_hooks/scratchpad/before_vm_start.py index 2e5ddc6..0ff3876 100755 --- a/vdsm_hooks/scratchpad/before_vm_start.py +++ b/vdsm_hooks/scratchpad/before_vm_start.py @@ -33,7 +33,7 @@ Create image file ''' command = ['/usr/bin/qemu-img', 'create', '-f', 'raw', path, size] - retcode, out, err = hooking.execCmd(command, sudo=False, raw=True) + retcode, out, err = hooking.execCmd(command, raw=True) if retcode != 0: sys.stderr.write('scratchpad: error running command %s, err = %s\n' % (' '.join(command), err))
Francesco Romani has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 1: Code-Review-1
(1 comment)
Good cleanup, but looks like the parameter was accidentally took away from the function definition too (since the default here is sudo=False as well).
http://gerrit.ovirt.org/#/c/24145/1/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 555: def __del__(self): Line 556: self._poller.close() Line 557: Line 558: Line 559: def execCmd(command, cwd=None, data=None, raw=False, logErr=True, Well, we need the sudo parameter here! :) Line 560: printable=None, env=None, sync=True, nice=None, ioclass=None, Line 561: ioclassdata=None, setsid=False, execCmdLogger=logging.root, Line 562: deathSignal=0, childUmask=None): Line 563: """
Vinzenz Feenstra has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 1: Code-Review-1
(1 comment)
http://gerrit.ovirt.org/#/c/24145/1/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 555: def __del__(self): Line 556: self._poller.close() Line 557: Line 558: Line 559: def execCmd(command, cwd=None, data=None, raw=False, logErr=True,
Well, we need the sudo parameter here! :)
good catch ;) Line 560: printable=None, env=None, sync=True, nice=None, ioclass=None, Line 561: ioclassdata=None, setsid=False, execCmdLogger=logging.root, Line 562: deathSignal=0, childUmask=None): Line 563: """
Yaniv Bronhaim has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 1: Code-Review-1
(1 comment)
http://gerrit.ovirt.org/#/c/24145/1//COMMIT_MSG Commit Message:
Line 8: Line 9: These are remnants from the days where we had a crazier default. Line 10: Cleaned with Line 11: sed -i 's/execCmd(.*), sudo=False/execCmd\1/' Line 12: and a couple of manual additions. I don't understand the reason for it . we still use the sudo parameter, and until discarding all the sudo=True usages, I prefer to leave the default and specify sudo=True when required. unless you have better reason for removing the crazy but meaningful default Line 13: Line 14: Change-Id: Id8c0e30756113ddb35d2e51984c3ea72bbe45a44
oVirt Jenkins CI Server has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 1: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6200/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7093/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_storage_functional_tests_localfs/119/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_storage_functional_tests_nfs/58/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/6979/ : SUCCESS
Vinzenz Feenstra has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/24145/1//COMMIT_MSG Commit Message:
Line 8: Line 9: These are remnants from the days where we had a crazier default. Line 10: Cleaned with Line 11: sed -i 's/execCmd(.*), sudo=False/execCmd\1/' Line 12: and a couple of manual additions.
I don't understand the reason for it . we still use the sudo parameter, and
This patch is removing all the specification of sudo=False in the usage of execCmd, which is now the default. Line 13: Line 14: Change-Id: Id8c0e30756113ddb35d2e51984c3ea72bbe45a44
Yaniv Bronhaim has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/24145/1//COMMIT_MSG Commit Message:
Line 8: Line 9: These are remnants from the days where we had a crazier default. Line 10: Cleaned with Line 11: sed -i 's/execCmd(.*), sudo=False/execCmd\1/' Line 12: and a couple of manual additions.
This patch is removing all the specification of sudo=False in the usage of
yes, i saw the omit of sudo=False in execCmd which caught my eye. just put it back and im in Line 13: Line 14: Change-Id: Id8c0e30756113ddb35d2e51984c3ea72bbe45a44
Vinzenz Feenstra has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/24145/1//COMMIT_MSG Commit Message:
Line 8: Line 9: These are remnants from the days where we had a crazier default. Line 10: Cleaned with Line 11: sed -i 's/execCmd(.*), sudo=False/execCmd\1/' Line 12: and a couple of manual additions.
yes, i saw the omit of sudo=False in execCmd which caught my eye. just put
That must have been an accident ;) Line 13: Line 14: Change-Id: Id8c0e30756113ddb35d2e51984c3ea72bbe45a44
Dan Kenigsberg has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/24145/1/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 555: def __del__(self): Line 556: self._poller.close() Line 557: Line 558: Line 559: def execCmd(command, cwd=None, data=None, raw=False, logErr=True,
good catch ;)
oh my. "sed" is a bad programmer. Line 560: printable=None, env=None, sync=True, nice=None, ioclass=None, Line 561: ioclassdata=None, setsid=False, execCmdLogger=logging.root, Line 562: deathSignal=0, childUmask=None): Line 563: """
oVirt Jenkins CI Server has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 2: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6219/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7109/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_storage_functional_tests_localfs/125/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_storage_functional_tests_nfs/64/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/6998/ : SUCCESS
Francesco Romani has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 2: Code-Review+1
Looks OK. Not 100% confident about the storage, which I know less, but AFAIK is ok. Moreover, jenkins failures seem unrelated.
Yaniv Bronhaim has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 2: Code-Review+1
Nir Soffer has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 2: Code-Review+1
Vinzenz Feenstra has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 2: Code-Review-1
(3 comments)
http://gerrit.ovirt.org/#/c/24145/2/vdsm/storage/blockSD.py File vdsm/storage/blockSD.py:
Line 217: ioclass This can be two lines now instead of three
http://gerrit.ovirt.org/#/c/24145/2/vdsm/storage/clusterlock.py File vdsm/storage/clusterlock.py:
Line 64: Line 65: def initLock(self): Line 66: lockUtil = os.path.join(self.lockUtilPath, "safelease") Line 67: initCommand = [lockUtil, "release", "-f", self._leasesPath, "0"] Line 68: rc, out, err = misc.execCmd(initCommand, Fits into one line now Line 69: cwd=self.lockUtilPath) Line 70: if rc != 0: Line 71: self.log.warn("could not initialise spm lease (%s): %s", rc, out) Line 72: raise se.ClusterLockInitError()
http://gerrit.ovirt.org/#/c/24145/2/vdsm/storage/volume.py File vdsm/storage/volume.py:
Line 1032: # can also accept size in M or G with appropriate suffix Line 1033: # +1 is so that odd numbers will round upwards. Line 1034: cmd += [volume, "%uK" % ((size + 1) / 2)] Line 1035: Line 1036: (rc, out, err) = misc.execCmd(cmd, cwd=cwd, Fits into one line now Line 1037: deathSignal=signal.SIGKILL) Line 1038: if rc: Line 1039: raise se.VolumeCreationError(out) Line 1040: return True
Vinzenz Feenstra has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 2:
(3 comments)
http://gerrit.ovirt.org/#/c/24145/2/vdsm/storage/blockSD.py File vdsm/storage/blockSD.py:
Line 217: ioclass
This can be two lines now instead of three
Done
http://gerrit.ovirt.org/#/c/24145/2/vdsm/storage/clusterlock.py File vdsm/storage/clusterlock.py:
Line 64: Line 65: def initLock(self): Line 66: lockUtil = os.path.join(self.lockUtilPath, "safelease") Line 67: initCommand = [lockUtil, "release", "-f", self._leasesPath, "0"] Line 68: rc, out, err = misc.execCmd(initCommand,
Fits into one line now
Done Line 69: cwd=self.lockUtilPath) Line 70: if rc != 0: Line 71: self.log.warn("could not initialise spm lease (%s): %s", rc, out) Line 72: raise se.ClusterLockInitError()
http://gerrit.ovirt.org/#/c/24145/2/vdsm/storage/volume.py File vdsm/storage/volume.py:
Line 1032: # can also accept size in M or G with appropriate suffix Line 1033: # +1 is so that odd numbers will round upwards. Line 1034: cmd += [volume, "%uK" % ((size + 1) / 2)] Line 1035: Line 1036: (rc, out, err) = misc.execCmd(cmd, cwd=cwd,
Fits into one line now
Done Line 1037: deathSignal=signal.SIGKILL) Line 1038: if rc: Line 1039: raise se.VolumeCreationError(out) Line 1040: return True
oVirt Jenkins CI Server has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 3: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6239/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7129/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_storage_functional_tests_localfs/126/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_storage_functional_tests_nfs/65/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7018/ : SUCCESS
Dan Kenigsberg has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 3:
Vinzenz, personally I am not into the line condensation business, but I do not really mind.
However, I do mind your posting onto my patch, clearing 3 hard-earned acks. Please avoid that in the future.
Francesco Romani has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 3: Code-Review+1
Nir Soffer has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 3: Code-Review+1
This is better now, thanks Vinzenz!
Yaniv Bronhaim has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 3: Code-Review+1
Dan Kenigsberg has posted comments on this change.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
Patch Set 3: Verified+1 Code-Review+2
Dan Kenigsberg has submitted this change and it was merged.
Change subject: execCmd: no need to pass the default sudo=False value ......................................................................
execCmd: no need to pass the default sudo=False value
These are remnants from the days where we had a crazier default. Cleaned with sed -i 's/execCmd(.*), sudo=False/execCmd\1/' and a couple of manual additions.
Change-Id: Id8c0e30756113ddb35d2e51984c3ea72bbe45a44 Signed-off-by: Dan Kenigsberg danken@redhat.com Signed-off-by: Vinzenz Feenstra vfeenstr@redhat.com Reviewed-on: http://gerrit.ovirt.org/24145 Reviewed-by: Francesco Romani fromani@redhat.com Reviewed-by: Nir Soffer nsoffer@redhat.com Reviewed-by: Yaniv Bronhaim ybronhei@redhat.com --- M lib/vdsm/utils.py M tests/functional/virtTests.py M tests/miscTests.py M tests/utilsTests.py M vdsm/hooks.py M vdsm/ksm.py M vdsm/storage/blockSD.py M vdsm/storage/clusterlock.py M vdsm/storage/devicemapper.py M vdsm/storage/fuser.py M vdsm/storage/misc.py M vdsm/storage/multipath.py M vdsm/storage/storage_mailbox.py M vdsm/storage/volume.py M vdsm/supervdsmServer M vdsm/tc.py M vdsm_hooks/checkimages/before_vm_start.py M vdsm_hooks/hostusb/after_vm_destroy.py M vdsm_hooks/hostusb/before_vm_start.py M vdsm_hooks/scratchpad/before_vm_start.py 20 files changed, 43 insertions(+), 47 deletions(-)
Approvals: Nir Soffer: Looks good to me, but someone else must approve Yaniv Bronhaim: Looks good to me, but someone else must approve Dan Kenigsberg: Verified; Looks good to me, approved Francesco Romani: Looks good to me, but someone else must approve
vdsm-patches@lists.fedorahosted.org