From 31b3a1bdc327e0afa0019b9d7ed5108661465ba1 Mon Sep 17 00:00:00 2001 From: Arnon Yaari Date: Wed, 25 Jul 2018 23:10:01 +0300 Subject: [PATCH] Differentiate between partitions and devices using sysfs disk_io_counters skips devices that have partitions, and to differentiate partitions from devices we can't use the names - we need to look in sysfs. Partitions and devices have entries in /sys/class/block and only devices have entries in /sys/block This fixes counters of NVMe devices. Also fixed looking up the sector size of partitions. --- psutil/_pslinux.py | 54 ++++++++++++++++++++---------- psutil/tests/test_linux.py | 68 +++++++++++++++++++++----------------- 2 files changed, 74 insertions(+), 48 deletions(-) diff --git a/psutil/_pslinux.py b/psutil/_pslinux.py index ffa3a8a32..e4845ff79 100644 --- a/psutil/_pslinux.py +++ b/psutil/_pslinux.py @@ -252,12 +252,33 @@ def file_flags_to_mode(flags): return mode +def get_partition_device(partition): + """given a partition, return the device that the partition is a part of. + If "parition" is already a name of a device, return None + """ + # https://unix.stackexchange.com/questions/226420 + # /sys/class/block/sda1 -> + # ../../devices/pci0000:00/0000:00:10.0/host2/target2:0:0/ + # 2:0:0:0/block/sda/sda1 + # for NVMe: + # /sys/class/block/nvme0n1p1 -> + # ../../devices/pci0000:00/0000:00:17.0/0000:13:00.0/ + # nvme/nvme0/nvme0n1/nvme0n1p1 + device_path = os.readlink("/sys/class/block/" + partition) + parent = device_path.split(os.sep)[-2] + # "/sys/block" doesn't contain partitions, but does contain devices + if os.path.exists("/sys/block/" + parent): + return parent + return None + + def get_sector_size(partition): - """Return the sector size of a partition. + """Return the sector size of a device or partition. Used by disk_io_counters(). """ + device = get_partition_device(partition) or partition try: - with open("/sys/block/%s/queue/hw_sector_size" % partition, "rt") as f: + with open("/sys/block/%s/queue/hw_sector_size" % device, "rt") as f: return int(f.read()) except (IOError, ValueError): # man iostat states that sectors are equivalent with blocks and @@ -1036,20 +1057,19 @@ def disk_io_counters(): def get_partitions(): partitions = [] with open_text("%s/partitions" % get_procfs_path()) as f: - lines = f.readlines()[2:] - for line in reversed(lines): - _, _, _, name = line.split() - if name[-1].isdigit(): - # we're dealing with a partition (e.g. 'sda1'); 'sda' will - # also be around but we want to omit it + for line in f.readlines()[2:]: + _, _, _, name = line.split() partitions.append(name) - else: - if not partitions or not partitions[-1].startswith(name): - # we're dealing with a disk entity for which no - # partitions have been defined (e.g. 'sda' but - # 'sda1' was not around), see: - # https://github.com/giampaolo/psutil/issues/338 - partitions.append(name) + # when we sum all disk counters, we need to ignore devices + # that have partitions, to avoid multiple counting. + # e.g. if we have "sda" and "sda1", we don't want "sda" + # note that a "partition in partitions" can be a name of a device - + # "get_partition_device" will return None for these names, and they + # will not be in "parent_devices" if they don't have partitions + parent_devices = set(get_partition_device(partition) + for partition in partitions) + partitions = [partition for partition in partitions if + partition not in parent_devices] return partitions retdict = {} @@ -1079,12 +1099,12 @@ def get_partitions(): (reads_merged, rbytes, rtime, writes, writes_merged, wbytes, wtime, _, busy_time, _) = map(int, fields[4:14]) elif fields_len == 14: - # Linux 2.6+, line referring to a disk + # Linux 2.6+, line referring to a disk or partition name = fields[2] (reads, reads_merged, rbytes, rtime, writes, writes_merged, wbytes, wtime, _, busy_time, _) = map(int, fields[3:14]) elif fields_len == 7: - # Linux 2.6+, line referring to a partition + # Linux >=2.6, <2.6.25, line referring to a partition name = fields[2] reads, rbytes, writes, wbytes = map(int, fields[3:]) rtime = wtime = reads_merged = writes_merged = busy_time = 0 diff --git a/psutil/tests/test_linux.py b/psutil/tests/test_linux.py index 9ea59b617..c36b3f25c 100755 --- a/psutil/tests/test_linux.py +++ b/psutil/tests/test_linux.py @@ -995,16 +995,18 @@ def test_disk_io_counters_kernel_2_4_mocked(self): with mock_open_content( '/proc/diskstats', " 3 0 1 hda 2 3 4 5 6 7 8 9 10 11 12"): - ret = psutil.disk_io_counters(nowrap=False) - self.assertEqual(ret.read_count, 1) - self.assertEqual(ret.read_merged_count, 2) - self.assertEqual(ret.read_bytes, 3 * SECTOR_SIZE) - 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 * SECTOR_SIZE) - self.assertEqual(ret.write_time, 8) - self.assertEqual(ret.busy_time, 10) + with mock.patch('psutil._pslinux.os.readlink', + return_value='block/hda'): + ret = psutil.disk_io_counters(nowrap=False) + self.assertEqual(ret.read_count, 1) + self.assertEqual(ret.read_merged_count, 2) + self.assertEqual(ret.read_bytes, 3 * SECTOR_SIZE) + 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 * SECTOR_SIZE) + self.assertEqual(ret.write_time, 8) + self.assertEqual(ret.busy_time, 10) def test_disk_io_counters_kernel_2_6_full_mocked(self): # Tests /proc/diskstats parsing format for 2.6 kernels, @@ -1020,16 +1022,18 @@ def test_disk_io_counters_kernel_2_6_full_mocked(self): with mock_open_content( '/proc/diskstats', " 3 0 hda 1 2 3 4 5 6 7 8 9 10 11"): - ret = psutil.disk_io_counters(nowrap=False) - self.assertEqual(ret.read_count, 1) - self.assertEqual(ret.read_merged_count, 2) - self.assertEqual(ret.read_bytes, 3 * SECTOR_SIZE) - 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 * SECTOR_SIZE) - self.assertEqual(ret.write_time, 8) - self.assertEqual(ret.busy_time, 10) + with mock.patch('psutil._pslinux.os.readlink', + return_value='block/hda'): + ret = psutil.disk_io_counters(nowrap=False) + self.assertEqual(ret.read_count, 1) + self.assertEqual(ret.read_merged_count, 2) + self.assertEqual(ret.read_bytes, 3 * SECTOR_SIZE) + 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 * SECTOR_SIZE) + self.assertEqual(ret.write_time, 8) + self.assertEqual(ret.busy_time, 10) def test_disk_io_counters_kernel_2_6_limited_mocked(self): # Tests /proc/diskstats parsing format for 2.6 kernels, @@ -1047,17 +1051,19 @@ def test_disk_io_counters_kernel_2_6_limited_mocked(self): with mock_open_content( '/proc/diskstats', " 3 1 hda 1 2 3 4"): - ret = psutil.disk_io_counters(nowrap=False) - self.assertEqual(ret.read_count, 1) - self.assertEqual(ret.read_bytes, 2 * SECTOR_SIZE) - self.assertEqual(ret.write_count, 3) - self.assertEqual(ret.write_bytes, 4 * SECTOR_SIZE) - - 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) - self.assertEqual(ret.busy_time, 0) + with mock.patch('psutil._pslinux.os.readlink', + return_value='block/hda'): + ret = psutil.disk_io_counters(nowrap=False) + self.assertEqual(ret.read_count, 1) + self.assertEqual(ret.read_bytes, 2 * SECTOR_SIZE) + self.assertEqual(ret.write_count, 3) + self.assertEqual(ret.write_bytes, 4 * SECTOR_SIZE) + + 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) + self.assertEqual(ret.busy_time, 0) # =====================================================================