Re: [PATCH] ovl: Support FS_IOC_[SG]ETFLAGS and FS_IOC_FS[SG]ETXATTR ioctls on directories

[Xiao Yang <yangx.jy@cn.fujitsu.com>]

On 2020/9/22 20:01, Amir Goldstein wrote:
> On Tue, Sep 22, 2020 at 11:15 AM Xiao Yang<yangx.jy@cn.fujitsu.com>  wrote:
>> On 2020/9/21 17:09, Amir Goldstein wrote:
>>> On Mon, Sep 21, 2020 at 11:55 AM Xiao Yang<yangx.jy@cn.fujitsu.com>   wrote:
>>>> On 2020/9/21 16:17, Amir Goldstein wrote:
>>>>> On Mon, Sep 21, 2020 at 10:41 AM Xiao Yang<yangx.jy@cn.fujitsu.com>    wrote:
>>>>>> Factor out ovl_ioctl() and ovl_compat_ioctl() and take use of them for
>>>>>> directories.
>>>>>>
>>>>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>>>>>> ---
>>>>> This change is buggy. I had already posted it and self NACKed myself [1].
>>>>>
>>>>> You can find an hopefully non-buggy version of it on my ovl-shutdown [2] branch.
>>>>>
>>>>> As long as you are changing ovl_ioctl(), please also take the second
>>>>> commit on that
>>>>> branch to replace the open coded capability check with the
>>>>> vfs_ioc_setflags_prepare()
>>>>> generic helper.
>>>> Hi Amir,
>>>>
>>>> Thanks a lot for your quick reply. :-)
>>>> I will try to read and understand the metioned patches on your
>>>> ovl-shutdown branch.
>>> Please also verify my claim in the patch commit message, that the
>>> the test result of xfstest generic/079 changes from "notrun" to "success".
>> Hi Amir,
>>
>> With your patches, I have confirmed that generic/079 actually changed from
>> "notrun" to "success".  Besides, one minor issue:
>> Could we avoid the following compiler warning?
>> -------------------------------------------------
>> fs/overlayfs/readdir.c: In function ‘ovl_dir_real_file’:
>> fs/overlayfs/readdir.c:883:37: warning: passing argument 1 of
>> ‘ovl_dir_open_realfile’ discards ‘const’ qualifier from pointer target
>> type [-Wdiscarded-qualifiers]
>>     883 |    realfile = ovl_dir_open_realfile(file,&upperpath);
>>         |                                     ^~~~
>> fs/overlayfs/readdir.c:842:56: note: expected ‘struct file *’ but
>> argument is of type ‘const struct file *’
>>     842 | static struct file *ovl_dir_open_realfile(struct file *file,
>>         |                                           ~~~~~~~~~~~~~^~~~
>> -------------------------------------------------
>>
> Shouldn't be a problem to change ovl_dir_open_realfile()
> to take a const struct file * argument I think.
Hi Amir,

Other than the compiler warning I tested your patches on our
enviroment and didn't find any issue, so add:
Reviewed-by: Xiao Yang <yangx.jy@.cn.fujisu.com>

Thank you for sharing these patches again. :-)

Best Regards,
Xiao Yang
> Thanks,
> Amir.
>
>
> .
>