Details
-
Bug
-
Resolution: Fixed
-
Blocker
-
Lustre 2.4.0, Lustre 2.1.5
-
3
-
6799
Description
With the introduction of lum_layout_version field to lov_user_md, using "tar --xattr" to backup and restore the Lustre layout (LOV EA) will incorrectly pass the lum_layout_version value back to ll_setxattr() as lum_stripe_offset. This will incorrectly turn the layout version into the starting OST index, for cases where the version is within [1,max_ost_idx].
This may cause, for example, all files that have been migrated once to be restored by tar onto OST1.
Attachments
Issue Links
Activity
Jinshan, my reservation about that approach is that you are removing functionality from the getxattr interface. On platforms that use function shipping, [gs]etxattr may be the only option because the needed ioctls are not supported. Also, having inconsistent semantics between ioctl and getxattr is confusing.
Having said all that, I don't know whether applications on function-shipping clients would ever need to know they layout generation number. But in general it seems better to leave open the possibility in case we need it later.
patch is at: http://review.whamcloud.com/5664
In the above patch, layout_gen is cleared for getxattr
hmm.. do you think it's a good idea to pack layout_gen into low 12bit of lmm_stripe_size. Usually we don't need to know the exact version of layout_gen because it's only used to detect layout change. That being said, we don't need to keep layout_gen when restoring a file.
Ned, yes this does seem like a good path forward. There have been requests to allow specifying the exact OST index for all of the stripes.
Andreas, yes we could do this in the context on LU-2182. I was thinking we could handle the simple case first, which is to set the offset of stripe 0. llapi_layout_ost_index_set() would store the OST index of stripe N in lum.lmm_objects[N].l_ost_idx. For now, ll_setxattr would copy lmm_objects[0].l_ost_idx to lum.lum_stripe_offset if lmm_magic has the flag set. This approach would let us match the functionality that exists today without changing any server code. Eventually we can extend this to support stripes > 0 without changing the API, but that would be more invasive (i.e. requiring changes to lod). Does this approach sound reasonable?
Emoly, you can reproduce the problem without modifying liblustreapi by swapping the layout of two files with the llapi_swap_layouts() function, which will force their generation numbers to increment. Then tarring and untarring them with --xattr will force their offset to be at index 1.
Emoly, I think your reproducer exactly demonstrates the problem - if lum_layout_gen is non-zero it will be confused with lum_stripe_offset.
Ned, I think your solution is interesting, and is also related to LOV_MAGIC_V[13]_DEF magic that already exists in the code. Should this new flag be implemented in the context of LU-2182?
It's improper to set lum_layout_gen explicitly because stripe_offset and layout_gen share in a union, for writing and reading separately.
I'm just wondering how to reproduce this problem. I try to use "tar --xattrs" mentioned in description, but even I set "lum.u.lum_layout_gen = 2", after untar, layout_gen turns back to zero and it isn't misread as stripe_offset.
I did a test to reproduce the problem in the description by setting lum_layout_gen explicitly.
diff --git a/lustre/utils/liblustreapi.c b/lustre/utils/liblustreapi.c index 4067bd4..d0991c0 100644 --- a/lustre/utils/liblustreapi.c +++ b/lustre/utils/liblustreapi.c @@ -582,6 +582,7 @@ int llapi_file_open_pool(const char *name, int flags, int mode, lum.lmm_stripe_size = stripe_size; lum.lmm_stripe_count = stripe_count; lum.lmm_stripe_offset = stripe_offset; + lum.u.lum_layout_gen = 2; if (pool_name != NULL) { strncpy(lum.lmm_pool_name, pool_name, LOV_MAXPOOLNAME); } else {
When running "lfs setstripe -c -1 -i $i /mnt/lustre/testfile$i" on 4 OSTs, the output is
testfile0 lmm_stripe_count: 4 lmm_stripe_size: 1048576 lmm_layout_gen: 0 lmm_stripe_offset: 2 obdidx objid objid group 2 37 0x25 0 3 37 0x25 0 0 37 0x25 0 1 38 0x26 0 testfile1 lmm_stripe_count: 4 lmm_stripe_size: 1048576 lmm_layout_gen: 0 lmm_stripe_offset: 2 obdidx objid objid group 2 38 0x26 0 3 38 0x26 0 0 38 0x26 0 1 39 0x27 0 testfile2 lmm_stripe_count: 4 lmm_stripe_size: 1048576 lmm_layout_gen: 0 lmm_stripe_offset: 2 obdidx objid objid group 2 39 0x27 0 3 39 0x27 0 0 39 0x27 0 1 40 0x28 0 testfile3 lmm_stripe_count: 4 lmm_stripe_size: 1048576 lmm_layout_gen: 0 lmm_stripe_offset: 2 obdidx objid objid group 2 40 0x28 0 3 40 0x28 0 0 40 0x28 0 1 41 0x29 0
It shows stripe_offset is overwritten by lum_layout_gen.
getxattr should take the returning data as opaque unless the extended attributes are system defined. Ioctl and getxattr are already two separate interfaces so I tend to think it's okay to return different data, if we have defined the return value clearly.
I worked out this patch because it'd be the simplest way to keep compatibility. Also this solution won't last long because v4 version of layout is on its way, at that time, layout gen will no longer share field with others.