lkml.org 
[lkml]   [2022]   [May]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 2/3] fat: add renameat2 RENAME_EXCHANGE flag support
From
On 5/31/22 14:41, OGAWA Hirofumi wrote:
> 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.
>

Yes, git bisect-ability is why I mentioned that we could do it in separate patches
but I'll integrate your changes now and see what approach I take depending on how
the code looks then.

>>>> + /* 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.
>

Sure.

--
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

\
 
 \ /
  Last update: 2022-05-31 14:46    [W:0.062 / U:0.276 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site