Change in vdsm[master]: pylint: vdsClientGluster: get dictionary element properly
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has uploaded a new change for review.
Change subject: pylint: vdsClientGluster: get dictionary element properly
......................................................................
pylint: vdsClientGluster: get dictionary element properly
Change-Id: I149824bca1de057c24a737a6bdc043e93a902aad
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com>
---
M client/vdsClientGluster.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/39/69339/1
diff --git a/client/vdsClientGluster.py b/client/vdsClientGluster.py
index ff9b45a..b818f29 100644
--- a/client/vdsClientGluster.py
+++ b/client/vdsClientGluster.py
@@ -178,7 +178,7 @@
def do_glusterVolumeRemoveBrickStart(self, args):
params = self._eqSplit(args)
volumeName = params.get('volumeName', '')
- brickList = params('bricks', '').split(',')
+ brickList = params.get('bricks', '').split(',')
replicaCount = params.get('replica', '')
status = self.s.glusterVolumeRemoveBrickStart(volumeName,
--
To view, visit https://gerrit.ovirt.org/69339
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I149824bca1de057c24a737a6bdc043e93a902aad
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
6 years, 5 months
Change in vdsm[master]: tests: CountThreadsPlugin
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has uploaded a new change for review.
Change subject: tests: CountThreadsPlugin
......................................................................
tests: CountThreadsPlugin
Enable this test plugin (via --with-countthreads) to verify that neither
your test, nor the code it tests, leak threads. Unfortunately, this
plugin cannot be enabled by default, since we do have buggy test and
code that do leak.
Change-Id: I4347f5a6ccf02e80d86174c979eb85a0c5076e7a
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com>
---
M tests/testValidation.py
M tests/testlib.py
2 files changed, 22 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/40/69040/1
diff --git a/tests/testValidation.py b/tests/testValidation.py
index c9b147d..a4b6789 100644
--- a/tests/testValidation.py
+++ b/tests/testValidation.py
@@ -23,6 +23,7 @@
from functools import wraps
from nose.plugins import Plugin
import subprocess
+import threading
class SlowTestsPlugin(Plugin):
@@ -88,6 +89,24 @@
StressTestsPlugin.enabled = True
+class CountThreadsPlugin(Plugin):
+ """
+ Check whether a test (or the code it triggers) leaks threads
+ """
+ name = 'countthreads'
+
+ def _thread_names(self):
+ return frozenset(t.name for t in threading.enumerate())
+
+ def startTest(self, test):
+ self._start_thread_names = self._thread_names()
+
+ def stopTest(self, test):
+ diff = self._thread_names() - self._start_thread_names
+ if diff:
+ raise Exception('thread leak: %s ' % diff)
+
+
def ValidateRunningAsRoot(f):
@wraps(f)
def wrapper(*args, **kwargs):
diff --git a/tests/testlib.py b/tests/testlib.py
index b67b377..8a55e6e 100644
--- a/tests/testlib.py
+++ b/tests/testlib.py
@@ -58,7 +58,8 @@
from virt import vmxml
from monkeypatch import Patch
-from testValidation import SlowTestsPlugin, StressTestsPlugin
+from testValidation import (
+ SlowTestsPlugin, StressTestsPlugin, CountThreadsPlugin)
# /tmp may use tempfs filesystem, not suitable for some of the test assuming a
# filesystem with direct io support.
@@ -433,6 +434,7 @@
plugins=core.DefaultPluginManager())
conf.plugins.addPlugin(SlowTestsPlugin())
conf.plugins.addPlugin(StressTestsPlugin())
+ conf.plugins.addPlugin(CountThreadsPlugin())
runner = VdsmTestRunner(stream=conf.stream,
verbosity=conf.verbosity,
--
To view, visit https://gerrit.ovirt.org/69040
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4347f5a6ccf02e80d86174c979eb85a0c5076e7a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
6 years, 5 months
Change in vdsm[master]: automation: avoid yappi skips
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has uploaded a new change for review.
Change subject: automation: avoid yappi skips
......................................................................
automation: avoid yappi skips
I am not sure that testing yappi is worth the time consumed by it, but
I'd like to consider removing of
SKIP: yappi is not installed
messages from the test stdout.
Change-Id: I7ade1636b38e7b8361279e230ca9c238c41e0b6e
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com>
---
M automation/check-patch.sh
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/89/68089/1
diff --git a/automation/check-patch.sh b/automation/check-patch.sh
index ee9052f..d01161d 100755
--- a/automation/check-patch.sh
+++ b/automation/check-patch.sh
@@ -8,7 +8,7 @@
export OVIRT_CI=1
easy_install pip
-pip install -U tox==2.1.1
+pip install -U tox==2.1.1 yappi
./autogen.sh --system --enable-hooks --enable-vhostmd
--
To view, visit https://gerrit.ovirt.org/68089
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7ade1636b38e7b8361279e230ca9c238c41e0b6e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
6 years, 5 months
Change in vdsm[master]: pytlint: drop ivdsm
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has uploaded a new change for review.
Change subject: pytlint: drop ivdsm
......................................................................
pytlint: drop ivdsm
Not used for years, completely replaceable by "manhole".
Change-Id: I0478614a8ddc1a664fbafb59545b3078178ad92a
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com>
---
D contrib/ivdsm.py
1 file changed, 0 insertions(+), 56 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/38/69338/1
diff --git a/contrib/ivdsm.py b/contrib/ivdsm.py
deleted file mode 100644
index a6dcd91..0000000
--- a/contrib/ivdsm.py
+++ /dev/null
@@ -1,56 +0,0 @@
-#
-# Copyright 2011 Red Hat, Inc.
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-#
-# Refer to the README and COPYING files for full details of the license
-#
-
-import os
-import pwd
-import logging
-import threading
-import sys
-
-
-def runVdsm(baseDir="/usr/share/vdsm/", configFilePath="/etc/vdsm/vdsm.conf",
- loggerConfigurationPath='/etc/vdsm/logger.conf'):
- """
- Start a VDSM instance in a new thread.
-
- Return a tuple ``(ClientIF, Thread Running VDSM)``
- """
- if pwd.getpwuid(os.geteuid())[0] != "vdsm":
- raise Exception("You can't run vdsm with any user other then 'vdsm'.")
-
- sys.path.append(baseDir)
-
- from vdsm.config import config
- from logging import config as lconfig
- import clientIF
-
- loggerConfFile = loggerConfigurationPath
- lconfig.fileConfig(loggerConfFile, disable_existing_loggers=False)
- log = logging.getLogger('vds')
-
- config.read(configFilePath)
-
- cif = clientIF.clientIF(log)
-
- t = threading.Thread(target=cif.serve)
- t.setDaemon(True)
- t.start()
-
- return (cif, t)
--
To view, visit https://gerrit.ovirt.org/69338
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0478614a8ddc1a664fbafb59545b3078178ad92a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
6 years, 5 months
Change in vdsm[master]: pylint: storageServer: ExampleConnection: improve documented...
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has uploaded a new change for review.
Change subject: pylint: storageServer: ExampleConnection: improve documented interface
......................................................................
pylint: storageServer: ExampleConnection: improve documented interface
ExampleConnection serves as a "documentation class". This patch fixes
the signature of its __eq__ and __ne__ methods.
Signed-off-by Dan Kenigsberg <danken(a)redhat.com>
Change-Id: I2dd21331439a5fcb911c8e86be2aa78fc0c12b21
---
M vdsm/storage/storageServer.py
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/37/69337/1
diff --git a/vdsm/storage/storageServer.py b/vdsm/storage/storageServer.py
index a40767f..dc3fd80 100644
--- a/vdsm/storage/storageServer.py
+++ b/vdsm/storage/storageServer.py
@@ -89,12 +89,12 @@
Don't use this to monitor connection health"""
pass
- def __eq__(self):
+ def __eq__(self, other):
"""Implement this! It will be used to detect multiple connections to
the same target"""
pass
- def __ne__(self):
+ def __ne__(self, other):
"""Must be implemented otherwise != operator will return True for equal
objects"""
--
To view, visit https://gerrit.ovirt.org/69337
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2dd21331439a5fcb911c8e86be2aa78fc0c12b21
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
6 years, 5 months
Change in vdsm[master]: pylint: storage/fileSD: fix typo
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has uploaded a new change for review.
Change subject: pylint: storage/fileSD: fix typo
......................................................................
pylint: storage/fileSD: fix typo
commit 166bda1 introduced a typo that is hereby fixed.
It's time we introduce static checkers to the project, to avoid similar
cases.
Change-Id: Ia441c59ca994a9d15836b165a1a56e67a5e12ca6
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com>
---
M vdsm/storage/fileSD.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/36/69336/1
diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py
index b364b06..5c055b7 100644
--- a/vdsm/storage/fileSD.py
+++ b/vdsm/storage/fileSD.py
@@ -754,7 +754,7 @@
try:
proc.truncateFile(path, size, METADATA_PERMISSIONS, creatExcl=True)
except OSError as e:
- if e.rrrno == errno.EEXIST:
+ if e.errno == errno.EEXIST:
self.log.info("Reusing external leases volume %s", path)
--
To view, visit https://gerrit.ovirt.org/69336
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia441c59ca994a9d15836b165a1a56e67a5e12ca6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
6 years, 5 months
Change in vdsm[master]: profiling: Rename cpuProfileTests.py
by Nir Soffer
Nir Soffer has uploaded a new change for review.
Change subject: profiling: Rename cpuProfileTests.py
......................................................................
profiling: Rename cpuProfileTests.py
Use new naming convention for tests modules: cpu_profile_test.py
Change-Id: I10eb0afe0d8d8d6c6db88d4df05159f93385eec0
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M tests/Makefile.am
R tests/cpu_profile_test.py
2 files changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/51/65251/1
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1ee0b45..a2f0b58 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -51,8 +51,8 @@
commands_test.py \
concurrentTests.py \
config_test.py \
+ cpu_profile_test.py \
cpuinfo_test.py \
- cpuProfileTests.py \
deviceTests.py \
domainDescriptorTests.py \
domain_manifest_test.py \
diff --git a/tests/cpuProfileTests.py b/tests/cpu_profile_test.py
similarity index 100%
rename from tests/cpuProfileTests.py
rename to tests/cpu_profile_test.py
--
To view, visit https://gerrit.ovirt.org/65251
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I10eb0afe0d8d8d6c6db88d4df05159f93385eec0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
6 years, 5 months
Change in vdsm[master]: testValidation: Add ProcesLeakChecker plugin
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: testValidation: Add ProcesLeakChecker plugin
......................................................................
Patch Set 2:
(4 comments)
https://gerrit.ovirt.org/#/c/69324/1/tests/testValidation.py
File tests/testValidation.py:
Line 113: self._start_processes = self._child_processes()
Line 114:
Line 115: def stopTest(self, test):
Line 116: leaked_processes = self._child_processes() - self._start_processes
Line 117: if leaked_processes:
> This will be less efficient, see https://gerrit.ovirt.org/69331
this surprises me, even though I remember the old Unix mantra of "do one thing and do it well".
are you sure you counted cpu eaten by subprocesses?
Line 118: info = [dict(pid=pid, cmdline=utils.getCmdArgs(pid))
Line 119: for pid in leaked_processes]
Line 120: raise AssertionError("Test leaked child processes:\n" +
Line 121: json.dumps(info, indent=4))
https://gerrit.ovirt.org/#/c/69324/2/tests/testValidation.py
File tests/testValidation.py:
Line 27:
Line 28: from nose.plugins.skip import SkipTest
Line 29: from nose.plugins import Plugin
Line 30:
Line 31: from vdsm import utils
argh, sorry. I did this myself in another patch.
would you rebase on top of my --with-leaked-thread-check patch?
Line 32: from vdsm.compat import CPopen
Line 33:
Line 34:
Line 35: class SlowTestsPlugin(Plugin):
Line 120: raise AssertionError("Test leaked child processes:\n" +
Line 121: json.dumps(info, indent=4))
Line 122:
Line 123: def _child_processes(self):
Line 124: cmd = ["pgrep", "-P", "%s" % os.getpid()]
you asked me to define a not-so-different constant at the class level, to avoid reevaluation of getpid()!
Line 125: proc = CPopen(cmd, stdin=None, stdout=subprocess.PIPE,
Line 126: stderr=subprocess.PIPE)
Line 127: out, err = proc.communicate()
Line 128: # EXIT STATUS
Line 123: def _child_processes(self):
Line 124: cmd = ["pgrep", "-P", "%s" % os.getpid()]
Line 125: proc = CPopen(cmd, stdin=None, stdout=subprocess.PIPE,
Line 126: stderr=subprocess.PIPE)
Line 127: out, err = proc.communicate()
do you have a grudge against execCmd()?
/me really wonder why you opted against the simple path.
Line 128: # EXIT STATUS
Line 129: # 0 One or more processes matched the criteria.
Line 130: # 1 No processes matched.
Line 131: if proc.returncode not in (0, 1):
--
To view, visit https://gerrit.ovirt.org/69324
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I451490d56f70ea8a6a685569d984495798e8b297
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
6 years, 5 months
Change in vdsm[master]: vdsm-client: Remove unused code
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: vdsm-client: Remove unused code
......................................................................
vdsm-client: Remove unused code
The vdsmclient/client.py module is not a script any more, and does not
need the #!/usr/bin/python or handling of __main__.
The module is used by importing and calling the main() function.
Change-Id: I7f009f9c813f7331ae979ff297fd26e11532ef3c
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M lib/vdsmclient/client.py
1 file changed, 0 insertions(+), 5 deletions(-)
Approvals:
Nir Soffer: Verified
Yaniv Bronhaim: Looks good to me, but someone else must approve
Jenkins CI: Passed CI tests
Irit Goihman: Looks good to me, but someone else must approve
Dan Kenigsberg: Looks good to me, approved
--
To view, visit https://gerrit.ovirt.org/69064
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I7f009f9c813f7331ae979ff297fd26e11532ef3c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
6 years, 5 months
Change in vdsm[master]: vdsm-client: Remove unused code
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: vdsm-client: Remove unused code
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://gerrit.ovirt.org/69064
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f009f9c813f7331ae979ff297fd26e11532ef3c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
6 years, 5 months