[Devel] [PATCH vz9 v2 2/2] fs: use is_sb_ve_accessible() for VE-filtered sync

Konstantin Khorenko khorenko at virtuozzo.com
Tue Nov 9 22:45:56 MSK 2021


On 02.11.2021 19:03, Nikita Yushchenko wrote:
> 02.11.2021 19:00, Kirill Tkhai wrote:
>> On 02.11.2021 17:45, Nikita Yushchenko wrote:
>>> Remove dedicated implementation of VE-filtered sync.
>>>
>>> Instead, implement VE-filtered sync within common sync code, by passing
>>> VE to sync_inodes_one_sb() and sync_fs_one_sb(), and filtering SBs to
>>> sync there via is_sb_ve_accessible().
>>>
>>> This makes VE-filtered sync to include mounts from VE's non-root mount
>>> namespaces.
>>>
>>> https://jira.sw.ru/browse/PSBM-44684
>>> Signed-off-by: Nikita Yushchenko <nikita.yushchenko at virtuozzo.com>
>>
>> This is still difficult to review. We should use a single patch for a single change.
>> Refactorings should go in separate patches to not distract our attention from really important places.
>> Could you please move sync_arg introduction in a separate patch like in attached file I did?
> 
> I considered that but abandoned.
> This has no meaning outside of the rest of the patch.

Nikita, i tend to agree with Kirill, argument change can easily be done in a separate patch and this 
will make the review easier.

Moreover, as you effectively refactor quite a number of original code,
can you please separate the part which should be merged with the original patch
e6acaa7d3aff ("ve/fs/sync: Per containter sync and syncfs and fs.fsync-enable sysctl")
effectively killing dropped functions.

Or may be revert the original patch completely and create a whole new series for this feature as the 
original commit is also a holy merged mess.

Thank you.


More information about the Devel mailing list