Change in vdsm[master]: openstacknet: add MidoNet support
by automation＠ovirt.org
gerrit-hooks has posted comments on this change.
Change subject: openstacknet: add MidoNet support
......................................................................
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/59255
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id5cb84c155b46603e05951238ad16182081dbe10
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <toni+ovirt(a)midokura.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 11 months
Change in vdsm[master]: Introduce reports package
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: Introduce reports package
......................................................................
Patch Set 38: Code-Review+2
Yaniv, this was verified before the config help text changes, I guess it is ready?
--
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: 38
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yaniv Kaul <ykaul(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 11 months
Change in vdsm[master]: Add hawkular reporter to reports
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: Add hawkular reporter to reports
......................................................................
Patch Set 7:
(2 comments)
https://gerrit.ovirt.org/#/c/58660/7/lib/vdsm/reports/hawkular.py
File lib/vdsm/reports/hawkular.py:
Line 19: #
Line 20:
Line 21: from __future__ import absolute_import
Line 22:
Line 23: from collections import deque
Please import collections, there is no need to import the name deque. This makes imports easier to manage.
Line 24: import logging
Line 25: import threading
Line 26:
Line 27: import six
Line 39: def start(address):
Line 40: global _running
Line 41: if _running:
Line 42: raise RuntimeError('trying to start reporter while running')
Line 43: else:
else is not needed. The whole point in the early return style is to keep the normal flow simple, avoiding unneeded indentation.
if-else is god when we want to handle two cases. Here we handle only one case, error, by raising.
Line 44: logging.info("Starting hawkular reporter")
Line 45: concurrent.thread(_run, name='hawkular', address=address).start()
Line 46: _running = True
Line 47:
--
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: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yaniv Kaul <ykaul(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 11 months
Change in vdsm[master]: Add hawkular reporter to reports
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: Add hawkular reporter to reports
......................................................................
Patch Set 6:
(1 comment)
https://gerrit.ovirt.org/#/c/58660/6/lib/vdsm/reports/hawkular.py
File lib/vdsm/reports/hawkular.py:
Line 76: items = _queue.pop()
Line 77: if items is _STOP:
Line 78: break
Line 79: client.put(items)
Line 80: _running = False
> I don't agree, as only after we pop the _STOP we actually stopped running.
Ok, modifying _running here has an advantage, although we don't care about it.
However logging when a thread start and stop is very important. Without it, how can you know that the thread started are existed?
--
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yaniv Kaul <ykaul(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 11 months
Change in vdsm[master]: net tests: Support the ability to choose the tested switch
by edwardh＠redhat.com
Edward Haas has posted comments on this change.
Change subject: net tests: Support the ability to choose the tested switch
......................................................................
Patch Set 4: Verified+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: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 11 months
Change in vdsm[master]: net tests: Support the ability to choose the tested switch
by automation＠ovirt.org
gerrit-hooks has posted comments on this change.
Change subject: net tests: Support the ability to choose the tested switch
......................................................................
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/58970
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I06e230633dfefcdc4a4b545eb2139ffe190c6f35
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 11 months
Change in vdsm[master]: net tests: Support the ability to choose the tested switch
by edwardh＠redhat.com
Edward Haas has posted comments on this change.
Change subject: net tests: Support the ability to choose the tested switch
......................................................................
Patch Set 2:
(2 comments)
https://gerrit.ovirt.org/#/c/58970/2/tests/functional/networkTests.py
File tests/functional/networkTests.py:
Line 409: self.assertEqual(netinfo.bondings[bondName]['active_slave'], '')
Line 410:
Line 411: def setupNetworks(self, networks, bonds, options, **kwargs):
Line 412: test_kernel_config = kwargs.pop('test_kernel_config', True)
Line 413: switch = os.environ.get('VDSM_TESTER_SWITCH_TYPE')
> we can initialize this on module/class-level
Not sure it is worth it, it does not seem to cost anything.
I used memoized, the class level fixture is somehow colliding with monkeypatches.
Line 414: if switch:
Line 415: for net, attrs in networks.items():
Line 416: tested_switch = attrs.get('switch')
Line 417: if tested_switch != switch:
Line 412: test_kernel_config = kwargs.pop('test_kernel_config', True)
Line 413: switch = os.environ.get('VDSM_TESTER_SWITCH_TYPE')
Line 414: if switch:
Line 415: for net, attrs in networks.items():
Line 416: tested_switch = attrs.get('switch')
> i think we have to add default value 'legacy'
Yea, I did in patch3.
Line 417: if tested_switch != switch:
Line 418: raise SkipTest('{} switch tests'.format(tested_switch))
Line 419: status, msg = self.vdsm_net.setupNetworks(networks, bonds, options)
Line 420: unified = (
--
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: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 11 months
Change in vdsm[master]: xmlrpc: Hide fenceNode password in the log
by mzamazal＠redhat.com
Milan Zamazal has posted comments on this change.
Change subject: xmlrpc: Hide fenceNode password in the log
......................................................................
Patch Set 4:
(2 comments)
https://gerrit.ovirt.org/#/c/58833/4/lib/vdsm/rpc/bindingxmlrpc.py
File lib/vdsm/rpc/bindingxmlrpc.py:
Line 1197: def wrapApiMethod(f):
Line 1198: def wrapper(*args, **kwargs):
Line 1199: try:
Line 1200: logLevel = logging.DEBUG
Line 1201: suppress_args = f.__name__ in ('fenceNode',)
> not sure we need this for more verbs, in this case we could have the soil p
Indeed, it's a form of documentation that we can add more verbs here, which we'll probably do. One of the candidates are the verbs with passwords in parameters below, other ones are verbs with large parameter values.
Line 1202: suppress_response = f.__name__ in ('getAllVmStats',)
Line 1203:
Line 1204: # TODO: This password protection code is fragile and ugly. Password
Line 1205: # protection should be done in the wrapped methods, and logging
Line 1198: def wrapper(*args, **kwargs):
Line 1199: try:
Line 1200: logLevel = logging.DEBUG
Line 1201: suppress_args = f.__name__ in ('fenceNode',)
Line 1202: suppress_response = f.__name__ in ('getAllVmStats',)
> ditto
This one is unrelated to this change, so it would be for a separate patch. But we already have got a proposed patch to remove this line altogether so there is no need to care about it.
Line 1203:
Line 1204: # TODO: This password protection code is fragile and ugly. Password
Line 1205: # protection should be done in the wrapped methods, and logging
Line 1206: # shold be done in the next layers, similar to storage logs.
--
To view, visit https://gerrit.ovirt.org/58833
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I53318ed8ce425f042b5f783af779b540c1dfd7b5
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <toni+ovirt(a)midokura.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 11 months
Change in vdsm[master]: Adding report_stats to host.api
by igoihman＠redhat.com
Irit Goihman has posted comments on this change.
Change subject: Adding report_stats to host.api
......................................................................
Patch Set 8: -Verified
--
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: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 11 months
Change in vdsm[master]: Adding report_stats to host.api
by igoihman＠redhat.com
Irit Goihman has posted comments on this change.
Change subject: Adding report_stats to host.api
......................................................................
Patch Set 8: Verified+1
--
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: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 11 months