Skip to content
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.

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

vdev_id: Support daisy-chained JBODs in multipath mode #11520

Closed
wants to merge 3 commits into from

Conversation

arshad512
Copy link
Contributor

@arshad512 arshad512 commented Jan 25, 2021

Description

Within function sas_handler() userspace commands like
'/usr/sbin/multipath' have been replaced with sourcing
device details from within sysfs which reduced a
significant amount of overhead and processing time.
Multiple JBOD enclosures and their order are sourced
from the bsg driver (/sys/class/enclosure) to isolate
chassis top-level expanders, which are then dynamically
indexed based on host channel of the multipath subordinate
disk member device being processed. Additionally added a
"mixed" mode for slot identification for environments where
a ZFS server system may contain SAS disk slots where there
is no expander (direct connect to HBA) while an attached
external JBOD with an expander have different slot identifier
methods.

How Has This Been Tested?

Testing was performed on a AMD EPYC based dual-server
high-availability multipath environment with multiple
HBAs per ZFS server and four SAS JBODs. The two primary
JBODs were multipath/cross-connected between the two
ZFS-HA servers. The secondary JBODs were daisy-chained
off of the primary JBODs using aligned SAS expander
channels (JBOD-0 expanderA--->JBOD-1 expanderA,
JBOD-0 expanderB--->JBOD-1 expanderB, etc).
Pools were created, exported and re-imported, imported
globally with 'zpool import -a -d /dev/disk/by-vdev'.
Low level udev debug outputs were traced to isolate
and resolve errors.

Result:

Initial testing of a previous version of this change
showed how reliance on userspace utilities like
'/usr/sbin/multipath' and '/usr/bin/lsscsi' were
exacerbated by increasing numbers of disks and JBODs.
With four 60-disk SAS JBODs and 240 disks the time to
process a udevadm trigger was 3 minutes 30 seconds
during which nearly all CPU cores were above 80%
utilization. By switching reliance on userspace
utilities to sysfs in this version, the udevadm
trigger processing time was reduced to 12.2 seconds
and negligible CPU load.

This patch also fixes few shellcheck complains.

Signed-off-by: Jeff Johnson jeff.johnson@aeoncomputing.com
Signed-off-by: Arshad Hussain arshad.hussain@aeoncomputing.com

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

Within function sas_handler() userspace commands like
'/usr/sbin/multipath' have been replaced with sourcing
device details from within sysfs which reduced a
significant amount of overhead and processing time.
Multiple JBOD enclosures and their order are sourced
from the bsg driver (/sys/class/enclosure) to isolate
chassis top-level expanders, which are then dynamically
indexed based on host channel of the multipath subordinate
disk member device being processed. Additionally added a
"mixed" mode for slot identification for environments where
a ZFS server system may contain SAS disk slots where there
is no expander (direct connect to HBA) while an attached
external JBOD with an expander have different slot identifier
methods.

How Has This Been Tested?
~~~~~~~~~~~~~~~~~~~~~~

Testing was performed on a AMD EPYC based dual-server
high-availability multipath environment with multiple
HBAs per ZFS server and four SAS JBODs. The two primary
JBODs were multipath/cross-connected between the two
ZFS-HA servers. The secondary JBODs were daisy-chained
off of the primary JBODs using aligned SAS expander
channels (JBOD-0 expanderA--->JBOD-1 expanderA,
          JBOD-0 expanderB--->JBOD-1 expanderB, etc).
Pools were created, exported and re-imported, imported
globally with 'zpool import -a -d /dev/disk/by-vdev'.
Low level udev debug outputs were traced to isolate
and resolve errors.

Result:
~~~~~~~

Initial testing of a previous version of this change
showed how reliance on userspace utilities like
'/usr/sbin/multipath' and '/usr/bin/lsscsi' were
exacerbated by increasing numbers of disks and JBODs.
With four 60-disk SAS JBODs and 240 disks the time to
process a udevadm trigger was 3 minutes 30 seconds
during which nearly all CPU cores were above 80%
utilization. By switching reliance on userspace
utilities to sysfs in this version, the udevadm
trigger processing time was reduced to 12.2 seconds
and negligible CPU load.

This patch also fixes few shellcheck complains.

Signed-off-by: Jeff Johnson <jeff.johnson@aeoncomputing.com>
Signed-off-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
AeonJJohnson and others added 2 commits January 25, 2021 19:06
Within function sas_handler() userspace commands like
'/usr/sbin/multipath' have been replaced with sourcing
device details from within sysfs which reduced a
significant amount of overhead and processing time.
Multiple JBOD enclosures and their order are sourced
from the bsg driver (/sys/class/enclosure) to isolate
chassis top-level expanders, which are then dynamically
indexed based on host channel of the multipath subordinate
disk member device being processed. Additionally added a
"mixed" mode for slot identification for environments where
a ZFS server system may contain SAS disk slots where there
is no expander (direct connect to HBA) while an attached
external JBOD with an expander have different slot identifier
methods.

How Has This Been Tested?
~~~~~~~~~~~~~~~~~~~~~~

Testing was performed on a AMD EPYC based dual-server
high-availability multipath environment with multiple
HBAs per ZFS server and four SAS JBODs. The two primary
JBODs were multipath/cross-connected between the two
ZFS-HA servers. The secondary JBODs were daisy-chained
off of the primary JBODs using aligned SAS expander
channels (JBOD-0 expanderA--->JBOD-1 expanderA,
          JBOD-0 expanderB--->JBOD-1 expanderB, etc).
Pools were created, exported and re-imported, imported
globally with 'zpool import -a -d /dev/disk/by-vdev'.
Low level udev debug outputs were traced to isolate
and resolve errors.

Result:
~~~~~~~

Initial testing of a previous version of this change
showed how reliance on userspace utilities like
'/usr/sbin/multipath' and '/usr/bin/lsscsi' were
exacerbated by increasing numbers of disks and JBODs.
With four 60-disk SAS JBODs and 240 disks the time to
process a udevadm trigger was 3 minutes 30 seconds
during which nearly all CPU cores were above 80%
utilization. By switching reliance on userspace
utilities to sysfs in this version, the udevadm
trigger processing time was reduced to 12.2 seconds
and negligible CPU load.

This patch also fixes few shellcheck complains.

Signed-off-by: Jeff Johnson <jeff.johnson@aeoncomputing.com>
Signed-off-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jan 25, 2021
@gdevenyi
Copy link
Contributor

This looks great, thanks for the hard work de-bashifying the code.

There's a few checkbashism complaints

Run make checkstyle
possible bashism in ./cmd/vdev_id/vdev_id line 158 (local foo=bar):
	local MAPPED_CHAN=
possible bashism in ./cmd/vdev_id/vdev_id line 159 (local foo=bar):
	local PCI_ID=$1
possible bashism in ./cmd/vdev_id/vdev_id line 160 (local foo=bar):
	local PORT=$2
make: *** [checkbashisms] Error 1
Makefile:1485: recipe for target 'checkbashisms' failed
Error: Process completed with exit code 2.

@behlendorf
Copy link
Contributor

@arshad512 can you rebase this on master and force update the PR. In the process it'd be ideal if you could also split this in to two commits. One which makes the POSIX changes, and another which adds the new functionality.

@gdevenyi
Copy link
Contributor

gdevenyi commented Jan 25, 2021

And here's the current state of shellcheck, I think there's some more fixups that could be made:


In cmd/vdev_id/vdev_id line 146:
	CHANNEL=$2
        ^-----^ SC2034: CHANNEL appears unused. Verify use (or export if used externally).


In cmd/vdev_id/vdev_id line 149:
	MAPPED_SLOT=$(awk '$1 == "slot" && $2 == "${LINUX_SLOT}" && \
                                                                    ^-- SC1004: This backslash+linefeed is literal. Break outside single quotes if you just want to break the line.


In cmd/vdev_id/vdev_id line 154:
	printf "%d" ${MAPPED_SLOT}
                    ^------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	printf "%d" "${MAPPED_SLOT}"


In cmd/vdev_id/vdev_id line 158:
	local MAPPED_CHAN=
        ^---------------^ SC2039: In POSIX sh, 'local' is undefined.


In cmd/vdev_id/vdev_id line 159:
	local PCI_ID=$1
        ^----------^ SC2039: In POSIX sh, 'local' is undefined.


In cmd/vdev_id/vdev_id line 160:
	local PORT=$2
        ^--------^ SC2039: In POSIX sh, 'local' is undefined.


In cmd/vdev_id/vdev_id line 164:
		MAPPED_CHAN=$(awk -v port=$PORT \
                                          ^---^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		MAPPED_CHAN=$(awk -v port="$PORT" \


In cmd/vdev_id/vdev_id line 165:
			'$1 == "channel" && $2 == ${PORT} \
                                                          ^-- SC1004: This backslash+linefeed is literal. Break outside single quotes if you just want to break the line.


In cmd/vdev_id/vdev_id line 169:
		MAPPED_CHAN=$(awk -v pciID=$PCI_ID -v port=$PORT \
                                           ^-----^ SC2086: Double quote to prevent globbing and word splitting.
                                                           ^---^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		MAPPED_CHAN=$(awk -v pciID="$PCI_ID" -v port="$PORT" \


In cmd/vdev_id/vdev_id line 170:
			'$1 == "channel" && $2 == pciID && $3 == port \
                                                                      ^-- SC1004: This backslash+linefeed is literal. Break outside single quotes if you just want to break the line.


In cmd/vdev_id/vdev_id line 174:
	printf "%s" ${MAPPED_CHAN}
                    ^------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	printf "%s" "${MAPPED_CHAN}"


In cmd/vdev_id/vdev_id line 187:
	DEVEXP=$(ls -l /sys/block/$DEV/device/ | grep enclos | awk -F/ '{print $(NF-1) }')
                 ^-- SC2010: Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.
                                  ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	DEVEXP=$(ls -l /sys/block/"$DEV"/device/ | grep enclos | awk -F/ '{print $(NF-1) }')


In cmd/vdev_id/vdev_id line 190:
	set -- $(ls -l /sys/class/enclosure | grep host${HOSTCHAN}/port-${HOSTCHAN}:${PORT} | awk '{print $9}')
               ^-- SC2046: Quote this to prevent word splitting.
                 ^-- SC2010: Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.
                                                       ^---------^ SC2086: Double quote to prevent globbing and word splitting.
                                                                        ^---------^ SC2086: Double quote to prevent globbing and word splitting.
                                                                                    ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	set -- $(ls -l /sys/class/enclosure | grep host"${HOSTCHAN}"/port-"${HOSTCHAN}":"${PORT}" | awk '{print $9}')


In cmd/vdev_id/vdev_id line 193:
	JBOD=$@
             ^-- SC2124: Assigning an array to a string! Assign as array, or use * instead of @ to concatenate.


In cmd/vdev_id/vdev_id line 195:
	for i in "${JBOD}"
                 ^-------^ SC2066: Since you double quoted this, it will not word split, and the loop will only run once.


In cmd/vdev_id/vdev_id line 202:
	printf "%d" ${MAPPED_JBOD}
                    ^------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	printf "%d" "${MAPPED_JBOD}"


In cmd/vdev_id/vdev_id line 207:
		PHYS_PER_PORT=$(awk '$1 == "phys_per_port" \
                                                           ^-- SC1004: This backslash+linefeed is literal. Break outside single quotes if you just want to break the line.


In cmd/vdev_id/vdev_id line 212:
	if ! echo $PHYS_PER_PORT | grep -q -E '^[0-9]+$' ; then
                  ^------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	if ! echo "$PHYS_PER_PORT" | grep -q -E '^[0-9]+$' ; then


In cmd/vdev_id/vdev_id line 218:
		MULTIPATH_MODE=$(awk '$1 == "multipath" \
                                                        ^-- SC1004: This backslash+linefeed is literal. Break outside single quotes if you just want to break the line.


In cmd/vdev_id/vdev_id line 223:
		MULTIJBOD_MODE=$(awk '$1 == "multijbod" \
                                                        ^-- SC1004: This backslash+linefeed is literal. Break outside single quotes if you just want to break the line.


In cmd/vdev_id/vdev_id line 231:
			DM_NAME=$(ls -l --full-time /dev/mapper |
                                  ^---------------------------^ SC2012: Use find instead of ls to better handle non-alphanumeric filenames.


In cmd/vdev_id/vdev_id line 242:
			PART=$(echo $DM_NAME | awk -Fp '/p/{print "-part"$2}')
                                    ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
			PART=$(echo "$DM_NAME" | awk -Fp '/p/{print "-part"$2}')


In cmd/vdev_id/vdev_id line 246:
		DM_NAME=$(echo $DM_NAME | sed 's/p[0-9][0-9]*$//')
                               ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		DM_NAME=$(echo "$DM_NAME" | sed 's/p[0-9][0-9]*$//')


In cmd/vdev_id/vdev_id line 253:
		DMDEV=$(echo $DM_NAME |awk '{gsub("../", " "); print $NF}')
                             ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		DMDEV=$(echo "$DM_NAME" |awk '{gsub("../", " "); print $NF}')


In cmd/vdev_id/vdev_id line 260:
		DEV=$(ls /sys/block/$DMDEV/slaves | awk '
                      ^-------------------------^ SC2012: Use find instead of ls to better handle non-alphanumeric filenames.
                                    ^----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		DEV=$(ls /sys/block/"$DMDEV"/slaves | awk '


In cmd/vdev_id/vdev_id line 272:
	if echo $DEV | grep -q ^/devices/ ; then
                ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	if echo "$DEV" | grep -q ^/devices/ ; then


In cmd/vdev_id/vdev_id line 275:
		sys_path=$(udevadm info -q path -p /sys/block/$DEV 2>/dev/null)
                                                              ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		sys_path=$(udevadm info -q path -p /sys/block/"$DEV" 2>/dev/null)


In cmd/vdev_id/vdev_id line 279:
	set -- $(echo "$sys_path" | tr / ' ')
               ^----------------------------^ SC2046: Quote this to prevent word splitting.


In cmd/vdev_id/vdev_id line 287:
		d=$(eval echo \${$i})
                                ^-- SC1083: This { is literal. Check expression (missing ;/\n?) or quote it.
                                   ^-- SC1083: This } is literal. Check expression (missing ;/\n?) or quote it.


In cmd/vdev_id/vdev_id line 289:
		echo $d | grep -q -E '^host[0-9]+$' && break
                     ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		echo "$d" | grep -q -E '^host[0-9]+$' && break


In cmd/vdev_id/vdev_id line 294:
	HOSTCHAN=$(echo $d | awk -F/ '{ gsub("host","",$NF); print $NF}')
                        ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	HOSTCHAN=$(echo "$d" | awk -F/ '{ gsub("host","",$NF); print $NF}')


In cmd/vdev_id/vdev_id line 300:
	PCI_ID=$(eval echo \${$((i -1))} | awk -F: '{print $2":"$3}')
                             ^-- SC1083: This { is literal. Check expression (missing ;/\n?) or quote it.
                                       ^-- SC1083: This } is literal. Check expression (missing ;/\n?) or quote it.


In cmd/vdev_id/vdev_id line 316:
		port_dir="$port_dir/$(eval echo \${$i})"
                                                  ^-- SC1083: This { is literal. Check expression (missing ;/\n?) or quote it.
                                                     ^-- SC1083: This } is literal. Check expression (missing ;/\n?) or quote it.


In cmd/vdev_id/vdev_id line 320:
	PHY=$(ls -d $port_dir/phy* 2>/dev/null | head -1 | awk -F: '{print $NF}')
              ^-- SC2012: Use find instead of ls to better handle non-alphanumeric filenames.
                    ^-------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	PHY=$(ls -d "$port_dir"/phy* 2>/dev/null | head -1 | awk -F: '{print $NF}')


In cmd/vdev_id/vdev_id line 331:
		d=$(eval echo \${$i})
                                ^-- SC1083: This { is literal. Check expression (missing ;/\n?) or quote it.
                                   ^-- SC1083: This } is literal. Check expression (missing ;/\n?) or quote it.


In cmd/vdev_id/vdev_id line 333:
		if echo $d | grep -q '^end_device' ; then
                        ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		if echo "$d" | grep -q '^end_device' ; then


In cmd/vdev_id/vdev_id line 349:
		SLOT=$(cat $end_device_dir/bay_identifier 2>/dev/null)
                           ^-------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		SLOT=$(cat "$end_device_dir"/bay_identifier 2>/dev/null)


In cmd/vdev_id/vdev_id line 352:
		if [ $(cat $end_device_dir/bay_identifier 2>/dev/null) ] ; then
                     ^-- SC2046: Quote this to prevent word splitting.
                           ^-------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		if [ $(cat "$end_device_dir"/bay_identifier 2>/dev/null) ] ; then


In cmd/vdev_id/vdev_id line 353:
			SLOT=$(cat $end_device_dir/bay_identifier 2>/dev/null)
                                   ^-------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
			SLOT=$(cat "$end_device_dir"/bay_identifier 2>/dev/null)


In cmd/vdev_id/vdev_id line 355:
			SLOT=$(cat $end_device_dir/phy_identifier 2>/dev/null)
                                   ^-------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
			SLOT=$(cat "$end_device_dir"/phy_identifier 2>/dev/null)


In cmd/vdev_id/vdev_id line 359:
		SLOT=$(cat $end_device_dir/phy_identifier 2>/dev/null)
                           ^-------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		SLOT=$(cat "$end_device_dir"/phy_identifier 2>/dev/null)


In cmd/vdev_id/vdev_id line 362:
		d=$(eval echo \${$i})
                                ^-- SC1083: This { is literal. Check expression (missing ;/\n?) or quote it.
                                   ^-- SC1083: This } is literal. Check expression (missing ;/\n?) or quote it.


In cmd/vdev_id/vdev_id line 363:
		SLOT=$(echo $d | sed -e 's/^.*://')
                            ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		SLOT=$(echo "$d" | sed -e 's/^.*://')


In cmd/vdev_id/vdev_id line 367:
		d=$(eval echo \${$i})
                                ^-- SC1083: This { is literal. Check expression (missing ;/\n?) or quote it.
                                   ^-- SC1083: This } is literal. Check expression (missing ;/\n?) or quote it.


In cmd/vdev_id/vdev_id line 368:
		SLOT=$(echo $d | sed -e 's/^.*://')
                            ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		SLOT=$(echo "$d" | sed -e 's/^.*://')


In cmd/vdev_id/vdev_id line 372:
		d=$(eval echo \${$i})
                                ^-- SC1083: This { is literal. Check expression (missing ;/\n?) or quote it.
                                   ^-- SC1083: This } is literal. Check expression (missing ;/\n?) or quote it.


In cmd/vdev_id/vdev_id line 373:
		SLOT=$(echo $d | sed -e 's/^.*://')
                            ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		SLOT=$(echo "$d" | sed -e 's/^.*://')


In cmd/vdev_id/vdev_id line 378:
		sas_address=$(cat $end_device_dir/sas_address 2>/dev/null)
                                  ^-------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		sas_address=$(cat "$end_device_dir"/sas_address 2>/dev/null)


In cmd/vdev_id/vdev_id line 382:
			set -- $(sg_ses -p aes $enclosure | \
                               ^-- SC2046: Quote this to prevent word splitting.
                                               ^--------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
			set -- $(sg_ses -p aes "$enclosure" | \


In cmd/vdev_id/vdev_id line 398:
		CHAN=$(map_channel $PCI_ID $PORT)
                                   ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		CHAN=$(map_channel "$PCI_ID" $PORT)


In cmd/vdev_id/vdev_id line 399:
		SLOT=$(map_slot $SLOT $CHAN)
                                ^---^ SC2086: Double quote to prevent globbing and word splitting.
                                      ^---^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		SLOT=$(map_slot "$SLOT" "$CHAN")


In cmd/vdev_id/vdev_id line 400:
		JBOD=$(map_jbod $DEV)
                                ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		JBOD=$(map_jbod "$DEV")


In cmd/vdev_id/vdev_id line 405:
		echo ${CHAN}-${JBOD}-${SLOT}${PART}
                     ^-----^ SC2086: Double quote to prevent globbing and word splitting.
                             ^-----^ SC2086: Double quote to prevent globbing and word splitting.
                                     ^-----^ SC2086: Double quote to prevent globbing and word splitting.
                                            ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		echo "${CHAN}"-"${JBOD}"-"${SLOT}""${PART}"


In cmd/vdev_id/vdev_id line 407:
		CHAN=$(map_channel $PCI_ID $PORT)
                                   ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		CHAN=$(map_channel "$PCI_ID" $PORT)


In cmd/vdev_id/vdev_id line 408:
		SLOT=$(map_slot $SLOT $CHAN)
                                ^---^ SC2086: Double quote to prevent globbing and word splitting.
                                      ^---^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		SLOT=$(map_slot "$SLOT" "$CHAN")


In cmd/vdev_id/vdev_id line 413:
		echo ${CHAN}${SLOT}${PART}
                     ^-----^ SC2086: Double quote to prevent globbing and word splitting.
                            ^-----^ SC2086: Double quote to prevent globbing and word splitting.
                                   ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		echo "${CHAN}""${SLOT}""${PART}"


In cmd/vdev_id/vdev_id line 419:
		FIRST_BAY_NUMBER=$(awk '$1 == "first_bay_number" \
                                                                 ^-- SC1004: This backslash+linefeed is literal. Break outside single quotes if you just want to break the line.


In cmd/vdev_id/vdev_id line 425:
		PHYS_PER_PORT=$(awk '$1 == "phys_per_port" \
                                                           ^-- SC1004: This backslash+linefeed is literal. Break outside single quotes if you just want to break the line.


In cmd/vdev_id/vdev_id line 430:
	if ! echo $PHYS_PER_PORT | grep -q -E '^[0-9]+$' ; then
                  ^------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	if ! echo "$PHYS_PER_PORT" | grep -q -E '^[0-9]+$' ; then


In cmd/vdev_id/vdev_id line 436:
		MULTIPATH_MODE=$(awk '$1 == "multipath" \
                                                        ^-- SC1004: This backslash+linefeed is literal. Break outside single quotes if you just want to break the line.


In cmd/vdev_id/vdev_id line 444:
			DM_NAME=$(ls -l --full-time /dev/mapper |
                                  ^---------------------------^ SC2012: Use find instead of ls to better handle non-alphanumeric filenames.


In cmd/vdev_id/vdev_id line 455:
			PART=$(echo $DM_NAME | awk -Fp '/p/{print "-part"$2}')
                                    ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
			PART=$(echo "$DM_NAME" | awk -Fp '/p/{print "-part"$2}')


In cmd/vdev_id/vdev_id line 459:
		DM_NAME=$(echo $DM_NAME | sed 's/p[0-9][0-9]*$//')
                               ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		DM_NAME=$(echo "$DM_NAME" | sed 's/p[0-9][0-9]*$//')


In cmd/vdev_id/vdev_id line 466:
		DEV=$(multipath -ll $DM_NAME |
                                    ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		DEV=$(multipath -ll "$DM_NAME" |


In cmd/vdev_id/vdev_id line 473:
	if echo $DEV | grep -q ^/devices/ ; then
                ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	if echo "$DEV" | grep -q ^/devices/ ; then


In cmd/vdev_id/vdev_id line 476:
		sys_path=$(udevadm info -q path -p /sys/block/$DEV 2>/dev/null)
                                                              ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		sys_path=$(udevadm info -q path -p /sys/block/"$DEV" 2>/dev/null)


In cmd/vdev_id/vdev_id line 483:
	set -- $(echo "$sys_path" | tr / ' ')
               ^----------------------------^ SC2046: Quote this to prevent word splitting.


In cmd/vdev_id/vdev_id line 491:
		d=$(eval echo \${$i})
                                ^-- SC1083: This { is literal. Check expression (missing ;/\n?) or quote it.
                                   ^-- SC1083: This } is literal. Check expression (missing ;/\n?) or quote it.


In cmd/vdev_id/vdev_id line 494:
		echo $d | grep -q -E '^host[0-9]+$' && break
                     ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		echo "$d" | grep -q -E '^host[0-9]+$' && break


In cmd/vdev_id/vdev_id line 502:
	PCI_ID=$(eval echo \${$((i -1))} | awk -F: '{print $2":"$3}')
                             ^-- SC1083: This { is literal. Check expression (missing ;/\n?) or quote it.
                                       ^-- SC1083: This } is literal. Check expression (missing ;/\n?) or quote it.


In cmd/vdev_id/vdev_id line 511:
		port_dir="$port_dir/$(eval echo \${$i})"
                                                  ^-- SC1083: This { is literal. Check expression (missing ;/\n?) or quote it.
                                                     ^-- SC1083: This } is literal. Check expression (missing ;/\n?) or quote it.


In cmd/vdev_id/vdev_id line 515:
	set -- $(echo $port_dir | sed -e 's/^.*:\([^:]*\):\([^:]*\)$/\1 \2/')
               ^-- SC2046: Quote this to prevent word splitting.
                      ^-------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	set -- $(echo "$port_dir" | sed -e 's/^.*:\([^:]*\):\([^:]*\)$/\1 \2/')


In cmd/vdev_id/vdev_id line 523:
	CHAN=$(map_channel $PCI_ID $PORT)
                           ^-----^ SC2086: Double quote to prevent globbing and word splitting.
                                   ^---^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	CHAN=$(map_channel "$PCI_ID" "$PORT")


In cmd/vdev_id/vdev_id line 524:
	SLOT=$(map_slot $SLOT $CHAN)
                              ^---^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	SLOT=$(map_slot $SLOT "$CHAN")


In cmd/vdev_id/vdev_id line 529:
	echo ${CHAN}${SLOT}${PART}
             ^-----^ SC2086: Double quote to prevent globbing and word splitting.
                    ^-----^ SC2086: Double quote to prevent globbing and word splitting.
                           ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	echo "${CHAN}""${SLOT}""${PART}"


In cmd/vdev_id/vdev_id line 533:
enclosure_handler () {
^-- SC2120: enclosure_handler references arguments, but none are ever passed.


In cmd/vdev_id/vdev_id line 539:
	ENC=$(basename $(readlink -m "/sys/$DEVPATH/../.."))
                       ^-- SC2046: Quote this to prevent word splitting.


In cmd/vdev_id/vdev_id line 540:
	if [ ! -d /sys/class/enclosure/$ENC ] ; then
                                       ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	if [ ! -d /sys/class/enclosure/"$ENC" ] ; then


In cmd/vdev_id/vdev_id line 548:
	ENC_DEVICE=$(readlink /sys/class/enclosure/$ENC)
                                                   ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	ENC_DEVICE=$(readlink /sys/class/enclosure/"$ENC")


In cmd/vdev_id/vdev_id line 552:
	PORT_DIR=$(echo $ENC_DEVICE | grep -Eo '.+host[0-9]+/port-[0-9]+:[0-9]+')
                        ^---------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	PORT_DIR=$(echo "$ENC_DEVICE" | grep -Eo '.+host[0-9]+/port-[0-9]+:[0-9]+')


In cmd/vdev_id/vdev_id line 555:
	PORT_ID=$(echo $PORT_DIR | grep -Eo "[0-9]+$")
                       ^-------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	PORT_ID=$(echo "$PORT_DIR" | grep -Eo "[0-9]+$")


In cmd/vdev_id/vdev_id line 559:
	PCI_ID_LONG=$(basename $(readlink -m "/sys/$PORT_DIR/../.."))
                               ^-- SC2046: Quote this to prevent word splitting.


In cmd/vdev_id/vdev_id line 565:
	NAME=$(awk "/channel/{if ($1 == "channel" && $2 == "$PCI_ID" && \
                                         ^-----^ SC2140: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A\"B\"C"?
                                                            ^-----^ SC2027: The surrounding quotes actually unquote this. Remove or escape them.
                                                            ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	NAME=$(awk "/channel/{if ($1 == "channel" && $2 == ""$PCI_ID"" && \


In cmd/vdev_id/vdev_id line 566:
		$3 == "$PORT_ID") {print ${4}int(count[$4])}; count[$4]++}" $CONFIG)
                       ^------^ SC2027: The surrounding quotes actually unquote this. Remove or escape them.
                       ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		$3 == ""$PORT_ID"") {print ${4}int(count[$4])}; count[$4]++}" $CONFIG)


In cmd/vdev_id/vdev_id line 602:
	if echo $DM_NAME | grep -q -E 'p[0-9][0-9]*$' ; then
                ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	if echo "$DM_NAME" | grep -q -E 'p[0-9][0-9]*$' ; then


In cmd/vdev_id/vdev_id line 604:
			DM_PART=$(echo $DM_NAME | awk -Fp '/p/{print "-part"$2}')
                                       ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
			DM_PART=$(echo "$DM_NAME" | awk -Fp '/p/{print "-part"$2}')


In cmd/vdev_id/vdev_id line 612:
			link=$(echo $link | sed 's/p[0-9][0-9]*$//')
                                    ^---^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
			link=$(echo "$link" | sed 's/p[0-9][0-9]*$//')


In cmd/vdev_id/vdev_id line 615:
		for l in $link $(basename $link) ; do
                                          ^---^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		for l in $link $(basename "$link") ; do


In cmd/vdev_id/vdev_id line 616:
			if [ ! -z $l ]; then
                             ^-- SC2236: Use -n instead of ! -z.
                                  ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
			if [ ! -z "$l" ]; then


In cmd/vdev_id/vdev_id line 617:
				alias=$(awk -v var=$l '($1 == "alias") && \
                                                   ^-- SC2086: Double quote to prevent globbing and word splitting.
                                                                          ^-- SC1004: This backslash+linefeed is literal. Break outside single quotes if you just want to break the line.

Did you mean: 
				alias=$(awk -v var="$l" '($1 == "alias") && \


In cmd/vdev_id/vdev_id line 618:
					($3 == var) \
                                                    ^-- SC1004: This backslash+linefeed is literal. Break outside single quotes if you just want to break the line.


In cmd/vdev_id/vdev_id line 621:
					echo ${alias}${DM_PART}
                                             ^------^ SC2086: Double quote to prevent globbing and word splitting.
                                                     ^--------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
					echo "${alias}""${DM_PART}"


In cmd/vdev_id/vdev_id line 631:
	case ${OPTION} in
        ^-- SC2220: Invalid flags are not handled. Add a *) case.


In cmd/vdev_id/vdev_id line 643:
	ENCLOSURE_MODE=$(awk '{if ($1 == "enclosure_symlinks") \
                                                               ^-- SC1004: This backslash+linefeed is literal. Break outside single quotes if you just want to break the line.


In cmd/vdev_id/vdev_id line 644:
		print $2}' $CONFIG)
                           ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
		print $2}' "$CONFIG")


In cmd/vdev_id/vdev_id line 668:
if [ ! -r $CONFIG ] ; then
          ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
if [ ! -r "$CONFIG" ] ; then


In cmd/vdev_id/vdev_id line 679:
	TOPOLOGY=$(awk '($1 == "topology") {print $2; exit}' $CONFIG)
                                                             ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	TOPOLOGY=$(awk '($1 == "topology") {print $2; exit}' "$CONFIG")


In cmd/vdev_id/vdev_id line 683:
	BAY=$(awk '($1 == "slot") {print $2; exit}' $CONFIG)
                                                    ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	BAY=$(awk '($1 == "slot") {print $2; exit}' "$CONFIG")


In cmd/vdev_id/vdev_id line 690:
	ID_ENCLOSURE=$(enclosure_handler)
                       ^---------------^ SC2119: Use enclosure_handler "$@" if function's $1 should mean script's $1.


In cmd/vdev_id/vdev_id line 696:
	ENCLOSURE_PREFIX=$(awk '/enclosure_symlinks_prefix/{print $2}' $CONFIG)
                                                                       ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	ENCLOSURE_PREFIX=$(awk '/enclosure_symlinks_prefix/{print $2}' "$CONFIG")

For more information:
  https://www.shellcheck.net/wiki/SC2066 -- Since you double quoted this, it ...
  https://www.shellcheck.net/wiki/SC1083 -- This { is literal. Check expressi...
  https://www.shellcheck.net/wiki/SC2010 -- Don't use ls | grep. Use a glob o...

Attaching as well because github literal embed seems to still break formatting.
shellcheck.log

@arshad512
Copy link
Contributor Author

Latest code moved to #11526. I am closing this PR. - Thanks

@arshad512 arshad512 closed this Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Code Review Needed Ready for review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants