Re: [PATCH v3 2/3] fat: add renameat2 RENAME_EXCHANGE flag support

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

Javier Martinez Canillas <javierm@redhat.com> writes:

>> Main purpose of me is to consolidate helpers with vfat_rename(), and
>> tweak coding style to use existent fat codes.
>>
>
> Indeed. What do you think of the following plan for v4 ?
>
> 1) Keep patch "fat: add a vfat_rename2() and make existing .rename callback a helper"
>    as the first patch of the series.
> 2) Add a patch #2 with your authorship that adds the helper and use them in the
>    vfat_rename() function.
> 3) Make this patch "fat: add renameat2 RENAME_EXCHANGE flag support" to be patch #3
>    and use the helpers introduced in patch #2.
> 4) Make patch #4 to not only add a test for RENAME_EXCHANGE but also for renameat()
>    and renameat2(..., RENAME_NOREPLACE). That way it will also cover your changes in
>    patch #2.

I don't care much about it because whole is not big (in short, I'm ok
with even one patch), so the point is the patches should be able to
bisect easily if separated.

>>> +	/* update inode version and timestamps */
>>> +	inode_inc_iversion(old_inode);
>>> +	inode_inc_iversion(new_inode);
>> 
>> Why do we need to update iversion of those inodes? I couldn't get intent
>> of this.
>>
>
> To be honest, I wasn't sure about this either but I saw that the implementation
> of RENAME_EXCHANGE in other filesystems did. For example btrfs_rename_exchange().

Ok. If I'm not overlooking, it looks like only btrfs. Please remove
those inode_inc_iversion() for {new,old}_inode.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>