On 5/29/20 6:23 PM, Goffredo Baroncelli wrote:
> On 5/28/20 11:03 PM, Hans van Kranenburg wrote:
>> Hi,
>>
>> On 5/26/20 10:19 PM, Goffredo Baroncelli wrote:
>>> On 5/25/20 7:14 PM, David Sterba wrote:
>>>> I'll start with the data structures
> [...]
>>>>
>>>> This looks like a copy of btrfs_chunk and stripe, only removing items
>>>> not needed for the chunk information. Rather than copying the
>>>> unnecessary fileds like dev_uuid in stripe, this should be designed for
>>>> data exchange with the usecase in mind.
>>>
>>> There are two clients for this api:
>>> - btrfs fi us
>>> - btrfs dev us
>>>
>>> We can get rid of:
>>> - "offset" fields (2x)
>>> - "uuid" fields
>>>
>>> However the "offset" fields can be used to understand where a logical map
>>> is on the physical disks. I am thinking about a graphical tool to show this
>>> mapping, which doesn't exits yet -).
>>> The offset field may be used as key to get further information (like the chunk
>>> usage, see below)
>>>
>>> Regarding the UUID field, I agree it can be removed because it is redundant (there
>>> is already the devid)
>>>
>>>
>>>>
>>>> The format does not need follow the exact layout that kernel uses, ie.
>>>> chunk info with one embedded stripe and then followed by variable length
>>>> array of further stripes. This is convenient in one way but not in
>>>> another one. Alternatively each chunk can be emitted as a single entry,
>>>> duplicating part of the common fields and adding the stripe-specific
>>>> ones. This is for consideration.
>>>>
>>>> I've looked at my old code doing the chunk dump based on the search
>>>> ioctl and found that it also allows to read the chunk usage, with one
>>>> extra search to the block group item where the usage is stored. As this
>>>> is can be slow, it should be optional. Ie. the main ioctl structure
>>>> needs flags where this can be requested.
>>>
>>> This info could be very useful. I think to something like a balance of
>>> chunks which are near filled (or near empty). The question is if we
>>> should have a different ioctl.
>>
>> Do you mean that you want to allow to a non root user to run btrfs balance?
> No at all. The balance is an expensive operation that IMHO need root
> privileges to be started.
>
>>
>> Otherwise, no. IMO convenience functions for quickly retrieving a
>> specific subset of data should be created as reusable library functions
>> in the calling code, not as a redundant extra IOCTL that has to be
>> maintained.
>
> I think that there is a misunderstood. There is no intention to take the
> place of the current balance ioctl.
Ok, I'll rephrase. Using it to improve balance is not an argument for
adding a new IOCTL, since it has to run as root anyway, and then you can
use the SEARCH IOCTL. And as long as there's a few helper functions
which do the plumbing, SEARCH isn't that bad at just getting some chunk
and block group info.
-# python3
>>> import btrfs
>>> with btrfs.FileSystem('/') as fs:
... for chunk in fs.chunks():
... print(fs.block_group(chunk.vaddr))
...
block group vaddr 13631488 length 8388608 flags DATA used 8388608
used_pct 100
block group vaddr 22020096 length 8388608 flags SYSTEM|DUP used 114688
used_pct 1
block group vaddr 30408704 length 1073741824 flags METADATA|DUP used
889061376 used_pct 83
block group vaddr 1104150528 length 1073741824 flags DATA used
1073741824 used_pct 100
block group vaddr 2177892352 length 1073741824 flags DATA used
1073741824 used_pct 100
[...]
> The aim of this ioctl is only to get information about the chunk distribution.
> Getting the chunk information could help to perform a better balance.
> I.e. a balance which start from the chunk more empty the go forward
> processing the chunk more filled.
An example of this is the existing btrfs-balance-least-used program.
> Another case use is to visulize how
> the chunk are filled, or how the disks are used..
An example of that is btrfs-heatmap.
Hfgl,
Hans
