Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

image-erofs: introduce basic support for erofs #247

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ee-smuxel
Copy link
Contributor

Erofs is a read-only file-system supported by the Linux Kernel since version 5.4.

This patchset adds basic support, a test & documentation for the filesystem.
I'm tempted to add some of the command-line options as dedicated config
nodes, but i'm unsure if it should be done.

  • block-size: Defaults to the system page size, will likely be specified most of the time in real images
  • compression: the latest release 1.7.1 supports lz4, lzma and lz4hc with various different options, specifying
    these in extra-args will likely be easier.

I'm also not 100% sure about the testcase so i'm happy for comments

Copy link
Member

@michaelolbrich michaelolbrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly good. Just a few minor things.

genimage.c Outdated Show resolved Hide resolved
@@ -0,0 +1,5 @@
image test.erofs {
erofs {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set the filesystem uuid here with extraargs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should i just hardcode the UUID to a specific value? I tried checking the other occurances of uuid's in tests and they mostly seem to revolve around the partition table uuid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's also the possibility of adding a dedicated uuid parameter as the commit introduces new public API anyway

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for a dedicated parameter. We don't have it anywhere else either. Just put a random uuid here to make the output of dump.erofs more predictable.

test_expect_success mkfs_erofs,fsck_erofs "erofs" "
run_genimage_root erofs.config test.erofs &&
fsck.erofs -p images/test.erofs | tee erofs.log &&
test_must_fail grep -q 'Filesystem was changed' erofs.log
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like something to see that the content actually reached the filesystem.

  1. use check_size
  2. dump.erofs -s -S --ls --path=/ test.erofs | grep -v 'Filesystem created' > dump
    test_cmp "${testdir}/erofs.dump" "dump"

That will not check everything, but that contains the file count and the dir listing of the toplevel dir.

Tip: I insert a "cp dump ${testdir}/erofs.dump" so save the expected output on the first try and remove the line again afterwards.
And don't forget to add the new file to EXTRA_DIST in Makefile.am

Signed-off-by: Sebastian Muxel <sebastian.muxel@entner-electronics.com>
Signed-off-by: Sebastian Muxel <sebastian.muxel@entner-electronics.com>
Signed-off-by: Sebastian Muxel <sebastian.muxel@entner-electronics.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants