Re: [PATCH 6/8] btrfs-progs: qgroups: introduce btrfs_qgroup_query

[Jeff Mahoney <jeffm@suse.com>]

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

On 3/7/18 12:58 AM, Qu Wenruo wrote:
> 
> 
> On 2018年03月03日 02:47, jeffm@suse.com wrote:
>> diff --git a/qgroup.c b/qgroup.c
>> index b1be3311..2d0a6947 100644
>> --- a/qgroup.c
>> +++ b/qgroup.c
>> @@ -1267,6 +1249,66 @@ static int __qgroups_search(int fd, struct qgroup_lookup *qgroup_lookup)
>>  	return ret;
>>  }
>>  
>> +static int qgroups_search_all(int fd, struct qgroup_lookup *qgroup_lookup)
>> +{
>> +	struct btrfs_ioctl_search_args args = {
>> +		.key = {
>> +			.tree_id = BTRFS_QUOTA_TREE_OBJECTID,
>> +			.max_type = BTRFS_QGROUP_RELATION_KEY,
>> +			.min_type = BTRFS_QGROUP_STATUS_KEY,
>> +			.max_objectid = (u64)-1,
>> +			.max_offset = (u64)-1,
>> +			.max_transid = (u64)-1,
>> +			.nr_items = 4096,
>> +		},
>> +	};
>> +	int ret;
>> +
>> +	ret = __qgroups_search(fd, &args, qgroup_lookup);
>> +	if (ret == -ENOTTY)
>> +		error("can't list qgroups: quotas not enabled");
>> +	else if (ret < 0)
>> +		error("can't list qgroups: %s", strerror(-ret));
>> +	return ret;
>> +}
>> +
>> +int btrfs_qgroup_query(int fd, u64 qgroupid, struct btrfs_qgroup_stats *stats)
>> +{
>> +	struct btrfs_ioctl_search_args args = {
>> +		.key = {
>> +			.tree_id = BTRFS_QUOTA_TREE_OBJECTID,
>> +			.min_type = BTRFS_QGROUP_INFO_KEY,
>> +			.max_type = BTRFS_QGROUP_LIMIT_KEY,
>> +			.max_objectid = 0,
>> +			.max_offset = qgroupid,
>> +			.max_transid = (u64)-1,
>> +			.nr_items = 4096, /* should be 2, i think */
> 
> 2 is not correct in fact.
> 
> As QGROUP_INFO is smaller than QGROUP_LIMIT, to get a slice of all what
> we need, we need to include all other unrelated items.
> 
> One example will be:
> 	item 1 key (0 QGROUP_INFO 0/5) itemoff 16211 itemsize 40
> 	item 2 key (0 QGROUP_INFO 0/257) itemoff 16171 itemsize 40
> 	item 3 key (0 QGROUP_INFO 1/1) itemoff 16131 itemsize 40
> 	item 4 key (0 QGROUP_LIMIT 0/5) itemoff 16091 itemsize 40
> 	item 5 key (0 QGROUP_LIMIT 0/257) itemoff 16051 itemsize 40
> 	item 6 key (0 QGROUP_LIMIT 1/1) itemoff 16011 itemsize 40
> 
> To query qgroup info about 0/257, above setup will get the following slice:
> 	item 1 key (0 QGROUP_INFO 0/5) itemoff 16211 itemsize 40
> 	item 2 key (0 QGROUP_INFO 0/257) itemoff 16171 itemsize 40
> 	item 3 key (0 QGROUP_INFO 1/1) itemoff 16131 itemsize 40
> 	item 4 key (0 QGROUP_LIMIT 0/5) itemoff 16091 itemsize 40
> 	item 5 key (0 QGROUP_LIMIT 0/257) itemoff 16051 itemsize 40
> So we still need that large @nr_items.
> 
> Despite this comment it looks good.

Of course.  I use TREE_SEARCH so infrequently that I forget about this
every time so the pain is always fresh.

It should be .min_offset = qgroupid, .nr_items = 2, but of course that
doesn't work either for different reasons.  __qgroups_search's loop will
loop until it comes back with no more results and it sets the nr_items
itself to 4096 at the end of the loop.  The key comparison in the ioctl
only does a regular key comparison and offset doesn't get evaluated if
the types aren't equal.  That works fine when doing tree insertion or
searches for a single key but is wrong for searching for a range.  I
have a TREE_SEARCH_V3 lying around somewhere to address this ridiculous
behavior and should probably finish it up at some point.

This hasn't mattered for __qgroup_search until now since it hasn't used
anything other than -1 for the offset and objectid so I'll just add a
filter there.

-Jeff

-- 
Jeff Mahoney
SUSE Labs


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