Change in vdsm[master]: virt net: Change graphics type from network to address

[automation]

gerrit-hooks has posted comments on this change.

Change subject: virt net: Change graphics type from network to 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/59336
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0724627bd6c37ce11f943c8818fcd5b559fdf070
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: Martin Polednik 
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]: virt net: Change graphics type from network to address

[edwardh]

Edward Haas has posted comments on this change.

Change subject: virt net: Change graphics type from network to address
..


Patch Set 4:

(3 comments)

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

PS3, Line 13: effor
> effort
Done


https://gerrit.ovirt.org/#/c/59336/3/vdsm/virt/vmdevices/graphics.py
File vdsm/virt/vmdevices/graphics.py:

PS3, Line 148: vice
> vice
Done


Line 147: # and all other hosts in the cluster have the migration hook 
installed.
Line 148: # The migration hook is responsible to convert ip to net and 
vice versa
Line 149: display_network = self.specParams.get('displayNetwork')
Line 150: if (display_network and
Line 151: self.specParams.get('displayIp', '0') != '0' and
> displayIp is obtained from libvirt_networks().
Right, I guess you wanted to point out that we do not have ovs networks as 
libvirt networks.
Well, except for a small bug in _getNetworkIP, if it is not found, the network 
name is used as the iface/dev. (done originally to support bridgeless networks, 
and now extended for our ovs case where the libvirt network is not found).

Please let me know if you find a problem with this logic.
Line 152: supervdsm.getProxy().ovs_bridge(display_network)):
Line 153: graphics.appendChildWithArgs(
Line 154: 'listen', type='address', 
address=self.specParams['displayIp'])
Line 155: elif display_network:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0724627bd6c37ce11f943c8818fcd5b559fdf070
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: Martin Polednik 
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 virt: Support connecting VM/s to an ovs bridge

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net virt: Support connecting VM/s to an ovs bridge
..


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/57508/6/tests/deviceTests.py
File tests/deviceTests.py:

PS6, Line 430: ovirtmgmt
> we should pass something else here to test if MockedProxy.ovs_bridge() was 
Sorry, I do not follow.
Obviously it is used, otherwise I would not need to mock getProxy and the 
expected result (interfaceXML) would not match actual.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id233c3b46f0d3a546d1524456fc681d16873ad50
Gerrit-PatchSet: 6
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[ovirt-4.0]: api: Use warnings to report schema inconsistencies

[Jenkins CI]

Jenkins CI has posted comments on this change.

Change subject: api: Use warnings to report schema inconsistencies
..


Patch Set 2: Continuous-Integration+1

Propagate review hook: Continuous Integration value inherited from patch 1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5077d77b3b0e18edafa5f85073feb39cb87448cb
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Nir Soffer 
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[ovirt-4.0]: api: Use warnings to report schema inconsistencies

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: api: Use warnings to report schema inconsistencies
..


Patch Set 2: Verified+1

Verified on master.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5077d77b3b0e18edafa5f85073feb39cb87448cb
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Nir Soffer 
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[ovirt-4.0]: api: Use warnings to report schema inconsistencies

[automation]

gerrit-hooks has posted comments on this change.

Change subject: api: Use warnings to report schema inconsistencies
..


Patch Set 2: -Verified

* #1348256::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1348256::OK, public bug
* Check Product::#1348256::OK, Correct classification oVirt
* Check TM::#1348256::ERROR, wrong target milestone for stable branch, --- 
should match ^.*4.0.*
* Check merged to previous::OK, change not open on any previous branch

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5077d77b3b0e18edafa5f85073feb39cb87448cb
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Jenkins CI
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]: api: Use warnings to report schema inconsistencies

[automation]

gerrit-hooks has posted comments on this change.

Change subject: api: Use warnings to report schema inconsistencies
..


Patch Set 1: Verified-1

* Update tracker: IGNORE, no Bug-Url found

* Check Bug-Url::ERROR, At least one bug-url is required for the stable branch
* Check merged to previous::OK, change not open on any previous branch

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5077d77b3b0e18edafa5f85073feb39cb87448cb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: 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]: api: Use warnings to report schema inconsistencies

[nsoffer]

Hello Piotr Kliczewski,

I'd like you to do a code review.  Please visit

https://gerrit.ovirt.org/59512

to review the following change.

Change subject: api: Use warnings to report schema inconsistencies
..

api: Use warnings to report schema inconsistencies

Previously we spammed the log with warnings about conflicts between the
schema and the real usage of the apis. These warnings are useful for
developers but have negative value for users.

Now we use warnings.warn instead of logging.warning, so they do not
appear in the log by default. To enable warnings in the log, change the
configuration:

[devel]
python_warnings_enable = true

When enabled, the warnings are little more verbose now, using Python
standard warning style:

jsonrpc.Executor/2::WARNING::2016-06-01
22:23:55,551::vdsmapi::148::py.warnings::(_report_inconsistency)
/usr/lib/python2.7/site-packages/api/vdsmapi.py:148: Inconsistency: No
default value specified for systemProductName parameter in
Host.getHardwareInfo
  warnings.warn(message, Inconsistency)

Change-Id: I5077d77b3b0e18edafa5f85073feb39cb87448cb
Signed-off-by: Nir Soffer 
Reviewed-on: https://gerrit.ovirt.org/58494
Continuous-Integration: Jenkins CI
Reviewed-by: Piotr Kliczewski 
---
M lib/api/vdsmapi.py
1 file changed, 6 insertions(+), 1 deletion(-)


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

diff --git a/lib/api/vdsmapi.py b/lib/api/vdsmapi.py
index a7a85c1..73ce548 100644
--- a/lib/api/vdsmapi.py
+++ b/lib/api/vdsmapi.py
@@ -23,6 +23,7 @@
 import logging
 import os
 import six
+import warnings
 import yaml
 
 from vdsm.config import config
@@ -54,6 +55,10 @@
 
 class MethodNotFound(Exception):
 pass
+
+
+class Inconsistency(Warning):
+""" API usage conflicts with the schema """
 
 
 def find_schema(schema_name='vdsm-api'):
@@ -140,7 +145,7 @@
 if self._strict_mode:
 raise JsonRpcInvalidParamsError(message)
 else:
-self.log.warning(message)
+warnings.warn(message, Inconsistency)
 
 def verify_args(self, class_name, method_name, args):
 try:


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5077d77b3b0e18edafa5f85073feb39cb87448cb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: rpc: Remove empty command infos

[nsoffer]

Nir Soffer has uploaded a new change for review.

Change subject: rpc: Remove empty command infos
..

rpc: Remove empty command infos

Empty command info is useless; leaving it in the code lead to creation
of more of these.

Change-Id: I86048271e462c107043c52bf1c635fb14c20d774
Signed-off-by: Nir Soffer 
---
M lib/vdsm/rpc/Bridge.py
1 file changed, 0 insertions(+), 3 deletions(-)


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

diff --git a/lib/vdsm/rpc/Bridge.py b/lib/vdsm/rpc/Bridge.py
index 2187684..af9e6f1 100644
--- a/lib/vdsm/rpc/Bridge.py
+++ b/lib/vdsm/rpc/Bridge.py
@@ -343,9 +343,6 @@
 'Host_getStorageDomains': {'ret': 'domlist'},
 'Host_getStorageRepoStats': {'ret': Host_getStorageRepoStats_Ret},
 'Host_hostdevListByCaps': {'ret': 'deviceList'},
-'Host_hostdevChangeNumvfs': {},
-'Host_startMonitoringDomain': {},
-'Host_stopMonitoringDomain': {},
 'Host_getVMList': {'call': Host_getVMList_Call, 'ret': 'vmList'},
 'Host_getVMFullList': {'call': Host_getVMFullList_Call, 'ret': 'vmList'},
 'Host_getAllVmStats': {'ret': 'statsList'},


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

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

Change in vdsm[master]: rpc: Remove empty command infos

[automation]

gerrit-hooks has posted comments on this change.

Change subject: rpc: Remove empty command infos
..


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/59511
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I86048271e462c107043c52bf1c635fb14c20d774
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: 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: Add inplace virt-sparsify support

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Add inplace virt-sparsify support
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/57347/5/lib/vdsm/rpc/Bridge.py
File lib/vdsm/rpc/Bridge.py:

Line 358: 'Image_download': {'ret': 'uuid'},
Line 359: 'Image_mergeSnapshots': {'ret': 'uuid'},
Line 360: 'Image_move': {'ret': 'uuid'},
Line 361: 'Image_reconcileVolumeChain': {'ret': 'volumes'},
Line 362: 'Image_sparsifyInplace': {},
> empty are not needed. It works fine without them.
Thanks, posted https://gerrit.ovirt.org/59511
Line 363: 'Image_syncData': {'ret': 'uuid'},
Line 364: 'Image_upload': {'ret': 'uuid'},
Line 365: 'Image_prepare': {'ret': Image_prepare_Ret},
Line 366: 'ISCSIConnection_discoverSendTargets': {'ret': 'fullTargets'},


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ac2bb1fbd2acbe0fc47694d17313c6ccd01a227
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shmuel Melamud 
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]: vdsm.conf: Add drop-in dir

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: vdsm.conf: Add drop-in dir
..


Patch Set 18:

(19 comments)

https://gerrit.ovirt.org/#/c/58728/18/lib/vdsm/config.py.in
File lib/vdsm/config.py.in:

Line 506: print(twp.fill("%s = %s" % (key, value)))
Line 507: print('')
Line 508: 
Line 509: 
Line 510: def load(name='vdsm'):
Why do we need a default value?
Line 511: cfg = configparser.ConfigParser()
Line 512: set_defaults(cfg)
Line 513: read_configs(cfg, name)
Line 514: return cfg


Line 537: dropins.sort(key=os.path.basename)
Line 538: cfg.read(dropins)
Line 539: 
Line 540: 
Line 541: config = load('vdsm')
If we have a default, we don't need to use it here.
Line 542: 
Line 543: if __name__ == '__main__':


https://gerrit.ovirt.org/#/c/58728/18/lib/vdsm/tool/configurators/libvirt.py
File lib/vdsm/tool/configurators/libvirt.py:

Line 1
Line 2
The changes in this file are not related to supporting dropins. Please separate 
this clean to another patch before the dropin patch. This patch is too big and 
big patches are harder to review and easier to break stuff.


Line 55
Line 56
Line 57
Line 58
Line 59
Not clear what was wrong with the old code.


Line 38: requires = frozenset(('certificates',))
Line 39: 
Line 40: services = ("vdsmd", "supervdsmd", "libvirtd")
Line 41: 
Line 42: _vdsm_cfg = config
Why do we need this? I think we should access vdsm config in in the same way we 
access it in any other vdsm code - using config.getxxx.
Line 43: 
Line 44: 
Line 45: def _getFile(fname):
Line 46: return FILES[fname]['path']


Line 109: """
Line 110: return True if libvirt configuration files match ssl 
configuration of
Line 111: vdsm.conf.
Line 112: """
Line 113: ssl = _vdsm_cfg.getboolean('vars', 'ssl')
Same
Line 114: 
Line 115: lconf_p = ParserWrapper({
Line 116: 'listen_tcp': '0',
Line 117: 'auth_tcp': 'sasl',


https://gerrit.ovirt.org/#/c/58728/18/lib/vdsm/vdscli.py
File lib/vdsm/vdscli.py:

Line 77
Line 78
Line 79
Line 80
Line 81
Not sure why the old code was reading the config here when config was jst read 
when importing config, but this change is also not related to dropins, and 
should be splitted to the patch cleaning up libvirt configurator.


https://gerrit.ovirt.org/#/c/58728/18/tests/config_test.py
File tests/config_test.py:

Line 28: (
Line 29: 'test',
Line 30: [
Line 31: ('ssl', 'true', None),
Line 32: ('num', '0', None),
Why use boolean and integer? using strings we can make sure that each type of 
configuration is different, minimizing chance of errors in the tests.

For example, we can use:

- defaults: "default value"
- default conf: "default conf value"
- admin dropin: "admin value"

And so on. These tests will never pass by mistake with wrong code.
Line 33: ],
Line 34: )
Line 35: ]
Line 36: 


Line 36: 
Line 37: 
Line 38: @contextmanager
Line 39: def fakedirs():
Line 40: with namedTemporaryDir() as path:
path is not specific enough, use something like tmpdir.
Line 41: admin_dir = os.path.join(path, 'etc/')
Line 42: vendor_dir = os.path.join(path, 'usr/lib')
Line 43: runtime_dir = os.path.join(path, 'run/')
Line 44: paths = (admin_dir, vendor_dir, runtime_dir)


Line 37: 
Line 38: @contextmanager
Line 39: def fakedirs():
Line 40: with namedTemporaryDir() as path:
Line 41: admin_dir = os.path.join(path, 'etc/')
No need for trailing slash in paths.
Line 42: vendor_dir = os.path.join(path, 'usr/lib')
Line 43: runtime_dir = os.path.join(path, 'run/')
Line 44: paths = (admin_dir, vendor_dir, runtime_dir)
Line 45: with MonkeyPatchScope(


Line 38: @contextmanager
Line 39: def fakedirs():
Line 40: with namedTemporaryDir() as path:
Line 41: admin_dir = os.path.join(path, 'etc/')
Line 42: vendor_dir = os.path.join(path, 'usr/lib')
Use:

os.path.join(tmpdir, "usr", "lib")
Line 43: runtime_dir = os.path.join(path, 'run/')
Line 44: paths = (admin_dir, vendor_dir, runtime_dir)
Line 45: with MonkeyPatchScope(
Line 46: [(config, '_SYSCONFDIR', admin_dir),


Line 39: def fakedirs():
Line 40: with namedTemporaryDir() as path:
Line 41: admin_dir = os.path.join(path, 'etc/')
Line 42: vendor_dir = os.path.join(path, 'usr/lib')
Line 43: runtime_dir = os.path.join(path, 'run/')
Remove trailing slash
Line 44: paths = (admin_dir, vendor_dir, runtime_dir)
Line 45: with MonkeyPatchScope(
Line 46: [(config, '_SYSCONFDIR', admin_dir),
Line 47:  (config, '_DROPPIN_BASES', paths),


Line 40: with namedTemporaryDir() as path:
Line 41: admin_dir = os.path.join(path, 'etc/')
Line 42: vendor_dir = os.path.join(path, 'usr/lib')
Line 43: runtime_dir = os.path.join(path, 

Change in vdsm[master]: storage: Add inplace virt-sparsify support

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: storage: Add inplace virt-sparsify support
..


Patch Set 5:

(2 comments)

https://gerrit.ovirt.org/#/c/57347/5/client/vdsClient.py
File client/vdsClient.py:

Line 1386: if status['status']['code']:
Line 1387: return status['status']['code'], 
status['status']['message']
Line 1388: return 0, status['uuid']
Line 1389: 
Line 1390: def sparsifyImageInplace(self, args):
> Why add support in this deprecated client when we are not going to support 
We can remove it. Still if we wanted to have it here it would be not enough to 
add it.
Line 1391: (imgUUID, spUUID, sdUUID, volUUID) = args
Line 1392: status = self.s.sparsifyImageInplace(imgUUID, spUUID, 
sdUUID, volUUID)
Line 1393: if status['status']['code']:
Line 1394: return status['status']['code'], 
status['status']['message']


https://gerrit.ovirt.org/#/c/57347/5/lib/vdsm/rpc/Bridge.py
File lib/vdsm/rpc/Bridge.py:

Line 358: 'Image_download': {'ret': 'uuid'},
Line 359: 'Image_mergeSnapshots': {'ret': 'uuid'},
Line 360: 'Image_move': {'ret': 'uuid'},
Line 361: 'Image_reconcileVolumeChain': {'ret': 'volumes'},
Line 362: 'Image_sparsifyInplace': {},
> I hoped that you can tell me if these empty transformations are needed.
empty are not needed. It works fine without them.
Line 363: 'Image_syncData': {'ret': 'uuid'},
Line 364: 'Image_upload': {'ret': 'uuid'},
Line 365: 'Image_prepare': {'ret': Image_prepare_Ret},
Line 366: 'ISCSIConnection_discoverSendTargets': {'ret': 'fullTargets'},


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ac2bb1fbd2acbe0fc47694d17313c6ccd01a227
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shmuel Melamud 
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]: storage: Add inplace virt-sparsify support

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Add inplace virt-sparsify support
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/57347/5/client/vdsClient.py
File client/vdsClient.py:

Line 1386: if status['status']['code']:
Line 1387: return status['status']['code'], 
status['status']['message']
Line 1388: return 0, status['uuid']
Line 1389: 
Line 1390: def sparsifyImageInplace(self, args):
> We may want to have it for now. This method is not enough to add new verb s
Why add support in this deprecated client when we are not going to support it 
in bingindxmlrpc.py?
Line 1391: (imgUUID, spUUID, sdUUID, volUUID) = args
Line 1392: status = self.s.sparsifyImageInplace(imgUUID, spUUID, 
sdUUID, volUUID)
Line 1393: if status['status']['code']:
Line 1394: return status['status']['code'], 
status['status']['message']


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ac2bb1fbd2acbe0fc47694d17313c6ccd01a227
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shmuel Melamud 
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]: Send vm and host reports each sampling interval from HostMon...

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: Send vm and host reports each sampling interval from HostMonitor
..


Patch Set 3:

(2 comments)

https://gerrit.ovirt.org/#/c/59067/3/lib/vdsm/virt/sampling.py
File lib/vdsm/virt/sampling.py:

Line 549: sample = HostSample(self._pid)
Line 550: self._samples.append(sample)
Line 551: 
Line 552: if self._cif:
Line 553: tmp_hoststats = hostapi.get_stats(self._cif, 
self._samples.stats())
Why not call it stats? tmp_hoststats is too ugly.
Line 554: hostapi.report_stats(tmp_hoststats)
Line 555: tmp_vmstats = self._cif.getAllVmStats()
Line 556: vmstats.report_stats(tmp_vmstats)
Line 557: 


Line 551: 
Line 552: if self._cif:
Line 553: tmp_hoststats = hostapi.get_stats(self._cif, 
self._samples.stats())
Line 554: hostapi.report_stats(tmp_hoststats)
Line 555: tmp_vmstats = self._cif.getAllVmStats()
> this works, but I'm not sure it is the best solution. We want to send the r
This value is also suppressed, so it need special treatment to access the 
stats. It is also strange to report vm stats under the current hosts running 
them, vm can run on many hosts during its lifetime, and you probably care about 
the entire history.

I think we need to rethink how we report vms stats.
Line 556: vmstats.report_stats(tmp_vmstats)
Line 557: 
Line 558: second_last = self._samples.last(nth=2)
Line 559: if second_last is None:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I724199135b89f85ac8abe7dc8073412f4764bc51
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
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 test: Create an external ifcfg bond in a context

[phoracek]

Petr Horáček has posted comments on this change.

Change subject: net test: Create an external ifcfg bond in a context
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7295a9a1eafeef9a2643dd8f3aa64b25797d3810
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: 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]: Adding report_stats to virt.stats

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: Adding report_stats to virt.stats
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/59066/4/lib/vdsm/virt/vmstats.py
File lib/vdsm/virt/vmstats.py:

Line 214: 
Line 215: reports.send(report)
Line 216: except KeyError as e:
Line 217: logging.exception('Report host stat failed to report key: %s' 
%
Line 218:   (e.args[0],))
Same issue with e.args, see my comment here:
https://gerrit.ovirt.org/#/c/58661/9/lib/vdsm/host/api.py@137
Line 219: 
Line 220: 
Line 221: def _nic_traffic(vm_obj, name, model, mac,
Line 222:  start_sample, start_index,


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf494c6a3087d04c12731c587b619f253dd51165
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
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]: suppport: 3.5 not supported in 4.x

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: suppport: 3.5 not supported in 4.x
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3ea717cc7a5dab7bf6834da52cb65c7898f3ae1e
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: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Oved Ourfali 
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]: Adding report_stats to health reports

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: Adding report_stats to health reports
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I885720b0b70620469c985d0079c4cbe9da74edf6
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
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]: Adding report_stats to host.api

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: Adding report_stats to host.api
..


Patch Set 9: Code-Review-1

(5 comments)

https://gerrit.ovirt.org/#/c/58661/9/lib/vdsm/host/api.py
File lib/vdsm/host/api.py:

Line 21: 
Line 22: import errno
Line 23: import logging
Line 24: import time
Line 25: from . import stats, uuid
Please import one thing per line, make it easier to work with the code.
Line 26: from vdsm import cpuarch
Line 27: from vdsm import hooks
Line 28: from vdsm import utils
Line 29: from vdsm import reports


Line 122: if_info = hoststats['network'][interface]
Line 123: net_prefix = prefix + '.network_interfaces.' + 
interface
Line 124: report[net_prefix + '.speed'] = if_info['speed']
Line 125: report[net_prefix + '.rx_rate'] = if_info['rxRate']
Line 126: report[net_prefix + '.tx_rate'] = if_info['txRate']
rxRate and txRange are need only for old engine (< 3.6), we probably should not 
report them.
Line 127: report[net_prefix + '.rx_errors'] = 
if_info['rxErrors']
Line 128: report[net_prefix + '.tx_errors'] = 
if_info['txErrors']
Line 129: report[net_prefix + '.rx_dropped'] = 
if_info['rxDropped']
Line 130: report[net_prefix + '.tx_dropped'] = 
if_info['txDropped']


Line 133: 
Line 134: reports.send(report)
Line 135: except KeyError as e:
Line 136: logging.exception('Report host stat failed to report key: %s' 
%
Line 137:   (e.args[0],))
e.args[0] not needed, and bad idea in general, we should not make assumptions 
about exceptions attributes.

Since we are logging an exception, we will see this without any code:

KeyError: rxRate

So I think only need minimal context:

try:
...
except KeyError:
logging.exception("Reporting host stats failed")
Line 138: 
Line 139: 
Line 140: def _readSwapTotalFree():
Line 141: meminfo = utils.readMemInfo()


https://gerrit.ovirt.org/#/c/58661/9/lib/vdsm/host/stats.py
File lib/vdsm/host/stats.py:

Line 234
Line 235
Line 236
Line 237
Line 238
We are not supporting < 3.6 even in 4.0, you can remove these - better in 
another patch.


https://gerrit.ovirt.org/#/c/58661/9/tests/hoststatsTests.py
File tests/hoststatsTests.py:

Line 174
Line 175
Line 176
Line 177
Line 178
These are needed only by older engine (< 3.6), so we probably do not need to 
report them.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie65c865669422b7652b028492b1380157fc5e177
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
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: Create an external bond in a context

[phoracek]

Petr Horáček has posted comments on this change.

Change subject: net test: Create an external bond in a context
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I401fcbb8047c215300309c991415d263979f8b9e
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: 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]: Add hawkular reporter to reports

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: Add hawkular reporter to reports
..


Patch Set 8: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/58660/8/lib/vdsm/reports/hawkular.py
File lib/vdsm/reports/hawkular.py:

Line 74: with _cond:
Line 75: while not _queue:
Line 76: _cond.wait()
Line 77: while _queue:
Line 78: items = _queue.pop()
Should be popleft() - this removes the last element instead of the first.
Line 79: if items is _STOP:
Line 80: break
Line 81: client.put(items)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I71f538184855f7c58bba66acd7b6dea3a53db71b
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
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]: build: Make sure run_tests*.sh scripts are executable

[igoihman]

Irit Goihman has posted comments on this change.

Change subject: build: Make sure run_tests*.sh scripts are executable
..


Patch Set 5: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc1e3dc8ace7f69801b765262352903020cc8aef
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Tomas Golembiovsky 
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]: Introduce reports package

[automation]

gerrit-hooks has posted comments on this change.

Change subject: Introduce reports package
..


Patch Set 40:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I23c1141f097f740441d085f99e0bf76eb7f718c9
Gerrit-PatchSet: 40
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
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]: Introduce reports package

[nsoffer]

Nir Soffer has submitted this change and it was merged.

Change subject: Introduce reports package
..


Introduce reports package

reports logic allows to send report to external providers by start close
and send functions. The report itself is simply key value attribute.

This patch includes statsd reporter which implements the send method to
statsd listener by python-statsd.
To enable reports add the following to vdsm.conf:

[reports]
enabled = false|true (disabled by default)
collector_type = statsd (following patch adds more providers)
collector_address = localhost|any hostname (currently one destination)

We still miss python-stastd for python3 so we don't require the package
in spec. If metric collector is configured to be enable and the type
implementation is not available ImportError should be raised.

Change-Id: I23c1141f097f740441d085f99e0bf76eb7f718c9
Signed-off-by: Yaniv Bronhaim 
Reviewed-on: https://gerrit.ovirt.org/56880
Reviewed-by: Piotr Kliczewski 
Reviewed-by: Nir Soffer 
Reviewed-by: Martin Polednik 
Continuous-Integration: Jenkins CI
---
M configure.ac
M lib/vdsm/Makefile.am
M lib/vdsm/config.py.in
A lib/vdsm/reports/Makefile.am
A lib/vdsm/reports/__init__.py
A lib/vdsm/reports/statsd.py
M vdsm.spec.in
M vdsm/vdsm
8 files changed, 130 insertions(+), 1 deletion(-)

Approvals:
  Piotr Kliczewski: Looks good to me, approved
  Nir Soffer: Looks good to me, approved
  Yaniv Bronhaim: Verified
  Jenkins CI: Passed CI tests
  Martin Polednik: Looks good to me, but someone else must approve



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I23c1141f097f740441d085f99e0bf76eb7f718c9
Gerrit-PatchSet: 40
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
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]: sp: allow executing upgradePool even if there is a pending u...

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: sp: allow executing upgradePool even if there is a pending 
update
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/57188/3/vdsm/storage/sp.py
File vdsm/storage/sp.py:

Line 439: self.log.debug("Registering with state change event")
Line 440: self.domainMonitor.onDomainStateChange.register(
Line 441: self._upgradeCallback)
Line 442: self.log.debug("Running initial domain upgrade threads")
Line 443: for sdUUID in self._domainsToUpgrade:
> I've meant to move it out properly (by making a copy in the lock scope). Th
Not needed, if nobody can modify the list we don't need to make a copy. Lets 
minimize the changes.
Line 444: concurrent.thread(target=self._upgradeCallback,
Line 445:  args=(sdUUID, True),
Line 446:  kwargs={"__securityOverride": 
True}).start()
Line 447: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
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]: logstat: Fix build by distributing contrib/logstat

[automation]

gerrit-hooks has posted comments on this change.

Change subject: logstat: Fix build by distributing contrib/logstat
..


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/59507
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8a9a87fe94f9dcbba062a9aa50299311366681e6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: 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]: logstat: Fix build by distributing contrib/logstat

[nsoffer]

Nir Soffer has uploaded a new change for review.

Change subject: logstat: Fix build by distributing contrib/logstat
..

logstat: Fix build by distributing contrib/logstat

Commit 8eef20198a26 (logstat: Print statistics from vdsm log) added a
new script but did not add it to the main Makefile. For some reason
jenkins was happy with the original patch, but now it is failing.

Seems that the EXTRA_DIST list should move to contrib/Makefile to
prevent these errors in the future.

Change-Id: I8a9a87fe94f9dcbba062a9aa50299311366681e6
Signed-off-by: Nir Soffer 
---
M Makefile.am
1 file changed, 1 insertion(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/07/59507/1

diff --git a/Makefile.am b/Makefile.am
index 9ac2515..edf37a6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -43,6 +43,7 @@
autogen.sh \
build-aux/pkg-version \
contrib/logdb \
+   contrib/logstat \
contrib/profile-stats \
contrib/repoplot \
pylintrc \


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

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

Change in vdsm[master]: suppport: 3.5 not supported in 4.x

[michal . skrivanek]

Michal Skrivanek has posted comments on this change.

Change subject: suppport: 3.5 not supported in 4.x
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3ea717cc7a5dab7bf6834da52cb65c7898f3ae1e
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: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Oved Ourfali 
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]: api: Move after_get_all_vm_stats hook from API to clientIF

[automation]

gerrit-hooks has posted comments on this change.

Change subject: api: Move after_get_all_vm_stats hook from API to clientIF
..


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/59393
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf9cdf57e7fc4e72733b195acdd84a10602df7ca
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
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]: rpc: Log also error codes of RPC calls

[automation]

gerrit-hooks has posted comments on this change.

Change subject: rpc: Log also error codes of RPC calls
..


Patch Set 10:

* 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/59234
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I776e667fcfd1a20a9ef5f6c638d6c3d950672314
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
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]: rpc: Log RPC call summary on info level

[automation]

gerrit-hooks has posted comments on this change.

Change subject: rpc: Log RPC call summary on info level
..


Patch Set 12:

* 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/59080
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idde2f1ba7394f16770543f5ca13411e8c2339cc6
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
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]: logging: throttledlog introduced

[automation]

gerrit-hooks has posted comments on this change.

Change subject: logging: throttledlog introduced
..


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/59461
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief96f74eb84880827ccf24c3e7ae854403090b8a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
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]: rpc: Log important info from VM stats

[automation]

gerrit-hooks has posted comments on this change.

Change subject: rpc: Log important info from VM stats
..


Patch Set 24:

* #1339291::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1339291::OK, public bug
* Check Product::#1339291::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/58465
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcbac615323b62fb9a27e5c0f5a4e98990076146
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
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]: net: Setup ipv6autoconf with OVS switch

[phoracek]

Petr Horáček has posted comments on this change.

Change subject: net: Setup ipv6autoconf with OVS switch
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/59444/2/lib/vdsm/sysctl.py
File lib/vdsm/sysctl.py:

Line 80: def disable_ipv6_local_auto(dev):
Line 81: return _set_ipv6_local_auto(dev, False)
Line 82: 
Line 83: 
Line 84: def _set_ipv6_local_auto(dev, state):
why does the setter returns True/False?
Line 85: setstate = '1' if state else '0'
Line 86: try:
Line 87: if is_disabled_ipv6(dev):
Line 88: if state:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ab57857ff9f5c9ed582ae4aaacc3477f2fcd94d
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: 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]: sp: allow executing upgradePool even if there is a pending u...

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: sp: allow executing upgradePool even if there is a pending 
update
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/57188/5/vdsm/storage/sp.py
File vdsm/storage/sp.py:

Line 418: panic("Unrecoverable errors during SPM stop process.")
Line 419: 
Line 420: self.spmRole = SPM_FREE
Line 421: 
Line 422: def _upgradePool(self, targetDomVersion, lock_timeout=None):
> Agreed, I'll separate this to a separate change.
OK, lest leave the exception handling here.
Line 423: try:
Line 424: with rmanager.acquireResource(sc.STORAGE, "upgrade_" + 
self.spUUID,
Line 425:   rm.LockType.exclusive, 
timeout=lock_timeout):
Line 426: sd.validateDomainVersion(targetDomVersion)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
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: Support the ability to choose the tested switch

[phoracek]

Petr Horáček has posted comments on this change.

Change subject: net tests: Support the ability to choose the tested switch
..


Patch Set 9: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I06e230633dfefcdc4a4b545eb2139ffe190c6f35
Gerrit-PatchSet: 9
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]: sp: allow executing upgradePool even if there is a pending u...

[laravot]

Liron Aravot has posted comments on this change.

Change subject: sp: allow executing upgradePool even if there is a pending 
update
..


Patch Set 5:

(3 comments)

https://gerrit.ovirt.org/#/c/57188/3/vdsm/storage/sp.py
File vdsm/storage/sp.py:

Line 439: 
Line 440: self.log.debug("Registering with state change event")
Line 441: self.domainMonitor.onDomainStateChange.register(
Line 442: self._upgradeCallback)
Line 443: self.log.debug("Running initial domain upgrade 
threads")
> It should, otherwise the list may be modified while you iterated over it, w
I've meant to move it out properly (by making a copy in the lock scope). The 
reason to do is that when no upgrade thread will start till the loop ends and 
the lock will be released. neither of the options seems perfect - what do you 
think?
Line 444: # We need to copy the list as the domain monitor 
registered
Line 445: # callback and the initiated update threads may 
modify the list
Line 446: # while we iterate over it.
Line 447: # http://bugzilla.redhat.com/1319523


https://gerrit.ovirt.org/#/c/57188/5/vdsm/storage/sp.py
File vdsm/storage/sp.py:

Line 174: self.log.error("Error while preparing domain `%s` 
upgrade", sdUUID,
Line 175:exc_info=True)
Line 176: return
Line 177: 
Line 178: with rmanager.acquireResource(STORAGE, "upgrade_" + 
self.spUUID,
> Are you sure about this change? sc.XXX is now our common way to refer to st
its rebase foobar (as we still didn't use sc.XXX on patch #1). will return.
Line 179:   rm.LockType.shared):
Line 180: with rmanager.acquireResource(STORAGE, sdUUID,
Line 181:   rm.LockType.exclusive):
Line 182: if sdUUID not in self._domainsToUpgrade:


Line 418: panic("Unrecoverable errors during SPM stop process.")
Line 419: 
Line 420: self.spmRole = SPM_FREE
Line 421: 
Line 422: def _upgradePool(self, targetDomVersion, lock_timeout=None):
> I think we agreed that this should be a separate patch?
Agreed, I'll separate this to a separate change.


As regards to the second comment, I think its preferred to leave it here for 
several reasons:
1. We should have the same treatment for resource timeout when failing to 
acquire lock - if tomorrow we'll add a call to this method from another place 
(or start using timeout from the spmStart flow) we'll need to keep in mind to 
catch and raise the PoolUpgradeInProgres exception

2. Theoretically speaking, we could use another lock here which will require a 
different raised exception.

3. This method encapsulates the upgrade and the raised exceptions, if we stop 
using the rmanager and do a different check one day (as was done before), we'll 
need to update the callers.
Line 423: try:
Line 424: with rmanager.acquireResource(sc.STORAGE, "upgrade_" + 
self.spUUID,
Line 425:   rm.LockType.exclusive, 
timeout=lock_timeout):
Line 426: sd.validateDomainVersion(targetDomVersion)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
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]: virt net: Change graphics type from network to address

[phoracek]

Petr Horáček has posted comments on this change.

Change subject: virt net: Change graphics type from network to address
..


Patch Set 3: Code-Review-1

(3 comments)

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

PS3, Line 13: effor
effort


https://gerrit.ovirt.org/#/c/59336/3/vdsm/virt/vmdevices/graphics.py
File vdsm/virt/vmdevices/graphics.py:

PS3, Line 148: vise
vice


Line 147: # and all other hosts in the cluster have the migration hook 
installed.
Line 148: # The migration hook is responsible to convert ip to net and 
vise versa
Line 149: display_network = self.specParams.get('displayNetwork')
Line 150: if (display_network and
Line 151: self.specParams.get('displayIp', '0') != '0' and
displayIp is obtained from libvirt_networks().

take a look at _getNetworkIp()
Line 152: supervdsm.getProxy().ovs_bridge(display_network)):
Line 153: graphics.appendChildWithArgs(
Line 154: 'listen', type='address', 
address=self.specParams['displayIp'])
Line 155: elif display_network:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0724627bd6c37ce11f943c8818fcd5b559fdf070
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
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 virt: Support connecting VM/s to an ovs bridge

[phoracek]

Petr Horáček has posted comments on this change.

Change subject: net virt: Support connecting VM/s to an ovs bridge
..


Patch Set 6: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/57508/6/tests/deviceTests.py
File tests/deviceTests.py:

PS6, Line 430: ovirtmgmt
we should pass something else here to test if MockedProxy.ovs_bridge() was used.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id233c3b46f0d3a546d1524456fc681d16873ad50
Gerrit-PatchSet: 6
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]: suppport: 3.5 not supported in 4.x

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: suppport: 3.5 not supported in 4.x
..


Patch Set 2: Verified+1

This version removes the unused import.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3ea717cc7a5dab7bf6834da52cb65c7898f3ae1e
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: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Oved Ourfali 
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]: suppport: 3.5 not supported in 4.x

[automation]

gerrit-hooks has posted comments on this change.

Change subject: suppport: 3.5 not supported in 4.x
..


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/59504
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3ea717cc7a5dab7bf6834da52cb65c7898f3ae1e
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: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Oved Ourfali 
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]: suppport: 3.5 not supported in 4.x

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: suppport: 3.5 not supported in 4.x
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3ea717cc7a5dab7bf6834da52cb65c7898f3ae1e
Gerrit-PatchSet: 1
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: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Oved Ourfali 
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]: suppport: 3.5 not supported in 4.x

[automation]

gerrit-hooks has posted comments on this change.

Change subject: suppport: 3.5 not supported in 4.x
..


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/59504
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3ea717cc7a5dab7bf6834da52cb65c7898f3ae1e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: 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]: suppport: 3.5 not supported in 4.x

[nsoffer]

Nir Soffer has uploaded a new change for review.

Change subject: suppport: 3.5 not supported in 4.x
..

suppport: 3.5 not supported in 4.x

The 4.x release supports only Engine 3.6 and cluster level 3.6. Remove
3.5 from supported versions.

Since supported versions are the same for all platforms, remove the
special code for ppc64le.

Change-Id: I3ea717cc7a5dab7bf6834da52cb65c7898f3ae1e
Signed-off-by: Nir Soffer 
---
M lib/vdsm/dsaversion.py.in
1 file changed, 2 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/04/59504/1

diff --git a/lib/vdsm/dsaversion.py.in b/lib/vdsm/dsaversion.py.in
index f9c5649..104853f 100644
--- a/lib/vdsm/dsaversion.py.in
+++ b/lib/vdsm/dsaversion.py.in
@@ -35,9 +35,6 @@
 'version_name': version_name,
 'software_version': software_version,
 'software_revision': software_revision,
-'supportedENGINEs': ['3.5', '3.6', '4.0', '4.1'],
-'clusterLevels': ['3.5', '3.6', '4.0', '4.1'],
+'supportedENGINEs': ['3.6', '4.0', '4.1'],
+'clusterLevels': ['3.6', '4.0', '4.1'],
 }
-
-if cpuarch.real() == cpuarch.PPC64LE:
-version_info['clusterLevels'] = ['3.6', '4.0', '4.1']


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

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

Change in vdsm[master]: sp: _upgradeStoragePool - log domains pending for update

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: sp: _upgradeStoragePool - log domains pending for update
..


Patch Set 3:

ping

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2f0b9c262f4a671ff55ec1da12db2e10d1c9ee24
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
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]: sp: allow executing upgradePool even if there is a pending u...

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: sp: allow executing upgradePool even if there is a pending 
update
..


Patch Set 5:

(4 comments)

https://gerrit.ovirt.org/#/c/57188/5/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 3337: # This lock has to be mutual with the pool metadata 
operations (like
Line 3338: # activateSD/deactivateSD) as the operation uses the pool 
metadata.
Line 3339: vars.task.getExclusiveLock(STORAGE, spUUID)
Line 3340: pool = self.getPool(spUUID)
Line 3341: pool._upgradePool(targetDomVersion, lock_timeout=0)
Plase separate this to another patch. Tring to upgrade while another thread is 
holding the pool _upgrade lock and is stuck is a theoretical edge case, and we 
like to have small patches specially when we deal with delicate lockinging in 
legacy code.
Line 3342: return {"upgradeStatus": "started"}
Line 3343: 
Line 3344: def _getDomsStats(self, domainMonitor, doms):
Line 3345: domInfo = {}


https://gerrit.ovirt.org/#/c/57188/5/vdsm/storage/sp.py
File vdsm/storage/sp.py:

Line 349
Line 350
Line 351
Line 352
Line 353
Either this code is wrong (sc.STORAGE), or your new code (using STORAGE), or we 
have a useless import of STORAGE.


Line 174: self.log.error("Error while preparing domain `%s` 
upgrade", sdUUID,
Line 175:exc_info=True)
Line 176: return
Line 177: 
Line 178: with rmanager.acquireResource(STORAGE, "upgrade_" + 
self.spUUID,
Are you sure about this change? sc.XXX is now our common way to refer to 
storage constants, same as se.YYY for storage excetions.
Line 179:   rm.LockType.shared):
Line 180: with rmanager.acquireResource(STORAGE, sdUUID,
Line 181:   rm.LockType.exclusive):
Line 182: if sdUUID not in self._domainsToUpgrade:


Line 418: panic("Unrecoverable errors during SPM stop process.")
Line 419: 
Line 420: self.spmRole = SPM_FREE
Line 421: 
Line 422: def _upgradePool(self, targetDomVersion, lock_timeout=None):
I think we agreed that this should be a separate patch?

Also, we can make this much easier if we handle the error in the method calling 
this. 

Since we call with timeout=0 in hsm, and timeout=None will never raise, we can 
add the error handling in hsm.
Line 423: try:
Line 424: with rmanager.acquireResource(sc.STORAGE, "upgrade_" + 
self.spUUID,
Line 425:   rm.LockType.exclusive, 
timeout=lock_timeout):
Line 426: sd.validateDomainVersion(targetDomVersion)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
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: Move after_get_all_vm_stats hook from API to clientIF

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: api: Move after_get_all_vm_stats hook from API to clientIF
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/59393/1/vdsm/API.py
File vdsm/API.py:

Line 1339
Line 1340
Line 1341
Line 1342
Line 1343
> Agreed. The best long term fix IMO is https://gerrit.ovirt.org/#/c/53101/
We want to merge these logging patches now, we are not looking for long term 
solution.

I prefer to keep the log in API.py for now, and find a better place for hooking 
later.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf9cdf57e7fc4e72733b195acdd84a10602df7ca
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
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]: rpc: Log RPC call summary on info level

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: rpc: Log RPC call summary on info level
..


Patch Set 11:

(1 comment)

https://gerrit.ovirt.org/#/c/59080/11/lib/yajsonrpc/__init__.py
File lib/yajsonrpc/__init__.py:

Line 538
Line 539
Line 540
Line 541
Line 542
Needs a rebase to return the missing line here


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idde2f1ba7394f16770543f5ca13411e8c2339cc6
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
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]: rpc: Log also error codes of RPC calls

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: rpc: Log also error codes of RPC calls
..


Patch Set 9:

(7 comments)

https://gerrit.ovirt.org/#/c/59234/9/lib/yajsonrpc/__init__.py
File lib/yajsonrpc/__init__.py:

Line 517
Line 518
Line 519
Line 520
Line 521
This should raise an exception, not create a response.


Line 523
Line 524
Line 525
Line 526
Line 527
Other exceptions here are not logged or reported


Line 537
Line 538
Line 539
Line 540
Line 541
This error is not logged, make debugging such issues impossible, you have to 
look into engine log.


Line 540
Line 541
Line 542
Line 543
Line 544
We don't log any detail about this error, pretty useless for debugging.


Line 272: return self._server_address
Line 273: 
Line 274: @property
Line 275: def responses(self):
Line 276: return self._responses
Unneeded now
Line 277: 
Line 278: def sendReply(self):
Line 279: if len(self._requests) > 0:
Line 280: return


Line 513: response_log = "succeeded"
Line 514: else:
Line 515: response_log = "failed (error %s)" % (error.code,)
Line 516: else:
Line 517: response_log = "without response"
Do we care about this? I think that "succeeded" is ok for both cases.

So we can simplify this to:

error = getattr(response, "error", None)
if error is None:
result = "succeeded"
else:
result = "failed (error %s)" % error.code
Line 518: self.log.info("RPC call %s %s in %.2f seconds",
Line 519:   req.method, response_log, monotonic_time() - 
start_time)
Line 520: if response is not None:
Line 521: ctx.requestDone(response)


Line 515: response_log = "failed (error %s)" % (error.code,)
Line 516: else:
Line 517: response_log = "without response"
Line 518: self.log.info("RPC call %s %s in %.2f seconds",
Line 519:   req.method, response_log, monotonic_time() - 
start_time)
In later patch, I think we should eliminate all error handling in this 
function, since it seems to be the same for any error any way, or just wrong 
(error not reported or not logged, see comments in _handle_request) and do  
error handling in _serveRequest.

This will also simplify logging or error codes:

start_time = ...
try:
response = self._handle_request(ctx, req)
except Exception as e:
extract error code...
log failure with error code...
response = ...
else:
log success...

send response...

Since this is more risky, I think we should do this only on master for now.

Piotr, what do you think?
Line 520: if response is not None:
Line 521: ctx.requestDone(response)
Line 522: 
Line 523: def _handle_request(self, ctx, req):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I776e667fcfd1a20a9ef5f6c638d6c3d950672314
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
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]: rpc: Log important info from VM stats

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: rpc: Log important info from VM stats
..


Patch Set 23: Code-Review+1

Waiting for more acks.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcbac615323b62fb9a27e5c0f5a4e98990076146
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
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]: rpc: Log important info from VM stats

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: rpc: Log important info from VM stats
..


Patch Set 20:

(1 comment)

https://gerrit.ovirt.org/#/c/58465/20/lib/vdsm/throttledlog.py
File lib/vdsm/throttledlog.py:

Line 100: else:
Line 101: if not periodic.should_log():
Line 102: return
Line 103: 
Line 104: _logger.log(level, message, *args)
> > It would be nice to provide the standard loging methods (debug, info...).
Right, the simple wrappers you added are more clear anyway.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcbac615323b62fb9a27e5c0f5a4e98990076146
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
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]: logging: throttledlog introduced

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: logging: throttledlog introduced
..


Patch Set 2: Code-Review+1

Waiting for Piotr ack.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief96f74eb84880827ccf24c3e7ae854403090b8a
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
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]: virt: Add info about the number of run tasks in executor wor...

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: virt: Add info about the number of run tasks in executor workers
..


Patch Set 4: Code-Review+1

Waiting for Francesco and Pitor ack.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id5175f6d7d004bec97f604036ceb3f1662277224
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
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]: virt: Add docstrings about Executor parameters and behavior

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: virt: Add docstrings about Executor parameters and behavior
..


Patch Set 10: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bda87cdead2f926cfb09664e87bdff1a87906dd
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
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]: virt: Improve thread safety of discarding executor workers

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: virt: Improve thread safety of discarding executor workers
..


Patch Set 9: Code-Review+1

Waiting for Francesco and Piotr ack

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3a4cec1a4b3c1e1aa341d90c0e02f46d0b776e9c
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
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]: virt: Improve thread safety of discarding executor workers

[mzamazal]

Milan Zamazal has posted comments on this change.

Change subject: virt: Improve thread safety of discarding executor workers
..


Patch Set 9:

(2 comments)

I reworded and clarified the commit message.

https://gerrit.ovirt.org/#/c/58146/8/lib/vdsm/executor.py
File lib/vdsm/executor.py:

Line 302: if self._discarded:
Line 303: raise _WorkerDiscarded()
Line 304: 
Line 305: def _discard_after(self, timeout):
Line 306: if timeout is not None:
> We don't need this line - functools.partial is copying the current value. T
Done
Line 307: discard = functools.partial(self._discard, 
self._task_counter)
Line 308: return self._scheduler.schedule(timeout, discard)
Line 309: return None
Line 310: 


Line 304: 
Line 305: def _discard_after(self, timeout):
Line 306: if timeout is not None:
Line 307: discard = functools.partial(self._discard, 
self._task_counter)
Line 308: return self._scheduler.schedule(timeout, discard)
> task_number -> self._task_counter
Done
Line 309: return None
Line 310: 
Line 311: def _discard(self, task_number):
Line 312: with self._lock:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3a4cec1a4b3c1e1aa341d90c0e02f46d0b776e9c
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
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]: virt: Add info about the number of run tasks in executor wor...

[automation]

gerrit-hooks has posted comments on this change.

Change subject: virt: Add info about the number of run tasks in executor workers
..


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/59048
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id5175f6d7d004bec97f604036ceb3f1662277224
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
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]: virt: Improve thread safety of discarding executor workers

[automation]

gerrit-hooks has posted comments on this change.

Change subject: virt: Improve thread safety of discarding executor workers
..


Patch Set 9:

* 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/58146
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3a4cec1a4b3c1e1aa341d90c0e02f46d0b776e9c
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
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]: virt: Block stuck VMs in executor

[automation]

gerrit-hooks has posted comments on this change.

Change subject: virt: Block stuck VMs in executor
..


Patch Set 19:

* 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/57755
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3de9a404c16fd213363c40a9e800633d1d50f61
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
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: hardcode release number

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: spec: hardcode release number
..


Patch Set 1:

(2 comments)

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

Line 8: 
Line 9: It should be upgraded manually, and for master branch
Line 10: should be always 0 (zero), while it should be 1-N for stable
Line 11: branches.
Line 12: 
Please explain that this requires a spec patch for each build in the stable 
branches.
Line 13: Change-Id: I457e8b4f280d9773788a0652ced1e3770cca9bca
Line 14: Backport-To: 4.0


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

Line 44: %global debug_package %{nil}
Line 45: 
Line 46: Name:   %{vdsm_name}
Line 47: Version:@PACKAGE_VERSION@
Line 48: Release:0.@PACKAGE_RELEASE@%{?dist}%{?extra_release}
There is a problem with this solution - we are using @PACKAGE_RELEASE@ in 
lib/vdsm/dsaversion.py.in, and this cause vdsm revision to show wrong values.

I think we have to fix the pkg_version script, and leave PACKAGE_RELEASE as is.

We need to check how we build vdsm for downstream, maybe this issue already 
exists in downstream builds (spec and dsaversion with different values).
Line 49: Summary:Virtual Desktop Server Manager
Line 50: 
Line 51: Group:  Applications/System
Line 52: License:GPLv2+


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I457e8b4f280d9773788a0652ced1e3770cca9bca
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Barak Korren 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Rafael Martins 
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]: build: use only the top git hash as release

[fromani]

Francesco Romani has posted comments on this change.

Change subject: build: use only the top git hash as release
..


Patch Set 1:

Nir, good tip. Let's hear what other reviewers think.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida06ea25a994ca01dcdd093be38028a069d7eb30
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
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]: virt: Improve thread safety of discarding executor workers

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: virt: Improve thread safety of discarding executor workers
..


Patch Set 8:

(6 comments)

https://gerrit.ovirt.org/#/c/58146/8//COMMIT_MSG
Commit Message:

Line 6: 
Line 7: virt: Improve thread safety of discarding executor workers
Line 8: 
Line 9: Some important guarantees are currently missing when processing executor
Line 10: tasks and handling discarded workers.  Consider the following scenario:
Lets be more specific, "Some important guarantees" is not clear.
Line 11: 
Line 12: - _execute_task cancels discard timer but the timer is already starting
Line 13:   discard processing.
Line 14: - However `self._discarded = True' statement hasn't been reached yet.


Line 18: - _discard is executed in the meantime.
Line 19: - The other processed task stalls and discard processing is initiated
Line 20:   again.
Line 21: - _discard checks for self._discarded (which is True now) and raises
Line 22:   AssertionError.
Did you actually see this exception? Worth a note that this fixes a rare or 
theoretical edge case. Unless you have other evidence.
Line 23: 
Line 24: This is not the only thread safety problem and things may get worse in 
a
Line 25: followup patch limiting the number of workers by groups.  We shouldn't
Line 26: hit the problems often, but if they do then bad things happen.  So we


Line 23: 
Line 24: This is not the only thread safety problem and things may get worse in 
a
Line 25: followup patch limiting the number of workers by groups.  We shouldn't
Line 26: hit the problems often, but if they do then bad things happen.  So we
Line 27: should fix it.
I'm not sure what do you mean by this section. Either describe the additional 
issues or remove it.
Line 28: 
Line 29: We introduce a lock for the worker discarding process in each execution
Line 30: step.  The lock prevents to run _discard at unexpected times, namely:
Line 31: 


Line 32: - If _execute_task decides to continue normally, no discard processing
Line 33:   of the given task is performed anymore.
Line 34: 
Line 35: - We always run Executor._worker_discarded before
Line 36:   Executor._worker_stopped.
Please update the commit message to reflect the current code.
Line 37: 
Line 38: Change-Id: I3a4cec1a4b3c1e1aa341d90c0e02f46d0b776e9c


https://gerrit.ovirt.org/#/c/58146/8/lib/vdsm/executor.py
File lib/vdsm/executor.py:

Line 302: if self._discarded:
Line 303: raise _WorkerDiscarded()
Line 304: 
Line 305: def _discard_after(self, timeout):
Line 306: task_number = self._task_counter
We don't need this line - functools.partial is copying the current value. This 
was needed when we used a nested function to pass the current value.
Line 307: if timeout is not None:
Line 308: discard = functools.partial(self._discard, task_number)
Line 309: return self._scheduler.schedule(timeout, discard)
Line 310: return None


Line 304: 
Line 305: def _discard_after(self, timeout):
Line 306: task_number = self._task_counter
Line 307: if timeout is not None:
Line 308: discard = functools.partial(self._discard, task_number)
task_number -> self._task_counter
Line 309: return self._scheduler.schedule(timeout, discard)
Line 310: return None
Line 311: 
Line 312: def _discard(self, task_number):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3a4cec1a4b3c1e1aa341d90c0e02f46d0b776e9c
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
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]: build: we don't use the VERSION file anymore

[fromani]

Francesco Romani has posted comments on this change.

Change subject: build: we don't use the VERSION file anymore
..


Patch Set 1: Code-Review-2

we _DO_ use VERSION in the "make dist" flow, so this patch is harmful.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d88f9ca4089973f5fdab0fa1bae3246480aa0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
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]: build: Make sure run_tests*.sh scripts are executable

[mzamazal]

Milan Zamazal has posted comments on this change.

Change subject: build: Make sure run_tests*.sh scripts are executable
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/55949/5/build-aux/Makefile.subs
File build-aux/Makefile.subs:

Line 22: 
Line 23: # Reference:
Line 24: # http://www.gnu.org/software/automake/manual/html_node/Scripts.html
Line 25: PATHSUBST = sed \
Line 26:-e "s,[@]top_srcdir[@],$(top_srcdir),g" \
> Yes and no. If we don't replace top_srcdir with the path provided in the Ma
We had exactly that problem in former versions of this patch. Tomáš was kind to 
provide this line as a solution and now the patch works fine. So it is very 
related. :-)
Line 27:-e "s,[@]BACKUPDIR[@],$(vdsmbackupdir),g" \
Line 28:-e "s,[@]BINDIR[@],$(bindir),g" \
Line 29:-e "s,[@]CONFDIR[@],$(vdsmconfdir),g" \
Line 30:-e "s,[@]HOOKSDIR[@],$(vdsmhooksdir),g" \


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc1e3dc8ace7f69801b765262352903020cc8aef
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Tomas Golembiovsky 
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]: jsonrpc: Return back mistakenly removed line in _serveRequest

[mzamazal]

Milan Zamazal has abandoned this change.

Change subject: jsonrpc: Return back mistakenly removed line in _serveRequest
..


Abandoned

Piotr was faster. :-)

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ia379837b35a6c2ee8ec234c538b567928463decb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
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]: jsonrpc: Return back mistakenly removed line in _serveRequest

[automation]

gerrit-hooks has posted comments on this change.

Change subject: jsonrpc: Return back mistakenly removed line in _serveRequest
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia379837b35a6c2ee8ec234c538b567928463decb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
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-3.6]: build: pkgversion: minimum release should be 1

[automation]

gerrit-hooks has posted comments on this change.

Change subject: build: pkgversion: minimum release should be 1
..


Patch Set 2:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idace2ac53d6aafe674c7ad11f611d0de54fcbe5f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Rafael Martins 
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-3.6]: build: pkgversion: minimum release should be 1

[fromani]

Francesco Romani has submitted this change and it was merged.

Change subject: build: pkgversion: minimum release should be 1
..


build: pkgversion: minimum release should be 1

For stable branches, we should never have release < 1.
E.g. we should avoid

v4.y.z-0

instead we should have

v4.y.z-1

Change-Id: Idace2ac53d6aafe674c7ad11f611d0de54fcbe5f
Signed-off-by: Francesco Romani 
Reviewed-on: https://gerrit.ovirt.org/59337
Continuous-Integration: Jenkins CI
Reviewed-by: Rafael Martins 
Reviewed-by: Nir Soffer 
Reviewed-by: Piotr Kliczewski 
Reviewed-on: https://gerrit.ovirt.org/59495
---
M build-aux/pkg-version
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Jenkins CI: Passed CI tests
  Rafael Martins: Looks good to me, but someone else must approve
  Francesco Romani: Verified; Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Idace2ac53d6aafe674c7ad11f611d0de54fcbe5f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Rafael Martins 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-3.6]: build: pkgversion: minimum release should be 1

[fromani]

Francesco Romani has posted comments on this change.

Change subject: build: pkgversion: minimum release should be 1
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idace2ac53d6aafe674c7ad11f611d0de54fcbe5f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Rafael Martins 
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-3.6]: build: pkgversion: minimum release should be 1

[fromani]

Francesco Romani has posted comments on this change.

Change subject: build: pkgversion: minimum release should be 1
..


Patch Set 1:

raising score

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idace2ac53d6aafe674c7ad11f611d0de54fcbe5f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Rafael Martins 
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]: virt: Improve thread safety of discarding executor workers

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: virt: Improve thread safety of discarding executor workers
..


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/58146/6/lib/vdsm/executor.py
File lib/vdsm/executor.py:

Line 286
Line 287
Line 288
Line 289
Line 290
> What bad could happen here if it was inside the critical section? We just p
Right, now it does not matter, previously it ensured that _worker_discarded 
will be called first.


Line 298
Line 299
Line 300
Line 301
Line 302
> Done.
Maybe it was used in earlier versions, but I think having a consistent api is 
good, when a worker has some event, you want to know which worker. There is no 
possible deadlock in current code since we don't call this holding a worker 
lock.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3a4cec1a4b3c1e1aa341d90c0e02f46d0b776e9c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
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]: build: Make sure run_tests*.sh scripts are executable

[Tomas Golembiovsky]

Tomas Golembiovsky has posted comments on this change.

Change subject: build: Make sure run_tests*.sh scripts are executable
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/55949/5/build-aux/Makefile.subs
File build-aux/Makefile.subs:

Line 22: 
Line 23: # Reference:
Line 24: # http://www.gnu.org/software/automake/manual/html_node/Scripts.html
Line 25: PATHSUBST = sed \
Line 26:-e "s,[@]top_srcdir[@],$(top_srcdir),g" \
> this part still seems not related
Yes and no. If we don't replace top_srcdir with the path provided in the 
Makefile, config.status will replace it later. But config.status will always 
replace it with '.' instead of the real path to the root directory. That will 
lead to broken paths in all generated .sh files that are not in the root 
directory (e.g. those in the tests/).
Line 27:-e "s,[@]BACKUPDIR[@],$(vdsmbackupdir),g" \
Line 28:-e "s,[@]BINDIR[@],$(bindir),g" \
Line 29:-e "s,[@]CONFDIR[@],$(vdsmconfdir),g" \
Line 30:-e "s,[@]HOOKSDIR[@],$(vdsmhooksdir),g" \


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc1e3dc8ace7f69801b765262352903020cc8aef
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Tomas Golembiovsky 
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]: vdsm: changed relative imports to absolute imports

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: vdsm: changed relative imports to absolute imports
..


Patch Set 3:

Piotr, importing absolute_imports fixes bad defaults in python 2.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf42bbe589b1e18b789ad8e3137d320e3a3966a7
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: Add inplace virt-sparsify support

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Add inplace virt-sparsify support
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/57347/5/lib/vdsm/rpc/Bridge.py
File lib/vdsm/rpc/Bridge.py:

Line 358: 'Image_download': {'ret': 'uuid'},
Line 359: 'Image_mergeSnapshots': {'ret': 'uuid'},
Line 360: 'Image_move': {'ret': 'uuid'},
Line 361: 'Image_reconcileVolumeChain': {'ret': 'volumes'},
Line 362: 'Image_sparsifyInplace': {},
> it is not needed. Please remove.
I hoped that you can tell me if these empty transformations are needed.
Line 363: 'Image_syncData': {'ret': 'uuid'},
Line 364: 'Image_upload': {'ret': 'uuid'},
Line 365: 'Image_prepare': {'ret': Image_prepare_Ret},
Line 366: 'ISCSIConnection_discoverSendTargets': {'ret': 'fullTargets'},


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ac2bb1fbd2acbe0fc47694d17313c6ccd01a227
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shmuel Melamud 
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[ovirt-3.6]: migration: downtime: add tunable for back compat

[fromani]

Francesco Romani has posted comments on this change.

Change subject: migration: downtime: add tunable for back compat
..


Patch Set 2: Verified+1

verified like 4.0 and master patches.
We are not touching production code, hence removing gerrit-hooks score and not 
requiring BZ

we figured out why we didn't need this befoerwe:
 the build I looked was in old jenkins, and there we use source 
tarballs, not tags
 and tarballs built from git checkout apparently have correct 
release number
 fromani, in last build I used the build_artifacts job from new 
jenkins

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If477f268354a9609ecca216dad1eee4f40910c8f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Tomas Jelinek 
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-3.6]: build: pkgversion: minimum release should be 1

[rmartins]

Rafael Martins has posted comments on this change.

Change subject: build: pkgversion: minimum release should be 1
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idace2ac53d6aafe674c7ad11f611d0de54fcbe5f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Rafael Martins 
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-3.6]: build: pkgversion: minimum release should be 1

[fromani]

Francesco Romani has posted comments on this change.

Change subject: build: pkgversion: minimum release should be 1
..


Patch Set 1: Verified+1

verified like 4.0 and master patches.
We are not touching production code, hence removing gerrit-hooks score and not 
requiring BZ

we figured out why we didn't need this befoerwe:
 the build I looked was in old jenkins, and there we use source 
tarballs, not tags
 and tarballs built from git checkout apparently have correct 
release number
 fromani, in last build I used the build_artifacts job from new 
jenkins

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idace2ac53d6aafe674c7ad11f611d0de54fcbe5f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Rafael Martins 
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-3.6]: migration: downtime: add tunable for back compat

[fromani]

Francesco Romani has posted comments on this change.

Change subject: migration: downtime: add tunable for back compat
..


Patch Set 2: -Verified

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If477f268354a9609ecca216dad1eee4f40910c8f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Tomas Jelinek 
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-3.6]: migration: downtime: add tunable for back compat

[fromani]

Francesco Romani has posted comments on this change.

Change subject: migration: downtime: add tunable for back compat
..


Patch Set 2: Verified-1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If477f268354a9609ecca216dad1eee4f40910c8f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Tomas Jelinek 
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-3.6]: migration: downtime: add tunable for back compat

[fromani]

Francesco Romani has posted comments on this change.

Change subject: migration: downtime: add tunable for back compat
..


Patch Set 2:

drop my last comment: wrong patch.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If477f268354a9609ecca216dad1eee4f40910c8f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Tomas Jelinek 
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-3.6]: build: pkgversion: minimum release should be 1

[automation]

gerrit-hooks has posted comments on this change.

Change subject: build: pkgversion: minimum release should be 1
..


Patch Set 1: Verified-1

* Update tracker: IGNORE, no Bug-Url found

* Check Bug-Url::ERROR, At least one bug-url is required for the stable branch
* Check merged to previous::OK, change not open on any previous branch

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idace2ac53d6aafe674c7ad11f611d0de54fcbe5f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Rafael Martins 
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-3.6]: build: pkgversion: minimum release should be 1

[fromani]

Hello Piotr Kliczewski, Nir Soffer, Rafael Martins,

I'd like you to do a code review.  Please visit

https://gerrit.ovirt.org/59495

to review the following change.

Change subject: build: pkgversion: minimum release should be 1
..

build: pkgversion: minimum release should be 1

For stable branches, we should never have release < 1.
E.g. we should avoid

v4.y.z-0

instead we should have

v4.y.z-1

Change-Id: Idace2ac53d6aafe674c7ad11f611d0de54fcbe5f
Signed-off-by: Francesco Romani 
Reviewed-on: https://gerrit.ovirt.org/59337
Continuous-Integration: Jenkins CI
Reviewed-by: Rafael Martins 
Reviewed-by: Nir Soffer 
Reviewed-by: Piotr Kliczewski 
---
M build-aux/pkg-version
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/95/59495/1

diff --git a/build-aux/pkg-version b/build-aux/pkg-version
index c290d76..67fa81f 100755
--- a/build-aux/pkg-version
+++ b/build-aux/pkg-version
@@ -19,7 +19,7 @@
 AWK_RELEASE='
 BEGIN { FS="-"; OFS="." }
 /^v[0-9]/ {
-  if (NF == 1) print 0
+  if (NF == 1) print 1
   else if (NF == 2) print $2
   else if (NF == 3) print $2, "git" substr($3, 2)
   else if (NF == 4) print $2, $3, "git" substr($4, 2)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idace2ac53d6aafe674c7ad11f611d0de54fcbe5f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Rafael Martins 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: jsonrpc: Return back mistakenly removed line in _serveRequest

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: jsonrpc: Return back mistakenly removed line in _serveRequest
..


Patch Set 1: Code-Review-1

Pleae fix the patch that removed this line.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia379837b35a6c2ee8ec234c538b567928463decb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
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]: hooks: openstack hook should wait a while for nic activation

[fromani]

Francesco Romani has posted comments on this change.

Change subject: hooks: openstack hook should wait a while for nic activation
..


Patch Set 3: Code-Review+1

as long as we know the sleep() code is fragile and temporary, it's ok for me. 
Deeper review later.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4f455789909f090a039863fdcfeb61b9db1042f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Marcin Mirecki 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
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]: build: Make sure run_tests*.sh scripts are executable

[ybronhei]

Yaniv Bronhaim has posted comments on this change.

Change subject: build: Make sure run_tests*.sh scripts are executable
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/55949/5/build-aux/Makefile.subs
File build-aux/Makefile.subs:

Line 22: 
Line 23: # Reference:
Line 24: # http://www.gnu.org/software/automake/manual/html_node/Scripts.html
Line 25: PATHSUBST = sed \
Line 26:-e "s,[@]top_srcdir[@],$(top_srcdir),g" \
this part still seems not related
Line 27:-e "s,[@]BACKUPDIR[@],$(vdsmbackupdir),g" \
Line 28:-e "s,[@]BINDIR[@],$(bindir),g" \
Line 29:-e "s,[@]CONFDIR[@],$(vdsmconfdir),g" \
Line 30:-e "s,[@]HOOKSDIR[@],$(vdsmhooksdir),g" \


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc1e3dc8ace7f69801b765262352903020cc8aef
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Tomas Golembiovsky 
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]: jsonrpc: Return back mistakenly removed line in _serveRequest

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: jsonrpc: Return back mistakenly removed line in _serveRequest
..


Patch Set 1: Code-Review-1

thanks, but I think https://gerrit.ovirt.org/#/c/59469/ proposes this already

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia379837b35a6c2ee8ec234c538b567928463decb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
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]: api: unexpected response when a verb returns nothing

[automation]

gerrit-hooks has posted comments on this change.

Change subject: api: unexpected response when a verb returns nothing
..


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/59469
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5630052bea05fb8ea4fa3f2ff1dfedf6e1b2841d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Roy Golan 
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]: api: unexpected response when a verb returns nothing

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: api: unexpected response when a verb returns nothing
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5630052bea05fb8ea4fa3f2ff1dfedf6e1b2841d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Roy Golan 
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]: api: unexpected response when a verb returns nothing

[automation]

gerrit-hooks has posted comments on this change.

Change subject: api: unexpected response when a verb returns nothing
..


Patch Set 3:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5630052bea05fb8ea4fa3f2ff1dfedf6e1b2841d
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Roy Golan 
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]: api: unexpected response when a verb returns nothing

[danken]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: api: unexpected response when a verb returns nothing
..


api: unexpected response when a verb returns nothing

We used to send True for verbs where there is no response. Now we send
None which causes NPE in the engine when parsing StorageDomain.activate
response. This patch fixes this regression.


Change-Id: I5630052bea05fb8ea4fa3f2ff1dfedf6e1b2841d
Signed-off-by: Piotr Kliczewski 
Related-To: https://gerrit.ovirt.org/59078
Reviewed-on: https://gerrit.ovirt.org/59469
Continuous-Integration: Jenkins CI
Tested-by: Freddy Rolland 
Reviewed-by: Freddy Rolland 
Reviewed-by: Dan Kenigsberg 
---
M lib/yajsonrpc/__init__.py
1 file changed, 1 insertion(+), 0 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Dan Kenigsberg: Looks good to me, approved
  Freddy Rolland: Verified; Looks good to me, but someone else must approve



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I5630052bea05fb8ea4fa3f2ff1dfedf6e1b2841d
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Roy Golan 
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]: Revert "NetworkManager: configure to monitor ifcfg/connectio...

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: Revert "NetworkManager: configure to monitor ifcfg/connection 
files"
..


Patch Set 2: Verified+1

copying Burman's score

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I392a918112c3a7b5bfee6fccb2a51b038d5a543f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Anonymous Coward #1001177
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michael Burman 
Gerrit-Reviewer: Ondřej Svoboda 
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]: Revert "NetworkManager: configure to monitor ifcfg/connectio...

[automation]

gerrit-hooks has posted comments on this change.

Change subject: Revert "NetworkManager: configure to monitor ifcfg/connection 
files"
..


Patch Set 2:

* #1326798::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1326798::OK, public bug
* Check Product::#1326798::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/59260
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I392a918112c3a7b5bfee6fccb2a51b038d5a543f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Anonymous Coward #1001177
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michael Burman 
Gerrit-Reviewer: Ondřej Svoboda 
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]: jsonrpc: Return back mistakenly removed line in _serveRequest

[frolland]

Freddy Rolland has posted comments on this change.

Change subject: jsonrpc: Return back mistakenly removed line in _serveRequest
..


Patch Set 1:

already in https://gerrit.ovirt.org/59469

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia379837b35a6c2ee8ec234c538b567928463decb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
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]: virt: clientIF: extract vmContainer into a module

[automation]

gerrit-hooks has posted comments on this change.

Change subject: virt: clientIF: extract vmContainer into a module
..


Patch Set 9:

* 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/53101
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iacd2ae6c5e9ca6a73c0fed978c78c9ebb001c46d
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
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]: sp: allow executing upgradePool even if there is a pending u...

[automation]

gerrit-hooks has posted comments on this change.

Change subject: sp: allow executing upgradePool even if there is a pending 
update
..


Patch Set 5:

* #1260428::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1260428::OK, public bug
* Check Public Bug::#1319523::ERROR, private bug
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
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: migrate tests to python3

[igoihman]

Irit Goihman has posted comments on this change.

Change subject: tests: migrate tests to python3
..


Patch Set 9:

(2 comments)

https://gerrit.ovirt.org/#/c/57251/9/tests/Makefile.am
File tests/Makefile.am:

Line 143
Line 144
Line 145
Line 146
Line 147
> what about alignmentScanTests.py? you can mention what still fails after th
I modernized the test file but it still not compatible with python3


Line 148
Line 149
Line 150
Line 151
Line 152
> what about cPopenTests.py? you modernized it as well..
same


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iab8fda7a289aeaba9dd29ece6efad24bfecb2320
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: 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: 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]: sp: allow executing upgradePool even if there is a pending u...

[automation]

gerrit-hooks has posted comments on this change.

Change subject: sp: allow executing upgradePool even if there is a pending 
update
..


Patch Set 4:

* #1260428::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1260428::OK, public bug
* Check Public Bug::#1319523::ERROR, private bug
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
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]: api: unexpected response when a verb returns nothing

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: api: unexpected response when a verb returns nothing
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/59469/1/lib/yajsonrpc/__init__.py
File lib/yajsonrpc/__init__.py:

PS1, Line 543:  
> Why btw not make those API verbs to return True if None isn't a valid answe
Yes, this is how the api used to work.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5630052bea05fb8ea4fa3f2ff1dfedf6e1b2841d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Roy Golan 
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]: hooks: openstack hook should wait a while for nic activation

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hooks: openstack hook should wait a while for nic activation
..


Patch Set 3:

* #1314371::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1314371::OK, public bug
* Check Product::#1314371::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/58917
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4f455789909f090a039863fdcfeb61b9db1042f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Marcin Mirecki 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
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