Skip to content

Commit

Permalink
Merge pull request #1979 from phillxnet/1978_regression_in_dashboard_…
Browse files Browse the repository at this point in the history 
…disk_activity_widge

regression in dashboard disk activity widget. Fixes #1978
  • Loading branch information
@schakrava
schakrava authored Nov 1, 2018
2 parents ccbd952 + 045e748 commit 8ceef40
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 3 deletions.
3 changes: 3 additions & 0 deletions src/rockstor/smart_manager/data_collector.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,9 @@ def disk_stats(prev_stats):
stats_file_path = '/proc/diskstats'
cur_stats = {}
interval = 1
# TODO: Consider refactoring the following to use Disk.temp_name or
# TODO: building byid_disk_map from the same. Ideally we would have
# TODO: performance testing in place prior to this move.
# Build a list of our db's disk names, now in by-id type format.
disks = [d.name for d in Disk.objects.all()]
# /proc/diskstats has lines of the following form:
Expand Down
4 changes: 2 additions & 2 deletions src/rockstor/system/osi.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1620,7 +1620,7 @@ def get_dev_byid_name(device_name, remove_path=False):


def get_byid_name_map():
"""Simple wrapper around 'ls -l /dev/disk/by-id' which returns a current
"""Simple wrapper around 'ls -lr /dev/disk/by-id' which returns a current
mapping of all attached by-id device names to their sdX counterparts. When
multiple by-id names are found for the same sdX device then the longest is
preferred, or when equal in length then the first listed is used. Intended
Expand All @@ -1636,7 +1636,7 @@ def get_byid_name_map():
was encountered by run_command or no by-id type names were encountered.
"""
byid_name_map = {}
out, err, rc = run_command([LS, '-l', '/dev/disk/by-id'],
out, err, rc = run_command([LS, '-lr', '/dev/disk/by-id'],
throw=True)
if rc == 0:
for each_line in out:
Expand Down
131 changes: 130 additions & 1 deletion src/rockstor/system/tests/test_osi.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import unittest
from mock import patch

from system.osi import get_dev_byid_name, Disk, scan_disks
from system.osi import get_dev_byid_name, Disk, scan_disks, get_byid_name_map


class Pool(object):
Expand Down Expand Up @@ -1551,3 +1551,132 @@ def test_scan_disks_nvme_sys_disk(self):
'returned = ({}).\n '
'expected = ({}).'.format(returned,
expected))

def test_get_byid_name_map_prior_command_mock(self):
"""
Test get_byid_name_map() for prior mapping between canonical
and by-id device name mapping.
Note that the 'new' variant of this behaviour, tested later, only
involved the change of the mocked run_command output.
"""
# The following test data of 'ls -l' contains; sdd, sde, and sda3 with
# same length by-id mappings that are also their only mappings.
# This allows testing for returning the expected wwn-..., not the
# regression tested here of "scsi-... type name. As per
# get_dev_byid_name()'s reverse lexicographical return priority we
# would now expect the wwn-... type names if there are not others
# available and all available by-id are of the same length.
# Thanks to forum member juchong for submitting this command output.

out = [
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-HGST_HUH728080ALE600_2EKXANGP -> ../../sdb', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-VMware_Virtual_SATA_CDRW_Drive_00000000000000000001 -> ../../sr0', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68L0BN1_WD-WX11D6651995 -> ../../sdg', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68L0BN1_WD-WX31DB58YJF0 -> ../../sdh', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68MYMN1_WD-WX11DB4H8VJJ -> ../../sdf', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68MYMN1_WD-WX21DC42ELAT -> ../../sdc', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68MYMN1_WD-WXM1H84CXAUJ -> ../../sdi', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 scsi-35000cca252017870 -> ../../sdd', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 scsi-35000cca252017ef4 -> ../../sde', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636 -> ../../sda', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636-part1 -> ../../sda1', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636-part2 -> ../../sda2', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636-part3 -> ../../sda3', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x5000cca23bf728eb -> ../../sdb', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x5000cca252017870 -> ../../sdd', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x5000cca252017ef4 -> ../../sde', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee20b77be35 -> ../../sdf', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee20dd22144 -> ../../sdg', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee260e5c696 -> ../../sdi', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee26114d4cb -> ../../sdc', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee26293bbea -> ../../sdh', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636 -> ../../sda', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636-part1 -> ../../sda1', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636-part2 -> ../../sda2', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636-part3 -> ../../sda3', # noqa E501
]
err = ['']
rc = 0
expected = {
'sda1': 'scsi-36000c29df1181dd53db36b41b3582636-part1',
'sdf': 'ata-WDC_WD60EFRX-68MYMN1_WD-WX11DB4H8VJJ',
'sdd': 'scsi-35000cca252017870',
'sde': 'scsi-35000cca252017ef4',
'sr0': 'ata-VMware_Virtual_SATA_CDRW_Drive_00000000000000000001',
'sdg': 'ata-WDC_WD60EFRX-68L0BN1_WD-WX11D6651995',
'sda2': 'scsi-36000c29df1181dd53db36b41b3582636-part2',
'sda': 'scsi-36000c29df1181dd53db36b41b3582636',
'sdb': 'ata-HGST_HUH728080ALE600_2EKXANGP',
'sdc': 'ata-WDC_WD60EFRX-68MYMN1_WD-WX21DC42ELAT',
'sdh': 'ata-WDC_WD60EFRX-68L0BN1_WD-WX31DB58YJF0',
'sdi': 'ata-WDC_WD60EFRX-68MYMN1_WD-WXM1H84CXAUJ',
'sda3': 'scsi-36000c29df1181dd53db36b41b3582636-part3'}
self.mock_run_command.return_value = (out, err, rc)
returned = get_byid_name_map()
self.maxDiff = None
self.assertDictEqual(returned, expected)

def test_get_byid_name_map(self):
"""
Test get_byid_name_map() for expected by-id device name mapping.
"""
# The following test data of 'ls -lr' contains; sdd, sde, and sda3 with
# same length by-id mappings that are also their only mappings.
# This allows testing for returning the expected wwn-..., not the
# "scsi-... type name. As per get_dev_byid_name()'s revised reverse
# lexicographical return priority we would now expect the wwn-... type
# names if there are no others available and all available by-id are
# of the same length.
# This ls -lr output is derived from the ls -l output supplied by forum
# member juchong.

out = [
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636-part3 -> ../../sda3', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636-part2 -> ../../sda2', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636-part1 -> ../../sda1', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636 -> ../../sda', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee26293bbea -> ../../sdh', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee26114d4cb -> ../../sdc', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee260e5c696 -> ../../sdi', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee20dd22144 -> ../../sdg', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee20b77be35 -> ../../sdf', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x5000cca252017ef4 -> ../../sde', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x5000cca252017870 -> ../../sdd', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x5000cca23bf728eb -> ../../sdb', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636-part3 -> ../../sda3', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636-part2 -> ../../sda2', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636-part1 -> ../../sda1', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636 -> ../../sda', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 scsi-35000cca252017ef4 -> ../../sde', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 scsi-35000cca252017870 -> ../../sdd', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68MYMN1_WD-WXM1H84CXAUJ -> ../../sdi', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68MYMN1_WD-WX21DC42ELAT -> ../../sdc', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68MYMN1_WD-WX11DB4H8VJJ -> ../../sdf', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68L0BN1_WD-WX31DB58YJF0 -> ../../sdh', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68L0BN1_WD-WX11D6651995 -> ../../sdg', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-VMware_Virtual_SATA_CDRW_Drive_00000000000000000001 -> ../../sr0', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-HGST_HUH728080ALE600_2EKXANGP -> ../../sdb', # noqa E501
]
err = ['']
rc = 0
# The order of the following is unimportant as it's a dictionary but is
# preserved on the index to aid comparison with:
# test_get_byid_name_map_prior_command_mock()
expected = {
'sda1': 'wwn-0x6000c29df1181dd53db36b41b3582636-part1',
'sdf': 'ata-WDC_WD60EFRX-68MYMN1_WD-WX11DB4H8VJJ',
'sdd': 'wwn-0x5000cca252017870',
'sde': 'wwn-0x5000cca252017ef4',
'sr0': 'ata-VMware_Virtual_SATA_CDRW_Drive_00000000000000000001',
'sdg': 'ata-WDC_WD60EFRX-68L0BN1_WD-WX11D6651995',
'sda2': 'wwn-0x6000c29df1181dd53db36b41b3582636-part2',
'sda': 'wwn-0x6000c29df1181dd53db36b41b3582636',
'sdb': 'ata-HGST_HUH728080ALE600_2EKXANGP',
'sdc': 'ata-WDC_WD60EFRX-68MYMN1_WD-WX21DC42ELAT',
'sdh': 'ata-WDC_WD60EFRX-68L0BN1_WD-WX31DB58YJF0',
'sdi': 'ata-WDC_WD60EFRX-68MYMN1_WD-WXM1H84CXAUJ',
'sda3': 'wwn-0x6000c29df1181dd53db36b41b3582636-part3'}
self.mock_run_command.return_value = (out, err, rc)
returned = get_byid_name_map()
self.maxDiff = None
self.assertDictEqual(returned, expected)

0 comments on commit 8ceef40

Please sign in to comment.