Re: [PATCH v2 1/6] idmapped-mount: split setgid test from test-core

["xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com>]

on 2022/4/8 9:20, xuyang2018.jy@fujitsu.com wrote:
> on 2022/4/7 20:55, Christian Brauner wrote:
>> On Thu, Apr 07, 2022 at 08:09:30PM +0800, Yang Xu wrote:
>>> Since we plan to increase setgid test covertage, it will find new bug
>>> , so add a new test group test-setgid is better.
>>>
>>> Also add a new test case to test test-setgid instead of miss it.
>>>
>>> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
>>> ---
>>>    src/idmapped-mounts/idmapped-mounts.c | 19 +++++++++++++++----
>>>    tests/generic/999                     | 26 ++++++++++++++++++++++++++
>>>    tests/generic/999.out                 |  2 ++
>>
>> I actually didn't mean to split out the existing setgid tests. I mean
>> adding new ones for the test-cases you're adding. But how you did it
>> works for me too and is a bit nicer. I don't have a strong opinion so as
>> long as Dave and Darrick are fine with it then this seems good to me.
> Ok, let's listen ..
When I write v3, I add mknodat patch as 1st patch and tmpfile as 2nd 
patch(by using a file doesn't under DIR1 directory, so I don't need to 
concern about xfs_irix_sgid_inherit_enabled), errno reset to 0 as 3st 
patch. It seems this way can't introduce the new failure for generic/633.

So I will add a new group for umask and acl and add new case for them 
instead of split setgid case from test-core group.

ps: I doubt whether I need to send two patch sets(one is about 
mknodat,tmpfile,errno, the another is about umask,acl,new case).
What do you think about this?

Best Regards
Yang Xu
>>
>> One note about the test name/numbering though. It seems you haven't
>> added the test using the provided xfstest infrastructure to do that.
>> Instead of manually adding the test you should run the "new" script.
>>
>> You should run:
>>
>>           ~/src/git/xfstests$ ./new generic
>>
>>           Next test id is 678
>>           Append a name to the ID? Test name will be 678-$name. y,[n]:
>>           Creating test file '678'
>>           Add to group(s) [auto] (separate by space, ? for list): auto quick attr idmapped mount perms
>>           Creating skeletal script for you to edit ...
>>
>> that'll automatically figure out the correct test number etc.
> Thanks, TBH, I don't know this usage. I don't name to 678 because
> fstests patchwork has others new case(in reviewing), so I add a biger
> number.
>
> Best Regards
> Yang Xu