Re: [PATCH] btrfs-progs: qgroup: swap the argument in the caller of update_qgroup_relation

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1624 bytes --]



On 2018年05月07日 16:50, Lu Fengqi wrote:
> The QGROUP_RELATION item is very special, it always exists in pairs
> (objectid and offset exchange). Its objectid and offset are the ids of a
> pair of parent and child qgroups, respectively. The larger one is
> parent and the smaller one is child. After the following commit, the order
> of the parameters is wrong and causes qgroup show to output the wrong
> qgroup parent-child relationship.
> 
> Fixes: aaf2dac5ef37 ("btrfs-progs: qgroup: split update_qgroup to reduce arguments")
> Issue: #129
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
> ---
>  qgroup.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/qgroup.c b/qgroup.c
> index 11659e8394dd..e7e127daf5ce 100644
> --- a/qgroup.c
> +++ b/qgroup.c
> @@ -1122,11 +1122,16 @@ static int __qgroups_search(int fd, struct qgroup_lookup *qgroup_lookup)
>  				qgroupid = btrfs_search_header_offset(sh);
>  				qgroupid1 = btrfs_search_header_objectid(sh);
>  
> -				if (qgroupid < qgroupid1)
> +				if (qgroupid <= qgroupid1)
>  					break;
>  
> +				/*
> +				 * because of qgroupid > qgroupid1, qgroupid is
> +				 * the id of parent, and qgroupid1 is the id of
> +				 * child.
> +				 */

Instead of such comment, renaming @qgroupid to @parent, and @qgroupid1
to @child makes more sense.

And a test case would definitely help in this case.

Thanks,
Qu

>  				ret = update_qgroup_relation(qgroup_lookup,
> -							qgroupid, qgroupid1);
> +							qgroupid1, qgroupid);
>  				break;
>  			default:
>  				return ret;
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]