Francesco Romani has uploaded a new change for review.

Change subject: vmstats: network: avoid ZeroDivisionError
......................................................................

vmstats: network: avoid ZeroDivisionError

For still under investigation case, after migration
sometimes vmstats code is fed with zero-intervalled samples.
While we investigate the root cause, handle gracefully ZeroDivisionError
adding guards in the public functions that have an 'interval' argument.

The only left one was 'networks', so this patch fixes that.

It is not good enough to generalize the check in vmstats.produce()
because the relevant functions (cpu, disks, networks) are part of
the public API of the module, so they should handle bad input properly.

To improve testability, we fix the return value of the aforementioned
functions. The return value is meant to be used only in testing.

Change-Id: I1ff99e3706212179ef9dc56e92e7d5decdd6bd7a
Signed-off-by: Francesco Romani <[email protected]>
---
M tests/vmStatsTests.py
M vdsm/virt/vmstats.py
2 files changed, 45 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/64/44764/1

diff --git a/tests/vmStatsTests.py b/tests/vmStatsTests.py
index 9259335..5db3af1 100644
--- a/tests/vmStatsTests.py
+++ b/tests/vmStatsTests.py
@@ -217,6 +217,7 @@
         self.assertEqual(len(all_indexes), len(self.samples))
 
 
+@expandPermutations
 class NetworkStatsTests(VmStatsTestCase):
 
     # TODO: grab them from the schema
@@ -261,6 +262,35 @@
                          self.interval)
         self.assertRepeatedStatsHaveKeys(nics, stats['network'])
 
+    def test_networks_good_interval(self):
+        nics = (
+            FakeNic(name='vnet0', model='virtio',
+                    mac_addr='00:1a:4a:16:01:51'),
+        )
+        vm = FakeVM(nics=nics)
+
+        stats = {}
+        self.assertTrue(
+            vmstats.networks(vm, stats,
+                             self.bulk_stats, self.bulk_stats,
+                             1)
+        )
+
+    @permutations([[-42], [0]])
+    def test_networks_bad_interval(self, interval):
+        nics = (
+            FakeNic(name='vnet0', model='virtio',
+                    mac_addr='00:1a:4a:16:01:51'),
+        )
+        vm = FakeVM(nics=nics)
+
+        stats = {}
+        self.assertTrue(
+            vmstats.networks(vm, stats,
+                             self.bulk_stats, self.bulk_stats,
+                             0) is None
+        )
+
 
 class DiskStatsTests(VmStatsTestCase):
 
diff --git a/vdsm/virt/vmstats.py b/vdsm/virt/vmstats.py
index 455586c..cf25574 100644
--- a/vdsm/virt/vmstats.py
+++ b/vdsm/virt/vmstats.py
@@ -101,12 +101,12 @@
     stats['cpuSys'] = 0.0
 
     if first_sample is None or last_sample is None:
-        return
+        return None
     if interval <= 0:
         logging.warning(
             'invalid interval %i when computing CPU stats',
             interval)
-        return
+        return None
 
     try:
         stats['cpuUsage'] = str(last_sample['cpu.system'] +
@@ -124,6 +124,8 @@
 
     except KeyError as e:
         logging.exception("CPU stats not available: %s", e)
+
+    return stats
 
 
 def balloon(vm, stats, sample):
@@ -208,7 +210,12 @@
     stats['network'] = {}
 
     if first_sample is None or last_sample is None:
-        return
+        return None
+    if interval <= 0:
+        logging.warning(
+            'invalid interval %i when computing network stats for vm %s',
+            interval, vm.id)
+        return None
 
     first_indexes = _find_bulk_stats_reverse_map(first_sample, 'net')
     last_indexes = _find_bulk_stats_reverse_map(last_sample, 'net')
@@ -227,10 +234,12 @@
             last_sample, last_indexes[nic.name],
             interval)
 
+    return stats
+
 
 def disks(vm, stats, first_sample, last_sample, interval):
     if first_sample is None or last_sample is None:
-        return
+        return None
 
     first_indexes = _find_bulk_stats_reverse_map(first_sample, 'block')
     last_indexes = _find_bulk_stats_reverse_map(last_sample, 'block')
@@ -284,6 +293,8 @@
     if disk_stats:
         stats['disks'] = disk_stats
 
+    return stats
+
 
 def _disk_rate(first_sample, first_index, last_sample, last_index, interval):
     return {


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1ff99e3706212179ef9dc56e92e7d5decdd6bd7a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to