All of lore.kernel.org
 help / color / mirror / Atom feed
* About the code style requirement
@ 2021-09-29  5:41 Qu Wenruo
  2021-09-29  6:10 ` Daniel Axtens
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Qu Wenruo @ 2021-09-29  5:41 UTC (permalink / raw)
  To: Grub-devel, linux-btrfs

Hi,

I'm recently considering to cross-port btrfs-progs/U-boot btrfs code to 
GRUB, so that we can have more unified code base, with more features 
(and of-course bug fixes)

But the first blockage I'm hitting is the code style.

It looks like GRUB is using its own code style which is not found in 
kernel/btrfs-progs/U-boot.

But I didn't find where the code style doc is, so is it a hard 
requirement to follow the existing style even we're cross-porting most 
code unmodified from other projects?

Thanks,
Qu


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: About the code style requirement
  2021-09-29  5:41 About the code style requirement Qu Wenruo
@ 2021-09-29  6:10 ` Daniel Axtens
  2021-09-29  6:34 ` Vladimir 'phcoder' Serbinenko
  2021-09-30  5:33 ` Christoph Hellwig
  2 siblings, 0 replies; 12+ messages in thread
From: Daniel Axtens @ 2021-09-29  6:10 UTC (permalink / raw)
  To: The development of GNU GRUB, Grub-devel, linux-btrfs; +Cc: Qu Wenruo

Qu Wenruo via Grub-devel <grub-devel@gnu.org> writes:

> Hi,
>
> I'm recently considering to cross-port btrfs-progs/U-boot btrfs code to 
> GRUB, so that we can have more unified code base, with more features 
> (and of-course bug fixes)
>
> But the first blockage I'm hitting is the code style.
>
> It looks like GRUB is using its own code style which is not found in 
> kernel/btrfs-progs/U-boot.
>
> But I didn't find where the code style doc is, so is it a hard 
> requirement to follow the existing style even we're cross-porting most 
> code unmodified from other projects?

It's largely based on the GNU coding style. The `indent` program can do
most of the work for you.

Certain parts of grub that are imported more-or-less unmodified do
follow their own style, e.g. grub-core/lib/json/jsmn.h

I'm not a maintainer so I can't tell you how hard the style requirement
is.

Kind regards,
Daniel

> Thanks,
> Qu
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: About the code style requirement
  2021-09-29  5:41 About the code style requirement Qu Wenruo
  2021-09-29  6:10 ` Daniel Axtens
@ 2021-09-29  6:34 ` Vladimir 'phcoder' Serbinenko
  2021-09-29  7:04   ` Qu Wenruo
  2021-09-30  5:33 ` Christoph Hellwig
  2 siblings, 1 reply; 12+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2021-09-29  6:34 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 977 bytes --]

Le mer. 29 sept. 2021, 07:42, Qu Wenruo via Grub-devel <grub-devel@gnu.org>
a écrit :

> Hi,
>
> I'm recently considering to cross-port btrfs-progs/U-boot btrfs code to
> GRUB, so that we can have more unified code base, with more features
> (and of-course bug fixes)
>
Did you check the license compatibility first? GRUB is GPLv3+ project and
we can't accept code that is GPLv2-only. What license is the code that you
want to use?

>
> But the first blockage I'm hitting is the code style.
>
> It looks like GRUB is using its own code style which is not found in
> kernel/btrfs-progs/U-boot.
>
> But I didn't find where the code style doc is, so is it a hard
> requirement to follow the existing style even we're cross-porting most
> code unmodified from other projects?
>
> Thanks,
> Qu
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 1689 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: About the code style requirement
  2021-09-29  6:34 ` Vladimir 'phcoder' Serbinenko
@ 2021-09-29  7:04   ` Qu Wenruo
  2021-09-29 15:38     ` Lennart Sorensen
  0 siblings, 1 reply; 12+ messages in thread
From: Qu Wenruo @ 2021-09-29  7:04 UTC (permalink / raw)
  To: The development of GNU GRUB, Vladimir 'phcoder' Serbinenko



On 2021/9/29 14:34, Vladimir 'phcoder' Serbinenko wrote:
> 
> 
> Le mer. 29 sept. 2021, 07:42, Qu Wenruo via Grub-devel 
> <grub-devel@gnu.org <mailto:grub-devel@gnu.org>> a écrit :
> 
>     Hi,
> 
>     I'm recently considering to cross-port btrfs-progs/U-boot btrfs code to
>     GRUB, so that we can have more unified code base, with more features
>     (and of-course bug fixes)
> 
> Did you check the license compatibility first? GRUB is GPLv3+ project 
> and we can't accept code that is GPLv2-only. What license is the code 
> that you want to use?

Oh, didn't know there would be license problem.

Then it would be complex.

Quite some code is directly copied from kernel without modification, 
those should still be GPL-2.0 only.

For code already cross-ported to U-boot, it's GPL-2.0+, I guess it would 
be OK there.

But from what I see in btrfs-progs, most of them is still GPL 2.0.

I guess during the cross-port to U-boot, I have already broken the GPL 
2.0 requirement...

Any advice on this problem?

Thanks,
Qu

> 
> 
>     But the first blockage I'm hitting is the code style.
> 
>     It looks like GRUB is using its own code style which is not found in
>     kernel/btrfs-progs/U-boot.
> 
>     But I didn't find where the code style doc is, so is it a hard
>     requirement to follow the existing style even we're cross-porting most
>     code unmodified from other projects?
> 
>     Thanks,
>     Qu
> 
> 
>     _______________________________________________
>     Grub-devel mailing list
>     Grub-devel@gnu.org <mailto:Grub-devel@gnu.org>
>     https://lists.gnu.org/mailman/listinfo/grub-devel
>     <https://lists.gnu.org/mailman/listinfo/grub-devel>
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: About the code style requirement
  2021-09-29  7:04   ` Qu Wenruo
@ 2021-09-29 15:38     ` Lennart Sorensen
  2021-09-29 22:24       ` Qu Wenruo
  0 siblings, 1 reply; 12+ messages in thread
From: Lennart Sorensen @ 2021-09-29 15:38 UTC (permalink / raw)
  To: The development of GNU GRUB
  Cc: Vladimir 'phcoder' Serbinenko, Qu Wenruo

On Wed, Sep 29, 2021 at 03:04:59PM +0800, Qu Wenruo via Grub-devel wrote:
> Oh, didn't know there would be license problem.

There often is. :)

> Then it would be complex.
> 
> Quite some code is directly copied from kernel without modification, those
> should still be GPL-2.0 only.
> 
> For code already cross-ported to U-boot, it's GPL-2.0+, I guess it would be
> OK there.
> 
> But from what I see in btrfs-progs, most of them is still GPL 2.0.
> 
> I guess during the cross-port to U-boot, I have already broken the GPL 2.0
> requirement...
> 
> Any advice on this problem?

Does libbtrfsutils provide what you need?  It appears to be LGPLv2.1+.

-- 
Len Sorensen


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: About the code style requirement
  2021-09-29 15:38     ` Lennart Sorensen
@ 2021-09-29 22:24       ` Qu Wenruo
       [not found]         ` <CAEaD8JNeNV-8frJeA5qCig=SMsT3WOtBySZ1aBL0Vnm+Qa2YYw@mail.gmail.com>
  0 siblings, 1 reply; 12+ messages in thread
From: Qu Wenruo @ 2021-09-29 22:24 UTC (permalink / raw)
  To: Lennart Sorensen, The development of GNU GRUB
  Cc: Vladimir 'phcoder' Serbinenko



On 2021/9/29 23:38, Lennart Sorensen wrote:
> On Wed, Sep 29, 2021 at 03:04:59PM +0800, Qu Wenruo via Grub-devel wrote:
>> Oh, didn't know there would be license problem.
> 
> There often is. :)
> 
>> Then it would be complex.
>>
>> Quite some code is directly copied from kernel without modification, those
>> should still be GPL-2.0 only.
>>
>> For code already cross-ported to U-boot, it's GPL-2.0+, I guess it would be
>> OK there.
>>
>> But from what I see in btrfs-progs, most of them is still GPL 2.0.
>>
>> I guess during the cross-port to U-boot, I have already broken the GPL 2.0
>> requirement...
>>
>> Any advice on this problem?
> 
> Does libbtrfsutils provide what you need?  It appears to be LGPLv2.1+.
> 

Unfortunately no.

BTW, U-boot is GPL v2.0+, would it be possible to cross port from U-boot?

For the worst case I can craft a new interface from scratch, as GRUB 
doesn't seem to need any write operation for the fs, thus quite some 
code can be truncated and all transaction related functionality can be 
removed.

Thanks,
Qu



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: About the code style requirement
  2021-09-29  5:41 About the code style requirement Qu Wenruo
  2021-09-29  6:10 ` Daniel Axtens
  2021-09-29  6:34 ` Vladimir 'phcoder' Serbinenko
@ 2021-09-30  5:33 ` Christoph Hellwig
  2 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2021-09-30  5:33 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: Grub-devel, linux-btrfs

On Wed, Sep 29, 2021 at 01:41:09PM +0800, Qu Wenruo wrote:
> Hi,
> 
> I'm recently considering to cross-port btrfs-progs/U-boot btrfs code to
> GRUB, so that we can have more unified code base, with more features (and
> of-course bug fixes)

The GPLv2 only kernel code is not going be much use for the GPLv3+ grub
code.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: About the code style requirement
       [not found]             ` <ef50b770-442c-3f96-73b0-730c6ac5bd04@suse.com>
@ 2021-10-01  6:08               ` Vladimir 'phcoder' Serbinenko
  2021-10-01  7:14                 ` Qu Wenruo
  0 siblings, 1 reply; 12+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2021-10-01  6:08 UTC (permalink / raw)
  To: Qu Wenruo, The development of GRUB 2

On Fri, Oct 1, 2021 at 2:13 AM Qu Wenruo <wqu@suse.com> wrote:
>
>
>
> On 2021/9/30 18:51, Qu Wenruo wrote:
> >
> >
> > On 2021/9/30 18:20, Vladimir 'phcoder' Serbinenko wrote:
> >>
> >>
> >> Le jeu. 30 sept. 2021, 00:24, Qu Wenruo <wqu@suse.com
> >> <mailto:wqu@suse.com>> a écrit :
> >>
> >>
> >>
> >>     On 2021/9/29 23:38, Lennart Sorensen wrote:
> >>      > On Wed, Sep 29, 2021 at 03:04:59PM +0800, Qu Wenruo via
> >>     Grub-devel wrote:
> >>      >> Oh, didn't know there would be license problem.
> >>      >
> >>      > There often is. :)
> >>      >
> >>      >> Then it would be complex.
> >>      >>
> >>      >> Quite some code is directly copied from kernel without
> >>     modification, those
> >>      >> should still be GPL-2.0 only.
> >>      >>
> >>      >> For code already cross-ported to U-boot, it's GPL-2.0+, I guess
> >>     it would be
> >>      >> OK there.
> >>      >>
> >>      >> But from what I see in btrfs-progs, most of them is still GPL
> >> 2.0.
> >>      >>
> >>      >> I guess during the cross-port to U-boot, I have already broken
> >>     the GPL 2.0
> >>      >> requirement...
> >>      >>
> >>      >> Any advice on this problem?
> >>      >
> >>      > Does libbtrfsutils provide what you need?  It appears to be
> >>     LGPLv2.1+.
> >>      >
> >>
> >>     Unfortunately no.
> >>
> >>     BTW, U-boot is GPL v2.0+, would it be possible to cross port from
> >>     U-boot?
> >>
> >>     For the worst case I can craft a new interface from scratch, as GRUB
> >>     doesn't seem to need any write operation for the fs, thus quite some
> >>     code can be truncated and all transaction related functionality
> >> can be
> >>     removed.
> >>
> >> Sorry but this still doesn't cut it.
> >
> > Isn't GPLv2.0+ of U-boot compatible with the GPLv3.0+ of GRUB?
> >
> >> If you use the code, even if you adapt it, you need permission from
> >> original authors, either as a license or as a relicensing permission.
> >> I sometimes get asked for relicensing of the code I wrote and I often
> >> grant this request. Asking original authors may be worth it.
> >
> > In this U-boot case, the code is cross-ported from btrfs-progs by
> > myself, I can think of 3 or 4 other authors involved doing small bug
> > fixes in U-boot realm, thus asking for permission of U-boot code should
> > be pretty simple.
> >
> > But there are still code copied from kernel into U-boot, without
> > modification.
> >
> > Does it still prevent those kernel code from being used in GRUB?
> >
> >> Otherwise you're allowed to study the documentation and write code
> >> based from it. What you're allowed more than this depends on
> >> jurisdiction.
> >
> > Sure, in fact since GRUB doesn't require write support for the fs, I can
> > craft a different interface and start from scratch.
> >
> > But I still hope to get the license thing more clear, other than
> > starting a new (and less functional) btrfs code.
>
> After some chats with guys in SUSE, it's definitely a no-no to share any
> code from those projects.
>
> So I'll just start from scratch to craft a read-only btrfs
> implementation inside GRUB.

GRUB already has a btrfs implementation. Writing new one from scratch
instead of existing one is unwelcome. From scratch means new bugs.
Do you have problems with existing implementation? If so I prefer you
to fix the exact bugs rather than attempt to rewrite the entire thing
from scratch


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: About the code style requirement
  2021-10-01  6:08               ` Vladimir 'phcoder' Serbinenko
@ 2021-10-01  7:14                 ` Qu Wenruo
  2021-10-07 13:51                   ` Daniel Kiper
  0 siblings, 1 reply; 12+ messages in thread
From: Qu Wenruo @ 2021-10-01  7:14 UTC (permalink / raw)
  To: Vladimir 'phcoder' Serbinenko, The development of GRUB 2



On 2021/10/1 14:08, Vladimir 'phcoder' Serbinenko wrote:
> On Fri, Oct 1, 2021 at 2:13 AM Qu Wenruo <wqu@suse.com> wrote:
>>
>>
>>
>> On 2021/9/30 18:51, Qu Wenruo wrote:
>>>
>>>
>>> On 2021/9/30 18:20, Vladimir 'phcoder' Serbinenko wrote:
>>>>
>>>>
>>>> Le jeu. 30 sept. 2021, 00:24, Qu Wenruo <wqu@suse.com
>>>> <mailto:wqu@suse.com>> a écrit :
>>>>
>>>>
>>>>
>>>>      On 2021/9/29 23:38, Lennart Sorensen wrote:
>>>>       > On Wed, Sep 29, 2021 at 03:04:59PM +0800, Qu Wenruo via
>>>>      Grub-devel wrote:
>>>>       >> Oh, didn't know there would be license problem.
>>>>       >
>>>>       > There often is. :)
>>>>       >
>>>>       >> Then it would be complex.
>>>>       >>
>>>>       >> Quite some code is directly copied from kernel without
>>>>      modification, those
>>>>       >> should still be GPL-2.0 only.
>>>>       >>
>>>>       >> For code already cross-ported to U-boot, it's GPL-2.0+, I guess
>>>>      it would be
>>>>       >> OK there.
>>>>       >>
>>>>       >> But from what I see in btrfs-progs, most of them is still GPL
>>>> 2.0.
>>>>       >>
>>>>       >> I guess during the cross-port to U-boot, I have already broken
>>>>      the GPL 2.0
>>>>       >> requirement...
>>>>       >>
>>>>       >> Any advice on this problem?
>>>>       >
>>>>       > Does libbtrfsutils provide what you need?  It appears to be
>>>>      LGPLv2.1+.
>>>>       >
>>>>
>>>>      Unfortunately no.
>>>>
>>>>      BTW, U-boot is GPL v2.0+, would it be possible to cross port from
>>>>      U-boot?
>>>>
>>>>      For the worst case I can craft a new interface from scratch, as GRUB
>>>>      doesn't seem to need any write operation for the fs, thus quite some
>>>>      code can be truncated and all transaction related functionality
>>>> can be
>>>>      removed.
>>>>
>>>> Sorry but this still doesn't cut it.
>>>
>>> Isn't GPLv2.0+ of U-boot compatible with the GPLv3.0+ of GRUB?
>>>
>>>> If you use the code, even if you adapt it, you need permission from
>>>> original authors, either as a license or as a relicensing permission.
>>>> I sometimes get asked for relicensing of the code I wrote and I often
>>>> grant this request. Asking original authors may be worth it.
>>>
>>> In this U-boot case, the code is cross-ported from btrfs-progs by
>>> myself, I can think of 3 or 4 other authors involved doing small bug
>>> fixes in U-boot realm, thus asking for permission of U-boot code should
>>> be pretty simple.
>>>
>>> But there are still code copied from kernel into U-boot, without
>>> modification.
>>>
>>> Does it still prevent those kernel code from being used in GRUB?
>>>
>>>> Otherwise you're allowed to study the documentation and write code
>>>> based from it. What you're allowed more than this depends on
>>>> jurisdiction.
>>>
>>> Sure, in fact since GRUB doesn't require write support for the fs, I can
>>> craft a different interface and start from scratch.
>>>
>>> But I still hope to get the license thing more clear, other than
>>> starting a new (and less functional) btrfs code.
>>
>> After some chats with guys in SUSE, it's definitely a no-no to share any
>> code from those projects.
>>
>> So I'll just start from scratch to craft a read-only btrfs
>> implementation inside GRUB.
> 
> GRUB already has a btrfs implementation. Writing new one from scratch
> instead of existing one is unwelcome. From scratch means new bugs.
> Do you have problems with existing implementation?

In fact, quite some:

- Can't read shared data extents of reflinked files
   This can be fixed without a big refactor.

- No csum verification
- No support for extra checksums (xxhash/sha256/blake2)
- No very basic tree block sanity check
   The only sanity check is bytenr and fsid, which is far from ideal.
   The most deadly case like parent-child tree block level mismatch is
   not handled at all.
   (which can easily cause a dead loop,
    e.g. tree_block1 -> tree_block2 -> tree_block 1)

> If so I prefer you
> to fix the exact bugs rather than attempt to rewrite the entire thing
> from scratch

If only for the read failure of the reflinked files, I'd be pretty happy 
just to fix that bug.

But for the long run, I really prefer to have a more-or-less common base 
which every btrfs developer can easily start their work easily.

Your work on the existing grub btrfs code is very appreciated, but I'm 
not sure if it is the best solution in the long run.

Thus please let us btrfs developers to contribute our experience to make 
a better and more unified implementation.
(Although still less unified due to license conflicts)

Thanks,
Qu



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: About the code style requirement
  2021-10-01  7:14                 ` Qu Wenruo
@ 2021-10-07 13:51                   ` Daniel Kiper
  2021-10-08  0:22                     ` Qu Wenruo
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Kiper @ 2021-10-07 13:51 UTC (permalink / raw)
  To: Qu Wenruo
  Cc: dja, lsorense, hch, Vladimir 'phcoder' Serbinenko, grub-devel

On Fri, Oct 01, 2021 at 03:14:02PM +0800, Qu Wenruo via Grub-devel wrote:
> On 2021/10/1 14:08, Vladimir 'phcoder' Serbinenko wrote:

[...]

> > GRUB already has a btrfs implementation. Writing new one from scratch
> > instead of existing one is unwelcome. From scratch means new bugs.
> > Do you have problems with existing implementation?
>
> In fact, quite some:
>
> - Can't read shared data extents of reflinked files
>   This can be fixed without a big refactor.
>
> - No csum verification
> - No support for extra checksums (xxhash/sha256/blake2)
> - No very basic tree block sanity check
>   The only sanity check is bytenr and fsid, which is far from ideal.
>   The most deadly case like parent-child tree block level mismatch is
>   not handled at all.
>   (which can easily cause a dead loop,
>    e.g. tree_block1 -> tree_block2 -> tree_block 1)
>
> > If so I prefer you
> > to fix the exact bugs rather than attempt to rewrite the entire thing
> > from scratch
>
> If only for the read failure of the reflinked files, I'd be pretty happy
> just to fix that bug.
>
> But for the long run, I really prefer to have a more-or-less common base
> which every btrfs developer can easily start their work easily.
>
> Your work on the existing grub btrfs code is very appreciated, but I'm not
> sure if it is the best solution in the long run.
>
> Thus please let us btrfs developers to contribute our experience to make a
> better and more unified implementation.
> (Although still less unified due to license conflicts)

I think it would be nice to have a kind of library which could be
imported into the GRUB source code in the similar way how it is done
e.g. for gnulib or JSON. Please take a look at grub-core/lib directory
and e.g. commit 528938d50 (json: Import upstream jsmn-1.1.0). Of course
it has to have compatible license with the GRUB. You can find more info
about licensing here [1].

WRT coding style, it is described here [2]. As Daniel A. said it is GNU
coding style with some (minor) exceptions. However, we do not enforce
this coding style for the code which is imported into the GRUB as libraries.

Anyway, we want to have support for latest Btrfs features in the GRUB.
So, I think your proposal is interesting. However, we have to get more
details how to solve various problems. So, if you could consider various
options and present their pros and cons then I think we can continue
discussion how to make Btrfs development for GRUB easier.

Daniel

[1] https://www.gnu.org/licenses/licenses.html.en
[2] https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: About the code style requirement
  2021-10-07 13:51                   ` Daniel Kiper
@ 2021-10-08  0:22                     ` Qu Wenruo
  2021-10-16  0:33                       ` Qu Wenruo
  0 siblings, 1 reply; 12+ messages in thread
From: Qu Wenruo @ 2021-10-08  0:22 UTC (permalink / raw)
  To: Daniel Kiper
  Cc: dja, lsorense, hch, Vladimir 'phcoder' Serbinenko, grub-devel



On 2021/10/7 21:51, Daniel Kiper wrote:
> On Fri, Oct 01, 2021 at 03:14:02PM +0800, Qu Wenruo via Grub-devel wrote:
>> On 2021/10/1 14:08, Vladimir 'phcoder' Serbinenko wrote:
> 
> [...]
> 
>>> GRUB already has a btrfs implementation. Writing new one from scratch
>>> instead of existing one is unwelcome. From scratch means new bugs.
>>> Do you have problems with existing implementation?
>>
>> In fact, quite some:
>>
>> - Can't read shared data extents of reflinked files
>>    This can be fixed without a big refactor.
>>
>> - No csum verification
>> - No support for extra checksums (xxhash/sha256/blake2)
>> - No very basic tree block sanity check
>>    The only sanity check is bytenr and fsid, which is far from ideal.
>>    The most deadly case like parent-child tree block level mismatch is
>>    not handled at all.
>>    (which can easily cause a dead loop,
>>     e.g. tree_block1 -> tree_block2 -> tree_block 1)
>>
>>> If so I prefer you
>>> to fix the exact bugs rather than attempt to rewrite the entire thing
>>> from scratch
>>
>> If only for the read failure of the reflinked files, I'd be pretty happy
>> just to fix that bug.
>>
>> But for the long run, I really prefer to have a more-or-less common base
>> which every btrfs developer can easily start their work easily.
>>
>> Your work on the existing grub btrfs code is very appreciated, but I'm not
>> sure if it is the best solution in the long run.
>>
>> Thus please let us btrfs developers to contribute our experience to make a
>> better and more unified implementation.
>> (Although still less unified due to license conflicts)
> 
> I think it would be nice to have a kind of library which could be
> imported into the GRUB source code in the similar way how it is done
> e.g. for gnulib or JSON. Please take a look at grub-core/lib directory
> and e.g. commit 528938d50 (json: Import upstream jsmn-1.1.0). Of course
> it has to have compatible license with the GRUB. You can find more info
> about licensing here [1].

Yep, it would be the best practice to import a good btrfs implementation 
directly to GRUB.

But as you know, currently we don't have such lib which is license 
compatible yet.

> 
> WRT coding style, it is described here [2]. As Daniel A. said it is GNU
> coding style with some (minor) exceptions. However, we do not enforce
> this coding style for the code which is imported into the GRUB as libraries. >
> Anyway, we want to have support for latest Btrfs features in the GRUB.
> So, I think your proposal is interesting. However, we have to get more
> details how to solve various problems. So, if you could consider various
> options and present their pros and cons then I think we can continue
> discussion how to make Btrfs development for GRUB easier.

Currently there seems to be 3 different plans:

1. Continue with current GRUB btrfs code base

    Pros: No need for a new project/code base

    Cons: Very few (if any) btrfs developers is interested in handling a
          completely different code base.
          Thus the code can easily (and is already) get out-of-sync with
          latest btrfs features.

2. Import code from U-boot

    Pros: The most unified code base (directly from btrfs-progs/kernel).
          More or less tested (only two bugs found after the cross-port
          to U-boot).
          Mostly feature rich (multi-device not implemented due to U-boot
          limit, lacking only blake2 hash support)

    Cons: License incompatible. The btrfs-progs code may be re-licensed,
          but there are still things like rbtree and list are copied from
          kernel, thus a full re-license seems unfeasible.

3. New btrfs FUSE based project <<< Trying to do in the long run

    Pros: More btrfs community members can get involved.
          Easier to test thanks to FUSE.
          License compatible (MIT planned).
          Extra educational purpose for new developers.

    Cons: A new project means new bugs.
          Still a very different code base, thus not that unified to
          btrfs-progs/kernel.
          Need extra infrastructure/interface to be cross-ported to GRUB.

> 
> Daniel
> 
> [1] https://www.gnu.org/licenses/licenses.html.en
> [2] https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html
> 
Thanks for all the info, it would definitely help when I start coding 
inside GRUB.

Thanks,
Qu



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: About the code style requirement
  2021-10-08  0:22                     ` Qu Wenruo
@ 2021-10-16  0:33                       ` Qu Wenruo
  0 siblings, 0 replies; 12+ messages in thread
From: Qu Wenruo @ 2021-10-16  0:33 UTC (permalink / raw)
  To: The development of GNU GRUB, Daniel Kiper
  Cc: dja, lsorense, hch, Vladimir 'phcoder' Serbinenko



On 2021/10/8 08:22, Qu Wenruo via Grub-devel wrote:
> 
> 
> On 2021/10/7 21:51, Daniel Kiper wrote:
>> On Fri, Oct 01, 2021 at 03:14:02PM +0800, Qu Wenruo via Grub-devel wrote:
>>> On 2021/10/1 14:08, Vladimir 'phcoder' Serbinenko wrote:
>>
>> [...]
>>
>>>> GRUB already has a btrfs implementation. Writing new one from scratch
>>>> instead of existing one is unwelcome. From scratch means new bugs.
>>>> Do you have problems with existing implementation?
>>>
>>> In fact, quite some:
>>>
>>> - Can't read shared data extents of reflinked files
>>>    This can be fixed without a big refactor.
>>>
>>> - No csum verification
>>> - No support for extra checksums (xxhash/sha256/blake2)
>>> - No very basic tree block sanity check
>>>    The only sanity check is bytenr and fsid, which is far from ideal.
>>>    The most deadly case like parent-child tree block level mismatch is
>>>    not handled at all.
>>>    (which can easily cause a dead loop,
>>>     e.g. tree_block1 -> tree_block2 -> tree_block 1)
>>>
>>>> If so I prefer you
>>>> to fix the exact bugs rather than attempt to rewrite the entire thing
>>>> from scratch
>>>
>>> If only for the read failure of the reflinked files, I'd be pretty happy
>>> just to fix that bug.
>>>
>>> But for the long run, I really prefer to have a more-or-less common base
>>> which every btrfs developer can easily start their work easily.
>>>
>>> Your work on the existing grub btrfs code is very appreciated, but 
>>> I'm not
>>> sure if it is the best solution in the long run.
>>>
>>> Thus please let us btrfs developers to contribute our experience to 
>>> make a
>>> better and more unified implementation.
>>> (Although still less unified due to license conflicts)
>>
>> I think it would be nice to have a kind of library which could be
>> imported into the GRUB source code in the similar way how it is done
>> e.g. for gnulib or JSON. Please take a look at grub-core/lib directory
>> and e.g. commit 528938d50 (json: Import upstream jsmn-1.1.0). Of course
>> it has to have compatible license with the GRUB. You can find more info
>> about licensing here [1].
> 
> Yep, it would be the best practice to import a good btrfs implementation 
> directly to GRUB.
> 
> But as you know, currently we don't have such lib which is license 
> compatible yet.
> 
>>
>> WRT coding style, it is described here [2]. As Daniel A. said it is GNU
>> coding style with some (minor) exceptions. However, we do not enforce
>> this coding style for the code which is imported into the GRUB as 
>> libraries. >
>> Anyway, we want to have support for latest Btrfs features in the GRUB.
>> So, I think your proposal is interesting. However, we have to get more
>> details how to solve various problems. So, if you could consider various
>> options and present their pros and cons then I think we can continue
>> discussion how to make Btrfs development for GRUB easier.
> 
> Currently there seems to be 3 different plans:
> 
> 1. Continue with current GRUB btrfs code base
> 
>     Pros: No need for a new project/code base
> 
>     Cons: Very few (if any) btrfs developers is interested in handling a
>           completely different code base.
>           Thus the code can easily (and is already) get out-of-sync with
>           latest btrfs features.

I pinned down the bug why GRUB fails to read some extents.

It turns out to be a much bigger problem, that GRUB can't even handle 
the soon-to-be-default features like NO_HOLES.
(And mixed inline with regular extents)

Sure, I will send out patches for that bug, but it really takes me too 
long time just to pin down the bug inside a completely different code base.

Thanks,
Qu

> 
> 2. Import code from U-boot
> 
>     Pros: The most unified code base (directly from btrfs-progs/kernel).
>           More or less tested (only two bugs found after the cross-port
>           to U-boot).
>           Mostly feature rich (multi-device not implemented due to U-boot
>           limit, lacking only blake2 hash support)
> 
>     Cons: License incompatible. The btrfs-progs code may be re-licensed,
>           but there are still things like rbtree and list are copied from
>           kernel, thus a full re-license seems unfeasible.
> 
> 3. New btrfs FUSE based project <<< Trying to do in the long run
> 
>     Pros: More btrfs community members can get involved.
>           Easier to test thanks to FUSE.
>           License compatible (MIT planned).
>           Extra educational purpose for new developers.
> 
>     Cons: A new project means new bugs.
>           Still a very different code base, thus not that unified to
>           btrfs-progs/kernel.
>           Need extra infrastructure/interface to be cross-ported to GRUB.
> 
>>
>> Daniel
>>
>> [1] https://www.gnu.org/licenses/licenses.html.en
>> [2] https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html
>>
> Thanks for all the info, it would definitely help when I start coding 
> inside GRUB.
> 
> Thanks,
> Qu
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-10-16  0:33 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29  5:41 About the code style requirement Qu Wenruo
2021-09-29  6:10 ` Daniel Axtens
2021-09-29  6:34 ` Vladimir 'phcoder' Serbinenko
2021-09-29  7:04   ` Qu Wenruo
2021-09-29 15:38     ` Lennart Sorensen
2021-09-29 22:24       ` Qu Wenruo
     [not found]         ` <CAEaD8JNeNV-8frJeA5qCig=SMsT3WOtBySZ1aBL0Vnm+Qa2YYw@mail.gmail.com>
     [not found]           ` <be1bdb41-6541-3810-b36b-26231317ddef@suse.com>
     [not found]             ` <ef50b770-442c-3f96-73b0-730c6ac5bd04@suse.com>
2021-10-01  6:08               ` Vladimir 'phcoder' Serbinenko
2021-10-01  7:14                 ` Qu Wenruo
2021-10-07 13:51                   ` Daniel Kiper
2021-10-08  0:22                     ` Qu Wenruo
2021-10-16  0:33                       ` Qu Wenruo
2021-09-30  5:33 ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.