Re: [PATCH] generic/631: Add a check for extended attributes

[Shreeya Patel <shreeya.patel@collabora.com>]

On 30/04/21 8:47 pm, Darrick J. Wong wrote:
> On Fri, Apr 30, 2021 at 03:56:56PM +0530, Shreeya Patel wrote:
>> Test case 631 fails for filesystems like exfat or vfat or any other
>> which does not support extended attributes.
>>
>> The main reason for failure is not being able to mount overlayfs
>> with filesystems that do not support extended attributes.
>> mount -t overlay overlay -o "$l,$u,$w,$i" $mergedir
>>
>> Above command would return an error as -
>> /var/mnt/scratch/merged0: wrong fs type, bad option, bad superblock on overlay,
>> missing codepage or helper program, or other error.
>>
>> dmesg log reports the following -
>> overlayfs: filesystem on '/var/mnt/scratch/upperdir1' not supported
>>
>> As per the overlayfs documentation -
>> "A wide range of filesystems supported by Linux can be the lower filesystem,
>> but not all filesystems that are mountable by Linux have the features needed
>> for OverlayFS to work. The lower filesystem does not need to be writable.
>> The lower filesystem can even be another overlayfs.
>> The upper filesystem will normally be writable and if it is it must support
>> the creation of trusted.* and/or user.* extended attributes, and must provide
>> valid d_type in readdir responses, so NFS is not suitable.
> Does this test also need to check for d_type support?


It already does that.


>> A read-only overlay of two read-only filesystems may use any filesystem type."
>>
>> As per the above statements from the overlayfs documentation, it is clear that
>> filesystems that do not support extended attributes would not work with overlayfs.
>> This is why we see the error in dmesg log for upperdir1 which had an exfat filesystem.
> (Please wrap the commit messages at 75 columns, per SubmittingPatches.)


Sure, will send a v2.


>> Hence, add a check for extended attributes which would avoid running this tests for
>> filesystems that are not supported.
>>
>> Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
> Regardless, this seems like a reasonable change, so:
>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>


Thanks


> --D
>
>> ---
>>   tests/generic/631 | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/tests/generic/631 b/tests/generic/631
>> index c43f3de3..c7f0190e 100755
>> --- a/tests/generic/631
>> +++ b/tests/generic/631
>> @@ -39,10 +39,12 @@ _cleanup()
>>   
>>   # get standard environment, filters and checks
>>   . ./common/rc
>> +. ./common/attr
>>   
>>   # real QA test starts here
>>   _supported_fs generic
>>   _require_scratch
>> +_require_attrs
>>   test "$FSTYP" = "overlay" && _notrun "Test does not apply to overlayfs."
>>   _require_extra_fs overlay
>>   
>> -- 
>> 2.30.2
>>