Change in vdsm[ovirt-4.0]: rpc: Log also error codes of RPC calls
by mzamazal＠redhat.com
Hello Piotr Kliczewski, Nir Soffer,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/59999
to review the following change.
Change subject: rpc: Log also error codes of RPC calls
......................................................................
rpc: Log also error codes of RPC calls
In one of the previous commits, we added logging of RPC calls on info
level. It'd be useful to have information about response codes there,
so this patch adds that information.
Change-Id: I776e667fcfd1a20a9ef5f6c638d6c3d950672314
Bug-Url: https://bugzilla.redhat.com/1351247
Backport-To: 4.0
Signed-off-by: Milan Zamazal <mzamazal(a)redhat.com>
Reviewed-on: https://gerrit.ovirt.org/59234
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com>
Reviewed-by: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Continuous-Integration: Jenkins CI
---
M lib/vdsm/rpc/bindingxmlrpc.py
M lib/yajsonrpc/__init__.py
2 files changed, 27 insertions(+), 13 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/99/59999/1
diff --git a/lib/vdsm/rpc/bindingxmlrpc.py b/lib/vdsm/rpc/bindingxmlrpc.py
index 732918f..c60e204 100644
--- a/lib/vdsm/rpc/bindingxmlrpc.py
+++ b/lib/vdsm/rpc/bindingxmlrpc.py
@@ -318,15 +318,18 @@
self.log.debug(fmt, *logargs)
+ res = {}
try:
- return f(*args, **kwargs)
+ res = f(*args, **kwargs)
except:
self.log.error("Unexpected exception", exc_info=True)
- return errCode['unexpected']
- finally:
- self.log.info("RPC call %s finished in %.2f seconds",
- f.__name__,
- utils.monotonic_time() - start_time)
+ res = errCode['unexpected']
+ self.log.info("RPC call %s finished (code=%s) in "
+ "%.2f seconds",
+ f.__name__,
+ res.get('status', {}).get('code'),
+ utils.monotonic_time() - start_time)
+ return res
wrapper.__name__ = f.__name__
wrapper.__doc__ = f.__doc__
@@ -1198,6 +1201,7 @@
def wrapApiMethod(f):
def wrapper(*args, **kwargs):
start_time = utils.monotonic_time()
+ res = {}
try:
logLevel = logging.DEBUG
suppress_args = f.__name__ in ('fenceNode',)
@@ -1258,18 +1262,23 @@
except libvirt.libvirtError as e:
f.__self__.cif.log.error("libvirt error", exc_info=True)
if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
- return errCode['noVM']
+ res = errCode['noVM']
else:
- return errCode['unexpected']
+ res = errCode['unexpected']
+ return res
except VdsmException as e:
f.__self__.cif.log.error("vdsm exception occured", exc_info=True)
- return e.response()
+ res = e.response()
+ return res
except:
f.__self__.cif.log.error("unexpected error", exc_info=True)
- return errCode['unexpected']
+ res = errCode['unexpected']
+ return res
finally:
- f.__self__.cif.log.info("RPC call %s finished in %.2f seconds",
+ f.__self__.cif.log.info("RPC call %s finished (code=%s) in "
+ "%.2f seconds",
f.__name__,
+ res.get('status', {}).get('code'),
utils.monotonic_time() - start_time)
wrapper.__name__ = f.__name__
wrapper.__doc__ = f.__doc__
diff --git a/lib/yajsonrpc/__init__.py b/lib/yajsonrpc/__init__.py
index ff8e4ae..3583dff 100644
--- a/lib/yajsonrpc/__init__.py
+++ b/lib/yajsonrpc/__init__.py
@@ -503,8 +503,13 @@
def _serveRequest(self, ctx, req):
start_time = monotonic_time()
response = self._handle_request(req, ctx.server_address)
- self.log.info("RPC call %s finished in %.2f seconds",
- req.method, monotonic_time() - start_time)
+ error = getattr(response, "error", None)
+ if error is None:
+ response_log = "succeeded"
+ else:
+ response_log = "failed (error %s)" % (error.code,)
+ self.log.info("RPC call %s %s in %.2f seconds",
+ req.method, response_log, monotonic_time() - start_time)
if response is not None:
ctx.requestDone(response)
--
To view, visit https://gerrit.ovirt.org/59999
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I776e667fcfd1a20a9ef5f6c638d6c3d950672314
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
7 years, 11 months
Change in vdsm[ovirt-4.0]: rpc: Log RPC call summary on info level
by mzamazal＠redhat.com
Hello Piotr Kliczewski, Nir Soffer,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/59998
to review the following change.
Change subject: rpc: Log RPC call summary on info level
......................................................................
rpc: Log RPC call summary on info level
We currently log relatively detailed information about all RPC calls.
This information is logged on the debug level. We are going to switch
to the info logging level sometimes in future and then information about
RPC calls won't be logged anymore. But we still want to see some very
basic information about the RPC calls, such as that they were performed
and how much time they took.
This patch adds the corresponding logging. We additionally add the
response to the context at a single place as a preparation for the
following patch that logs the response.
Change-Id: Idde2f1ba7394f16770543f5ca13411e8c2339cc6
Bug-Url: https://bugzilla.redhat.com/1351247
Backport-To: 4.0
Signed-off-by: Milan Zamazal <mzamazal(a)redhat.com>
Reviewed-on: https://gerrit.ovirt.org/59080
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com>
Reviewed-by: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Continuous-Integration: Jenkins CI
---
M lib/vdsm/rpc/bindingxmlrpc.py
M lib/yajsonrpc/__init__.py
2 files changed, 26 insertions(+), 11 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/98/59998/1
diff --git a/lib/vdsm/rpc/bindingxmlrpc.py b/lib/vdsm/rpc/bindingxmlrpc.py
index 2c4765a..732918f 100644
--- a/lib/vdsm/rpc/bindingxmlrpc.py
+++ b/lib/vdsm/rpc/bindingxmlrpc.py
@@ -302,6 +302,8 @@
def _registerFunctions(self):
def wrapIrsMethod(f):
def wrapper(*args, **kwargs):
+ start_time = utils.monotonic_time()
+
fmt = ""
logargs = []
@@ -321,6 +323,10 @@
except:
self.log.error("Unexpected exception", exc_info=True)
return errCode['unexpected']
+ finally:
+ self.log.info("RPC call %s finished in %.2f seconds",
+ f.__name__,
+ utils.monotonic_time() - start_time)
wrapper.__name__ = f.__name__
wrapper.__doc__ = f.__doc__
@@ -1191,6 +1197,7 @@
def wrapApiMethod(f):
def wrapper(*args, **kwargs):
+ start_time = utils.monotonic_time()
try:
logLevel = logging.DEBUG
suppress_args = f.__name__ in ('fenceNode',)
@@ -1260,6 +1267,10 @@
except:
f.__self__.cif.log.error("unexpected error", exc_info=True)
return errCode['unexpected']
+ finally:
+ f.__self__.cif.log.info("RPC call %s finished in %.2f seconds",
+ f.__name__,
+ utils.monotonic_time() - start_time)
wrapper.__name__ = f.__name__
wrapper.__doc__ = f.__doc__
return wrapper
diff --git a/lib/yajsonrpc/__init__.py b/lib/yajsonrpc/__init__.py
index 71db724..ff8e4ae 100644
--- a/lib/yajsonrpc/__init__.py
+++ b/lib/yajsonrpc/__init__.py
@@ -501,6 +501,14 @@
self._counter = 0
def _serveRequest(self, ctx, req):
+ start_time = monotonic_time()
+ response = self._handle_request(req, ctx.server_address)
+ self.log.info("RPC call %s finished in %.2f seconds",
+ req.method, monotonic_time() - start_time)
+ if response is not None:
+ ctx.requestDone(response)
+
+ def _handle_request(self, req, server_address):
self._attempt_log_stats()
logLevel = logging.DEBUG
@@ -510,8 +518,7 @@
self.log.info("In recovery, ignoring '%s' in bridge with %s",
req.method, req.params)
# TODO: take the response from the exception instead of via errCode
- ctx.requestDone(JsonRpcResponse(errCode['recovery'], None, req.id))
- return
+ return JsonRpcResponse(errCode['recovery'], None, req.id)
self.log.log(logLevel, "Calling '%s' in bridge with %s",
req.method, req.params)
@@ -519,32 +526,29 @@
method = self._bridge.dispatch(req.method)
except JsonRpcMethodNotFoundError as e:
if req.isNotification():
- return
+ return None
- ctx.requestDone(JsonRpcResponse(None, e, req.id))
- return
+ return JsonRpcResponse(None, e, req.id)
try:
params = req.params
- self._bridge.register_server_address(ctx.server_address)
+ self._bridge.register_server_address(server_address)
if isinstance(req.params, list):
res = method(*params)
else:
res = method(**params)
self._bridge.unregister_server_address()
except JsonRpcError as e:
- ctx.requestDone(JsonRpcResponse(None, e, req.id))
+ return JsonRpcResponse(None, e, req.id)
except Exception as e:
self.log.exception("Internal server error")
- ctx.requestDone(JsonRpcResponse(None,
- JsonRpcInternalError(str(e)),
- req.id))
+ return JsonRpcResponse(None, JsonRpcInternalError(str(e)), req.id)
else:
self.log.log(logLevel, "Return '%s' in bridge with %s",
req.method, res)
if isinstance(res, Suppressed):
res = res.value
- ctx.requestDone(JsonRpcResponse(res, None, req.id))
+ return JsonRpcResponse(res, None, req.id)
@traceback(on=log.name)
def serve_requests(self):
--
To view, visit https://gerrit.ovirt.org/59998
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idde2f1ba7394f16770543f5ca13411e8c2339cc6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
7 years, 11 months
Change in vdsm[ovirt-3.6]: hooks: openstack hook should wait a while for nic activation
by mmirecki＠redhat.com
Hello Dan Kenigsberg, Francesco Romani,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/60035
to review the following change.
Change subject: hooks: openstack hook should wait a while for nic activation
......................................................................
hooks: openstack hook should wait a while for nic activation
The vm will be stated in a paused state if a nic managed by
an external openstack network provider is present. The openstack
hook after_vm_start must then wait a given time to allow the
nic to be activated before unpausing the vm.
Change-Id: Ib4f455789909f090a039863fdcfeb61b9db1042f
Bug-Url: https://bugzilla.redhat.com/1314371
Signed-off-by: mirecki <mmirecki(a)redhat.com>
Reviewed-on: https://gerrit.ovirt.org/58917
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
Reviewed-by: Francesco Romani <fromani(a)redhat.com>
---
A vdsm_hooks/openstacknet/after_vm_start.py
M vdsm_hooks/openstacknet/before_device_create.py
M vdsm_hooks/openstacknet/openstacknet_utils.py
3 files changed, 84 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/35/60035/1
diff --git a/vdsm_hooks/openstacknet/after_vm_start.py b/vdsm_hooks/openstacknet/after_vm_start.py
new file mode 100644
index 0000000..6f5c1e5
--- /dev/null
+++ b/vdsm_hooks/openstacknet/after_vm_start.py
@@ -0,0 +1,53 @@
+#!/usr/bin/python
+
+'''
+OpenStack Network Hook (Post vm start)
+=============================================
+The hook unpauses a vm if it was started in the paused state.
+
+Syntax:
+ { 'vmId': 'VM_ID', 'vnic_id': 'port_id' }
+Where:
+ VM_ID should be replaced with the vm id.'''
+
+import libvirt
+import os
+import time
+import traceback
+import hooking
+from openstacknet_utils import MARK_FOR_UNPAUSE_PATH
+from openstacknet_utils import VM_ID_KEY
+from vdsm import vdscli
+
+
+OPENSTACK_NIC_WAIT_TIME = 15
+
+
+def resume_paused_vm(vm_id):
+ unpause_file = MARK_FOR_UNPAUSE_PATH % vm_id
+ if os.path.isfile(unpause_file):
+ vdscli.connect().cont(vm_id)
+ os.remove(unpause_file)
+
+
+def main():
+
+ # TODO (HACK):
+ # This code waits for the nic to be attached to neutron for a
+ # certain amount of time. This is one way of going around the
+ # race between the code and the vm nic becoming active. It is
+ # a very fragile hack, as there is no guarantee the nic will
+ # actually be ready after this.
+ vm_id = os.environ[VM_ID_KEY]
+ launch_flags = hooking.load_vm_launch_flags_from_file(vm_id)
+ if launch_flags == libvirt.VIR_DOMAIN_START_PAUSED:
+ time.sleep(OPENSTACK_NIC_WAIT_TIME)
+ resume_paused_vm(vm_id)
+
+
+if __name__ == '__main__':
+ try:
+ main()
+ except:
+ hooking.exit_hook('openstacknet hook: [unexpected error]: %s\n' %
+ traceback.format_exc())
diff --git a/vdsm_hooks/openstacknet/before_device_create.py b/vdsm_hooks/openstacknet/before_device_create.py
index c37f81a..3cc8376 100755
--- a/vdsm_hooks/openstacknet/before_device_create.py
+++ b/vdsm_hooks/openstacknet/before_device_create.py
@@ -31,6 +31,7 @@
'''
from __future__ import print_function
+import libvirt
import os
import sys
import traceback
@@ -39,12 +40,14 @@
import hooking
from openstacknet_utils import DUMMY_BRIDGE
from openstacknet_utils import INTEGRATION_BRIDGE
+from openstacknet_utils import MARK_FOR_UNPAUSE_PATH
from openstacknet_utils import OPENSTACK_NET_PROVIDER_TYPE
from openstacknet_utils import PLUGIN_TYPE_KEY
from openstacknet_utils import PROVIDER_TYPE_KEY
from openstacknet_utils import PT_BRIDGE
from openstacknet_utils import PT_OVS
from openstacknet_utils import SECURITY_GROUPS_KEY
+from openstacknet_utils import VM_ID_KEY
from openstacknet_utils import VNIC_ID_KEY
from openstacknet_utils import devName
from openstacknet_utils import setUpSecurityGroupVnic
@@ -120,6 +123,16 @@
hooking.exit_hook("Unknown plugin type: %s" % pluginType)
+def mark_for_unpause(vm_id):
+ unpause_file = MARK_FOR_UNPAUSE_PATH % vm_id
+ try:
+ os.makedirs(os.path.dirname(unpause_file))
+ except OSError:
+ pass
+ with open(unpause_file, mode='w') as f:
+ f.write("true")
+
+
def main():
if PROVIDER_TYPE_KEY not in os.environ:
return
@@ -135,6 +148,15 @@
addOpenstackVnic(domxml, pluginType, vNicId, hasSecurityGroups)
hooking.write_domxml(domxml)
+ vm_id = os.environ[VM_ID_KEY]
+ PAUSE_FLAG = libvirt.VIR_DOMAIN_START_PAUSED
+ flags = hooking.load_vm_launch_flags_from_file(vm_id)
+
+ if (flags & PAUSE_FLAG) != PAUSE_FLAG:
+ flags |= PAUSE_FLAG
+ hooking.dump_vm_launch_flags_to_file(vm_id, flags)
+ mark_for_unpause(vm_id)
+
def test(ovs, withSecurityGroups):
domxml = minidom.parseString("""<?xml version="1.0" encoding="utf-8"?>
diff --git a/vdsm_hooks/openstacknet/openstacknet_utils.py b/vdsm_hooks/openstacknet/openstacknet_utils.py
index 11623e7..d5245ad 100644
--- a/vdsm_hooks/openstacknet/openstacknet_utils.py
+++ b/vdsm_hooks/openstacknet/openstacknet_utils.py
@@ -2,11 +2,14 @@
from __future__ import print_function
import hooking
+import os
import subprocess
+from vdsm import constants
from vdsm.netinfo import DUMMY_BRIDGE
from vdsm.utils import CommandPath
# Constants for hook's API
+VM_ID_KEY = 'vmId'
PROVIDER_TYPE_KEY = 'provider_type'
OPENSTACK_NET_PROVIDER_TYPE = 'OPENSTACK_NETWORK'
VNIC_ID_KEY = 'vnic_id'
@@ -26,6 +29,12 @@
ovs_vsctl = CommandPath('ovs-vsctl',
'/usr/sbin/ovs-vsctl',
'/usr/bin/ovs-vsctl')
+MARK_FOR_UNPAUSE_FILE = 'marked_for_unpause'
+MARK_FOR_UNPAUSE_PATH = os.path.join(
+ constants.P_VDSM_RUN,
+ '%s',
+ MARK_FOR_UNPAUSE_FILE,
+)
# Make pyflakes happy
DUMMY_BRIDGE
--
To view, visit https://gerrit.ovirt.org/60035
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib4f455789909f090a039863fdcfeb61b9db1042f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Marcin Mirecki <mmirecki(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
7 years, 11 months
Change in vdsm[ovirt-4.0]: rpc: Log important info from VM stats
by mzamazal＠redhat.com
Hello Nir Soffer,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/59997
to review the following change.
Change subject: rpc: Log important info from VM stats
......................................................................
rpc: Log important info from VM stats
Currently, we don't log results of getAllVmStats API calls. This is not
to fill logs with a lot of uninteresting information. But information
contained in the VM stats is useful and we'd like to have it somewhere.
A possible solution to these contradictory requirements is to log just
important information and to log it only occasionally. In this patch,
we introduce logging VM ids and states in getAllVmStats calls from time
to time.
We implement this as a rather general functionality. We are going to
use the same mechanism for other large API outputs in followup patches.
There may be performance concerns but please consider that we perform
data extraction only for calls that are global (not per VM) and that are
not very frequent (like every second or so). So the processing overhead
shouldn't hurt much and shouldn't be much big in comparison to creating
the returned data inside Vdsm.
Change-Id: Ifcbac615323b62fb9a27e5c0f5a4e98990076146
Signed-off-by: Milan Zamazal <mzamazal(a)redhat.com>
Bug-Url: https://bugzilla.redhat.com/1351247
Backport-To: 4.0
Backport-To: 3.6
Reviewed-on: https://gerrit.ovirt.org/58465
Continuous-Integration: Jenkins CI
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com>
---
M lib/vdsm/logUtils.py
M tests/Makefile.am
A tests/logutils_test.py
M vdsm/API.py
4 files changed, 54 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/97/59997/1
diff --git a/lib/vdsm/logUtils.py b/lib/vdsm/logUtils.py
index 853889d..3794edd 100644
--- a/lib/vdsm/logUtils.py
+++ b/lib/vdsm/logUtils.py
@@ -219,3 +219,9 @@
def __repr__(self):
return '(suppressed)'
+
+
+class AllVmStatsValue(Suppressed):
+
+ def __repr__(self):
+ return repr({vm.get('vmId'): vm.get('status') for vm in self._value})
diff --git a/tests/Makefile.am b/tests/Makefile.am
index db35bdd..d9ffbf7 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -76,6 +76,7 @@
iscsiTests.py \
jobsTests.py \
libvirtconnectionTests.py \
+ logutils_test.py \
lvmTests.py \
main.py \
miscTests.py \
diff --git a/tests/logutils_test.py b/tests/logutils_test.py
new file mode 100644
index 0000000..ba9ef41
--- /dev/null
+++ b/tests/logutils_test.py
@@ -0,0 +1,40 @@
+#
+# Copyright 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+from vdsm.logUtils import AllVmStatsValue
+
+from testlib import VdsmTestCase as TestCaseBase
+
+
+class TestAllVmStats(TestCaseBase):
+
+ _STATS = [{'foo': 'bar',
+ 'status': 'Up',
+ 'vmId': u'43f02a2d-e563-4f11-a7bc-9ee191cfeba1'},
+ {'foo': 'bar',
+ 'status': 'Powering up',
+ 'vmId': u'bd0d066b-971e-42f8-8bc6-d647ab7e0e70'}]
+ _SIMPLIFIED = ({u'43f02a2d-e563-4f11-a7bc-9ee191cfeba1': 'Up',
+ u'bd0d066b-971e-42f8-8bc6-d647ab7e0e70': 'Powering up'})
+
+ def test_allvmstats(self):
+ data = AllVmStatsValue(self._STATS)
+ result = str(data)
+ self.assertEqual(eval(result), self._SIMPLIFIED)
diff --git a/vdsm/API.py b/vdsm/API.py
index 753f6be..9d8f8ab 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -34,10 +34,11 @@
from vdsm import hostdev
from vdsm import response
from vdsm import supervdsm
+from vdsm import throttledlog
from vdsm import jobs
from vdsm import v2v
from vdsm.host import api as hostapi
-from vdsm.logUtils import Suppressed
+from vdsm.logUtils import AllVmStatsValue, Suppressed
from vdsm.storage import clusterlock
from vdsm.storage import misc
from vdsm.storage import constants as sc
@@ -64,6 +65,9 @@
# default message for system shutdown, will be displayed in guest
USER_SHUTDOWN_MESSAGE = 'System going down'
+
+
+throttledlog.throttle('getAllVmStats', 100)
def updateTimestamp():
@@ -1343,6 +1347,8 @@
hooks.before_get_all_vm_stats()
statsList = self._cif.getAllVmStats()
statsList = hooks.after_get_all_vm_stats(statsList)
+ throttledlog.info('getAllVmStats', "Current getAllVmStats: %s",
+ AllVmStatsValue(statsList))
return {'status': doneCode, 'statsList': Suppressed(statsList)}
def hostdevListByCaps(self, caps=None):
--
To view, visit https://gerrit.ovirt.org/59997
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifcbac615323b62fb9a27e5c0f5a4e98990076146
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
7 years, 11 months
Change in vdsm[ovirt-4.0]: logging: Introduce throttledlog
by mzamazal＠redhat.com
Hello Piotr Kliczewski, Nir Soffer, Francesco Romani,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/59995
to review the following change.
Change subject: logging: Introduce throttledlog
......................................................................
logging: Introduce throttledlog
In some situations, we want to log interesting information. A natural
place to retrieve and log the information is where it is created.
However the corresponding code may be called quite often and we don't
need logging the information that often and fill logs with it
excessively. It's sufficient to log it just from time to time.
This patch provides means for occasional logging. The provided
functions actually log only every given number of invocations or every
given time span. We're going to use the functionality in followup
patches.
Change-Id: Ief96f74eb84880827ccf24c3e7ae854403090b8a
Signed-off-by: Milan Zamazal <mzamazal(a)redhat.com>
Bug-Url: https://bugzilla.redhat.com/1351247
Reviewed-on: https://gerrit.ovirt.org/59461
Continuous-Integration: Jenkins CI
Reviewed-by: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Reviewed-by: Francesco Romani <fromani(a)redhat.com>
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com>
---
M lib/vdsm/Makefile.am
A lib/vdsm/throttledlog.py
M tests/Makefile.am
A tests/throttledlog_test.py
M vdsm.spec.in
5 files changed, 201 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/95/59995/1
diff --git a/lib/vdsm/Makefile.am b/lib/vdsm/Makefile.am
index 3fb3a65..c49ee61 100644
--- a/lib/vdsm/Makefile.am
+++ b/lib/vdsm/Makefile.am
@@ -60,6 +60,7 @@
supervdsm.py \
sysctl.py \
taskset.py \
+ throttledlog.py \
udevadm.py \
utils.py \
v2v.py \
diff --git a/lib/vdsm/throttledlog.py b/lib/vdsm/throttledlog.py
new file mode 100644
index 0000000..04cc104
--- /dev/null
+++ b/lib/vdsm/throttledlog.py
@@ -0,0 +1,113 @@
+#
+# Copyright 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+from __future__ import absolute_import
+
+import logging
+
+from vdsm.utils import monotonic_time
+
+
+_DEFAULT_TIMEOUT_SEC = 3600
+_DEFAULT_LOGGING_LEVEL = logging.DEBUG
+
+_logger = logging.getLogger('throttled')
+
+_periodic = {}
+
+
+class _Periodic(object):
+
+ def __init__(self, interval, timeout):
+ self._interval = interval
+ self._timeout = timeout
+ self._counter = 0
+ self._last_time = 0
+
+ def tick(self):
+ now = monotonic_time()
+ result = self._result(now)
+ self._counter = (self._counter + 1) % self._interval
+ if result:
+ self._last_time = now
+ return result
+
+ def _result(self, now):
+ return (self._counter == 0 or
+ (now - self._last_time) >= self._timeout)
+
+
+def throttle(name, interval, timeout=_DEFAULT_TIMEOUT_SEC):
+ """
+ Throttle log messages for `name`, logging at most one message per
+ `interval` calls or always after `timeout` seconds of silence. Throttling
+ applies only to logging performed via `log()` function of this module. The
+ first call of `log()` never throttles the log, following calls are
+ throttled according to the given parameters.
+
+ If this function has already been called for `name`, replace the throttling
+ parameters for `name` with the new ones given here and start throttling
+ from beginning.
+
+ :param name: Arbitrary identifier to be matched in `log()` calls.
+ :type name: basestring
+ :param interval: The number of `log()` calls that should log at least once.
+ :type interval: int
+ :param timeout: The number of seconds without log emitted after which
+ `log()` should always unthrottle the next message.
+ :type timeout: int
+ """
+ _periodic[name] = _Periodic(interval, timeout)
+
+
+def log(name, level, message, *args):
+ """
+ Log `message` and `args` if throttling settings for `name` allow it.
+ See `throttle()` for information about throttling and `name`.
+ `level`, `message` and `args` are passed to `logging.Logger.log()`
+ unchanged.
+
+ :param name: Arbitrary identifier to be matched by `throttle()` settings.
+ :type name: basestring
+
+ .. note::
+
+ Depending on throttling settings and the current logging level `message`
+ and `args` may not be logged at all. So don't perform expensive
+ preprocessing of `args` before calling this function. If you need to
+ modify it before logging it, you may want to use something like
+ `vdsm.logUtils.Suppressed` or its subclasses.
+ """
+ try:
+ periodic = _periodic[name]
+ except KeyError:
+ pass # unthrottled
+ else:
+ if not periodic.tick():
+ return
+
+ _logger.log(level, message, *args)
+
+
+def debug(name, message, *args):
+ log(name, logging.DEBUG, message, *args)
+
+
+def info(name, message, *args):
+ log(name, logging.INFO, message, *args)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6dac859..db35bdd 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -128,6 +128,7 @@
toolTests.py \
toolBondingTests.py \
transportWrapperTests.py \
+ throttledlog_test.py \
unicode_test.py \
utilsTests.py \
vdscliTests.py \
diff --git a/tests/throttledlog_test.py b/tests/throttledlog_test.py
new file mode 100644
index 0000000..9085d07
--- /dev/null
+++ b/tests/throttledlog_test.py
@@ -0,0 +1,85 @@
+#
+# Copyright 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+import logging
+
+from vdsm import throttledlog
+
+from monkeypatch import MonkeyPatch
+from testlib import VdsmTestCase
+
+
+class FakeLogger(object):
+
+ def __init__(self, level):
+ self.level = level
+ self.messages = []
+
+ def isEnabledFor(self, level):
+ return level >= self.level
+
+ def log(self, level, message, *args):
+ if not self.isEnabledFor(level):
+ return
+ self.messages.append(message % args)
+
+
+class FakeTime(object):
+
+ def __init__(self):
+ self.time = 0.0
+
+ def __call__(self):
+ return self.time
+
+
+class TestThrottledLogging(VdsmTestCase):
+
+ @MonkeyPatch(throttledlog, "_logger", FakeLogger(logging.DEBUG))
+ def test_throttled_logging(self):
+ throttledlog.throttle('test', 3)
+ for i in range(5):
+ throttledlog.debug('test', "Cycle: %s", i)
+ self.assertEqual(throttledlog._logger.messages,
+ ['Cycle: 0', 'Cycle: 3'])
+
+ @MonkeyPatch(throttledlog, "_logger", FakeLogger(logging.INFO))
+ def test_no_logging(self):
+ throttledlog.throttle('test', 3)
+ for i in range(5):
+ throttledlog.debug('test', "Cycle: %s", i)
+ self.assertEqual(throttledlog._logger.messages, [])
+
+ @MonkeyPatch(throttledlog, "_logger", FakeLogger(logging.DEBUG))
+ def test_default(self):
+ throttledlog.throttle('test', 3)
+ for i in range(5):
+ throttledlog.debug('other', "Cycle: %s", i)
+ self.assertEqual(throttledlog._logger.messages,
+ ['Cycle: %s' % (i,) for i in range(5)])
+
+ @MonkeyPatch(throttledlog, "_logger", FakeLogger(logging.DEBUG))
+ @MonkeyPatch(throttledlog, "monotonic_time", FakeTime())
+ def test_timeout(self):
+ throttledlog.throttle('test', 10, timeout=7)
+ for i in range(12):
+ throttledlog.debug('test', "Cycle: %s", i)
+ throttledlog.monotonic_time.time += 1.0
+ self.assertEqual(throttledlog._logger.messages,
+ ['Cycle: %s' % (i,) for i in (0, 7, 10,)])
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 6f2157d..7bb8f2a 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -1276,6 +1276,7 @@
%{python_sitelib}/%{vdsm_name}/supervdsm.py*
%{python_sitelib}/%{vdsm_name}/sysctl.py*
%{python_sitelib}/%{vdsm_name}/taskset.py*
+%{python_sitelib}/%{vdsm_name}/throttledlog.py*
%{python_sitelib}/%{vdsm_name}/udevadm.py*
%{python_sitelib}/%{vdsm_name}/utils.py*
%{python_sitelib}/%{vdsm_name}/v2v.py*
--
To view, visit https://gerrit.ovirt.org/59995
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ief96f74eb84880827ccf24c3e7ae854403090b8a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
7 years, 11 months
Change in vdsm[ovirt-4.0]: rpc: Use Suppressed class instead of logging workarounds
by mzamazal＠redhat.com
Hello Nir Soffer,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/59994
to review the following change.
Change subject: rpc: Use Suppressed class instead of logging workarounds
......................................................................
rpc: Use Suppressed class instead of logging workarounds
We have got logging workarounds for getAllVmStats RPC calls.
The workarounds are there to suppress logging large data regularly.
We'd like to get rid of the workarounds, it's better to handle the large
data in the API functions, where we can use specific handling of each
kind of large data.
This patch introduces a wrapper for large values, with the purpose of
suppressing writing them to the log. The wrapper is used for
getAllVmStats so that we get rid of the logging workarounds.
There are other API calls that may produce large outputs, we will handle
them in followup patches.
Change-Id: Idaf00e557fccb8f08fa3aeb38d51cb4bbe0ffe53
Signed-off-by: Milan Zamazal <mzamazal(a)redhat.com>
Bug-Url: https://bugzilla.redhat.com/1351247
Backport-To: 4.0
Backport-To: 3.6
Reviewed-on: https://gerrit.ovirt.org/59078
Continuous-Integration: Jenkins CI
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com>
---
M lib/api/vdsmapi.py
M lib/vdsm/logUtils.py
M lib/vdsm/rpc/bindingxmlrpc.py
M lib/yajsonrpc/__init__.py
M vdsm/API.py
5 files changed, 30 insertions(+), 10 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/94/59994/1
diff --git a/lib/api/vdsmapi.py b/lib/api/vdsmapi.py
old mode 100644
new mode 100755
index 73ce548..04a48b9
--- a/lib/api/vdsmapi.py
+++ b/lib/api/vdsmapi.py
@@ -27,6 +27,7 @@
import yaml
from vdsm.config import config
+from vdsm.logUtils import Suppressed
from yajsonrpc import JsonRpcInvalidParamsError
@@ -303,6 +304,8 @@
ret_args = self.get_ret_param(class_name, method_name)
if ret_args:
+ if isinstance(ret, Suppressed):
+ ret = ret.value
self._verify_type(ret_args.get('type'), ret, class_name,
method_name)
except JsonRpcInvalidParamsError:
diff --git a/lib/vdsm/logUtils.py b/lib/vdsm/logUtils.py
index 5a651c9..853889d 100644
--- a/lib/vdsm/logUtils.py
+++ b/lib/vdsm/logUtils.py
@@ -206,3 +206,16 @@
raise RuntimeError(
"Attempt to open log with incorrect credentials")
return logging.handlers.WatchedFileHandler._open(self)
+
+
+class Suppressed(object):
+
+ def __init__(self, value):
+ self._value = value
+
+ @property
+ def value(self):
+ return self._value
+
+ def __repr__(self):
+ return '(suppressed)'
diff --git a/lib/vdsm/rpc/bindingxmlrpc.py b/lib/vdsm/rpc/bindingxmlrpc.py
index 67f386a..2c4765a 100644
--- a/lib/vdsm/rpc/bindingxmlrpc.py
+++ b/lib/vdsm/rpc/bindingxmlrpc.py
@@ -34,6 +34,7 @@
from vdsm import utils
from vdsm import xmlrpc
from vdsm.define import doneCode, errCode
+from vdsm.logUtils import Suppressed
from vdsm.network.netinfo.addresses import getDeviceByIP
import API
from vdsm.exception import VdsmException
@@ -1192,7 +1193,6 @@
def wrapper(*args, **kwargs):
try:
logLevel = logging.DEBUG
- suppress_logging = f.__name__ in ('getAllVmStats',)
suppress_args = f.__name__ in ('fenceNode',)
# TODO: This password protection code is fragile and ugly. Password
@@ -1239,9 +1239,14 @@
res = f(*args, **kwargs)
else:
res = errCode['recovery']
- log_res = "(suppressed)" if suppress_logging else res
+
f.__self__.cif.log.log(logLevel, 'return %s with %s',
- f.__name__, log_res)
+ f.__name__, res)
+
+ # Ugly hack, but this code is going to be deleted soon.
+ if isinstance(res.get('statsList'), Suppressed):
+ res['statsList'] = res['statsList'].value
+
return res
except libvirt.libvirtError as e:
f.__self__.cif.log.error("libvirt error", exc_info=True)
diff --git a/lib/yajsonrpc/__init__.py b/lib/yajsonrpc/__init__.py
index 6d1eaf7..71db724 100644
--- a/lib/yajsonrpc/__init__.py
+++ b/lib/yajsonrpc/__init__.py
@@ -21,6 +21,7 @@
from vdsm.compat import json
+from vdsm.logUtils import Suppressed
from vdsm.password import protect_passwords, unprotect_passwords
from vdsm.utils import monotonic_time, traceback
from vdsm.define import errCode
@@ -469,8 +470,6 @@
class JsonRpcServer(object):
log = logging.getLogger("jsonrpc.JsonRpcServer")
- FILTERED_METHODS = frozenset(['Host.getAllVmStats'])
-
"""
Creates new JsonrRpcServer by providing a bridge, timeout in seconds
which defining how often we should log connections stats and thread
@@ -504,7 +503,6 @@
def _serveRequest(self, ctx, req):
self._attempt_log_stats()
logLevel = logging.DEBUG
- suppress_logging = req.method in self.FILTERED_METHODS
# VDSM should never respond to any request before all information about
# running VMs is recovered, see https://bugzilla.redhat.com/1339291
@@ -542,10 +540,10 @@
JsonRpcInternalError(str(e)),
req.id))
else:
- res = True if res is None else res
- log_res = "(suppressed)" if suppress_logging else res
self.log.log(logLevel, "Return '%s' in bridge with %s",
- req.method, log_res)
+ req.method, res)
+ if isinstance(res, Suppressed):
+ res = res.value
ctx.requestDone(JsonRpcResponse(res, None, req.id))
@traceback(on=log.name)
diff --git a/vdsm/API.py b/vdsm/API.py
index e37569d..753f6be 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -37,6 +37,7 @@
from vdsm import jobs
from vdsm import v2v
from vdsm.host import api as hostapi
+from vdsm.logUtils import Suppressed
from vdsm.storage import clusterlock
from vdsm.storage import misc
from vdsm.storage import constants as sc
@@ -1342,7 +1343,7 @@
hooks.before_get_all_vm_stats()
statsList = self._cif.getAllVmStats()
statsList = hooks.after_get_all_vm_stats(statsList)
- return {'status': doneCode, 'statsList': statsList}
+ return {'status': doneCode, 'statsList': Suppressed(statsList)}
def hostdevListByCaps(self, caps=None):
devices = hostdev.list_by_caps(caps)
--
To view, visit https://gerrit.ovirt.org/59994
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idaf00e557fccb8f08fa3aeb38d51cb4bbe0ffe53
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
7 years, 11 months
Change in vdsm[master]: vm: Refine needsDriveMonitoring docstring
by Nir Soffer
Nir Soffer has uploaded a new change for review.
Change subject: vm: Refine needsDriveMonitoring docstring
......................................................................
vm: Refine needsDriveMonitoring docstring
Change-Id: I84f3500f16caff2a71e8772179a0f30249747729
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/02/59902/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index c2b1b93..401d4e5 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -997,10 +997,10 @@
def needsDriveMonitoring(self):
"""
- Return True if vm needs drive monitoring for this cycle.
+ Return True if a vm needs drive monitoring in this cycle.
This is called every 2 seconds (configurable) by the periodic system.
- If this retruns True, the periodic system will invoke
+ If this returns True, the periodic system will invoke
extendDrivesIfNeeded during this periodic cycle.
"""
return self._driveMonitorEnabled and bool(self._chunkedDrives())
--
To view, visit https://gerrit.ovirt.org/59902
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I84f3500f16caff2a71e8772179a0f30249747729
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
7 years, 11 months
Change in vdsm[ovirt-4.0]: rpc: Log calls of API methods with possibly large results
by mzamazal＠redhat.com
Hello Nir Soffer, Francesco Romani, Michal Skrivanek,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/59993
to review the following change.
Change subject: rpc: Log calls of API methods with possibly large results
......................................................................
rpc: Log calls of API methods with possibly large results
We can prevent some API methods from logging their excessively large
results. But this doesn't mean we shouldn't log their calls at all, we
just shouldn't log the large values.
This patch re-enables logging calls of the methods with suppressed
logging, it just disables logging their outputs.
Change-Id: I85d7f0ceee7e120081e2d9ad4ca3c8148ee70554
Bug-Url: https://bugzilla.redhat.com/1351247
Backport-To: 4.0
Backport-To: 3.6
Signed-off-by: Milan Zamazal <mzamazal(a)redhat.com>
Reviewed-on: https://gerrit.ovirt.org/58464
Continuous-Integration: Jenkins CI
Reviewed-by: Francesco Romani <fromani(a)redhat.com>
Reviewed-by: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com>
Reviewed-on: https://gerrit.ovirt.org/58709
Continuous-Integration: Francesco Romani <fromani(a)redhat.com>
---
M lib/vdsm/rpc/bindingxmlrpc.py
M lib/yajsonrpc/__init__.py
2 files changed, 6 insertions(+), 6 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/93/59993/1
diff --git a/lib/vdsm/rpc/bindingxmlrpc.py b/lib/vdsm/rpc/bindingxmlrpc.py
index 5cb7dff..67f386a 100644
--- a/lib/vdsm/rpc/bindingxmlrpc.py
+++ b/lib/vdsm/rpc/bindingxmlrpc.py
@@ -1192,8 +1192,7 @@
def wrapper(*args, **kwargs):
try:
logLevel = logging.DEBUG
- if f.__name__ in ('getAllVmStats',):
- logLevel = logging.TRACE
+ suppress_logging = f.__name__ in ('getAllVmStats',)
suppress_args = f.__name__ in ('fenceNode',)
# TODO: This password protection code is fragile and ugly. Password
@@ -1240,8 +1239,9 @@
res = f(*args, **kwargs)
else:
res = errCode['recovery']
+ log_res = "(suppressed)" if suppress_logging else res
f.__self__.cif.log.log(logLevel, 'return %s with %s',
- f.__name__, res)
+ f.__name__, log_res)
return res
except libvirt.libvirtError as e:
f.__self__.cif.log.error("libvirt error", exc_info=True)
diff --git a/lib/yajsonrpc/__init__.py b/lib/yajsonrpc/__init__.py
index 1724a99..6d1eaf7 100644
--- a/lib/yajsonrpc/__init__.py
+++ b/lib/yajsonrpc/__init__.py
@@ -504,8 +504,7 @@
def _serveRequest(self, ctx, req):
self._attempt_log_stats()
logLevel = logging.DEBUG
- if req.method in self.FILTERED_METHODS:
- logLevel = logging.TRACE
+ suppress_logging = req.method in self.FILTERED_METHODS
# VDSM should never respond to any request before all information about
# running VMs is recovered, see https://bugzilla.redhat.com/1339291
@@ -544,8 +543,9 @@
req.id))
else:
res = True if res is None else res
+ log_res = "(suppressed)" if suppress_logging else res
self.log.log(logLevel, "Return '%s' in bridge with %s",
- req.method, res)
+ req.method, log_res)
ctx.requestDone(JsonRpcResponse(res, None, req.id))
@traceback(on=log.name)
--
To view, visit https://gerrit.ovirt.org/59993
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I85d7f0ceee7e120081e2d9ad4ca3c8148ee70554
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
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>
7 years, 11 months
Change in vdsm[ovirt-4.0]: rpc: Lower logging priority just for getAllVmStats
by mzamazal＠redhat.com
Hello Piotr Kliczewski, Francesco Romani,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/59992
to review the following change.
Change subject: rpc: Lower logging priority just for getAllVmStats
......................................................................
rpc: Lower logging priority just for getAllVmStats
Currently, we effectively disable logging of calls for several methods.
But we should consider whether it causes more harm to log the
corresponding information or not to log it.
We should avoid logging excessively large messages. This applies to
getAllVmStats that may report quite a lot of information for many VMs.
But there is probably no good reason not to log everything about the
other methods currently silenced. So with this patch we suppress
logging just of getAllVmStats.
We should be informed about that method as well, we handle that in
followup patches.
Change-Id: I5eefac9f88ede84df06c0cc39b188c8ae75e456b
Bug-Url: https://bugzilla.redhat.com/1351247
Backport-To: 4.0
Backport-To: 3.6
Signed-off-by: Milan Zamazal <mzamazal(a)redhat.com>
Reviewed-on: https://gerrit.ovirt.org/58463
Continuous-Integration: Jenkins CI
Reviewed-by: Francesco Romani <fromani(a)redhat.com>
Reviewed-by: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Reviewed-on: https://gerrit.ovirt.org/58708
Continuous-Integration: Francesco Romani <fromani(a)redhat.com>
---
M lib/vdsm/rpc/bindingxmlrpc.py
M lib/yajsonrpc/__init__.py
2 files changed, 2 insertions(+), 5 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/92/59992/1
diff --git a/lib/vdsm/rpc/bindingxmlrpc.py b/lib/vdsm/rpc/bindingxmlrpc.py
index a1e32e7..5cb7dff 100644
--- a/lib/vdsm/rpc/bindingxmlrpc.py
+++ b/lib/vdsm/rpc/bindingxmlrpc.py
@@ -1192,8 +1192,7 @@
def wrapper(*args, **kwargs):
try:
logLevel = logging.DEBUG
- if f.__name__ in ('getVMList', 'getAllVmStats', 'getStats',
- 'fenceNode', 'setKsmTune'):
+ if f.__name__ in ('getAllVmStats',):
logLevel = logging.TRACE
suppress_args = f.__name__ in ('fenceNode',)
diff --git a/lib/yajsonrpc/__init__.py b/lib/yajsonrpc/__init__.py
index ec3b5ed..1724a99 100644
--- a/lib/yajsonrpc/__init__.py
+++ b/lib/yajsonrpc/__init__.py
@@ -469,9 +469,7 @@
class JsonRpcServer(object):
log = logging.getLogger("jsonrpc.JsonRpcServer")
- FILTERED_METHODS = frozenset(['Host.getVMList', 'Host.getAllVmStats',
- 'Host.getStats', 'StorageDomain.getStats',
- 'VM.getStats', 'Host.fenceNode'])
+ FILTERED_METHODS = frozenset(['Host.getAllVmStats'])
"""
Creates new JsonrRpcServer by providing a bridge, timeout in seconds
--
To view, visit https://gerrit.ovirt.org/59992
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5eefac9f88ede84df06c0cc39b188c8ae75e456b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
7 years, 11 months
Change in vdsm[ovirt-3.6]: migration: downtime: add tunable for back compat
by fromani＠redhat.com
Francesco Romani has uploaded a new change for review.
Change subject: migration: downtime: add tunable for back compat
......................................................................
migration: downtime: add tunable for back compat
Add tunable option to disable the iteration-based
migration downtime adjustment, to preserve the
pre-3.6.8 behaviour.
Change-Id: If477f268354a9609ecca216dad1eee4f40910c8f
Label: ovirt-3.6-only
Bug-Url: https://bugzilla.redhat.com/1339521
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M lib/vdsm/config.py.in
M vdsm/virt/migration.py
2 files changed, 16 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/59467/1
diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index 8be3a22..0c02e5e 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -231,6 +231,10 @@
('migration_create_timeout', '600',
'Time in seconds defining how long we are going to wait for '
'create migration response.'),
+
+ ('migration_use_iterations', 'true',
+ 'Adjust migration downtime considering QEMU iterations, not '
+ 'elapsed time.'),
]),
# Section: [rpc]
diff --git a/vdsm/virt/migration.py b/vdsm/virt/migration.py
index c110503..9ed5076 100644
--- a/vdsm/virt/migration.py
+++ b/vdsm/virt/migration.py
@@ -442,11 +442,17 @@
self._steps = steps
self._stop = threading.Event()
+ self._use_qemu_iterations = config.getboolean(
+ 'vars', 'migration_use_iterations')
+
delay_per_gib = config.getint('vars', 'migration_downtime_delay')
memSize = int(vm.conf['memSize'])
- self._wait = min(
- delay_per_gib * memSize / (_MiB_IN_GiB * self._steps),
- self._WAIT_STEP_LIMIT)
+ if self._use_qemu_iterations:
+ self._wait = min(
+ delay_per_gib * memSize / (_MiB_IN_GiB * self._steps),
+ self._WAIT_STEP_LIMIT)
+ else:
+ self._wait = (delay_per_gib * max(memSize, 2048) + 1023) / 1024
# do not materialize, keep as generator expression
self._downtimes = exponential_downtime(self._downtime, self._steps)
# we need the first value to support set_initial_downtime
@@ -555,6 +561,8 @@
self._vm.log.debug('setting initial migration downtime')
self.downtime_thread.set_initial_downtime()
+ if not self._use_qemu_iterations:
+ self.downtime_thread.start()
while not self._stop.isSet():
stopped = self._stop.wait(self._MIGRATION_MONITOR_INTERVAL)
@@ -587,7 +595,7 @@
iterationCount += 1
self._vm.log.debug('new iteration detected: %i',
iterationCount)
- if iterationCount == 1:
+ if iterationCount == 1 and self._use_qemu_iterations:
# it does not make sense to do any adjustments before
# first iteration.
self.downtime_thread.start()
--
To view, visit https://gerrit.ovirt.org/59467
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If477f268354a9609ecca216dad1eee4f40910c8f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
7 years, 11 months