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 <mzama...@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 <nsof...@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 <mzama...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
