Change in vdsm[ovirt-3.6]: vmstats: network: avoid ZeroDivisionError

fromani at redhat.com fromani at redhat.com
Thu Nov 12 10:32:14 UTC 2015


Hello Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/48478

to review the following change.

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.

X-Backport-To: 3.6
Change-Id: I1ff99e3706212179ef9dc56e92e7d5decdd6bd7a
Signed-off-by: Francesco Romani <fromani at redhat.com>
Bug-Url: https://bugzilla.redhat.com/1280223
Reviewed-on: https://gerrit.ovirt.org/44764
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
Continuous-Integration: Dan Kenigsberg <danken at redhat.com>
---
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/78/48478/1

diff --git a/tests/vmStatsTests.py b/tests/vmStatsTests.py
index 44fe296..ef43524 100644
--- a/tests/vmStatsTests.py
+++ b/tests/vmStatsTests.py
@@ -287,6 +287,7 @@
         self.assertTrue(indexes)
 
 
+ at expandPermutations
 class NetworkStatsTests(VmStatsTestCase):
 
     # TODO: grab them from the schema
@@ -331,6 +332,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 1f89c3f..14d1f56 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/48478
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1ff99e3706212179ef9dc56e92e7d5decdd6bd7a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list