[LU-6234] lfs computes pool name length incorrectly

• Type: Bug
• Resolution: Fixed
• Priority: Minor
• Fix Version/s: Lustre 2.8.0
• Affects Version/s: Lustre 2.7.0
• Environment: lustre-master tag 2.6.93
• Severity: 3
• Assignee: Zhenyu Xu
• Reporter: James Nunez (Inactive)

Description

When dealing with pools, you need to specify the file system name in front of the pool name as <fsname>.<pool_name>. The pool name, without the file system name, has a max length of 15 characters and is correctly enforced:

# lctl pool_new scratch.test_82b
Pool scratch.test_82b created
# lctl pool_new scratch.test_82abcdefgh
Pool scratch.test_82abcdefgh created
# lctl pool_new scratch.test_82abcdefghi
poolname test_82abcdefghi is too long (length is 16 max is 15)
argument scratch.test_82abcdefghi must be <fsname>.<poolname>
pool_new: File name too long

Yet, when specifying a pool name in 'lfs setstripe', the file system name is used to compute the length of the pool name. The file system name is required when using the '-p' option in 'lfs setstripe' as in the second example below.

# /usr/bin/lfs setstripe -p scratch.test_82b -o 7936 /lustre/scratch/d82b.conf-sanity/f82b.conf-sanity
error: setstripe: pool name 'scratch.test_82b' is too long (max is 15 characters)
...
# /usr/bin/lfs setstripe -p test_82b -o 4 /lustre/scratch/d82b.conf-sanity/f82b.conf-sanity
can't find fs root for '/lustre/scratch/d82b.conf-sanity': -19
'/lustre/scratch/d82b.conf-sanity/f82b.conf-sanity' is not on a Lustre filesystem: No such device (19)
error: setstripe: create stripe file '/lustre/scratch/d82b.conf-sanity/f82b.conf-sanity' failed
...

This caused problems with the conf-sanity test 82b at https://testing.hpdd.intel.com/test_sets/e9152f7c-ade4-11e4-a0b6-5254006e85c2

Issue Links

• is related to: LU-7103 ost-pools test_7a: test failed to respond and timed out
• is related to: LU-7155 Interop2.7.0<->master ost-pools test_7b: FAIL: can create a pool with an invalid name
• is related to: LU-6081 hsm: add file migrate support

Comments

[Zhenyu Xu]

isalnum('.') == false, and current patch can handle it.

[Andreas Dilger]

I noticed this patch landed, but it would have been good to add '.' to the characters that are not allowed in a pool or filesystem name. Otherwise it will break "lctl {get,set}_param" using '.' as a separator.

[Peter Jones]

Landed for 2.8

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/13742/
Subject: LU-6234 util: check fsname and pool name
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 7c25eb1ba2b1db3009a0e88b3ecf229134f8ac92

[Stephen Champion]

This is annoying. What is stalling the patch?

[Frank Zago (Inactive)]

Thanks Jean-Charles.

I've updated the patch with tests in ost-pool.sh. See attachments.

[jacques-charles lafoucriere]

A pool specific test script has been added (ost-pools.sh). New pool tests should go to it.

[Frank Zago (Inactive)]

Patch looks good. I've added a few tests to sanity.sh. See attachment. Would you mind folding them into that patch?

[Gerrit Updater]

Bobi Jam ([e-mail]) uploaded a new patch: http://review.whamcloud.com/13742
Subject: LU-6234 lfs: check pool name length w/o fsname in it
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: a192f2660b86b3609b60bcc15911f4fd26d8ab7e

[Andreas Dilger]

It appears that specifying the pool name avoids the need to extract the fsname from the target directory, so it makes the code somewhat more efficient. Regardless, that is something that previously worked and was allowed, so it shouldn't be broken. It was only allowed to land because it happened that the fsname.pool_name was under the 15-char limit of the check. I'm OK with fixing the existing check to verify only the poolname part of the input is within the limits, or moving it lower in llapi to verify only the pool name after it has been stripped.

[Frank Zago (Inactive)]

But does it make sense to pass a fsname? After all, the fsname is found through the file given in parameter.
Should 6ec0b4 be reverted?