Change in vdsm[master]: cleanup: remove persist/unpersist calls for legacy node

[automation]

gerrit-hooks has posted comments on this change.

Change subject: cleanup: remove persist/unpersist calls for legacy node
..


Patch Set 4:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63715
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1cca0cfa0e2254a5a8af6b3591ffe47689410415
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: cleanup: remove persist/unpersist calls for legacy node

[automation]

gerrit-hooks has posted comments on this change.

Change subject: cleanup: remove persist/unpersist calls for legacy node
..


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63715
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1cca0cfa0e2254a5a8af6b3591ffe47689410415
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: cleanup: remove persist/unpersist calls for legacy node

[automation]

gerrit-hooks has posted comments on this change.

Change subject: cleanup: remove persist/unpersist calls for legacy node
..


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63715
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1cca0cfa0e2254a5a8af6b3591ffe47689410415
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: upgrade: Support upgrade to v4 from v3

[amureini]

Allon Mureinik has posted comments on this change.

Change subject: upgrade: Support upgrade to v4 from v3
..


Patch Set 7: Code-Review+1

(1 comment)

https://gerrit.ovirt.org/#/c/63547/7//COMMIT_MSG
Commit Message:

PS7, Line 7:  to v4 from v3
"from v3 to v4" sounds more natural to me


-- 
To view, visit https://gerrit.ovirt.org/63547
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8a66346b91e49480d391116ba8ee8b8294f53ec4
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: cleanup: remove persist/unpersist calls for legacy node

[dougsland]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: cleanup: remove persist/unpersist calls for legacy node
..


Patch Set 1: Verified+1

make rpm works in branch master, clicking verified.

-- 
To view, visit https://gerrit.ovirt.org/63715
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1cca0cfa0e2254a5a8af6b3591ffe47689410415
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: cleanup: remove persist/unpersist calls for legacy node

[automation]

gerrit-hooks has posted comments on this change.

Change subject: cleanup: remove persist/unpersist calls for legacy node
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63715
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1cca0cfa0e2254a5a8af6b3591ffe47689410415
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: cleanup: remove persist/unpersist calls for legacy node

[dougsland]

Douglas Schilling Landgraf has uploaded a new change for review.

Change subject: cleanup: remove persist/unpersist calls for legacy node
..

cleanup: remove persist/unpersist calls for legacy node

This patch removes all related to unpersist and persist
calls exclusive for oVirt Node Legacy. oVirt Node Next doesn't
require such approach.

Change-Id: I1cca0cfa0e2254a5a8af6b3591ffe47689410415
Signed-off-by: Douglas Schilling Landgraf 
---
M lib/vdsm/network/configurators/ifcfg.py
M lib/vdsm/tool/configfile.py
M lib/vdsm/tool/configurators/certificates.py
M lib/vdsm/tool/configurators/libvirt.py
M lib/vdsm/tool/configurators/multipath.py
M lib/vdsm/tool/configurators/passwd.py
M lib/vdsm/tool/register.py
M lib/vdsm/tool/upgrade.py
M lib/vdsm/tool/validate_ovirt_certs.py.in
M lib/vdsm/utils.py
M tests/toolTests.py
M vdsm.spec.in
M vdsm/Makefile.am
D vdsm/ovirt_functions.sh
M vdsm/vdsm-store-net-config.in
M vdsm_hooks/Makefile.am
M vdsm_hooks/fcoe/fcoe_before_network_setup.py
D vdsm_hooks/persist-vdsm-hooks.in
D vdsm_hooks/unpersist-vdsm-hook
19 files changed, 9 insertions(+), 219 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/15/63715/1

diff --git a/lib/vdsm/network/configurators/ifcfg.py 
b/lib/vdsm/network/configurators/ifcfg.py
index 2302ae1..0555034 100644
--- a/lib/vdsm/network/configurators/ifcfg.py
+++ b/lib/vdsm/network/configurators/ifcfg.py
@@ -55,9 +55,6 @@
 from vdsm.network.netinfo.cache import ifaceUsed
 from vdsm.network.netlink import waitfor
 
-if utils.isOvirtNode():
-from ovirt.node.utils import fs as node_fs
-
 from . import Configurator, getEthtoolOpts
 from ..errors import ConfigNetworkError, ERR_FAILED_IFUP
 from ..models import Nic, Bridge
@@ -297,10 +294,7 @@
 @staticmethod
 def _removeFile(filename):
 """Remove file (directly or using oVirt node's library)"""
-if utils.isOvirtNode():
-node_fs.Config().delete(filename)  # unpersists and shreds the file
-else:
-utils.rmFile(filename)
+utils.rmFile(filename)
 logging.debug("Removed file %s", filename)
 
 def createLibvirtNetwork(self, network, bridged=True, iface=None,
@@ -512,10 +506,6 @@
 except:
 logging.debug('ignoring restorecon error in case '
   'SElinux is disabled', exc_info=True)
-
-# make sure that ifcfg files are always persisted by the node
-if self.unifiedPersistence and utils.isOvirtNode():
-node_fs.Config().persist(fileName)
 
 def _createConfFile(self, conf, name, ipv4, ipv6, mtu, nameservers,
 **kwargs):
diff --git a/lib/vdsm/tool/configfile.py b/lib/vdsm/tool/configfile.py
index 0c49b19..bad0759 100644
--- a/lib/vdsm/tool/configfile.py
+++ b/lib/vdsm/tool/configfile.py
@@ -175,9 +175,7 @@
 if self._entries:
 self._writeEntries(f, oldentries)
 
-utils.unpersist(self._filename)
 os.rename(tname, self._filename)
-utils.persist(self._filename)
 
 if self._oldmod != os.stat(self._filename).st_mode:
 os.chmod(self._filename, self._oldmod)
diff --git a/lib/vdsm/tool/configurators/certificates.py 
b/lib/vdsm/tool/configurators/certificates.py
index 0462b4d..edc96b3 100644
--- a/lib/vdsm/tool/configurators/certificates.py
+++ b/lib/vdsm/tool/configurators/certificates.py
@@ -23,10 +23,8 @@
 from vdsm.config import config
 
 from . import YES, NO
-from vdsm.tool.validate_ovirt_certs import validate_ovirt_certs
 from vdsm.constants import P_VDSM_EXEC, SYSCONF_PATH
 from vdsm.commands import execCmd
-from vdsm.utils import isOvirtNode
 
 PKI_DIR = os.path.join(SYSCONF_PATH, 'pki/vdsm')
 CA_FILE = os.path.join(PKI_DIR, 'certs/cacert.pem')
@@ -59,8 +57,6 @@
 
 def configure():
 _exec_vdsm_gencerts()
-if isOvirtNode():
-validate_ovirt_certs()
 
 
 def isconfigured():
diff --git a/lib/vdsm/tool/configurators/libvirt.py 
b/lib/vdsm/tool/configurators/libvirt.py
index 40d4958..94c5da2 100644
--- a/lib/vdsm/tool/configurators/libvirt.py
+++ b/lib/vdsm/tool/configurators/libvirt.py
@@ -27,12 +27,8 @@
 from . import InvalidRun, NO, MAYBE
 from . certificates import CA_FILE, CERT_FILE, KEY_FILE
 from vdsm.tool.configfile import ConfigFile, ParserWrapper
-from vdsm.tool.validate_ovirt_certs import validate_ovirt_certs
 from vdsm import utils
 from vdsm import constants
-
-if utils.isOvirtNode():
-from ovirt.node.utils.fs import Config as NodeCfg
 
 
 requires = frozenset(('certificates',))
@@ -45,12 +41,6 @@
 
 
 def configure():
-if utils.isOvirtNode():
-if not os.path.exists(constants.P_VDSM_CERT):
-raise InvalidRun(
-"vdsm: Missing certificate, vdsm not registered")
-validate_ovirt_certs()
-
 # Remove a previous configuration (if present)
 

Change in vdsm[master]: upgrade: Support upgrade to v4 from v3

[automation]

gerrit-hooks has posted comments on this change.

Change subject: upgrade: Support upgrade to v4 from v3
..


Patch Set 7:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63547
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8a66346b91e49480d391116ba8ee8b8294f53ec4
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: spec: Require sanlock version handling EINTR

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: spec: Require sanlock version handling EINTR
..


Patch Set 11:

Still not available:

00:22:49 Error: Package: vdsm-4.18.999-532.git681d5f3.el7.centos.x86_64 
(/vdsm-4.18.999-532.git681d5f3.el7.centos.x86_64)
00:22:49Requires: sanlock >= 3.2.4-3.el7_2
00:22:49Available: sanlock-3.2.4-1.el7.x86_64 (centos-base)
00:22:49sanlock = 3.2.4-1.el7
00:22:49Available: sanlock-3.2.4-2.el7_2.x86_64 (centos-updates)
00:22:49sanlock = 3.2.4-2.el7_2

-- 
To view, visit https://gerrit.ovirt.org/61200
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1203ad58f0f0ed1789a1e85d7f0b364891ef5864
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storage: fix disconnecting multiple iSCSI sessions

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: fix disconnecting multiple iSCSI sessions
..


Patch Set 2:

Dan, can you check this from the networking side?

-- 
To view, visit https://gerrit.ovirt.org/55578
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0d576f6fe262d9576fb99a674ba921dc4a141a32
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Gashev 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: v2v: Do not give Xen a special treatment.

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: v2v: Do not give Xen a special treatment.
..


Patch Set 1:

Lets make a decision about these patches.

-- 
To view, visit https://gerrit.ovirt.org/57430
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I597118940d42ee953d9cdc83bb44afd13b5e882a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: spec: Require sanlock version handling EINTR

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: spec: Require sanlock version handling EINTR
..


Patch Set 11:

Package should be available today, retrying...

-- 
To view, visit https://gerrit.ovirt.org/61200
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1203ad58f0f0ed1789a1e85d7f0b364891ef5864
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: spec: Require sanlock version handling EINTR

[automation]

gerrit-hooks has posted comments on this change.

Change subject: spec: Require sanlock version handling EINTR
..


Patch Set 11:

* #1356676::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1356676::OK, public bug
* Check Product::#1356676::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61200
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1203ad58f0f0ed1789a1e85d7f0b364891ef5864
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: vdsm: add eventfd and EventFile synchronization

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: vdsm: add eventfd and EventFile synchronization
..


Patch Set 1:

Similar and simpler module was merged ages ago.

-- 
To view, visit https://gerrit.ovirt.org/33687
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0d237f13c42b1f4505c90d30c6d3c3ecbd1e9fa7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: vdsm: add eventfd and EventFile synchronization

[nsoffer]

Nir Soffer has abandoned this change.

Change subject: vdsm: add eventfd and EventFile synchronization
..


Abandoned

Not needed now.

-- 
To view, visit https://gerrit.ovirt.org/33687
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: I0d237f13c42b1f4505c90d30c6d3c3ecbd1e9fa7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: vdsm: add eventfd and EventFile synchronization

[automation]

gerrit-hooks has posted comments on this change.

Change subject: vdsm: add eventfd and EventFile synchronization
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found

-- 
To view, visit https://gerrit.ovirt.org/33687
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0d237f13c42b1f4505c90d30c6d3c3ecbd1e9fa7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: vdsm: add eventfd and EventFile synchronization

[Nir Soffer]

Nir Soffer has uploaded a new change for review.

Change subject: vdsm: add eventfd and EventFile synchronization
..

vdsm: add eventfd and EventFile synchronization

Change-Id: I0d237f13c42b1f4505c90d30c6d3c3ecbd1e9fa7
Signed-off-by: Federico Simoncelli 
---
M lib/vdsm/Makefile.am
A lib/vdsm/eventfd.py
M tests/Makefile.am
A tests/eventfdTests.py
4 files changed, 251 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/87/33687/1

diff --git a/lib/vdsm/Makefile.am b/lib/vdsm/Makefile.am
index 4bebf28..e712cad 100644
--- a/lib/vdsm/Makefile.am
+++ b/lib/vdsm/Makefile.am
@@ -25,6 +25,7 @@
__init__.py \
compat.py \
define.py \
+   eventfd.py \
exception.py \
ipwrapper.py \
libvirtconnection.py \
diff --git a/lib/vdsm/eventfd.py b/lib/vdsm/eventfd.py
new file mode 100644
index 000..b2a7084
--- /dev/null
+++ b/lib/vdsm/eventfd.py
@@ -0,0 +1,140 @@
+#
+# Copyright 2014 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
+#
+
+"""\
+This module provides the support for eventfd(2).
+
+More information about eventfd and usage examples can be found in the
+eventfd(2) man page.
+
+The EventFile class provides a single synchronization object exposing
+the python Event interface and associated eventfds.
+
+The eventfd() context manager returns a file descriptor that can be
+used to provide the event notice to select, poll and epoll, e.g.
+
+import os
+import sys
+import select
+import threading
+import time
+from vdsm.eventfd import EventFile, DATASIZE
+
+e = EventFile()
+p = select.epoll()
+
+threading.Timer(5, e.set).start()
+
+with e.eventfd() as efd:
+p.register(efd, select.EPOLLIN)
+p.register(sys.stdin.fileno(), select.EPOLLIN)
+
+print "Echoing lines until event is received"
+event_received = False
+
+while not event_received:
+for fileno, event in p.poll():
+if not event & select.EPOLLIN:
+continue
+
+if fileno == efd:
+os.read(efd, DATASIZE)
+event_received = True
+elif fileno == sys.stdin.fileno():
+print os.read(sys.stdin.fileno(), 1024),
+
+print "Event received!"
+
+
+The Event set() semantic is preserved in the eventfd context manager:
+if the event is set then the eventfd already contains the notification.
+This is both to maintain the semantic and to avoid possible races as:
+
+if not e.is_set():
+with e.eventfd() as efd:
+...
+"""
+
+import os
+import ctypes
+import threading
+
+from contextlib import contextmanager
+
+_libc = ctypes.CDLL('libc.so.6', use_errno=True)
+
+EFD_NONBLOCK = os.O_NONBLOCK
+EFD_CLOEXEC = 0200  # os.O_CLOEXEC in python 3.3
+EFD_SEMAPHORE = 0001
+
+DATASIZE = ctypes.sizeof(ctypes.c_ulonglong)
+
+
+def eventfd(initval, flags):
+return _libc.eventfd(initval, flags)
+
+
+class EventFile(object):
+def __init__(self, event=None):
+self.__lock = threading.Lock()
+self.__fds = set()
+self.__event = event or threading.Event()
+
+@staticmethod
+def __fire_event(fd):
+os.write(fd, ctypes.c_ulonglong(1))
+
+def open_eventfd(self):
+with self.__lock:
+fd = eventfd(0, 0)
+
+self.__fds.add(fd)
+
+if self.__event.is_set():
+self.__fire_event(fd)
+
+return fd
+
+@contextmanager
+def eventfd(self):
+fd = self.open_eventfd()
+
+yield fd
+
+with self.__lock:
+self.__fds.remove(fd)
+os.close(fd)
+
+def isSet(self):
+return self.__event.isSet()
+
+is_set = isSet
+
+def set(self):
+with self.__lock:
+self.__event.set()
+for fd in self.__fds:
+self.__fire_event(fd)
+
+def clear(self):
+self.__event.clear()
+
+def wait(self, timeout=None, balancing=True):
+self.__event.wait(timeout)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 449d7b1..120712e 100644

Change in vdsm[master]: upgrade: Support upgrade to v4 from v3

[mlipchuk]

Maor Lipchuk has posted comments on this change.

Change subject: upgrade: Support upgrade to v4 from v3
..


Patch Set 6: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/63547
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8a66346b91e49480d391116ba8ee8b8294f53ec4
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: formatConverter: Use module logger

[mlipchuk]

Maor Lipchuk has posted comments on this change.

Change subject: formatConverter: Use module logger
..


Patch Set 3: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/63630
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b55237a22f13151e391116ba8ee7a7183e31ca2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jsonrpcvdscli: fix results of several commands

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: jsonrpcvdscli: fix results of several commands
..


Patch Set 3:

(6 comments)

Unfortunately, this change the behavior in incompatible way. I did not check 
the schema, but even if it describes the new format this change is wrong since 
all clients are depending on the actual data returned from vdsm.

More: This is not a change in jsonrpsvdscli, but in the Bridge - it effects all 
jsonrpc clients.

https://gerrit.ovirt.org/#/c/63483/3/lib/vdsm/rpc/Bridge.py
File lib/vdsm/rpc/Bridge.py:

Line 345
Line 346
Line 347
Line 348
Line 349
This returned before a list of pool...


Line 345: 'Host_fenceNode': {'ret': Host_fenceNode_Ret},
Line 346: 'Host_getAllTasksInfo': {'ret': 'allTasksInfo'},
Line 347: 'Host_getAllTasksStatuses': {'ret': 'allTasksStatus'},
Line 348: 'Host_getCapabilities': {'ret': Host_getCapabilities_Ret},
Line 349: 'Host_getConnectedStoragePools': {'ret': remove_status_from_ret},
Now we get a dict with a 'poollist' key - schema conflict - and worse breaking 
all client of this api, expecting list of pools.
Line 350: 'Host_getDeviceList': {'ret': 'devList'},
Line 351: 'Host_getDevicesVisibility': {'ret': 'visible'},
Line 352: 'Host_getExternalVMs': {'ret': 'vmList'},
Line 353: 'Host_getExternalVMNames': {'ret': 'vmNames'},


Line 355: 'Host_getConvertedVm': {'ret': 'ovf'},
Line 356: 'Host_getHardwareInfo': {'ret': 'info'},
Line 357: 'Host_getLVMVolumeGroups': {'ret': 'vglist'},
Line 358: 'Host_getStats': {'ret': 'info'},
Line 359: 'Host_getStorageDomains': {'ret': remove_status_from_ret},
Same
Line 360: 'Host_getStorageRepoStats': {'ret': Host_getStorageRepoStats_Ret},
Line 361: 'Host_hostdevListByCaps': {'ret': 'deviceList'},
Line 362: 'Host_getVMList': {'call': Host_getVMList_Call,
Line 363:'ret': remove_status_from_ret},


Line 380: 'ISCSIConnection_discoverSendTargets': {'ret': 'fullTargets'},
Line 381: 'LVMVolumeGroup_create': {'ret': 'uuid'},
Line 382: 'LVMVolumeGroup_getInfo': {'ret': 'info'},
Line 383: 'StorageDomain_getFileStats': {'ret': 'fileStats'},
Line 384: 'StorageDomain_getImages': {'ret': remove_status_from_ret},
Same
Line 385: 'StorageDomain_getInfo': {'ret': 'info'},
Line 386: 'StorageDomain_getStats': {'ret': 'stats'},
Line 387: 'StorageDomain_getVolumes': {'ret': remove_status_from_ret},
Line 388: 'StorageDomain_resizePV': {'ret': 'size'},


Line 383: 'StorageDomain_getFileStats': {'ret': 'fileStats'},
Line 384: 'StorageDomain_getImages': {'ret': remove_status_from_ret},
Line 385: 'StorageDomain_getInfo': {'ret': 'info'},
Line 386: 'StorageDomain_getStats': {'ret': 'stats'},
Line 387: 'StorageDomain_getVolumes': {'ret': remove_status_from_ret},
Same
Line 388: 'StorageDomain_resizePV': {'ret': 'size'},
Line 389: 'StoragePool_connectStorageServer': {'ret': 'statuslist'},
Line 390: 'StoragePool_disconnectStorageServer': {'ret': 'statuslist'},
Line 391: 'StoragePool_fence': {'ret': 'spm_st'},


Line 427: 'VM_updateDevice': {'ret': 'vmList'},
Line 428: 'Volume_copy': {'ret': 'uuid'},
Line 429: 'Volume_create': {'ret': 'uuid'},
Line 430: 'Volume_delete': {'ret': 'uuid'},
Line 431: 'Volume_getInfo': {'ret': remove_status_from_ret},
Same
Line 432: 'Volume_getPath': {'ret': 'path'},
Line 433: 'Volume_getSize': {'ret': Volume_getsize_Ret},
Line 434: 'Volume_extendSize': {'ret': 'uuid'},
Line 435: 'Host_getAllTasks': {'ret': 'tasks'},


-- 
To view, visit https://gerrit.ovirt.org/63483
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f6b5862a69ea3d5140352fce7dda51bb95bab9c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jsonrpcvdscli: fix results of several commands

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: jsonrpcvdscli: fix results of several commands
..


Patch Set 3:

Irit, the previous patch created a conflict in the schema - the schema promise 
list of vms, and the patch return dict with one key (vmList).

Maybe this patch does not change the schema, but we must first fix the errors 
before we reuse the code from that patch.

-- 
To view, visit https://gerrit.ovirt.org/63483
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f6b5862a69ea3d5140352fce7dda51bb95bab9c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storage: Unify stoage logger name

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Unify stoage logger name
..


Patch Set 3: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/61261
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I653c41301393b5450de4ac6dee92b606a5f64838
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jobs: Autodelete

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: jobs: Autodelete
..


Patch Set 6: Code-Review-1

(2 comments)

https://gerrit.ovirt.org/#/c/62002/6/tests/jobsTests.py
File tests/jobsTests.py:

Line 290: delay, callable = self.scheduler.calls[0]
Line 291: self.assertEqual(10, delay)
Line 292: self.assertIn(job.id, jobs.info())
Line 293: callable()
Line 294: self.assertNotIn(job.id, jobs.info())
Nice tests
Line 295: 
Line 296: def test_autodelete_when_aborted(self):
Line 297: cfg = make_config([('jobs', 'autodelete_delay', '10')])
Line 298: job = AutodeleteJob()


Line 303: delay, callable = self.scheduler.calls[0]
Line 304: self.assertEqual(10, delay)
Line 305: self.assertIn(job.id, jobs.info())
Line 306: callable()
Line 307: self.assertNotIn(job.id, jobs.info())
Both tests are exactly the same except the line when we run or abort the job. 
We can refactor the common parts, maybe the code for checking that autodelete 
was called?
Line 308: 
Line 309: def test_autodelete_disabled(self):
Line 310: cfg = make_config([('jobs', 'autodelete_delay', '-1')])
Line 311: job = AutodeleteJob()


-- 
To view, visit https://gerrit.ovirt.org/62002
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4aafd2271397bd9bc4289dc1aab723398d475add
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jobs: Preserve aborted status after run finishes

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: jobs: Preserve aborted status after run finishes
..


Patch Set 1: Code-Review-1

(5 comments)

https://gerrit.ovirt.org/#/c/63713/1/lib/vdsm/jobs.py
File lib/vdsm/jobs.py:

Line 158
Line 159
Line 160
Line 161
Line 162
This is not safe, job could be aborted here.


Line 162: self._status = STATUS.FAILED
Line 163: else:
Line 164: with self._status_lock:
Line 165: if self._status == STATUS.RUNNING:
Line 166: self._status = STATUS.DONE
This is only partial fix, see comments in the previous patch. Also I think we 
can squash this into the previous patch, handling state changes.
Line 167: finally:
Line 168: if self.autodelete:
Line 169: self._autodelete()
Line 170: 


https://gerrit.ovirt.org/#/c/63713/1/tests/jobsTests.py
File tests/jobsTests.py:

Line 76: 
Line 77: class StuckJob(TestingJob):
Line 78: 
Line 79: def _run(self):
Line 80: while not self._aborted:
We should kill this flag, and use self._status != ABORTED.

Having multiple status variables is recipe for bugs.
Line 81: time.sleep(0.1)
Line 82: 
Line 83: 
Line 84: class FakeScheduler(object):


Line 77: class StuckJob(TestingJob):
Line 78: 
Line 79: def _run(self):
Line 80: while not self._aborted:
Line 81: time.sleep(0.1)
If jobs code is broken, this will create stuck test run.

How about this:

def __init__(self):
self.event = threading.Event()

def _run(self):
self.event.wait(1)
   
def _abort(self):
self.event.set()

Now I don't care about job _status management, and I can also check that _abort 
was called (assert job.event.is_set())
Line 82: 
Line 83: 
Line 84: class FakeScheduler(object):
Line 85: 


Line 194: job = StuckJob()
Line 195: jobs.add(job)
Line 196: t = threading.Thread(target=job.run)
Line 197: t.daemon = True
Line 198: t.start()
Use testlib.start_thread instead
Line 199: self.assertEqual(jobs.STATUS.RUNNING, job.status)
Line 200: jobs.abort(job.id)
Line 201: t.join()
Line 202: self.assertEqual(jobs.STATUS.ABORTED, job.status)


-- 
To view, visit https://gerrit.ovirt.org/63713
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2cc1ac16ef9608d287f076950c4fa67890f5d900
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: formatConverter: Use module logger

[automation]

gerrit-hooks has posted comments on this change.

Change subject: formatConverter: Use module logger
..


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63630
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b55237a22f13151e391116ba8ee7a7183e31ca2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: upgrade: Support upgrade to v4 from v3

[automation]

gerrit-hooks has posted comments on this change.

Change subject: upgrade: Support upgrade to v4 from v3
..


Patch Set 6:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63547
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8a66346b91e49480d391116ba8ee8b8294f53ec4
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jobs: Allow run and abort only from valid states

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: jobs: Allow run and abort only from valid states
..


Patch Set 1: Code-Review-1

(3 comments)

https://gerrit.ovirt.org/#/c/63711/1/lib/vdsm/jobs.py
File lib/vdsm/jobs.py:

Line 69: ''' Only pending jobs may be run '''
Line 70: name = 'CannotRunJob'
Line 71: 
Line 72: 
Line 73: class JobNotRunnable(ClientError):
Leftover? I don't see any use.
Line 74: ''' Job cannot be run from its current state '''
Line 75: name = 'JobNotRunnable'
Line 76: 
Line 77: 


Line 136: return self.status in (STATUS.PENDING, STATUS.RUNNING)
Line 137: 
Line 138: def abort(self):
Line 139: if not self.active:
Line 140: raise CannotAbortJob()
CannotAbortJob means that job was in a state which can be aborted, but the 
operation failed, while this job was simply finished.

Maybe JobNotActive? (similar to JobNotDone)

This  name make it more clear that there is no error on the side of the client 
or the server.
Line 141: self._status = STATUS.ABORTED
Line 142: logging.info('Job %r aborting...', self._id)
Line 143: self._abort()
Line 144: if self.autodelete:


Line 145: self._autodelete()
Line 146: 
Line 147: def run(self):
Line 148: if self.status != STATUS.PENDING:
Line 149: raise CannotRunJob()
We have two case here:

- job was aborted, there is no need to run it, and there is no error to raise.
- job was already finished - this a bug in the code, so we should raise 
AssertionError, or RuntimeError, not a ClientError. ClientError should be used 
when a user perform an invalid request.
Line 150: self._status = STATUS.RUNNING
Line 151: try:
Line 152: self._run()
Line 153: except Exception as e:


-- 
To view, visit https://gerrit.ovirt.org/63711
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8bc1ab264c5786b43fb409c3fab8913337778bf
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jobs: Guard against racy state changes

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: jobs: Guard against racy state changes
..


Patch Set 1: Code-Review-1

-- 
To view, visit https://gerrit.ovirt.org/63712
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5366fcb962e9d34da6fca47b5171a6b132cf2e3b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jobs: Guard against racy state changes

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: jobs: Guard against racy state changes
..


Patch Set 1:

(5 comments)

https://gerrit.ovirt.org/#/c/63712/1/lib/vdsm/jobs.py
File lib/vdsm/jobs.py:

Line 154
Line 155
Line 156
Line 157
Line 158
Before we set status here and in the else path, we should check if the job was 
aborted. If we aborted, we can return, since abort() does everything needed.

We can build the status here:

status = STATUS.FAILED
else:
status = STATUS.DONE


Line 158
Line 159
Line 160
Line 161
Line 162
Here we can set the status safely:

with self._status_lock:
if self._status == STATUS.ABORTED:
return
self._status = status


Line 179
Line 180
Line 181
Line 182
Line 183
To avoid races between jobs.stop() and _delete() called from autodelete, we can 
add a new state DELETED:

with self._status_lock:
if self._status == STATUS.DELETED:
return
self._status = STATUS.DELETED

And, call _delete from jobs.stop(), before clearing _jobs.


Line 146: if self.autodelete:
Line 147: self._autodelete()
Line 148: 
Line 149: def run(self):
Line 150: with self._status_lock:
If job was aborted we should not fail. I think this will ensure this (needs a 
test):

if self._status = STATUS.ABORTED:
return
Line 151: if self.status != STATUS.PENDING:
Line 152: raise CannotRunJob()
Line 153: self._status = STATUS.RUNNING
Line 154: try:


Line 147: self._autodelete()
Line 148: 
Line 149: def run(self):
Line 150: with self._status_lock:
Line 151: if self.status != STATUS.PENDING:
Use self._status for consistency
Line 152: raise CannotRunJob()
Line 153: self._status = STATUS.RUNNING
Line 154: try:
Line 155: self._run()


-- 
To view, visit https://gerrit.ovirt.org/63712
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5366fcb962e9d34da6fca47b5171a6b132cf2e3b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: properties: properties_test.py py2/3 compliance

[automation]

gerrit-hooks has posted comments on this change.

Change subject: properties: properties_test.py py2/3 compliance
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63714
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6f63b33a44e465806e00ed3de0b1e55e8a01005f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Leon Goldberg 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: formatConverter: Use module logger

[automation]

gerrit-hooks has posted comments on this change.

Change subject: formatConverter: Use module logger
..


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63630
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b55237a22f13151e391116ba8ee7a7183e31ca2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: upgrade: Support upgrade to v4 from v3

[automation]

gerrit-hooks has posted comments on this change.

Change subject: upgrade: Support upgrade to v4 from v3
..


Patch Set 5:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63547
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8a66346b91e49480d391116ba8ee8b8294f53ec4
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jobs: Allow run and abort only from valid states

[automation]

gerrit-hooks has posted comments on this change.

Change subject: jobs: Allow run and abort only from valid states
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63711
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8bc1ab264c5786b43fb409c3fab8913337778bf
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jobs: Autodelete

[automation]

gerrit-hooks has posted comments on this change.

Change subject: jobs: Autodelete
..


Patch Set 6:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/62002
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4aafd2271397bd9bc4289dc1aab723398d475add
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jobs: Preserve aborted status after run finishes

[alitke]

Adam Litke has uploaded a new change for review.

Change subject: jobs: Preserve aborted status after run finishes
..

jobs: Preserve aborted status after run finishes

Job.run was unconditionally setting Job.status to done whenever _run
returned without raising.  In the case of abort we want to preserve the
aborted status.

Change-Id: I2cc1ac16ef9608d287f076950c4fa67890f5d900
Signed-off-by: Adam Litke 
---
M lib/vdsm/jobs.py
M tests/jobsTests.py
2 files changed, 23 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/13/63713/1

diff --git a/lib/vdsm/jobs.py b/lib/vdsm/jobs.py
index 1f80cc2..7b3e2d8 100644
--- a/lib/vdsm/jobs.py
+++ b/lib/vdsm/jobs.py
@@ -161,7 +161,9 @@
 self._error = e
 self._status = STATUS.FAILED
 else:
-self._status = STATUS.DONE
+with self._status_lock:
+if self._status == STATUS.RUNNING:
+self._status = STATUS.DONE
 finally:
 if self.autodelete:
 self._autodelete()
diff --git a/tests/jobsTests.py b/tests/jobsTests.py
index 1f0aeed..79a0902 100644
--- a/tests/jobsTests.py
+++ b/tests/jobsTests.py
@@ -17,6 +17,8 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
+import threading
+import time
 import uuid
 
 from vdsm import exception, jobs, response, schedule
@@ -70,6 +72,13 @@
 @progress.setter
 def progress(self, value):
 self._progress = value
+
+
+class StuckJob(TestingJob):
+
+def _run(self):
+while not self._aborted:
+time.sleep(0.1)
 
 
 class FakeScheduler(object):
@@ -181,6 +190,17 @@
 self.assertEqual(jobs.STATUS.ABORTED, job.status)
 self.assertTrue(job._aborted)
 
+def test_abort_running_job(self):
+job = StuckJob()
+jobs.add(job)
+t = threading.Thread(target=job.run)
+t.daemon = True
+t.start()
+self.assertEqual(jobs.STATUS.RUNNING, job.status)
+jobs.abort(job.id)
+t.join()
+self.assertEqual(jobs.STATUS.ABORTED, job.status)
+
 @permutations([
 [jobs.STATUS.ABORTED, jobs.CannotAbortJob.name],
 [jobs.STATUS.DONE, jobs.CannotAbortJob.name],


-- 
To view, visit https://gerrit.ovirt.org/63713
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2cc1ac16ef9608d287f076950c4fa67890f5d900
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jobs: Guard against racy state changes

[alitke]

Adam Litke has uploaded a new change for review.

Change subject: jobs: Guard against racy state changes
..

jobs: Guard against racy state changes

When performing test and set operations with the job status we must use
a lock to prevent multiple threads from conflicting with each other.

Change-Id: I5366fcb962e9d34da6fca47b5171a6b132cf2e3b
Signed-off-by: Adam Litke 
---
M lib/vdsm/jobs.py
1 file changed, 9 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/12/63712/1

diff --git a/lib/vdsm/jobs.py b/lib/vdsm/jobs.py
index ba71008..1f80cc2 100644
--- a/lib/vdsm/jobs.py
+++ b/lib/vdsm/jobs.py
@@ -91,6 +91,7 @@
 self._id = job_id
 self._status = STATUS.PENDING
 self._description = description
+self._status_lock = threading.Lock()
 self._error = None
 
 @property
@@ -136,18 +137,20 @@
 return self.status in (STATUS.PENDING, STATUS.RUNNING)
 
 def abort(self):
-if not self.active:
-raise CannotAbortJob()
-self._status = STATUS.ABORTED
+with self._status_lock:
+if not self.active:
+raise CannotAbortJob()
+self._status = STATUS.ABORTED
 logging.info('Job %r aborting...', self._id)
 self._abort()
 if self.autodelete:
 self._autodelete()
 
 def run(self):
-if self.status != STATUS.PENDING:
-raise CannotRunJob()
-self._status = STATUS.RUNNING
+with self._status_lock:
+if self.status != STATUS.PENDING:
+raise CannotRunJob()
+self._status = STATUS.RUNNING
 try:
 self._run()
 except Exception as e:


-- 
To view, visit https://gerrit.ovirt.org/63712
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5366fcb962e9d34da6fca47b5171a6b132cf2e3b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jobs: Preserve aborted status after run finishes

[automation]

gerrit-hooks has posted comments on this change.

Change subject: jobs: Preserve aborted status after run finishes
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63713
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2cc1ac16ef9608d287f076950c4fa67890f5d900
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jobs: Guard against racy state changes

[automation]

gerrit-hooks has posted comments on this change.

Change subject: jobs: Guard against racy state changes
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63712
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5366fcb962e9d34da6fca47b5171a6b132cf2e3b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jobs: Allow run and abort only from valid states

[alitke]

Adam Litke has uploaded a new change for review.

Change subject: jobs: Allow run and abort only from valid states
..

jobs: Allow run and abort only from valid states

Before this patch we allow abort and run from any state even when it
does not make sense.  Abort is valid only for pending or running jobs
and run is valid only for pending jobs.  When an invalid call is
attempted raise a new error.

Change-Id: Ia8bc1ab264c5786b43fb409c3fab8913337778bf
Signed-off-by: Adam Litke 
---
M lib/vdsm/define.py
M lib/vdsm/exception.py
M lib/vdsm/jobs.py
M tests/jobsTests.py
4 files changed, 55 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/11/63711/1

diff --git a/lib/vdsm/define.py b/lib/vdsm/define.py
index d75e919..3edb881 100644
--- a/lib/vdsm/define.py
+++ b/lib/vdsm/define.py
@@ -88,6 +88,8 @@
 'migrateLimit': exception.MigrationLimitExceeded().response(),
 'recovery': exception.RecoveryInProgress().response(),
 'hostdevDetachErr': exception.HostdevDetachFailed().response(),
+'CannotAbortJob': exception.CannotAbortJob().response(),
+'CannotRunJob': exception.CannotRunJob().response(),
 }
 
 
diff --git a/lib/vdsm/exception.py b/lib/vdsm/exception.py
index 40d13f5..671067a 100644
--- a/lib/vdsm/exception.py
+++ b/lib/vdsm/exception.py
@@ -347,6 +347,16 @@
 message = 'Could not detach host device'
 
 
+class CannotAbortJob(VdsmException):
+code = 84
+message = 'Job cannot be aborted from this state'
+
+
+class CannotRunJob(VdsmException):
+code = 85
+message = 'Job cannot be run from this state'
+
+
 class RecoveryInProgress(VdsmException):
 code = 99
 message = 'Recovering from crash or Initializing'
diff --git a/lib/vdsm/jobs.py b/lib/vdsm/jobs.py
index fb81bdd..ba71008 100644
--- a/lib/vdsm/jobs.py
+++ b/lib/vdsm/jobs.py
@@ -60,6 +60,21 @@
 name = 'JobNotDone'
 
 
+class CannotAbortJob(ClientError):
+''' Job is not running or pending '''
+name = 'CannotAbortJob'
+
+
+class CannotRunJob(ClientError):
+''' Only pending jobs may be run '''
+name = 'CannotRunJob'
+
+
+class JobNotRunnable(ClientError):
+''' Job cannot be run from its current state '''
+name = 'JobNotRunnable'
+
+
 class AbortNotSupported(ClientError):
 ''' This type of job does not support aborting '''
 name = 'AbortNotSupported'
@@ -121,7 +136,8 @@
 return self.status in (STATUS.PENDING, STATUS.RUNNING)
 
 def abort(self):
-# TODO: Don't abort if not pending and not running
+if not self.active:
+raise CannotAbortJob()
 self._status = STATUS.ABORTED
 logging.info('Job %r aborting...', self._id)
 self._abort()
@@ -129,7 +145,8 @@
 self._autodelete()
 
 def run(self):
-# TODO: Don't run if aborted or not pending
+if self.status != STATUS.PENDING:
+raise CannotRunJob()
 self._status = STATUS.RUNNING
 try:
 self._run()
diff --git a/tests/jobsTests.py b/tests/jobsTests.py
index 2cb578c..1f0aeed 100644
--- a/tests/jobsTests.py
+++ b/tests/jobsTests.py
@@ -173,12 +173,24 @@
 self.assertEqual({}, jobs.info(job_type=bar.job_type,
job_ids=[foo.id]))
 
-def test_abort_job(self):
-job = TestingJob()
+@permutations([[jobs.STATUS.PENDING], [jobs.STATUS.RUNNING]])
+def test_abort_job(self, status):
+job = TestingJob(status)
 jobs.add(job)
 jobs.abort(job.id)
 self.assertEqual(jobs.STATUS.ABORTED, job.status)
 self.assertTrue(job._aborted)
+
+@permutations([
+[jobs.STATUS.ABORTED, jobs.CannotAbortJob.name],
+[jobs.STATUS.DONE, jobs.CannotAbortJob.name],
+[jobs.STATUS.FAILED, jobs.CannotAbortJob.name]
+])
+def test_abort_from_invalid_state(self, status, err):
+job = TestingJob(status)
+jobs.add(job)
+res = jobs.abort(job.id)
+self.assertEqual(response.error(err), res)
 
 def test_abort_unknown_job(self):
 self.assertEqual(response.error(jobs.NoSuchJob.name),
@@ -265,6 +277,16 @@
 self.run_job(job)
 self.assertEqual(jobs.STATUS.DONE, job.status)
 
+@permutations([
+[jobs.STATUS.RUNNING],
+[jobs.STATUS.ABORTED],
+[jobs.STATUS.DONE],
+[jobs.STATUS.FAILED]
+])
+def test_run_from_invalid_state(self, state):
+job = TestingJob(state)
+self.assertRaises(jobs.CannotRunJob, job.run)
+
 def test_default_exception(self):
 message = "testing failure"
 job = TestingJob(exception=Exception(message))


-- 
To view, visit https://gerrit.ovirt.org/63711
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia8bc1ab264c5786b43fb409c3fab8913337778bf
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: 

Change in vdsm[master]: properties: properties.py py2/3 metaclass compliance

[Jenkins CI]

Jenkins CI has posted comments on this change.

Change subject: properties: properties.py py2/3 metaclass compliance
..


Patch Set 2: Continuous-Integration-1

Propagate review hook: Continuous Integration value inherited from patch 1

-- 
To view, visit https://gerrit.ovirt.org/63710
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icac99f68a32c9218eb93031fb4c6e2808cb88fc3
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Leon Goldberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: properties: properties.py py2/3 string type check compliance

[automation]

gerrit-hooks has posted comments on this change.

Change subject: properties: properties.py py2/3 string type check compliance
..


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63709
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I63fb0b3f5fd5ae12710684aade233c80dbcc5f2a
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Leon Goldberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: properties: properties.py py2/3 metaclass compliance

[automation]

gerrit-hooks has posted comments on this change.

Change subject: properties: properties.py py2/3 metaclass compliance
..


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63710
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icac99f68a32c9218eb93031fb4c6e2808cb88fc3
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Leon Goldberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: properties: properties.py py2/3 metaclass compliance

[automation]

gerrit-hooks has posted comments on this change.

Change subject: properties: properties.py py2/3 metaclass compliance
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63710
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icac99f68a32c9218eb93031fb4c6e2808cb88fc3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Leon Goldberg 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: properties: properties.py py2/3 string type check compliance.

[automation]

gerrit-hooks has posted comments on this change.

Change subject: properties: properties.py py2/3 string type check compliance.
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63709
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I63fb0b3f5fd5ae12710684aade233c80dbcc5f2a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Leon Goldberg 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storage: Unify stoage logger name

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: Unify stoage logger name
..


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61261
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I653c41301393b5450de4ac6dee92b606a5f64838
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jsonrpcvdscli: fix results of several commands

[igoihman]

Irit Goihman has posted comments on this change.

Change subject: jsonrpcvdscli: fix results of several commands
..


Patch Set 3:

> This looks correct, if https://gerrit.ovirt.org/63408 was correcdt,
 > but
 > I think it was wrong, changing the return value from list of vms to
 > dict with a vmList key, conflicting the schema and possibly
 > breaking engine.

Nir, can we talk about it offline? I don't see any conflict in the schema and a 
dict is returned now and before (just with different keys). We can test it 
further if you'd like.

-- 
To view, visit https://gerrit.ovirt.org/63483
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f6b5862a69ea3d5140352fce7dda51bb95bab9c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: mounts: Use more meaningful names

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: mounts: Use more meaningful names
..


Patch Set 8: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/56551
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iac1ba474856c9cf9f66d05dcce7ed152cbe2147b
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: mount: Simplify deleted suffix stripping

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: mount: Simplify deleted suffix stripping
..


Patch Set 7: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/56548
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb08af0a6a5e7202741cda8b39fe3cddac468b21
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: mount: Remove EL 6 /etc/mtab support

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: mount: Remove EL 6 /etc/mtab support
..


Patch Set 8: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/56517
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I49a66b19924accc93f626eca98b2e91a3b7f5e80
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: properties: py3: properties.py and properties_test.py compli...

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: properties: py3: properties.py and properties_test.py compliance
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/63230/3/tests/properties_test.py
File tests/properties_test.py:

Line 471
Line 472
Line 473
Line 474
Line 475
> I think the correct fix is:
This test is confusing the reader to think that password content is ascii. We 
should use binary data in the test:

data = b"\x80\x81\x82\x83"
obj.password = ProtectedPassword(base64.b64encode(data))
self.assertEqual(data, obj.password.value)

Using this test data, incorrect decoding attempt will fail, alerting the 
developer:

>>> b"\x80\x81\x82\x83".decode()
Traceback (most recent call last):
  File "", line 1, in 
UnicodeDecodeError: 'ascii' codec can't decode byte 0x80 in position 0: 
ordinal not in range(128)

>>> b"\x80\x81\x82\x83".decode('utf8')
Traceback (most recent call last):
  File "", line 1, in 
  File "/usr/lib64/python2.7/encodings/utf_8.py", line 16, in decode
return codecs.utf_8_decode(input, errors, True)
UnicodeDecodeError: 'utf8' codec can't decode byte 0x80 in position 0: 
invalid start byte


-- 
To view, visit https://gerrit.ovirt.org/63230
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id3f49dd718d786c4a98b06c7b8757ce5a9eb6d2a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Leon Goldberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Tomas Golembiovsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: properties: py3: properties.py and properties_test.py compli...

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: properties: py3: properties.py and properties_test.py compliance
..


Patch Set 3:

You can remove the module from the blacklist in the last patch, or just send 
another one on top.

-- 
To view, visit https://gerrit.ovirt.org/63230
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id3f49dd718d786c4a98b06c7b8757ce5a9eb6d2a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Leon Goldberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Tomas Golembiovsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: properties: py3: properties.py and properties_test.py compli...

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: properties: py3: properties.py and properties_test.py compliance
..


Patch Set 3:

(5 comments)

https://gerrit.ovirt.org/#/c/63230/3/lib/vdsm/properties.py
File lib/vdsm/properties.py:

Line 151: 
Line 152: class String(Property):
Line 153: 
Line 154: def validate(self, value):
Line 155: if not isinstance(value, six.string_types):
This looks ok.
Line 156: raise ValueError("Invalid value %r for string property 
%s" %
Line 157:  (value, self.name))
Line 158: return value
Line 159: 


Line 230: 
Line 231: 
Line 232: def decode_base64(value):
Line 233: try:
Line 234: return base64.b64decode(value).decode()
decode assume default encoding - we cannot use such assumptions. Fortunately, 
we don't have to deal with encodings here. The contents of a secret is binary 
data - this is why we encode this in base64.

The result of decoding base64 must be bytes on both python 2 and 3, this is 
*not* a unicode string.
Line 235: except TypeError as e:
Line 236: raise ValueError("Unable to decode base64 value %s" % e)
Line 237: 
Line 238: 


Line 253: obj.check(instance)
Line 254: return instance
Line 255: 
Line 256: 
Line 257: class Owner(six.with_metaclass(OwnerType, object)):
This syntax is ugly, hopefully there is another way.
Line 258: """
Line 259: Base class for classes using properties
Line 260: 
Line 261: Inheriting from this class, all properties on this class and 
subclasses


https://gerrit.ovirt.org/#/c/63230/3/tests/properties_test.py
File tests/properties_test.py:

Line 471
Line 472
Line 473
Line 474
Line 475
I think the correct fix is:

self.assertEqual(b"12345678", obj.password.value)

We need to check that users of this code are not assuming by mistake that 
password.value is unicode value, and are not trying to encode it somehow.

Francesco, what do you think?


Line 470: password = 
properties.Password(decode=properties.decode_base64)
Line 471: 
Line 472: def test_decode(self):
Line 473: obj = self.Cls()
Line 474: obj.password = 
ProtectedPassword(base64.b64encode(b"12345678"))
Looks good
Line 475: self.assertEqual("12345678", obj.password.value)
Line 476: 
Line 477: def test_invalid(self):
Line 478: obj = self.Cls()


-- 
To view, visit https://gerrit.ovirt.org/63230
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id3f49dd718d786c4a98b06c7b8757ce5a9eb6d2a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Leon Goldberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Tomas Golembiovsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: properties: py3: properties.py and properties_test.py compli...

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: properties: py3: properties.py and properties_test.py compliance
..


Patch Set 3:

Looking in the failures when running this under python 3, we have 3 issues:

1. decoding passwords requires bytes instead of string

==
ERROR: test_decode (properties_test.PasswordDecodeTests)
--
Traceback (most recent call last):
  File "/home/nsoffer/src/vdsm/tests/properties_test.py", line 474, in 
test_decode
obj.password = ProtectedPassword(base64.b64encode("12345678"))
  File "/usr/lib64/python3.5/base64.py", line 62, in b64encode
encoded = binascii.b2a_base64(s)[:-1]
TypeError: a bytes-like object is required, not 'str'


2. basestring missing in python 3

==
ERROR: test_empty (properties_test.StringRequiredTests)
--
Traceback (most recent call last):
  File "/home/nsoffer/src/vdsm/tests/properties_test.py", line 201, in 
test_empty
obj = self.Cls("")
  File "/home/nsoffer/src/vdsm/tests/properties_test.py", line 195, in __init__
self.value = value
  File "/home/nsoffer/src/vdsm/lib/vdsm/properties.py", line 115, in __set__
value = self.validate(value)
  File "/home/nsoffer/src/vdsm/lib/vdsm/properties.py", line 154, in validate
if not isinstance(value, basestring):
NameError: name 'basestring' is not defined

3. metaclass verification not running

==
FAIL: test_required (properties_test.PropertyRequiredTests)
--
Traceback (most recent call last):
  File "/home/nsoffer/src/vdsm/tests/properties_test.py", line 65, in 
test_required
self.assertRaises(ValueError, self.Cls)
AssertionError: ValueError not raised by Cls

Can you break this patch to 3 smaller patches for each issue?

-- 
To view, visit https://gerrit.ovirt.org/63230
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id3f49dd718d786c4a98b06c7b8757ce5a9eb6d2a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Leon Goldberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Tomas Golembiovsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: kvm2ovirt: use None if no password is given

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: kvm2ovirt: use None if no password is given
..


Patch Set 2: Code-Review+1

-- 
To view, visit https://gerrit.ovirt.org/63679
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I16d61115fe66eb4e63ae8f31798b96d9f41e72e9
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: kvm2ovirt: Fixes issue in importing VMs with libvirt uri qem...

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: kvm2ovirt: Fixes issue in importing VMs with libvirt uri 
qemu+tcp
..


Patch Set 1: Code-Review+1

-- 
To view, visit https://gerrit.ovirt.org/63678
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcd111f8b490ac64893afecf3ec1aad4b4d05b09
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Nijin Ashok 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storage: Unify stoage logger name

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Unify stoage logger name
..


Patch Set 2:

I'll verify this patch tomorrow.

-- 
To view, visit https://gerrit.ovirt.org/61261
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I653c41301393b5450de4ac6dee92b606a5f64838
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: net tests: fix test_events_keys

[automation]

gerrit-hooks has posted comments on this change.

Change subject: net tests: fix test_events_keys
..


Patch Set 2:

* #1374328::Update tracker: OK
* Set MODIFIED::bug 1374328#1374328IGNORE, not all related patches are 
closed, check 63486

-- 
To view, visit https://gerrit.ovirt.org/63539
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1e9b84ebb68a881bed6a0c97a23ee4c6896434
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: tests: fix testGetBondingOptions

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: tests: fix testGetBondingOptions
..


Patch Set 1: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/63541
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If60c2e4fadf470e2325eb6014a6ddf051499b3ee
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: net tests: fix test_events_keys

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: net tests: fix test_events_keys
..


Patch Set 1: Code-Review+2 Verified+1

test passes

-- 
To view, visit https://gerrit.ovirt.org/63539
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1e9b84ebb68a881bed6a0c97a23ee4c6896434
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: tests: introducting broken_on_ci decorator

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: tests: introducting broken_on_ci decorator
..


Patch Set 1: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/63540
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I837fe23c9fcd461dc305c4c26cad759f8efa9f94
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: net tests: fix test_events_keys

[danken]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: net tests: fix test_events_keys
..


net tests: fix test_events_keys

On Jenkins CI Fedora 23 this test sometimes fails because of missing
del_neigh event. It is not needed and we can drop it from expected
events.

Change-Id: I5a1e9b84ebb68a881bed6a0c97a23ee4c6896434
Signed-off-by: Petr Horáček 
Reviewed-on: https://gerrit.ovirt.org/63406
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg 
Bug-Url: https://bugzilla.redhat.com/1374328
Reviewed-on: https://gerrit.ovirt.org/63539
Tested-by: Dan Kenigsberg 
---
M tests/network/link_test.py
1 file changed, 0 insertions(+), 1 deletion(-)

Approvals:
  Jenkins CI: Passed CI tests
  Dan Kenigsberg: Verified; Looks good to me, approved



-- 
To view, visit https://gerrit.ovirt.org/63539
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I5a1e9b84ebb68a881bed6a0c97a23ee4c6896434
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: build: Don't use fixed path to systemd directory

[danken]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: build: Don't use fixed path to systemd directory
..


build: Don't use fixed path to systemd directory

Since commit 9d6f46d9 the project can no longer be built and installed
by non-root user. There is probably nothing wrong with the patch itself.
It's rather the fact that we use fixed path to the systemd directory
which prevents us from installing into some other prefix.

This patch changes the value of SYSTEMD_UNIT_DIR to use ${prefix}
which is normaly set to "/usr" during packaging.

Change-Id: I6306e2e449d3762a7b9dcfac58d09970fac83574
Signed-off-by: Tomáš Golembiovský 
Reviewed-on: https://gerrit.ovirt.org/63687
Continuous-Integration: Jenkins CI
Reviewed-by: Martin Polednik 
Reviewed-by: Francesco Romani 
---
M configure.ac
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Jenkins CI: Passed CI tests
  Francesco Romani: Looks good to me, approved
  Tomas Golembiovsky: Verified
  Martin Polednik: Looks good to me, but someone else must approve



-- 
To view, visit https://gerrit.ovirt.org/63687
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I6306e2e449d3762a7b9dcfac58d09970fac83574
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Tomas Golembiovsky 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: build: Don't use fixed path to systemd directory

[automation]

gerrit-hooks has posted comments on this change.

Change subject: build: Don't use fixed path to systemd directory
..


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

-- 
To view, visit https://gerrit.ovirt.org/63687
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6306e2e449d3762a7b9dcfac58d09970fac83574
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Tomas Golembiovsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storage: Unify stoage logger name

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: storage: Unify stoage logger name
..


Patch Set 2: Code-Review+2

Oops, I've merged the follow-up readme before taking this one. I hope it is 
verified soon.

-- 
To view, visit https://gerrit.ovirt.org/61261
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I653c41301393b5450de4ac6dee92b606a5f64838
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: vdsm: Rely on system for logrotation

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: vdsm: Rely on system for logrotation
..


Patch Set 1:

(4 comments)

https://gerrit.ovirt.org/#/c/63682/1//COMMIT_MSG
Commit Message:

Line 10: is no real reason to invoke logrotate manualy.
Line 11: 
Line 12: Also if we rely on system to run logrotate we can remove the dependency
Line 13: on it from RPMs. It is enough to install the configuration file into 
the
Line 14: the directory with logrotate configuration.
What if logrotate is not installed on the system?
Line 15: 
Line 16: The patch also renames script vdsm-logrotate to vdsm-remove-logs 
because
Line 17: it doesn't run logrotate anymore. It only removes old log files.
Line 18: 


https://gerrit.ovirt.org/#/c/63682/1/static/Makefile.am
File static/Makefile.am:

Line 36:./etc/vdsm/mom.d/04-cputune.policy \
Line 37:./etc/vdsm/mom.d/05-iotune.policy \
Line 38:$(NULL)
Line 39: 
Line 40: vdsmconfrotatedir = $(sysconfdir)/logrotate.d
This looks more like syslogrotatedir - nothing in this path is related to vdsm, 
so better update the name.
Line 41: 
Line 42: vdsmconfrotate_DATA = \
Line 43:./etc/logrotate.d/vdsm \
Line 44:$(NULL)


https://gerrit.ovirt.org/#/c/63682/1/vdsm.spec.in
File vdsm.spec.in:

Line 954: %{_tmpfilesdir}/%{vdsm_name}.conf
Line 955: %{_sysconfdir}/dhcp/dhclient.d/sourceRoute.sh
Line 956: %{_sysconfdir}/modprobe.d/vdsm-bonding-modprobe.conf
Line 957: %{_sysconfdir}/sudoers.d/50_vdsm
Line 958: %{_sysconfdir}/cron.hourly/vdsm-remove-logs
Why do we need to remove logs if logrotate is handling the logs?
Line 959: %{_sysconfdir}/libvirt/hooks/qemu
Line 960: %{_libexecdir}/%{vdsm_name}/curl-img-wrap
Line 961: %{_libexecdir}/%{vdsm_name}/fc-scan
Line 962: %{_libexecdir}/%{vdsm_name}/persist-vdsm-hooks


https://gerrit.ovirt.org/#/c/63682/1/vdsm/vdsm-remove-logs
File vdsm/vdsm-remove-logs:

Line 20: EXITVALUE=$?
Line 21: if [ $EXITVALUE != 0 ]; then
Line 22: /usr/bin/logger -t logrotate "ALERT clean of old import log 
files exited abnormally with [$EXITVALUE]"
Line 23: fi
Line 24: fi
Can we use logrotate for this?
Line 25: 


-- 
To view, visit https://gerrit.ovirt.org/63682
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica888244bd7c65121f55983e5716a6eae5662879
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net tests: raise specific MonitorError on IPv6 timeout

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net tests: raise specific MonitorError on IPv6 timeout
..


Patch Set 3: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/63486/3/tests/network/netinfo_test.py
File tests/network/netinfo_test.py:

Line 419: self.assertEqual(2, len(ip_addrs))
Line 420: self.assertTrue(addresses.is_ipv6(ip_addrs[0]))
Line 421: self.assertTrue(not addresses.is_dynamic(ip_addrs[0]))
Line 422: 
Line 423: @broken_on_ci(exception=monitor.MonitorError)
Who is raising it?
the waitfor is silent, it does not raise exceptions.
And as mentioned in previous patch, it is wrongly waiting for the server IP.
Line 424: @ValidateRunningAsRoot
Line 425: def test_local_auto_with_dynamic_address_from_ra(self):
Line 426: IPV6_NETADDRESS = '2001:1:1:1'
Line 427: IPV6_NETPREFIX_LEN = '64'


-- 
To view, visit https://gerrit.ovirt.org/63486
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4748a65b4781f5b2309eddc8e4d0bc0e18bb85c7
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net tests: replace wait_for_ipv6 with waitfor.waitfor_ipv6_addr

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net tests: replace wait_for_ipv6 with waitfor.waitfor_ipv6_addr
..


Patch Set 1: Code-Review-1

(2 comments)

https://gerrit.ovirt.org/#/c/63665/1/tests/network/netinfo_test.py
File tests/network/netinfo_test.py:

Line 429: IPV6_ADDR_CIDR = self._cidr_form(IPV6_ADDR, 
IPV6_NETPREFIX_LEN)
Line 430: ipwrapper.addrAdd(server, IPV6_ADDR, IPV6_NETPREFIX_LEN, 
family=6)
Line 431: ipwrapper.linkSet(server, ['up'])
Line 432: with dnsmasq_run(server, 
ipv6_slaac_prefix=IPV6_NETADDRESS + '::'):
Line 433: with waitfor.waitfor_ipv6_addr(client, 
address=IPV6_ADDR_CIDR):
You are waiting for the server IP on the client side?
Line 434: ipwrapper.linkSet(client, ['up'])
Line 435: 
Line 436: # Expecting link and global addresses on client iface
Line 437: # The addresses are given randomly, so we sort them


https://gerrit.ovirt.org/#/c/63665/1/tests/network/nettestlib.py
File tests/network/nettestlib.py:

Line 433
Line 434
Line 435
Line 436
Line 437
This one is a bit different from the one in waitfor.
It also waits for link scope (can be chosen) and for 'new_addr' event.
I guess we can add it to the waitfor version.


-- 
To view, visit https://gerrit.ovirt.org/63665
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3ec777a362519aa9240823181f809019b71aeea9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: api: use the new devel_warn helper

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: api: use the new devel_warn helper
..


Patch Set 2:

Is this needed now?

-- 
To view, visit https://gerrit.ovirt.org/62210
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idc85dde59e09a5882e9d586fb0d37d2434f0a351
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: vdsm: drop the warnings module configuration

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: vdsm: drop the warnings module configuration
..


Patch Set 3: Code-Review+2

CI failure not relevant.

-- 
To view, visit https://gerrit.ovirt.org/62212
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I791882a2dd8ba2dda9135b087bca33610db8a20d
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: logging: add and use new devel logger

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: logging: add and use new devel logger
..


Patch Set 3:

(2 comments)

https://gerrit.ovirt.org/#/c/62209/3//COMMIT_MSG
Commit Message:

Line 10: module doesn't really fit the needs of Vdsm.
Line 11: 
Line 12: This patch adds and make the code use a new logger, 'devel',
Line 13: set by default to ERROR level, to be used for the messages
Line 14: that only the developers should see.
Please show an example how to enable the warnings:

[logger_devel]
level=WARN

And how to enable warnings in runtime:

vdsClient -s 0 setLogLevel devel WARNING
Line 15: 
Line 16: Change-Id: I5061e78dde7aceffce9ae90fe5e2c2ad8c00f886
Line 17: Related-To: https://bugzilla.redhat.com/1364149
Line 18: Bug-Url: https://bugzilla.redhat.com/1369822


https://gerrit.ovirt.org/#/c/62209/3/lib/api/vdsmapi.py
File lib/api/vdsmapi.py:

Line 175: def _report_inconsistency(self, message):
Line 176: if self._strict_mode:
Line 177: raise JsonRpcInvalidParamsError(message)
Line 178: else:
Line 179: _devel.warning(message)
This dangerous - if the message will contain formatting placeholders, the call 
may raise. Best use:

_devel.warning("%s", message)
Line 180: 
Line 181: def verify_args(self, rep, args):
Line 182: try:
Line 183: # check whether there are extra parameters


-- 
To view, visit https://gerrit.ovirt.org/62209
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5061e78dde7aceffce9ae90fe5e2c2ad8c00f886
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net test: test_ip_info - wait for IP settings with address

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net test: test_ip_info - wait for IP settings with address
..


Patch Set 4: Verified+1

(1 comment)

https://gerrit.ovirt.org/#/c/63660/2/lib/vdsm/network/netlink/waitfor.py
File lib/vdsm/network/netlink/waitfor.py:

Line 41: return
Line 42: 
Line 43: 
Line 44: @contextmanager
Line 45: def waitfor_ipv4_addr(iface, address=None, timeout=10):
> please add a comment that :address: must be in CIDR form (if prefix length 
Done
Line 46: """
Line 47: Silently block until an ipv4 global scope address message is 
detected from
Line 48: the kernel (through netlink).
Line 49: :param iface: The device name.


-- 
To view, visit https://gerrit.ovirt.org/63660
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1c2832c1ec7614e1df9c291a9f3ab2e4a1134242
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net test: test_ip_info - wait for IP settings with address

[automation]

gerrit-hooks has posted comments on this change.

Change subject: net test: test_ip_info - wait for IP settings with address
..


Patch Set 4:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63660
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1c2832c1ec7614e1df9c291a9f3ab2e4a1134242
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storage: outOfProcess: remove useless warning

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: outOfProcess: remove useless warning
..


Patch Set 1: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/63702
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I479ab4952d85abcde18f6472b8ca0e019c59bdb8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storage: fileUtils: drop redundant warning

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: fileUtils: drop redundant warning
..


Patch Set 3: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/62208
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ae070aa613b63897b140837cb68c1a6134947a8
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jobs: Move run from sdm.base.Job to jobs.Job

[automation]

gerrit-hooks has posted comments on this change.

Change subject: jobs: Move run from sdm.base.Job to jobs.Job
..


Patch Set 7:

* Update tracker: IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

-- 
To view, visit https://gerrit.ovirt.org/62001
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I194eb154d70e846b7ec5d462faa26b3aa35d2ff6
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jobs: Move run from sdm.base.Job to jobs.Job

[nsoffer]

Nir Soffer has submitted this change and it was merged.

Change subject: jobs: Move run from sdm.base.Job to jobs.Job
..


jobs: Move run from sdm.base.Job to jobs.Job

The run implementation in sdm.base.Job is generally useful.  It performs
state checking and error handling that can be applicable to all job
types.  Move it to job.Job.  The tests associated with this
functionality need to move to jobsTests.py which means we no longer need
storage_sdm_api_test.py or sdmtestlib.py

Change-Id: I194eb154d70e846b7ec5d462faa26b3aa35d2ff6
Signed-off-by: Adam Litke 
Reviewed-on: https://gerrit.ovirt.org/62001
Reviewed-by: Nir Soffer 
Continuous-Integration: Jenkins CI
---
M lib/vdsm/jobs.py
M tests/Makefile.am
M tests/jobsTests.py
D tests/sdmtestlib.py
D tests/storage_sdm_api_test.py
M tests/storage_sdm_copy_data_test.py
M tests/storage_sdm_create_volume_test.py
M tests/testlib.py
M vdsm/storage/sdm/api/base.py
9 files changed, 64 insertions(+), 119 deletions(-)

Approvals:
  Nir Soffer: Looks good to me, approved
  Adam Litke: Verified
  Jenkins CI: Passed CI tests



-- 
To view, visit https://gerrit.ovirt.org/62001
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I194eb154d70e846b7ec5d462faa26b3aa35d2ff6
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jobs: Move run from sdm.base.Job to jobs.Job

[automation]

gerrit-hooks has posted comments on this change.

Change subject: jobs: Move run from sdm.base.Job to jobs.Job
..


Patch Set 6:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/62001
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I194eb154d70e846b7ec5d462faa26b3aa35d2ff6
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: sdm: Drop support for thread-local job_id

[automation]

gerrit-hooks has posted comments on this change.

Change subject: sdm: Drop support for thread-local job_id
..


Patch Set 5:

* Update tracker: IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

-- 
To view, visit https://gerrit.ovirt.org/62083
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4697f2d3f5e6826e0e95e03a10445c2df2abddf
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: sdm: Drop support for thread-local job_id

[nsoffer]

Nir Soffer has submitted this change and it was merged.

Change subject: sdm: Drop support for thread-local job_id
..


sdm: Drop support for thread-local job_id

sdm.api.base.Job currently has support for setting the job_id in the
thread-local variables.  We are not using this feature so let's remove
it and simplify our _run code and tests.  This will also _run (and
associated tests) to move to the general vdsm.jobs.Job class without
dragging in storage-specific interfaces.

Change-Id: Ia4697f2d3f5e6826e0e95e03a10445c2df2abddf
Signed-off-by: Adam Litke 
Reviewed-on: https://gerrit.ovirt.org/62083
Reviewed-by: Nir Soffer 
Continuous-Integration: Jenkins CI
---
M tests/storage_sdm_api_test.py
M vdsm/storage/sdm/api/base.py
2 files changed, 0 insertions(+), 8 deletions(-)

Approvals:
  Nir Soffer: Looks good to me, approved
  Adam Litke: Verified
  Jenkins CI: Passed CI tests



-- 
To view, visit https://gerrit.ovirt.org/62083
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia4697f2d3f5e6826e0e95e03a10445c2df2abddf
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: vdsm: Pass scheduler to jobs.start

[nsoffer]

Nir Soffer has submitted this change and it was merged.

Change subject: vdsm: Pass scheduler to jobs.start
..


vdsm: Pass scheduler to jobs.start

In a subsequent patch the jobs module will want to defer calls to delete
by using the program's scheduler instance.  Pass this scheduler into the
jobs module when calling start().

Change-Id: I5004b49b992ae0f7171a0f0549465cc8782d05da
Signed-off-by: Adam Litke 
Reviewed-on: https://gerrit.ovirt.org/62000
Continuous-Integration: Jenkins CI
Reviewed-by: Nir Soffer 
Tested-by: Nir Soffer 
---
M lib/vdsm/jobs.py
M tests/jobsTests.py
M vdsm/vdsm
3 files changed, 6 insertions(+), 4 deletions(-)

Approvals:
  Nir Soffer: Verified; Looks good to me, approved
  Jenkins CI: Passed CI tests



-- 
To view, visit https://gerrit.ovirt.org/62000
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I5004b49b992ae0f7171a0f0549465cc8782d05da
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: vdsm: Pass scheduler to jobs.start

[automation]

gerrit-hooks has posted comments on this change.

Change subject: vdsm: Pass scheduler to jobs.start
..


Patch Set 5:

* Update tracker: IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

-- 
To view, visit https://gerrit.ovirt.org/62000
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5004b49b992ae0f7171a0f0549465cc8782d05da
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: vdsm: Fix stopping order

[automation]

gerrit-hooks has posted comments on this change.

Change subject: vdsm: Fix stopping order
..


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

-- 
To view, visit https://gerrit.ovirt.org/63662
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieef1797a864536b2cbe22729ad3882235d97c4ac
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: vdsm: Fix stopping order

[nsoffer]

Nir Soffer has submitted this change and it was merged.

Change subject: vdsm: Fix stopping order
..


vdsm: Fix stopping order

The jobs module uses the scheduler so it should be stopped before the
scheduler.

Change-Id: Ieef1797a864536b2cbe22729ad3882235d97c4ac
Signed-off-by: Nir Soffer 
Reviewed-on: https://gerrit.ovirt.org/63662
Continuous-Integration: Jenkins CI
Reviewed-by: Adam Litke 
---
M vdsm/vdsm
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Nir Soffer: Verified
  Adam Litke: Looks good to me, approved
  Jenkins CI: Passed CI tests



-- 
To view, visit https://gerrit.ovirt.org/63662
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ieef1797a864536b2cbe22729ad3882235d97c4ac
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: vdsm: Pass scheduler to jobs.start

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: vdsm: Pass scheduler to jobs.start
..


Patch Set 4: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/62000
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5004b49b992ae0f7171a0f0549465cc8782d05da
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: vdsm: Fix stopping order

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: vdsm: Fix stopping order
..


Patch Set 1: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/63662
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieef1797a864536b2cbe22729ad3882235d97c4ac
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: lib: vdscli: switch to the new devel_warn helper

[automation]

gerrit-hooks has posted comments on this change.

Change subject: lib: vdscli: switch to the new devel_warn helper
..


Patch Set 2:

* #1369822::Update tracker: OK

-- 
To view, visit https://gerrit.ovirt.org/62211
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1189097826e79290cb996657ccf34a7c2b611ee0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storage: outOfProcess: remove useless warning

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: outOfProcess: remove useless warning
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/63702
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I479ab4952d85abcde18f6472b8ca0e019c59bdb8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: lib: vdscli: switch to the new devel_warn helper

[fromani]

Francesco Romani has abandoned this change.

Change subject: lib: vdscli: switch to the new devel_warn helper
..


Abandoned

squashed in https://gerrit.ovirt.org/#/c/62209/

-- 
To view, visit https://gerrit.ovirt.org/62211
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: I1189097826e79290cb996657ccf34a7c2b611ee0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: vdsm: drop the warnings module configuration

[automation]

gerrit-hooks has posted comments on this change.

Change subject: vdsm: drop the warnings module configuration
..


Patch Set 3:

* #1369822::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1369822::OK, public bug
* Check Product::#1369822::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/62212
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I791882a2dd8ba2dda9135b087bca33610db8a20d
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: logging: add and use new devel logger

[automation]

gerrit-hooks has posted comments on this change.

Change subject: logging: add and use new devel logger
..


Patch Set 3:

* #1369822::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1369822::OK, public bug
* Check Product::#1369822::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/62209
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5061e78dde7aceffce9ae90fe5e2c2ad8c00f886
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storage: fileUtils: drop redundant warning

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: fileUtils: drop redundant warning
..


Patch Set 3:

* #1369822::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1369822::OK, public bug
* Check Product::#1369822::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/62208
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ae070aa613b63897b140837cb68c1a6134947a8
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storage: outOfProcess: remove useless warning

[fromani]

Francesco Romani has uploaded a new change for review.

Change subject: storage: outOfProcess: remove useless warning
..

storage: outOfProcess: remove useless warning

This usage of warnings module add no value, so we drop it

Change-Id: I479ab4952d85abcde18f6472b8ca0e019c59bdb8
Signed-off-by: Francesco Romani 
---
M vdsm/storage/outOfProcess.py
1 file changed, 0 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/02/63702/1

diff --git a/vdsm/storage/outOfProcess.py b/vdsm/storage/outOfProcess.py
index c6b2cee..4d431f5 100644
--- a/vdsm/storage/outOfProcess.py
+++ b/vdsm/storage/outOfProcess.py
@@ -28,7 +28,6 @@
 import weakref
 
 from functools import partial
-from warnings import warn
 
 from ioprocess import IOProcess
 
@@ -237,9 +236,6 @@
 except OSError as e:
 if e.errno != errno.ENOTEMPTY:
 raise
-
-warn("Renaming a non-empty directory is not an atomic operation",
- DeprecationWarning)
 
 _IOProcessFileUtils(self._iop).cleanupdir(newpath, False)
 self.mkdir(newpath)


-- 
To view, visit https://gerrit.ovirt.org/63702
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I479ab4952d85abcde18f6472b8ca0e019c59bdb8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: virt: Make DomainDescriptor use XML helpers

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: virt: Make DomainDescriptor use XML helpers
..


Patch Set 8:

(9 comments)

https://gerrit.ovirt.org/#/c/55769/8/vdsm/virt/vmxml.py
File vdsm/virt/vmxml.py:

Line 78: result = [element] + result
Line 79: return result
Line 80: 
Line 81: 
Line 82: def dom_find_tag(element, tag, index=0):
> TL;DR:
I'm not convinced by the argument about common patterns in client code.

First, api should be created for any code, not the code we currently have, and 
second, having lot of code using bad return None pattern Is even worse.

Finally, the module api should be consistent, we don't want to have both 
find_xxx and require_xxx for every helper in this module or some of the 
helpers. We should use consistent pattern for all helpers. They can all return 
None when there is no result, or raise NotFound.

A possible way to get both behaviours in a pythonic way is to have api like 
getattr(obj, name, default). If you want to ignore missing result, you use None 
or False in the default.
Line 83: """
Line 84: Find a DOM element specified by given arguments.
Line 85: 
Line 86: :param element: DOM object to be searched for given `tag`


Line 98: except IndexError:
Line 99: return None
Line 100: 
Line 101: 
Line 102: def dom_find_attr(element, tag, attribute=True, index=0):
> Same here
Why have an api accepting element when we want to find the attribute of a sub 
element? The caller should get the sub element and get the attribute of the sub 
element.
Line 103: """
Line 104: Find attribute values of a DOM element specified by given 
arguments.
Line 105: 
Line 106: :param element: DOM object to be searched for given `tag`


Line 125: if subelement is None:
Line 126: if isinstance(attribute, basestring):
Line 127: return ''
Line 128: else:
Line 129: return {}
Why we should care about argument type? Why should be return different return 
value, when both of them are "empty"?
Line 130: return dom_attr(subelement, attribute)
Line 131: 
Line 132: 
Line 133: def dom_tag(element):


Line 139: :returns: tag of the element
Line 140: :rtype: basestring
Line 141: 
Line 142: """
Line 143: return element.tagName
Why do we need this? what is the etree way that this should hide?
Line 144: 
Line 145: 
Line 146: def dom_attr(element, attribute):
Line 147: """


Line 151: :type element: DOM object
Line 152: :param attribute: attribute name to look for, or a sequence of 
attribute
Line 153:   names to look for, or True to return all attribute values of 
the given
Line 154:   element
Line 155: :type attribute: basestring, or sequence of basestrings, or True
Please accept only one type for attribute, this does not make any sense. Accept 
either string or list or boolean. Looks like you are trying to mix several 
helpers into one.
Line 156: :returns: the corresponding attribute values or empty string (if
Line 157:   `attribute` is basestring and no corresponding attribute is 
found)
Line 158: :rtype: basestring if `attribute` is string; dictionary of
Line 159:   attribute names as keys and their corresponding values otherwise


Line 159:   attribute names as keys and their corresponding values otherwise
Line 160: 
Line 161: """
Line 162: if attribute is True:
Line 163: result = {a: dom_attr(element, a) for a in 
element.attributes.keys()}
The user can call us with element.attributes.keys(), or we can just return all 
attributes if the user did not specify anything.
Line 164: elif isinstance(attribute, (list, tuple,)):
Line 165: result = {a: dom_attr(element, a) for a in attribute}
Line 166: else:
Line 167: # Minidom returns unicodes, except for empty strings.


Line 164: elif isinstance(attribute, (list, tuple,)):
Line 165: result = {a: dom_attr(element, a) for a in attribute}
Line 166: else:
Line 167: # Minidom returns unicodes, except for empty strings.
Line 168: result = element.getAttribute(attribute)
Accepting *attributes will make this helpr much more consistent and easy to use:

def dom_attrs(element, *attributes):
if not attributes:
attributes = element.attributes.keys()
return {a: dom_attr(element, a) for a in attributes}

This always return a dict, with some of the attributes and values you ask.
Line 169: return result
Line 170: 
Line 171: 
Line 172: def dom_text(element):


Line 180: :rtype: basestring or NoneType
Line 181: 
Line 182: """
Line 183: if element is None:
Line 184: return None
This is very bad idea, result of having functions returning None. The caller 
should check the value before calling.
Line 185: child_node = element.firstChild
Line 186: if child_node is None:
Line 187: return None
Line 188: 

Change in vdsm[master]: tests: Add GuardedLocks checking utility

[automation]

gerrit-hooks has posted comments on this change.

Change subject: tests: Add GuardedLocks checking utility
..


Patch Set 7:

* Update tracker: IGNORE, no Bug-Url found

-- 
To view, visit https://gerrit.ovirt.org/62467
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9a2b78e6439b75cd60f99f8e756306297cfe51c0
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: tests: Add GuardedLocks checking utility

[alitke]

Adam Litke has abandoned this change.

Change subject: tests: Add GuardedLocks checking utility
..


Abandoned

Not needed.

-- 
To view, visit https://gerrit.ovirt.org/62467
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: I9a2b78e6439b75cd60f99f8e756306297cfe51c0
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: api: use the new devel_warn helper

[fromani]

Francesco Romani has posted comments on this change.

Change subject: api: use the new devel_warn helper
..


Patch Set 2:

(2 comments)

https://gerrit.ovirt.org/#/c/62210/2/lib/api/vdsmapi.py
File lib/api/vdsmapi.py:

Line 44
Line 45
Line 46
Line 47
Line 48
> We use developer warnings only in this module, so better localize the solut
Agreed, done.


Line 172: def _report_inconsistency(self, message):
Line 173: if self._strict_mode:
Line 174: raise JsonRpcInvalidParamsError(message)
Line 175: else:
Line 176: devel_warn(message)
> We can use the _devel logger here:
Done
Line 177: 
Line 178: def verify_args(self, rep, args):
Line 179: try:
Line 180: # check whether there are extra parameters


-- 
To view, visit https://gerrit.ovirt.org/62210
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idc85dde59e09a5882e9d586fb0d37d2434f0a351
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: lib: logUtils: add devel_warn function

[fromani]

Francesco Romani has posted comments on this change.

Change subject: lib: logUtils: add devel_warn function
..


Patch Set 2:

devel_warn is too ugly, I rushed this patch too early, to not forget about this 
issue. Let's use the logger approach.

-- 
To view, visit https://gerrit.ovirt.org/62209
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5061e78dde7aceffce9ae90fe5e2c2ad8c00f886
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: vdsm: Fix stopping order

[alitke]

Adam Litke has posted comments on this change.

Change subject: vdsm: Fix stopping order
..


Patch Set 1: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/63662
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieef1797a864536b2cbe22729ad3882235d97c4ac
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: vdsm: Rely on system for logrotation

[fromani]

Francesco Romani has posted comments on this change.

Change subject: vdsm: Rely on system for logrotation
..


Patch Set 1: Code-Review+1

initial review

-- 
To view, visit https://gerrit.ovirt.org/63682
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica888244bd7c65121f55983e5716a6eae5662879
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: util: NoIntrPoll - replace time() with monotonic_time()

[automation]

gerrit-hooks has posted comments on this change.

Change subject: util: NoIntrPoll - replace time() with monotonic_time()
..


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

-- 
To view, visit https://gerrit.ovirt.org/63661
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15dc7cc82ec196644612a1199d372c70913f41b2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org