On 20.12.2017 07:13, Qu Wenruo wrote:
> btrfs_qgroup_inherit structure has two members, num_ref_copies and
> num_excl_copies, to info btrfs kernel modules to inherit (copy)
> rfer/excl numbers at snapshot/subvolume creation time.
>
> Since qgroup number is already hard to maintain for multi-level qgroup
> scenario, allowing user to manually manipulate qgroup inherit is quite
> easy to screw up qgroup numbers.
>
> Although btrfs-progs supports such inheritance specification, the
> options are hidden from user and not documented.
> So there is no need to allow user to manually specify inheritance in
> kernel.
>
> Reported-by: Omar Sandoval <osandov@xxxxxxxxxxx>
> Reported-by: Nikolay Borisov <nborisov@xxxxxxxx>
> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
Overall I'm ok with the patch but I'd like to hear the opinion of other
developer about whether we can rename the fields.
> ---
> changelog:
> v2:
> Don't return -ENOTTY, but just ignoring value set in
> num_ref/excl_copies, suggested by Nikolay.
> Don't modify the UAPI members, but add comment to deprecate them,
> suggested by Omar.
> And add Omar as first reporter.
> ---
> fs/btrfs/qgroup.c | 54 +++++++++++++---------------------------------
> include/uapi/linux/btrfs.h | 4 ++--
> 2 files changed, 17 insertions(+), 41 deletions(-)
>
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 168fd03ca3ac..42c6b35d8d7f 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -2158,9 +2158,22 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
> }
>
> if (inherit) {
> + /*
> + * num_excl/rfer_copies indicate how many qgroup pairs needs
> + * to be manually inherited (copy rfer or excl from src
> + * qgroup to dst, along with qgroup relationship added)
> + *
> + * Allowing user to manipulate inheritance can easily cause
> + * problem in multi-level qgroup scenario.
> + * And the ioctl interface is hidden in btrfs-progs for a long
> + * time, deprecate them should not be a big problem.
> + *
> + * Here we just ignore any value set in num_excl/rfer_copies,
> + * and only handles qgroup relationship specified by
> + * @num_qgroups.
> + */
> i_qgroups = (u64 *)(inherit + 1);
> - nums = inherit->num_qgroups + 2 * inherit->num_ref_copies +
> - 2 * inherit->num_excl_copies;
> + nums = inherit->num_qgroups;
> for (i = 0; i < nums; ++i) {
> srcgroup = find_qgroup_rb(fs_info, *i_qgroups);
>
> @@ -2286,43 +2299,6 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
> ++i_qgroups;
> }
>
> - for (i = 0; i < inherit->num_ref_copies; ++i, i_qgroups += 2) {
> - struct btrfs_qgroup *src;
> - struct btrfs_qgroup *dst;
> -
> - if (!i_qgroups[0] || !i_qgroups[1])
> - continue;
> -
> - src = find_qgroup_rb(fs_info, i_qgroups[0]);
> - dst = find_qgroup_rb(fs_info, i_qgroups[1]);
> -
> - if (!src || !dst) {
> - ret = -EINVAL;
> - goto unlock;
> - }
> -
> - dst->rfer = src->rfer - level_size;
> - dst->rfer_cmpr = src->rfer_cmpr - level_size;
> - }
> - for (i = 0; i < inherit->num_excl_copies; ++i, i_qgroups += 2) {
> - struct btrfs_qgroup *src;
> - struct btrfs_qgroup *dst;
> -
> - if (!i_qgroups[0] || !i_qgroups[1])
> - continue;
> -
> - src = find_qgroup_rb(fs_info, i_qgroups[0]);
> - dst = find_qgroup_rb(fs_info, i_qgroups[1]);
> -
> - if (!src || !dst) {
> - ret = -EINVAL;
> - goto unlock;
> - }
> -
> - dst->excl = src->excl + level_size;
> - dst->excl_cmpr = src->excl_cmpr + level_size;
> - }
> -
> unlock:
> spin_unlock(&fs_info->qgroup_lock);
> out:
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index ce615b75e855..a10342d491bc 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -80,8 +80,8 @@ struct btrfs_qgroup_limit {
> struct btrfs_qgroup_inherit {
> __u64 flags;
> __u64 num_qgroups;
> - __u64 num_ref_copies;
> - __u64 num_excl_copies;
> + __u64 num_ref_copies; /* DEPRECATED, will just be ignored */
> + __u64 num_excl_copies; /* DEPRECATED, will just be ignored */
> struct btrfs_qgroup_limit lim;
> __u64 qgroups[0];
> };
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
