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

pool create fails using the devices returned by the get_persistent_disk_name #15567

Merged
merged 3 commits into from
Nov 29, 2023

Conversation

vaibhav-delphix
Copy link
Contributor

@vaibhav-delphix vaibhav-delphix commented Nov 23, 2023

Motivation and Context

The following tests had the failure shown below:

[2023-10-08T09:06:59.799Z] Test (Linux): /cli_root/zpool_reopen/zpool_reopen_002_pos (run as root) [00:02] [FAIL]
[2023-10-08T09:06:59.799Z] Test (Linux): /cli_root/zpool_reopen/zpool_reopen_003_pos (run as root) [00:02] [FAIL]
[2023-10-08T09:07:00.066Z] Test (Linux): /cli_root/zpool_reopen/zpool_reopen_004_pos (run as root) [00:02] [FAIL]

Test Failure:

ASSERTION: Testing zpool reopen with no arguments
cannot use '/dev/disk/by-id/wwn': must be a block device or regular file
ERROR: zpool create -f testpool-be34c68f-a0b4 mirror wwn sdc exited 1
NOTE: Performing test-fail callback (/usr/share/zfs/zfs-tests/callbacks/zfs_dbgmsg.ksh)

Problem is in get_persistent_disk_name function in blkdev.shlib file.
function is fetching a unqiue id using udevadm utility. It scan the output with /disk/by-id and taking 3rd element from separation.

On awk use we get the two values from udevadm info command.
disk/by-id/scsi-36000c2983b8bf71d74b22eba4d8af792 disk/by-id/wwn/0x6000c2983b8bf71d74b22eba4d8af792

Until we get the scsi-36000c2983b8bf71d74b22eba4d8af792 test runs file but when it gets 3rd element wwn test fails.

Description

"udevadm info -q all -n $DEV_DSKDIR/$device | awk '/disk/by-id/ {print $2; exit}' "
command gives possible two output on system

  1. disk/by-id/scsi-
  2. disk/by-id/wwn/<ID_WWN_WITH_EXTENSION>

In output 2 if we use cut -d/ -f3 it provide wwn string. Because of this wherever we use it fails like:- zpool create
So instead using returning 3rd element better to return the entire string after second sepration.

How Has This Been Tested?

Test (Linux): /usr/share/zfs/zfs-tests/tests/functional/cli_root/zpool_reopen/zpool_reopen_002_pos (run as root) [03:18] [PASS]
17:35:34.71 ASSERTION: Testing zpool reopen with no arguments
17:35:36.23 SUCCESS: zpool create -f testpool-96ba77e0-a96a mirror wwn/0x3333333000000fa0 loop1
17:35:36.58 SUCCESS: zfs create testpool-96ba77e0-a96a/testfs
17:35:36.93 SUCCESS: zfs set mountpoint=/var/tmp/testdir testpool-96ba77e0-a96a/testfs
17:35:36.94 SUCCESS: eval echo 'offline' > /sys/block/sde/device/state
17:35:36.97 SUCCESS: eval echo '1' > /sys/block/sde/device/delete
17:38:37.55 SUCCESS: zpool reopen testpool-96ba77e0-a96a
17:38:37.56 SUCCESS: check_state testpool-96ba77e0-a96a wwn/0x3333333000000fa0 unavail
17:38:37.58 SUCCESS: generate_random_file /testpool-96ba77e0-a96a/data 10
17:38:37.63 SUCCESS: zpool sync testpool-96ba77e0-a96a
17:38:37.64 NOTE: /sys/class/scsi_host/host1/scan
17:38:37.64 SUCCESS: eval echo '- - -' > /sys/class/scsi_host/host1/scan
17:38:40.41 SUCCESS: zpool reopen testpool-96ba77e0-a96a
17:38:40.43 SUCCESS: check_state testpool-96ba77e0-a96a wwn/0x3333333000000fa0 online
17:38:49.51 SUCCESS: wait_for_resilver_end testpool-96ba77e0-a96a 40
17:38:50.02 SUCCESS: zpool destroy testpool-96ba77e0-a96a
17:38:50.09 Zpool reopen with no arguments test passed
17:38:50.09 NOTE: Performing local cleanup via log_onexit (cleanup)
17:38:50.09 NOTE: /sys/class/scsi_host/host1/scan
17:38:50.09 SUCCESS: eval echo '- - -' > /sys/class/scsi_host/host1/scan
17:38:52.64 failed to clear label for /dev/sde1
17:38:52.66 failed to clear label for /dev/loop1
Test (Linux): /usr/share/zfs/zfs-tests/tests/functional/cli_root/zpool_reopen/zpool_reopen_004_pos (run as root) [06:38] [PASS]
17:46:13.40 ASSERTION: Testing zpool reopen with pool name as argument
17:46:14.90 SUCCESS: zpool create -f testpool-96ba77e0-a96a mirror scsi-33333333000000fa0 loop1
17:46:15.23 SUCCESS: zfs create testpool-96ba77e0-a96a/testfs
17:46:15.69 SUCCESS: zfs set mountpoint=/var/tmp/testdir testpool-96ba77e0-a96a/testfs
17:46:15.70 SUCCESS: eval echo 'offline' > /sys/block/sde/device/state
17:46:15.72 SUCCESS: eval echo '1' > /sys/block/sde/device/delete
17:49:16.35 SUCCESS: zpool reopen -n testpool-96ba77e0-a96a
17:49:16.37 SUCCESS: check_state testpool-96ba77e0-a96a scsi-33333333000000fa0 unavail
17:49:16.74 SUCCESS: generate_random_file /testpool-96ba77e0-a96a/data 80
17:49:16.79 SUCCESS: zpool sync testpool-96ba77e0-a96a
17:49:16.80 Added handler 2 with the following properties:
17:49:16.80   pool: testpool-96ba77e0-a96a
17:49:16.80   vdev: cf73d00f17a239b0
17:49:16.80 SUCCESS: zinject -d loop1 -D125:1 testpool-96ba77e0-a96a
17:52:23.02 SUCCESS: zpool scrub testpool-96ba77e0-a96a
17:52:23.02 NOTE: /sys/class/scsi_host/host1/scan
17:52:23.02 SUCCESS: eval echo '- - -' > /sys/class/scsi_host/host1/scan
17:52:44.26 SUCCESS: zpool reopen -n testpool-96ba77e0-a96a
17:52:44.27 SUCCESS: check_state testpool-96ba77e0-a96a scsi-33333333000000fa0 online
17:52:44.27 removed all registered handlers
17:52:44.27 SUCCESS: zinject -c all
17:52:47.31 SUCCESS: wait_for_scrub_end testpool-96ba77e0-a96a 40
17:52:47.98 SUCCESS: is_deferred_scan_started testpool-96ba77e0-a96a
17:52:47.99 SUCCESS: wait_for_resilver_end testpool-96ba77e0-a96a 40
17:52:48.32 SUCCESS: zpool offline testpool-96ba77e0-a96a loop1
17:52:48.96 SUCCESS: zpool destroy testpool-96ba77e0-a96a
17:52:48.97 Zpool reopen test successful
17:52:48.97 NOTE: Performing local cleanup via log_onexit (cleanup)
17:52:48.97 removed all registered handlers
17:52:48.97 SUCCESS: zinject -c all
17:52:48.97 NOTE: /sys/class/scsi_host/host1/scan
17:52:48.97 SUCCESS: eval echo '- - -' > /sys/class/scsi_host/host1/scan

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:

  • [x ] My code follows the OpenZFS code style requirements.
  • I have updated the documentation accordingly.
  • [ x] I have read the contributing document.
  • I have added tests to cover my changes.
  • I have run the ZFS Test Suite with this change applied.
  • All commit messages are properly formatted and contain Signed-off-by.

@vaibhav-delphix vaibhav-delphix changed the title zpool create gets fail when we pass the devices returned by the get_persistent_disk_name zpool create gets fail when we use the devices returned by the get_persistent_disk_name Nov 23, 2023
@vaibhav-delphix vaibhav-delphix changed the title zpool create gets fail when we use the devices returned by the get_persistent_disk_name pool create fails using the devices returned by the get_persistent_disk_name Nov 23, 2023
@vaibhav-delphix vaibhav-delphix marked this pull request as draft November 23, 2023 18:12
@vaibhav-delphix vaibhav-delphix marked this pull request as ready for review November 23, 2023 18:37
Copy link
Member

@amotin amotin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to have sense.

@amotin amotin added the Status: Code Review Needed Ready for review and testing label Nov 25, 2023
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Nov 29, 2023
@behlendorf behlendorf merged commit 7d68900 into openzfs:master Nov 29, 2023
15 of 19 checks passed
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Dec 12, 2023
Instead of using only the 3rd element return the entire string after 
the split to handle device names with dashes.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Vaibhav Bhanawat <[email protected]>
Closes openzfs#15567
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants