#767 / linux: fix disk_io_counters() for 2.4 and 2.6 kernels

HISTORY.rst

@@ -33,7 +33,8 @@ Bug tracker at https://github.com/giampaolo/psutil/issues
 - #759: [Linux] Process.memory_maps() may return paths ending with " (deleted)"
 - #761: [Windows] psutil.boot_time() wraps to 0 after 49 days.
 - #764: [NetBSD] fix compilation on NetBSD-6.x.
-- #767: [Linux] disk_io_counters() is broken (ValueError) on Linux 2.4.
+- #767: [Linux] disk_io_counters() may raise ValueError on 2.6 kernels and it's
+  broken on 2.4 kernels.
 
 
 3.4.2 - 2016-01-20

psutil/_pslinux.py

@@ -761,26 +761,39 @@ def get_partitions():
     with open_text("%s/diskstats" % get_procfs_path()) as f:
         lines = f.readlines()
     for line in lines:
-        # http://www.mjmwired.net/kernel/Documentation/iostats.txt
+        # OK, this is a bit confusing. The format of /proc/diskstats can
+        # have 3 variations.
+        # On Linux 2.4 each line has always 15 fields, e.g.:
+        # "3     0   8 hda 8 8 8 8 8 8 8 8 8 8 8"
+        # On Linux 2.6+ each line *usually* has 14 fields, and the disk
+        # name is in another position, like this:
+        # "3    0   hda 8 8 8 8 8 8 8 8 8 8 8"
+        # ...unless (Linux 2.6) the line refers to a partition instead
+        # of a disk, in which case the line has less fields (7):
+        # "3    1   hda1 8 8 8 8"
+        # See:
         # https://www.kernel.org/doc/Documentation/iostats.txt
         fields = line.split()
         fields_len = len(fields)
-        if fields_len == 14:
-            # Linux 2.6+
-            name = fields[2]
-            (reads, reads_merged, rbytes, rtime, writes, writes_merged,
-                wbytes, wtime) = map(int, fields[3:11])
-        else:
+        if fields_len == 15:
             # Linux 2.4
-            if fields_len != 15:
-                raise RuntimeError(
-                    "not sure how to interpret line %r; expected numer of "
-                    "space-separated values is 15, I found %s"
-                    % (line, fields_len))
-            reads = int(fields[2])
             name = fields[3]
+            reads = int(fields[2])
             (reads_merged, rbytes, rtime, writes, writes_merged,
                 wbytes, wtime) = map(int, fields[4:11])
+        elif fields_len == 14:
+            # Linux 2.6+, line referring to a disk
+            name = fields[2]
+            (reads, reads_merged, rbytes, rtime, writes, writes_merged,
+                wbytes, wtime) = map(int, fields[3:11])
+        elif fields_len == 7:
+            # Linux 2.6+, line referring to a partition
+            name = fields[2]
+            reads, rbytes, writes, wbytes = map(int, fields[3:])
+            rtime = wtime = reads_merged = writes_merged = 0
+        else:
+            raise ValueError("not sure how to interpret line %r" % line)
+
         if name in partitions:
             rbytes = rbytes * SECTOR_SIZE
             wbytes = wbytes * SECTOR_SIZE

psutil/tests/test_linux.py

@@ -400,19 +400,18 @@ def test_disk_partitions_mocked(self):
                     self.assertEqual(ret[0].fstype, 'zfs')
 
     def test_disk_io_counters_kernel_2_4_mocked(self):
-        # This tests /proc/diskstats format which is different on 2.4
-        # kernels. The spec is here:
-        # https://www.kernel.org/doc/Documentation/iostats.txt
+        # Tests /proc/diskstats parsing format for 2.4 kernels, see:
+        # https://github.com/giampaolo/psutil/issues/767
         def open_mock(name, *args, **kwargs):
             if name == '/proc/partitions':
                 return io.StringIO(textwrap.dedent(u"""\
                     major minor  #blocks  name
 
-                       8        0  488386584 sda
+                       8        0  488386584 hda
                     """))
             elif name == '/proc/diskstats':
                 return io.StringIO(
-                    u("   3     0   1 sda 2 3 4 5 6 7 8 9 10 11 12"))
+                    u("   3     0   1 hda 2 3 4 5 6 7 8 9 10 11 12"))
             else:
                 return orig_open(name, *args, **kwargs)
             return orig_open(name, *args)
@@ -431,21 +430,72 @@ def open_mock(name, *args, **kwargs):
             self.assertEqual(ret.write_bytes, 7 * 512)
             self.assertEqual(ret.write_time, 8)
 
-    def test_disk_io_counters_malformed_mocked(self):
-        # Simulate a malformed /proc/diskstats file having a line with
-        # too many fields.
+    def test_disk_io_counters_kernel_2_6_full_mocked(self):
+        # Tests /proc/diskstats parsing format for 2.6 kernels,
+        # lines reporting all metrics:
+        # https://github.com/giampaolo/psutil/issues/767
         def open_mock(name, *args, **kwargs):
-            if name == '/proc/diskstats':
+            if name == '/proc/partitions':
+                return io.StringIO(textwrap.dedent(u"""\
+                    major minor  #blocks  name
+
+                       8        0  488386584 hda
+                    """))
+            elif name == '/proc/diskstats':
                 return io.StringIO(
-                    u("   3     0   1 sda 2 3 4 5 6 7 8 9 10 11 12 13 14"))
+                    u("   3    0   hda 1 2 3 4 5 6 7 8 9 10 11"))
             else:
                 return orig_open(name, *args, **kwargs)
             return orig_open(name, *args)
 
         orig_open = open
         patch_point = 'builtins.open' if PY3 else '__builtin__.open'
-        with mock.patch(patch_point, side_effect=open_mock):
-            self.assertRaises(RuntimeError, psutil.disk_io_counters)
+        with mock.patch(patch_point, side_effect=open_mock) as m:
+            ret = psutil.disk_io_counters()
+            assert m.called
+            self.assertEqual(ret.read_count, 1)
+            self.assertEqual(ret.read_merged_count, 2)
+            self.assertEqual(ret.read_bytes, 3 * 512)
+            self.assertEqual(ret.read_time, 4)
+            self.assertEqual(ret.write_count, 5)
+            self.assertEqual(ret.write_merged_count, 6)
+            self.assertEqual(ret.write_bytes, 7 * 512)
+            self.assertEqual(ret.write_time, 8)
+
+    def test_disk_io_counters_kernel_2_6_limited_mocked(self):
+        # Tests /proc/diskstats parsing format for 2.6 kernels,
+        # where one line of /proc/partitions return a limited
+        # amount of metrics when it bumps into a partition
+        # (instead of a disk). See:
+        # https://github.com/giampaolo/psutil/issues/767
+        def open_mock(name, *args, **kwargs):
+            if name == '/proc/partitions':
+                return io.StringIO(textwrap.dedent(u"""\
+                    major minor  #blocks  name
+
+                       8        0  488386584 hda
+                    """))
+            elif name == '/proc/diskstats':
+                return io.StringIO(
+                    u("   3    1   hda 1 2 3 4"))
+            else:
+                return orig_open(name, *args, **kwargs)
+            return orig_open(name, *args)
+
+        orig_open = open
+        patch_point = 'builtins.open' if PY3 else '__builtin__.open'
+        with mock.patch(patch_point, side_effect=open_mock) as m:
+            ret = psutil.disk_io_counters()
+            assert m.called
+            self.assertEqual(ret.read_count, 1)
+            self.assertEqual(ret.read_bytes, 2 * 512)
+            self.assertEqual(ret.write_count, 3)
+            self.assertEqual(ret.write_bytes, 4 * 512)
+
+            self.assertEqual(ret.read_merged_count, 0)
+            self.assertEqual(ret.read_time, 0)
+            self.assertEqual(ret.write_merged_count, 0)
+            self.assertEqual(ret.write_time, 0)
 
 
 # =====================================================================