[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-btrfs
Subject:    Re: [PATCH v4 1/2] btrfs: Move on-disk structure definition to btrfs_tree.h
From:       Qu Wenruo <wqu () suse ! com>
Date:       2020-04-30 7:29:51
Message-ID: 13573de2-acea-33db-202a-c06221bd2cfd () suse ! com
[Download RAW message or body]



On 2020/4/25 下午3:25, Qu Wenruo wrote:
> 
> 
> On 2020/4/25 下午3:14, Christoph Hellwig wrote:
>> On Fri, Apr 24, 2020 at 10:24:45PM +0200, David Sterba wrote:
>>> On Tue, Apr 21, 2020 at 07:41:40PM +0800, Qu Wenruo wrote:
>>>>
>>>>
>>>> On 2020/4/21 下午7:31, Christoph Hellwig wrote:
>>>>> On Wed, Apr 15, 2020 at 04:41:12PM +0800, Qu Wenruo wrote:
>>>>>> These structures all are on-disk format. Move them to btrfs_tree.h
>>>>>>
>>>>>> This allows us to sync the header to different projects, who need to
>>>>>> read btrfs filesystem, like U-boot, grub.
>>>>>
>>>>> Please use a separate header for that.  btrfs_tree.h is a UAPI header
>>>>> with strict ABI guarantees.  Just add a fs/btrfs/btrfs_format.h that
>>>>> can be copied into the projects where is needed.
>>>>>
>>>> Doesn't on-disk format itself need strict ABI guarantees?
>>>>
>>>> Thus it looks like the perfect location for on-disk format definitions.
>>>
>>> Right now I'm not sure if it's a good idea to put the on-disk format to
>>> the UAPI path or not. The exported code is to support the ioctls,
>>> there's an overlap with the on-disk format but providing all the on-disk
>>> structures for general might become an unnecessry burden.
>>>
>>> We know that there's a small number of projects that want to sync the
>>> on-disk format so I don't think it's going to be a problem for them to
>>> use a private header.
>>
>> And the usual way is to just ensure the format header is self-contained
>> and suitably licensed that they can easily copy it and rely on the
>> version they checked in.  That avoids the problems of
>>
>>  a) the tools relying on installed kernel headers new enough for the
>>     feature they want to support
>>  b) ifdef magic for newer features in the tools
>>  c) the need to keep changes to the kernel format header to be backwards
>>     compatible at the compiler level (as there can be disk format
>>     compatible changes that still break users, e.g. introducing a named
>>     union, or splitting / merging struct definitions)
>>
> One last question.
> 
> Btrfs has one ioctl, which allow users to search btrfs on-disk data.
> 
> And the ioctl returns the on-disk data directly to user space, which
> means the on-disk format is also used by ioctl.
> 
> In that case, do we still need to put the on-disk format internal other
> than as a uapi?

After some tries, there are still problems especially for some flags
shared by ioctl and on-disk data.

E.g. We have a qgroup related ioctl, which uses some flag like
BTRFS_QGROUP_LIMIT_MAX_RFER.

But we also use that flag on-disk (obviously, we don't want spend try
time/code to do mapping of these flags).

Putting such flags to ioctl header, then we need to sync two headers to
other projects, while we care nothing about the ioctl part.

Putting such flags to on-disk format header, and put that header inside
fs/btrfs other than uapi, then ioctl doesn't have the definition of such
flags, and break user space programms.

So it looks like we'd better keep a on-disk format uapi header, and put
such flags to on-disk format uapi header, and ioctl header just includes
on-disk format header.

For user space, all needed flags are still kept as is.
For bootloader projects, they only need the on-disk format header.

Any extra ideas/feedback on this?

Thanks,
Qu
[prev in list] [next in list] [prev in thread] [next in thread]