[LU-6234] lfs computes pool name length incorrectly

• Status: Resolved
• Project: Lustre
• Affects Version/s: Lustre 2.7.0
• Fix Version/s: Lustre 2.8.0
• Type: Bug
• Priority: Minor
• Reporter: James Nunez (Inactive)
• Assignee: Zhenyu Xu
• Resolution: Fixed

Environment

lustre-master tag 2.6.93

• Severity: 3

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-6081 hsm: add file migrate support
• is related to: LU-7103 ost-pools test_7a: test failed to res...
• is related to: LU-7155 Interop2.7.0<->master ost-pools test_...

Comments

[Andreas Dilger]

This problem was introduced by change http://review.whamcloud.com/13241 "LU-6081 lfs: check that pool name is not too long".

The problem is that the supplied pool name may include the FSNAME at the start, as is used in conf-sanity test_82b. In our test configuration we just happen to have the supplied pool name of "lustre.test_82b" which is shorter than the checked limit. The check in lfs_setstripe() should be changed to skip the leading fsname if present.

That also begs the question - do we exclude '.' (and '/' and other problematic characters) from being part of the pool name?

[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?

[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.

[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

[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?

[jacques-charles lafoucriere]

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

[Frank Zago (Inactive)]

Thanks Jean-Charles.

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

[Stephen Champion]

This is annoying. What is stalling the patch?

[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

[Peter Jones]

Landed for 2.8

[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.

[Zhenyu Xu]

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

[Andreas Dilger]

You are right, thanks.