Skip to content

abstract by-id device path in a function #1839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 16, 2017
Merged

abstract by-id device path in a function #1839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions src/rockstor/fs/btrfs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
import os
import shutil
from system.osi import run_command, create_tmp_dir, is_share_mounted, \
is_mounted, get_dev_byid_name, convert_to_kib, toggle_path_rw
is_mounted, get_dev_byid_name, convert_to_kib, toggle_path_rw, \
get_device_path
from system.exceptions import (CommandException)
from pool_scrub import PoolScrub
from django_ztask.decorators import task
Expand Down Expand Up @@ -50,7 +51,7 @@ def add_pool(pool, disks):
:param disks: list of by-id disk names without paths to make the pool from.
:return o, err, rc from last command executed.
"""
disks_fp = ['/dev/disk/by-id/' + d for d in disks]
disks_fp = [get_device_path(d) for d in disks]
draid = mraid = pool.raid
if pool.raid == 'single':
mraid = 'dup'
Expand Down Expand Up @@ -95,7 +96,7 @@ def get_pool_info(disk):
:return: a dictionary with keys of 'disks', 'label', and 'uuid';
disks keys a list of devices, while label and uuid keys are for strings.
"""
dpath = '/dev/disk/by-id/%s' % disk
dpath = get_device_path(disk)
cmd = [BTRFS, 'fi', 'show', dpath]
o, e, rc = run_command(cmd)
pool_info = {'disks': [], }
Expand Down Expand Up @@ -187,7 +188,7 @@ def resize_pool(pool, dev_list_byid, add=True):
:return: Tuple of results from run_command(generated command) or None if
the device member/pool sanity check fails.
"""
dev_list_byid = ['/dev/disk/by-id/' + d for d in dev_list_byid]
dev_list_byid = [get_device_path(d) for d in dev_list_byid]
root_mnt_pt = mount_root(pool)
cur_dev = cur_devices(root_mnt_pt)
resize_flag = 'add'
Expand Down Expand Up @@ -260,7 +261,7 @@ def mount_root(pool):
last_device = pool.disk_set.attached().last()
logger.info('Mount by label (%s) failed.' % mnt_device)
for device in pool.disk_set.attached():
mnt_device = ('/dev/disk/by-id/%s' % device.target_name)
mnt_device = get_device_path(device.target_name)
logger.info('Attempting mount by device (%s).' % mnt_device)
if (os.path.exists(mnt_device)):
mnt_cmd = [MOUNT, mnt_device, root_pool_mnt, ]
Expand Down Expand Up @@ -351,8 +352,8 @@ def mount_share(share, mnt_pt):
if (is_mounted(mnt_pt)):
return
mount_root(share.pool)
pool_device = ('/dev/disk/by-id/%s' %
share.pool.disk_set.attached().first().target_name)
pool_device = get_device_path(share.pool.disk_set.attached()
.first().target_name)
subvol_str = 'subvol=%s' % share.subvol_name
create_tmp_dir(mnt_pt)
toggle_path_rw(mnt_pt, rw=False)
Expand All @@ -361,8 +362,8 @@ def mount_share(share, mnt_pt):


def mount_snap(share, snap_name, snap_mnt=None):
pool_device = ('/dev/disk/by-id/%s' %
share.pool.disk_set.attached().first().target_name)
pool_device = get_device_path(share.pool.disk_set.attached()
.first().target_name)
share_path = ('%s%s' % (DEFAULT_MNT_DIR, share.name))
rel_snap_path = ('.snapshots/%s/%s' % (share.name, snap_name))
snap_path = ('%s%s/%s' %
Expand Down Expand Up @@ -658,7 +659,7 @@ def rollback_snap(snap_name, sname, subvol_name, pool):
shutil.move(snap_fp, '%s/%s/%s' % (DEFAULT_MNT_DIR, pool.name, sname))
create_tmp_dir(mnt_pt)
subvol_str = 'subvol=%s' % sname
dpath = '/dev/disk/by-id/%s' % pool.disk_set.attached().first().target_name
dpath = get_device_path(pool.disk_set.attached().first().target_name)
mnt_cmd = [MOUNT, '-t', 'btrfs', '-o', subvol_str, dpath, mnt_pt]
run_command(mnt_cmd)

Expand Down Expand Up @@ -1111,7 +1112,7 @@ def device_scan(dev_byid_list=['all']):
# Skip detached devices as we know they don't exist.
# Potential log point for early detached device discovery.
continue
dev_byid_withpath = ('/dev/disk/by-id/%s' % dev_byid)
dev_byid_withpath = get_device_path(dev_byid)
if os.path.exists(dev_byid_withpath): # only scan existing devices
# using throw=False, to process the rc != 0 logic
# afterwards. Without throw=False, when rc != 0, exception is
Expand All @@ -1130,7 +1131,7 @@ def device_scan(dev_byid_list=['all']):
def btrfs_uuid(disk):
"""return uuid of a btrfs filesystem"""
o, e, rc = run_command(
[BTRFS, 'filesystem', 'show', '/dev/disk/by-id/%s' % disk])
[BTRFS, 'filesystem', 'show', get_device_path(disk)])
return o[0].split()[3]


Expand Down
6 changes: 3 additions & 3 deletions src/rockstor/system/luks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from tempfile import mkstemp
import shutil
from system.exceptions import CommandException
from system.osi import run_command, get_uuid_name_map
from system.osi import run_command, get_uuid_name_map, get_device_path
import logging

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -142,7 +142,7 @@ def luks_format_disk(disk_byid, passphrase):
:param passphrase: luks passphrase used to encrypt master key.
:return: o, e, rc tuple as returned by cryptsetup luksFormat command.
"""
disk_byid_withpath = ('/dev/disk/by-id/%s' % disk_byid)
disk_byid_withpath = get_device_path(disk_byid)
# Create a temp file to pass our passphrase to our cryptsetup command.
tfo, npath = mkstemp()
# Pythons _candidate_tempdir_list() should ensure our npath temp file is
Expand Down Expand Up @@ -483,7 +483,7 @@ def establish_keyfile(dev_byid, keyfile_withpath, passphrase):
# keyfile has already been registered. UI will not ask for passphrase
# as it is assumed that an existing keyfile is already registered.
return True
dev_byid_withpath = '/dev/disk/by-id/%s' % dev_byid
dev_byid_withpath = get_device_path(dev_byid)
tfo, npath = mkstemp()
# Pythons _candidate_tempdir_list() should ensure our npath temp file is
# in memory (tmpfs). From https://docs.python.org/2/library/tempfile.html
Expand Down
32 changes: 23 additions & 9 deletions src/rockstor/system/osi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@ def wipe_disk(disk_byid):
:return: o, e, rc tuple returned by the run_command wrapper running the
locally generated wipefs command.
"""
disk_byid_withpath = ('/dev/disk/by-id/%s' % disk_byid)
disk_byid_withpath = get_device_path(disk_byid)
return run_command([WIPEFS, '-a', disk_byid_withpath])


Expand All @@ -839,7 +839,7 @@ def blink_disk(disk_byid, total_exec, read, sleep):
:param sleep: light off time.
:return: None.
"""
dd_cmd = [DD, 'if=/dev/disk/by-id/%s' % disk_byid, 'of=/dev/null',
dd_cmd = [DD, 'if=%s' % get_device_path(disk_byid), 'of=/dev/null',
'bs=512', 'conv=noerror']
p = subprocess.Popen(dd_cmd, shell=False, stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
Expand Down Expand Up @@ -1375,7 +1375,7 @@ def get_disk_power_status(dev_byid):
# hdparm -C -q /dev/sda
# drive state is: active/idle
out, err, rc = run_command(
[HDPARM, '-C', '-q', '/dev/disk/by-id/%s' % dev_byid], throw=False)
[HDPARM, '-C', '-q', get_device_path(dev_byid)], throw=False)
if len(err) != 1:
# In some instances an error can be returned even with rc=0.
# ie SG_IO: bad/missing sense data, sb[]: 70 00 05 00 00 00 00 0a ...
Expand Down Expand Up @@ -1410,7 +1410,7 @@ def get_disk_APM_level(dev_byid):
# APM_level<tab>= off
# APM_level<tab>= not supported
out, err, rc = run_command(
[HDPARM, '-B', '-q', '/dev/disk/by-id/%s' % dev_byid], throw=False)
[HDPARM, '-B', '-q', get_device_path(dev_byid)], throw=False)
if len(err) != 1:
# In some instances an error can be returned even with rc=0.
# ie SG_IO: bad/missing sense data, sb[]: 70 00 05 00 00 00 00 0a ...
Expand Down Expand Up @@ -1452,7 +1452,7 @@ def set_disk_spindown(dev_byid, spindown_time, apm_value,
return False
# hdparm -S works on partitions so base_dev is not needed, but it does
# require a full path ie /dev/disk/by-id/dev_byid; dev_by along is no good.
dev_byid_withpath = '/dev/disk/by-id/%s' % dev_byid
dev_byid_withpath = get_device_path(dev_byid)
# md devices arn't offered a spindown config: unknown status from hdparm
# -C. Their member disks are exposed on the Disks page so for the time
# being their spin down times are addressed as regular disks are. Don't
Expand Down Expand Up @@ -1642,6 +1642,20 @@ def get_byid_name_map():
return byid_name_map


def get_device_path(by_id):
"""
Return full path for given device id.
For testing and adaptations, this can be adjusted for supporting devices
(like nbd) which have no device-by-id entry. That said, DO NOT put
workarounds here if there is any other way to do it. The by-id treatment
is for having stable device names across reboots and unplugging and
re-plugging devices, our database can get confused if that consistency is
not there. See https://github.com/rockstor/rockstor-core/pull/1704 for
some discussion of this topic.
"""
return '/dev/disk/by-id/%s' % by_id


def get_whole_dev_uuid(dev_byid):
"""
Simple wrapper around "lsblk -n -o uuid <dev_name>" to retrieve a device's
Expand All @@ -1659,7 +1673,7 @@ def get_whole_dev_uuid(dev_byid):
which is quicker and more versatile than
"""
dev_uuid = ''
dev_byid_withpath = '/dev/disk/by-id/%s' % dev_byid
dev_byid_withpath = get_device_path(dev_byid)
out, err, rc = run_command([LSBLK, '-n', '-o', 'uuid', dev_byid_withpath],
throw=False)
if rc != 0:
Expand Down Expand Up @@ -1721,7 +1735,7 @@ def get_dev_temp_name(dev_byid):
:return: sda type device name without path or if no match is found then
dev_byid is returned.
"""
dev_byid_withpath = '/dev/disk/by-id/%s' % dev_byid
dev_byid_withpath = get_device_path(dev_byid)
try:
temp_name = os.readlink(dev_byid_withpath).split('/')[-1]
except OSError:
Expand Down Expand Up @@ -1939,7 +1953,7 @@ def read_hdparm_setting(dev_byid):
infile = '/etc/systemd/system/rockstor-hdparm.service'
if not os.path.isfile(infile):
return None
dev_byid_withpath = '/dev/disk/by-id/%s' % dev_byid
dev_byid_withpath = get_device_path(dev_byid)
dev_byid_found = False
with open(infile) as ino:
for line in ino.readlines():
Expand Down Expand Up @@ -1978,7 +1992,7 @@ def enter_standby(dev_byid):
:return: None or out, err, rc of command
"""
# TODO: candidate for move to system/hdparm
hdparm_command = [HDPARM, '-q', '-y', '/dev/disk/by-id/%s' % dev_byid]
hdparm_command = [HDPARM, '-q', '-y', get_device_path(dev_byid)]
return run_command(hdparm_command)


Expand Down
7 changes: 3 additions & 4 deletions src/rockstor/system/smart.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"""

import re
from osi import run_command, get_base_device_byid
from osi import run_command, get_base_device_byid, get_device_path
from tempfile import mkstemp
from shutil import move
import logging
Expand Down Expand Up @@ -408,7 +408,7 @@ def get_dev_options(dev_byid, custom_options=''):
# Empty custom_options or they have never been set so just return
# full path to base device as nothing else to do.
dev_options = [
'/dev/disk/by-id/%s' % get_base_device_byid(dev_byid, TESTMODE)]
get_device_path(get_base_device_byid(dev_byid, TESTMODE))]
else:
# Convert string of custom options into a list ready for run_command
# TODO: think this ascii should be utf-8 as that's kernel standard
Expand All @@ -419,8 +419,7 @@ def get_dev_options(dev_byid, custom_options=''):
if (re.search('/dev/tw|/dev/cciss/c|/dev/sg', custom_options) is None):
# add full path to our custom options as we see no raid target dev
dev_options += [
'/dev/disk/by-id/{}'.format(
get_base_device_byid(dev_byid, TESTMODE))
get_device_path(get_base_device_byid(dev_byid, TESTMODE))
]
# Note on raid controller target devices. /dev/twe#, or /dev/twa#, or
# /dev/twl# are 3ware controller targets devs respectively 3x-xxxx,
Expand Down