On Mon, May 07, 2018 at 07:19:32PM +0800, Qu Wenruo wrote:
>
>
>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 <[email protected]>
>> ---
>>  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.

Although we are not sure which one is the parent before this if statement,
renaming may indeed make the code clearer, so V2 is coming.

>
>And a test case would definitely help in this case.

Make sense.

-- 
Thanks,
Lu

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


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to