RFC: Split do_rootfs into image specific tasks?

RFC: Split do_rootfs into image specific tasks?

[Richard Purdie]

I've heard complaints from people trying to create more interesting
image types about how hard it is to understand the rootfs/image
generation code and that its a pain to develop/test/debug.

Having looked at it myself, the internal construction of shell
functions which then gets passed into a multiprocessing pool is rather
convoluted and it places rather odd constraints on when variables are
expanded. Its therefore no wonder people find it confusing/complex.

I've sent three patches to the OE-Core list which illustrate how we
could split the code in lib/oe/image into separate tasks, one per image
type. This removes the need for the simple task graph code and defers
to the bitbake task management code to handle this instead.

This is a good step forward in splitting up the monolithic code and
starting to make it more accessible to people. It should also make it
easier for people to hook in other tasks and processes into the rootfs
code.

Incidentally, the reason this code was all combined originally was due
to limitations of fakeroot where if you exited the session, you lost
permissions data. With pseudo this constraint was removed.

We did start to rework the rootfs/image code previously and got so far
with untangling it however we did prioritise some performance tweaks
over splitting into separate tasks and in hindsight, this was a mistake
and should have been done the other way around. That work was suspended
due to changes in the people working on the project but this split has
always been intended, now is the time to finish it IMO.

The code I've sent does work but is not finished yet and has only had
basic testing. Things that remain to be done:

* Handling of IMAGE_GEN_DEBUGFS needs reimplementing on the new code   
  structure
* Some variable dependencies on do_rootfs need moving to do_image
* References to do_rootfs need auditing to see if they should now be 
  do_image_complete
* Changes need documenting with migration notes.
* The layout of image.bbclass is pretty horrid and needs cleanup, along
  with moving functions to the right places
* We should only calculate the rootfs size once, not once per image 
  type.

This work could also be a stepping stone towards further changes to the
rootfs backends. Right now its pretty backwards compatible but we could
change the API a bit and create the image types as specific tasks right
from the start rather than hand crafting shell functions as we do now
for example. I believe there would be further benefits in debugging and
ease of understanding of the codebase if we did that.

I guess the real question is whether people like this direction and how
much should we change the API in order to improve things? Could we
afford a pretty heavy rewrite in this area to give a good end result?

Cheers,

Richard

Re: [Openembedded-architecture] RFC: Split do_rootfs into image specific tasks?

[Richard Purdie]

To put this in a different why which people might be able to relate to,
this changes:

do_rootfs

into:

do_rootfs
do_image
do_image_ext4
do_image_tar
do_image_complete

(for IMAGE_FSTYPES = "tar.bz2 ext4")

Cheers,

Richard

Re: RFC: Split do_rootfs into image specific tasks?

[Patrick Ohly]

On Mon, 2015-12-28 at 13:00 +0000, Richard Purdie wrote:
> I've heard complaints from people trying to create more interesting
> image types about how hard it is to understand the rootfs/image
> generation code and that its a pain to develop/test/debug.
[...]
> I guess the real question is whether people like this direction and how
> much should we change the API in order to improve things? Could we
> afford a pretty heavy rewrite in this area to give a good end result?

I think it is indeed a step in the right direction. Having to re-create
the rootfs directory each time one makes a change to an image creation
class or image configuration made experimenting with such changes
awkward and slow.

However, the question remains how far this rewrite should go. The
current semantic model of an "image" is essentially "rootfs + boot
loader + kernel", and the current "image" recipes define both the
content of the rootfs and additional parameter for turning that one
rootfs into an image.

That's arguably the most common scenario, but it is only a special case.
Other image layouts may have multiple partitions, each with certain
files. For example, there might be a read-only rootfs, an overlay
partition for the rootfs and a data partition for user files (for the
sake of the argument, assume that it needs to be pre-populated with
certain files).

In my opinion, the proposed changes allow implementing a more general
image class. Such a class would have to create additional tasks (like
do_overlayfs and do_userfs where it installs files based on some
additional recipe variables), then in do_image_foobar combine these
additional directories into the full disk image.

This can be done without using any existing code, but probably there are
overlaps in functionality and opportunities for code reuse. For example,
converting a directory into a ext2/ext3/ext4/btrfs file system is a
common problem. Having that in a Python utility method would be useful
(but not needed immediately).

The "populate rootfs" code may also be a candidate for moving into an
utility method, although it may be harder to determine exactly which
variables it depends on and might not even be needed.

So, bottom line, I think it makes sense to move ahead as proposed and
refine later in cooperation with distros trying to implement their own
special disk layout.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: RFC: Split do_rootfs into image specific tasks?

[Paul Eggleton]

Hi Richard,

On Mon, 28 Dec 2015 13:00:40 Richard Purdie wrote:
> I've heard complaints from people trying to create more interesting
> image types about how hard it is to understand the rootfs/image
> generation code and that its a pain to develop/test/debug.
> 
> Having looked at it myself, the internal construction of shell
> functions which then gets passed into a multiprocessing pool is rather
> convoluted and it places rather odd constraints on when variables are
> expanded. Its therefore no wonder people find it confusing/complex.
> 
> I've sent three patches to the OE-Core list which illustrate how we
> could split the code in lib/oe/image into separate tasks, one per image
> type. This removes the need for the simple task graph code and defers
> to the bitbake task management code to handle this instead.
> 
> This is a good step forward in splitting up the monolithic code and
> starting to make it more accessible to people. It should also make it
> easier for people to hook in other tasks and processes into the rootfs
> code.
> 
> Incidentally, the reason this code was all combined originally was due
> to limitations of fakeroot where if you exited the session, you lost
> permissions data. With pseudo this constraint was removed.
> 
> We did start to rework the rootfs/image code previously and got so far
> with untangling it however we did prioritise some performance tweaks
> over splitting into separate tasks and in hindsight, this was a mistake
> and should have been done the other way around. That work was suspended
> due to changes in the people working on the project but this split has
> always been intended, now is the time to finish it IMO.
> 
> The code I've sent does work but is not finished yet and has only had
> basic testing. Things that remain to be done:
> 
> * Handling of IMAGE_GEN_DEBUGFS needs reimplementing on the new code
>   structure
> * Some variable dependencies on do_rootfs need moving to do_image
> * References to do_rootfs need auditing to see if they should now be
>   do_image_complete
> * Changes need documenting with migration notes.
> * The layout of image.bbclass is pretty horrid and needs cleanup, along
>   with moving functions to the right places
> * We should only calculate the rootfs size once, not once per image
>   type.
> 
> This work could also be a stepping stone towards further changes to the
> rootfs backends. Right now its pretty backwards compatible but we could
> change the API a bit and create the image types as specific tasks right
> from the start rather than hand crafting shell functions as we do now
> for example. I believe there would be further benefits in debugging and
> ease of understanding of the codebase if we did that.
> 
> I guess the real question is whether people like this direction and how
> much should we change the API in order to improve things? Could we
> afford a pretty heavy rewrite in this area to give a good end result?

I like this a lot. I'd idly considered this model before (as I'm sure others 
had) as it simplifies a lot of the multi-format image construction handling, 
improves incremental performance, and the added granularity makes it slightly 
easier to see what the build system is doing. Obviously there is some minor 
cleanup work to be done on the patchset as you've noted.

The only "issue" I can think of is that bitbake -c rootfs will no longer build 
the image files as it did previously; however I'm not sure I'd structure things 
any differently to avoid that though. I suspect people will fairly easily adapt 
to this change as long as we publicise it effectively.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: RFC: Split do_rootfs into image specific tasks?

[Richard Purdie]

On Wed, 2016-01-06 at 10:30 +1300, Paul Eggleton wrote:
> I like this a lot. I'd idly considered this model before (as I'm sure
> others 
> had) as it simplifies a lot of the multi-format image construction
> handling, 
> improves incremental performance, and the added granularity makes it
> slightly 
> easier to see what the build system is doing. Obviously there is some
> minor 
> cleanup work to be done on the patchset as you've noted.
> 
> The only "issue" I can think of is that bitbake -c rootfs will no
> longer build 
> the image files as it did previously; however I'm not sure I'd
> structure things 
> any differently to avoid that though. I suspect people will fairly
> easily adapt 
> to this change as long as we publicise it effectively.

Thanks for the feedback. Just as a heads up for people, I posted an
updated patch set to the OE-Core list. This one has various issues
fixed (including the todos) and is ready for wider testing.

Cheers,

Richard

Re: RFC: Split do_rootfs into image specific tasks?

[Richard Purdie]

On Mon, 2016-01-04 at 11:55 +0100, Patrick Ohly wrote:
> On Mon, 2015-12-28 at 13:00 +0000, Richard Purdie wrote:
> I think it is indeed a step in the right direction. Having to re
> -create the rootfs directory each time one makes a change to an image
> creation class or image configuration made experimenting with such 
> changes awkward and slow.

Agreed, that alone probably makes this change worthwhile in some ways.

> However, the question remains how far this rewrite should go. The
> current semantic model of an "image" is essentially "rootfs + boot
> loader + kernel", and the current "image" recipes define both the
> content of the rootfs and additional parameter for turning that one
> rootfs into an image.
> 
> That's arguably the most common scenario, but it is only a special
> case.
> Other image layouts may have multiple partitions, each with certain
> files. For example, there might be a read-only rootfs, an overlay
> partition for the rootfs and a data partition for user files (for the
> sake of the argument, assume that it needs to be pre-populated with
> certain files).
> 
> In my opinion, the proposed changes allow implementing a more general
> image class. Such a class would have to create additional tasks (like
> do_overlayfs and do_userfs where it installs files based on some
> additional recipe variables), then in do_image_foobar combine these
> additional directories into the full disk image.

That is the intent, someone should be able to much more easily tap into
and do other things with the generated data. It works better with
rm_work for example too, since things don't get cleaned up until after
do_image_complete.

> This can be done without using any existing code, but probably there
> are
> overlaps in functionality and opportunities for code reuse. For
> example,
> converting a directory into a ext2/ext3/ext4/btrfs file system is a
> common problem. Having that in a Python utility method would be 
> useful (but not needed immediately).

I'd be more than happy to see a function library of these kinds of
useful things. This change sets the scene for more changes to the
image_types.bbclass, we could for example change the IMAGE_CMD
variables into tasks and drop IMAGE_CMD entirely (at least in
principle, I'm not saying we should/shouldn't).

> The "populate rootfs" code may also be a candidate for moving into an
> utility method, although it may be harder to determine exactly which
> variables it depends on and might not even be needed.

If you look at the code after the recent changes, the do_rootfs code is
already well abstracted into the lib/oe/rootfs.py and associated files
as a module.

> So, bottom line, I think it makes sense to move ahead as proposed and
> refine later in cooperation with distros trying to implement their 
> own special disk layout.

Thanks for the feedback. I think this is a good first step and then we
can consider if any subsequent ones make sense.

Cheers,

Richard