Change in vdsm[master]: vdsm.conf: Add drop-in dir
by automation＠ovirt.org
gerrit-hooks has posted comments on this change.
Change subject: vdsm.conf: Add drop-in dir
......................................................................
Patch Set 10:
* #1279555::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1279555::OK, public bug
* Check Product::#1279555::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/58728
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3849829aca50b30742e9c860d7c19296d6361015
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 10 months
Change in vdsm[master]: rpc: Log RPC call summary on info level
by mzamazal＠redhat.com
Milan Zamazal has posted comments on this change.
Change subject: rpc: Log RPC call summary on info level
......................................................................
Patch Set 1:
(4 comments)
https://gerrit.ovirt.org/#/c/59080/1/lib/vdsm/rpc/bindingxmlrpc.py
File lib/vdsm/rpc/bindingxmlrpc.py:
Line 1198:
Line 1199: def wrapApiMethod(f):
Line 1200: def wrapper(*args, **kwargs):
Line 1201: try:
Line 1202: start_time = time.time()
> This should be before the try, and use utils.monotonic_time
Done
Line 1203: logLevel = logging.DEBUG
Line 1204:
Line 1205: # TODO: This password protection code is fragile and ugly. Password
Line 1206: # protection should be done in the wrapped methods, and logging
Line 1269: f.__self__.cif.log.error("unexpected error", exc_info=True)
Line 1270: return errCode['unexpected']
Line 1271: finally:
Line 1272: f.__self__.cif.log.info("RPC call of %s finished; %s seconds",
Line 1273: f.__name__, time.time() - start_time)
> Use utils.monotonic_time
Good tip, thanks. Done.
Line 1274: wrapper.__name__ = f.__name__
Line 1275: wrapper.__doc__ = f.__doc__
Line 1276: return wrapper
Line 1277:
https://gerrit.ovirt.org/#/c/59080/1/lib/yajsonrpc/__init__.py
File lib/yajsonrpc/__init__.py:
Line 547: if isinstance(res, LargeValue):
Line 548: res = res.value()
Line 549: ctx.requestDone(JsonRpcResponse(res, None, req.id))
Line 550: finally:
Line 551: self.log.info("RPC call of %s finished; %s seconds",
> I want it together with the information about the verb. Milan, what about s
Good ideas. As for return code, I implemented it in a followup patch: https://gerrit.ovirt.org/59234. Regarding the parameters of the verbs, I need to think about them, also in relation to debug logging of the calls; I'm going to do something about them in followup patches.
Line 552: req.method, time.time() - start_time)
Line 553:
Line 554: @traceback(on=log.name)
Line 555: def serve_requests(self):
Line 548: res = res.value()
Line 549: ctx.requestDone(JsonRpcResponse(res, None, req.id))
Line 550: finally:
Line 551: self.log.info("RPC call of %s finished; %s seconds",
Line 552: req.method, time.time() - start_time)
> This is not good enough, since we have multiple exit points in this functio
> This is not good enough, since we have multiple exit points in this function. Better do this in the function calling this function,
Right. Since the function may be invoked asynchronously via _threadFactory, we can't do it simply at the place of invocation. So I used a wrapper function for logging.
> and use utils.stopwatch.
It doesn't fit this case.
Line 553:
Line 554: @traceback(on=log.name)
Line 555: def serve_requests(self):
Line 556: while True:
--
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
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, 10 months
Change in vdsm[master]: build: Make sure run_tests*.sh scripts are executable
by Tomas Golembiovsky
Tomas Golembiovsky has posted comments on this change.
Change subject: build: Make sure run_tests*.sh scripts are executable
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://gerrit.ovirt.org/#/c/55949/4/build-aux/Makefile.subs
File build-aux/Makefile.subs:
Line 39:
Line 40: CONFIGSUBST = $(top_builddir)/config.status --file=-
Line 41:
Line 42: %:: %.in
Line 43: @echo " SED $@"; $(PATHSUBST) $< |$(CONFIGSUBST) >$@
> my proposal should chmod the file after they are created during make.. why
You've got the docs almost right. It's linked from the URL you posted and it's here:
https://www.gnu.org/software/make/manual/html_node/Match_002dAnything-Rul...
The important paragraphs are:
If you do not mark the match-anything rule as terminal, then it is
non-terminal. A non-terminal match-anything rule cannot apply to a file
name that indicates a specific type of data. A file name indicates a
specific type of data if some non-match-anything implicit rule target
matches it.
...
Special built-in dummy pattern rules are provided solely to recognize
certain file names so that non-terminal match-anything rules will not
be considered. These dummy rules have no prerequisites and no recipes,
and they are ignored for all other purposes.
Unfortunately .sh files are amog them and that's why '%: %.in' rule will not match them.
Yaniv: The problem is that everytime you add new .sh file you have to modify the Makefile rule.
Milan: Are you sure it works without it? If yes, it could be related to specific version of make. You can try the following Makefile. Running `make prepare && make` should fail to build `foo.sh`.
all: bar baz.conf foo.sh
% : %.in
touch $@
prepare:
touch foo.sh.in bar.in baz.conf.in
Line 44: @if expr "$@" : ".*\\.sh" >/dev/null ; then \
Line 45: chmod a+x "$@" ; \
--
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: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Tomas Golembiovsky <tgolembi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 10 months
Change in vdsm[master]: vdsm.conf: Add drop-in dir
by automation＠ovirt.org
gerrit-hooks has posted comments on this change.
Change subject: vdsm.conf: Add drop-in dir
......................................................................
Patch Set 9:
* #1279555::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1279555::OK, public bug
* Check Product::#1279555::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/58728
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3849829aca50b30742e9c860d7c19296d6361015
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 10 months
Change in vdsm[master]: rpc: Log important info from VM stats
by mzamazal＠redhat.com
Milan Zamazal has posted comments on this change.
Change subject: rpc: Log important info from VM stats
......................................................................
Patch Set 16:
Just rebase for now, I'll reviewers' comments later.
--
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: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: 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: No
7 years, 10 months
Change in vdsm[master]: rpc: Use Suppressed class instead of logging workarounds
by mzamazal＠redhat.com
Milan Zamazal has posted comments on this change.
Change subject: rpc: Use Suppressed class instead of logging workarounds
......................................................................
Patch Set 3:
(1 comment)
https://gerrit.ovirt.org/#/c/59078/1/lib/vdsm/logUtils.py
File lib/vdsm/logUtils.py:
Line 207: "Attempt to open log with incorrect credentials")
Line 208: return logging.handlers.WatchedFileHandler._open(self)
Line 209:
Line 210:
Line 211: class Suppressed(object):
> Makes sense - how about Suppressed?
Sounds good, let's try. Done.
Line 212:
Line 213: def __init__(self, value):
Line 214: self._value = value
Line 215:
--
To view, visit https://gerrit.ovirt.org/59078
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idaf00e557fccb8f08fa3aeb38d51cb4bbe0ffe53
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: 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, 10 months
Change in vdsm[master]: rpc: Log also error codes of RPC calls
by automation＠ovirt.org
gerrit-hooks has posted comments on this change.
Change subject: rpc: Log also error codes of RPC calls
......................................................................
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/59234
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I776e667fcfd1a20a9ef5f6c638d6c3d950672314
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 10 months
Change in vdsm[master]: rpc: Use Suppressed class instead of logging workarounds
by automation＠ovirt.org
gerrit-hooks has posted comments on this change.
Change subject: rpc: Use Suppressed class instead of logging workarounds
......................................................................
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/59078
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idaf00e557fccb8f08fa3aeb38d51cb4bbe0ffe53
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: 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: No
7 years, 10 months
Change in vdsm[master]: rpc: Log RPC call summary on info level
by automation＠ovirt.org
gerrit-hooks has posted comments on this change.
Change subject: rpc: Log RPC call summary on info level
......................................................................
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/59080
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idde2f1ba7394f16770543f5ca13411e8c2339cc6
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(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: No
7 years, 10 months
Change in vdsm[master]: rpc: Log important info from VM stats
by automation＠ovirt.org
gerrit-hooks has posted comments on this change.
Change subject: rpc: Log important info from VM stats
......................................................................
Patch Set 16:
* #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: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: 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: No
7 years, 10 months