On Sat, Mar 02, 2024 at 06:51:58AM +1030, Qu Wenruo wrote: > 在 2024/3/1 22:31, David Sterba 写道: > > On Tue, Feb 27, 2024 at 01:45:35PM +1030, Qu Wenruo wrote: > >> [BUG] > >> Currently btrfs can create subvolume with an invalid qgroup inherit > >> without triggering any error: > >> > >> # mkfs.btrfs -O quota -f $dev > >> # mount $dev $mnt > >> # btrfs subvolume create -i 2/0 $mnt/subv1 > >> # btrfs qgroup show -prce --sync $mnt > >> Qgroupid Referenced Exclusive Path > >> -------- ---------- --------- ---- > >> 0/5 16.00KiB 16.00KiB <toplevel> > >> 0/256 16.00KiB 16.00KiB subv1 > >> > >> [CAUSE] > >> We only do a very basic size check for btrfs_qgroup_inherit structure, > >> but never really verify if the values are correct. > >> > >> Thus in btrfs_qgroup_inherit() function, we have to skip non-existing > >> qgroups, and never return any error. > >> > >> [FIX] > >> Fix the behavior and introduce extra checks: > >> > >> - Introduce early check for btrfs_qgroup_inherit structure > >> Not only the size, but also all the qgroup ids would be verifyed. > >> > >> And the timing is very early, so we can return error early. > >> This early check is very important for snapshot creation, as snapshot > >> is delayed to transaction commit. > >> > >> - Drop support for btrfs_qgroup_inherit::num_ref_copies and > >> num_excl_copies > >> Those two members are used to specify to copy refr/excl numbers from > >> other qgroups. > >> This would definitely mark qgroup inconsistent, and btrfs-progs has > >> dropped the support for them for a long time. > >> It's time to drop the support for kernel. > >> > >> - Verify the supported btrfs_qgroup_inherit::flags > >> Just in case we want to add extra flags for btrfs_qgroup_inherit. > >> > >> Now above subvolume creation would fail with -ENOENT other than silently > >> ignore the non-existing qgroup. > >> > >> CC: stable@xxxxxxxxxxxxxxx > >> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx> > > > > Reviewed-by: David Sterba <dsterba@xxxxxxxx> > > > Just one thing to notice, this would cause certain test cases to fail, > as previously any incorrect qgroup inherit would just be ignored, but > now it would error out explicitly. Ok, this is expected if we do fixes like that.