diff --git a/HISTORY.rst b/HISTORY.rst index 3932311ed..c76e4a9a6 100644 --- a/HISTORY.rst +++ b/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 diff --git a/psutil/_pslinux.py b/psutil/_pslinux.py index 15dcede2e..5bddb7bdb 100644 --- a/psutil/_pslinux.py +++ b/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 diff --git a/psutil/tests/test_linux.py b/psutil/tests/test_linux.py index 54980d0f4..1d19eff64 100644 --- a/psutil/tests/test_linux.py +++ b/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) # =====================================================================