On Tue, Apr 17, 2018 at 09:21:16AM +0900, Misono Tomohiro wrote: > On 2018/04/17 2:53, David Sterba wrote: > > On Wed, Apr 11, 2018 at 02:20:49PM +0900, Misono Tomohiro wrote: > >> Use btrfs_delete_subvolume() to refactor btrfs_ioctl_snap_destroy(). > >> The permission check is still done in btrfs_ioctl_snap_destroy(). Also, > >> call of d_delete() is still required since btrfs_delete_subvolume() > >> does not call it. > >> > >> As a result, btrfs_unlink_subvol() and may_destroy_subvol() > >> become static functions. No functional change happens. > >> > >> Signed-off-by: Tomohiro Misono <misono.tomoh...@jp.fujitsu.com> > > > > Why is this still split into two patches? Factoring out a function > > should happen in one patch, ie 2 and 3 in one go. Do you have a reason > > not to do it like that? > > I thought this reduces code change in one patch and might be good, but > I'm completely fine with merging 2nd and 3rd patches.
This makes the rewiew harder, if the code is moved the diff can be long but we have both pieces to compare. Moving code is considered one logical change and should not introduce any other changes, besides renaming or formatting fixups. > Should I send v5? Yes please, this is not a trivial change and the changelogs need to be adjusted. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
