From 8cc496d18e6d15e8b9d17720191c83fecac67ca0 Mon Sep 17 00:00:00 2001 From: Giampaolo Rodola Date: Sun, 14 Feb 2016 13:47:39 +0100 Subject: [PATCH 1/4] fix #756 / linux / disk_io_counters: add r/w merged count fields --- HISTORY.rst | 2 ++ docs/index.rst | 8 ++++++++ psutil/__init__.py | 5 +++-- psutil/_pslinux.py | 25 ++++++++++++++----------- psutil/tests/test_system.py | 5 +++++ 5 files changed, 32 insertions(+), 13 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 30fd1eecf..034bdabce 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -13,6 +13,8 @@ Bug tracker at https://github.com/giampaolo/psutil/issues - #755: Process.memory_percent() "memtype" parameter. - #758: tests now live in psutil namespace. - #760: expose OS constants (LINUX, OSX, etc.) +- #756: [Linux] disk_io_counters() return 2 new fields: read_merged_count and + write_merged_count. - #762: new sripts/procsmem.py script. **Bug fixes** diff --git a/docs/index.rst b/docs/index.rst index eb8b283bf..94cc623df 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -275,6 +275,11 @@ Disks - **read_time**: time spent reading from disk (in milliseconds) - **write_time**: time spent writing to disk (in milliseconds) + On Linux we get the following extra fields: + + - **read_merged_count** (Linux): number of merged reads (see `iostat doc `__). + - **write_merged_count** (Linux): number of merged writes (see `iostat doc `__). + If *perdisk* is ``True`` return the same information for every physical disk installed on the system as a dictionary with partition names as the keys and the namedtuple described above as the values. @@ -290,6 +295,9 @@ Disks 'sda2': sdiskio(read_count=18707, write_count=8830, read_bytes=6060, write_bytes=3443, read_time=24585, write_time=1572), 'sdb1': sdiskio(read_count=161, write_count=0, read_bytes=786432, write_bytes=0, read_time=44, write_time=0)} + .. versionchanged:: 4.0.0 *read_merged_count* and *write_merged_count* were + addded on Linux. + Network ------- diff --git a/psutil/__init__.py b/psutil/__init__.py index 169a5e8bd..297ef5cd3 100644 --- a/psutil/__init__.py +++ b/psutil/__init__.py @@ -1799,12 +1799,13 @@ def disk_io_counters(perdisk=False): rawdict = _psplatform.disk_io_counters() if not rawdict: raise RuntimeError("couldn't find any physical disk") + nt = getattr(_psplatform, "sdiskio", _common.sdiskio) if perdisk: for disk, fields in rawdict.items(): - rawdict[disk] = _common.sdiskio(*fields) + rawdict[disk] = nt(*fields) return rawdict else: - return _common.sdiskio(*[sum(x) for x in zip(*rawdict.values())]) + return nt(*[sum(x) for x in zip(*rawdict.values())]) # ===================================================================== diff --git a/psutil/_pslinux.py b/psutil/_pslinux.py index b060b1a07..797c6556e 100644 --- a/psutil/_pslinux.py +++ b/psutil/_pslinux.py @@ -217,6 +217,10 @@ def set_scputimes_ntuple(procfs_path): svmem = namedtuple( 'svmem', ['total', 'available', 'percent', 'used', 'free', 'active', 'inactive', 'buffers', 'cached']) +sdiskio = namedtuple('sdiskio', ['read_count', 'write_count', + 'read_bytes', 'write_bytes', + 'read_time', 'write_time', + 'read_merged_count', 'write_merged_count']) pmem = namedtuple('pmem', 'rss vms shared text lib data dirty') paddrspmem = namedtuple('paddrspmem', ['uss', 'pss', 'swap']) @@ -758,22 +762,21 @@ def get_partitions(): lines = f.readlines() for line in lines: # http://www.mjmwired.net/kernel/Documentation/iostats.txt + # https://www.kernel.org/doc/Documentation/iostats.txt fields = line.split() + name = fields[2] if len(fields) > 7: - _, _, name, reads, _, rbytes, rtime, writes, _, wbytes, wtime = \ - fields[:11] + (reads, reads_merged, rbytes, rtime, writes, writes_merged, + wbytes, wtime) = map(int, fields[3:11]) else: # from kernel 2.6.0 to 2.6.25 - _, _, name, reads, rbytes, writes, wbytes = fields - rtime, wtime = 0, 0 + reads, rbytes, writes, wbytes = map(int, fields[3:11]) + rtime, wtime, reads_merged, writes_merged = 0, 0, 0, 0 if name in partitions: - rbytes = int(rbytes) * SECTOR_SIZE - wbytes = int(wbytes) * SECTOR_SIZE - reads = int(reads) - writes = int(writes) - rtime = int(rtime) - wtime = int(wtime) - retdict[name] = (reads, writes, rbytes, wbytes, rtime, wtime) + rbytes = rbytes * SECTOR_SIZE + wbytes = wbytes * SECTOR_SIZE + retdict[name] = (reads, writes, rbytes, wbytes, rtime, wtime, + reads_merged, writes_merged) return retdict diff --git a/psutil/tests/test_system.py b/psutil/tests/test_system.py index b1e67d3fa..d2e4070e3 100644 --- a/psutil/tests/test_system.py +++ b/psutil/tests/test_system.py @@ -612,6 +612,11 @@ def check_ntuple(nt): self.assertEqual(nt[3], nt.write_bytes) self.assertEqual(nt[4], nt.read_time) self.assertEqual(nt[5], nt.write_time) + if LINUX: + self.assertEqual(nt[6], nt.read_merged_count) + self.assertEqual(nt[7], nt.write_merged_count) + assert nt.read_merged_count >= 0, nt + assert nt.write_merged_count >= 0, nt assert nt.read_count >= 0, nt assert nt.write_count >= 0, nt assert nt.read_bytes >= 0, nt From 5ae7fae396afdfc5d37766b9078c5c4518ad2df9 Mon Sep 17 00:00:00 2001 From: Giampaolo Rodola Date: Sun, 14 Feb 2016 14:43:14 +0100 Subject: [PATCH 2/4] #767: [Linux] disk_io_counters() is broken (ValueError) on Linux 2.4. --- HISTORY.rst | 1 + psutil/_pslinux.py | 19 +++++++++++---- psutil/tests/test_linux.py | 50 +++++++++++++++++++++++++++++--------- 3 files changed, 53 insertions(+), 17 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 034bdabce..3932311ed 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -33,6 +33,7 @@ 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. 3.4.2 - 2016-01-20 diff --git a/psutil/_pslinux.py b/psutil/_pslinux.py index 797c6556e..15dcede2e 100644 --- a/psutil/_pslinux.py +++ b/psutil/_pslinux.py @@ -764,14 +764,23 @@ def get_partitions(): # http://www.mjmwired.net/kernel/Documentation/iostats.txt # https://www.kernel.org/doc/Documentation/iostats.txt fields = line.split() - name = fields[2] - if len(fields) > 7: + 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: - # from kernel 2.6.0 to 2.6.25 - reads, rbytes, writes, wbytes = map(int, fields[3:11]) - rtime, wtime, reads_merged, writes_merged = 0, 0, 0, 0 + # 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_merged, rbytes, rtime, writes, writes_merged, + wbytes, wtime) = map(int, fields[4:11]) 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 13c2e4642..54980d0f4 100644 --- a/psutil/tests/test_linux.py +++ b/psutil/tests/test_linux.py @@ -399,18 +399,22 @@ def test_disk_partitions_mocked(self): assert ret self.assertEqual(ret[0].fstype, 'zfs') - # not sure why (doesn't fail locally) - # https://travis-ci.org/giampaolo/psutil/jobs/108629915 - @unittest.skipIf(TRAVIS, "fails on travis") - def test_disk_io_counters_mocked(self): - # From kernel 2.6.0 to 2.6.25 /proc/diskstats has less fields; - # we test psutil handles this case by setting read_time and - # write_time to 0. + 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 def open_mock(name, *args, **kwargs): - if name == ('/proc/partitions'): - return orig_open(name, *args, **kwargs) + if name == '/proc/partitions': + return io.StringIO(textwrap.dedent(u"""\ + major minor #blocks name + + 8 0 488386584 sda + """)) + elif name == '/proc/diskstats': + return io.StringIO( + u(" 3 0 1 sda 2 3 4 5 6 7 8 9 10 11 12")) else: - return io.StringIO(u("8 1 sda1 2 2 2 2\n")) + return orig_open(name, *args, **kwargs) return orig_open(name, *args) orig_open = open @@ -418,8 +422,30 @@ def open_mock(name, *args, **kwargs): with mock.patch(patch_point, side_effect=open_mock) as m: ret = psutil.disk_io_counters() assert m.called - self.assertEqual(ret.read_time, 0) - self.assertEqual(ret.write_time, 0) + 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_malformed_mocked(self): + # Simulate a malformed /proc/diskstats file having a line with + # too many fields. + def open_mock(name, *args, **kwargs): + if name == '/proc/diskstats': + return io.StringIO( + u(" 3 0 1 sda 2 3 4 5 6 7 8 9 10 11 12 13 14")) + 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) # ===================================================================== From 9fc207f78c8eb14a9391c2d5acbe2f1190052ec6 Mon Sep 17 00:00:00 2001 From: Giampaolo Rodola Date: Sun, 14 Feb 2016 18:35:17 +0100 Subject: [PATCH 3/4] #767 / linux: fix disk_io_counters() for 2.4 and 2.6 kernels --- HISTORY.rst | 3 +- psutil/_pslinux.py | 39 +++++++++++++------- psutil/tests/test_linux.py | 74 +++++++++++++++++++++++++++++++------- 3 files changed, 90 insertions(+), 26 deletions(-) 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) # ===================================================================== From f6e0374bed5810b3c6a8d68eeea481a7a24fe509 Mon Sep 17 00:00:00 2001 From: Giampaolo Rodola Date: Sun, 14 Feb 2016 18:39:48 +0100 Subject: [PATCH 4/4] #766: provide culprit line in case of error --- psutil/_pslinux.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/psutil/_pslinux.py b/psutil/_pslinux.py index 5bddb7bdb..d80dfba2b 100644 --- a/psutil/_pslinux.py +++ b/psutil/_pslinux.py @@ -579,14 +579,14 @@ def process_inet(self, file, family, type_, inodes, filter_pid=None): return with open_text(file, buffering=BIGGER_FILE_BUFFERING) as f: f.readline() # skip the first line - for line in f: + for lineno, line in enumerate(f, 1): try: _, laddr, raddr, status, _, _, _, _, _, inode = \ line.split()[:10] except ValueError: raise RuntimeError( - "error while parsing %s; malformed line %r" % ( - file, line)) + "error while parsing %s; malformed line %s %r" % ( + file, lineno, line)) if inode in inodes: # # We assume inet sockets are unique, so we error # # out if there are multiple references to the