mbox series

[v5,0/5] Introduce bmap-tools as target + host package

Message ID 20210607204221.124908-1-thomas.petazzoni@bootlin.com
Headers show
Series Introduce bmap-tools as target + host package | expand

Message

Thomas Petazzoni June 7, 2021, 8:42 p.m. UTC 
Hello,

Back in 2019, Nicolas Carrier started a patch series on bmap-tools,
which was kind of abandonned after 4 iterations. I'm now trying to
pick it up and revive it.

Compared to the v4 from Nicolas:

 - Checked the list of optional runtime dependencies that bmap-tools
   can use, as suggested by Romain Naour, and added some explanations
   in the Config.in help text about them. There is not much more we
   can do about them in Buildroot: they are not optional build time
   dependencies, but only runtime dependencies, and only needed when
   certain features are needed (so there is no reason to make them
   mandatory dependencies).

 - Added the missing pyexpat runtime dependency.

 - Drop incorrect build-time dependencies, which are only runtime ones
   (target python-setuptools)

 - Update to the latest version of bmap-tools

 - Misc improvements to the test cases, migration to a newer toolchain
   that doesn't exhibit an fallocate() bug. Moving to a newer
   toolchain requires updating the pre-built kernel image used for the
   runtime test infrastructure.

 - Added a host variant for the bmap-tools package, as this tool can
   also be very useful on the host to optimize the time needed to
   write an image. This host bmap-tools package comes with a test case
   as well.

Thanks!

Thomas Petazzoni

Nicolas Carrier (2):
  package/bmap-tools: new package
  support/testing/tests/package/test_bmap_tools: new test

Thomas Petazzoni (3):
  package/bmap-tools: enable host package
  support/testing/infra/emulator.py: update pre-built kernels
  support/testing/tests/package/test_bmap_tools: add test for host
    bmap-tools

 DEVELOPERS                                    |  6 ++
 package/Config.in                             |  1 +
 package/Config.in.host                        |  1 +
 package/bmap-tools/Config.in                  | 44 ++++++++++
 package/bmap-tools/Config.in.host             | 12 +++
 package/bmap-tools/bmap-tools.hash            |  3 +
 package/bmap-tools/bmap-tools.mk              | 14 +++
 support/testing/infra/emulator.py             |  8 +-
 .../tests/package/sample_bmap_tools.sh        | 15 ++++
 .../testing/tests/package/test_bmap_tools.py  | 88 +++++++++++++++++++
 10 files changed, 188 insertions(+), 4 deletions(-)
 create mode 100644 package/bmap-tools/Config.in
 create mode 100644 package/bmap-tools/Config.in.host
 create mode 100644 package/bmap-tools/bmap-tools.hash
 create mode 100644 package/bmap-tools/bmap-tools.mk
 create mode 100755 support/testing/tests/package/sample_bmap_tools.sh
 create mode 100644 support/testing/tests/package/test_bmap_tools.py

Comments

Nicolas Carrier June 9, 2021, 7:35 a.m. UTC | #1 
Hello,
I've tested the patch series in both the current master and by applying
it to our fork, which is based of 2020.02.11 and, after resolving the
conflicts with our branch (mainly in the DEVELOPERS file), I could test
it in both.

* the target package works fine
* so does the host package
* the tests TestPy3BmapTools and TestPy2BmapTools are failing at run
time.

The first error in the run log is:

qemu-system-arm: Invalid SD card size: 60 MiB
SD card size has to be a power of 2, e.g. 64 MiB.

So I set a fixed image size of 64M.
With this modification, the python 2 test passes, but with that, the
python 3 one, fails with a timeout issue:

...
  File "/usr/lib/python3/dist-packages/pexpect/expect.py", line 181, in
expect_loop
    return self.timeout(e)
  File "/usr/lib/python3/dist-packages/pexpect/expect.py", line 144, in
timeout
    raise exc
pexpect.exceptions.TIMEOUT: Timeout exceeded.
...

By increasing the timeout to 60 seconds, I could have all the tests
passing.

I attached my diff for reference.


And I have a question, is keeping python 2 compatibility still
required? Shouldn't we get rid of this test? It'd save quite an amount
of CPU time :)


On Mon, 2021-06-07 at 22:42 +0200, Thomas Petazzoni wrote:
> CAUTION: This email originated from outside of the organization.
> Do not click links or open attachments unless you recognize the
> sender and know the content is safe.
> 
> Hello,
> 
> Back in 2019, Nicolas Carrier started a patch series on bmap-tools,
> which was kind of abandonned after 4 iterations. I'm now trying to
> pick it up and revive it.
> 
> Compared to the v4 from Nicolas:
> 
>  - Checked the list of optional runtime dependencies that bmap-tools
>    can use, as suggested by Romain Naour, and added some explanations
>    in the Config.in help text about them. There is not much more we
>    can do about them in Buildroot: they are not optional build time
>    dependencies, but only runtime dependencies, and only needed when
>    certain features are needed (so there is no reason to make them
>    mandatory dependencies).
> 
>  - Added the missing pyexpat runtime dependency.
> 
>  - Drop incorrect build-time dependencies, which are only runtime
> ones
>    (target python-setuptools)
> 
>  - Update to the latest version of bmap-tools
> 
>  - Misc improvements to the test cases, migration to a newer
> toolchain
>    that doesn't exhibit an fallocate() bug. Moving to a newer
>    toolchain requires updating the pre-built kernel image used for
> the
>    runtime test infrastructure.
> 
>  - Added a host variant for the bmap-tools package, as this tool can
>    also be very useful on the host to optimize the time needed to
>    write an image. This host bmap-tools package comes with a test
> case
>    as well.
> 
> Thanks!
> 
> Thomas Petazzoni
> 
> Nicolas Carrier (2):
>   package/bmap-tools: new package
>   support/testing/tests/package/test_bmap_tools: new test
> 
> Thomas Petazzoni (3):
>   package/bmap-tools: enable host package
>   support/testing/infra/emulator.py: update pre-built kernels
>   support/testing/tests/package/test_bmap_tools: add test for host
>     bmap-tools
> 
>  DEVELOPERS                                    |  6 ++
>  package/Config.in                             |  1 +
>  package/Config.in.host                        |  1 +
>  package/bmap-tools/Config.in                  | 44 ++++++++++
>  package/bmap-tools/Config.in.host             | 12 +++
>  package/bmap-tools/bmap-tools.hash            |  3 +
>  package/bmap-tools/bmap-tools.mk              | 14 +++
>  support/testing/infra/emulator.py             |  8 +-
>  .../tests/package/sample_bmap_tools.sh        | 15 ++++
>  .../testing/tests/package/test_bmap_tools.py  | 88
> +++++++++++++++++++
>  10 files changed, 188 insertions(+), 4 deletions(-)
>  create mode 100644 package/bmap-tools/Config.in
>  create mode 100644 package/bmap-tools/Config.in.host
>  create mode 100644 package/bmap-tools/bmap-tools.hash
>  create mode 100644 package/bmap-tools/bmap-tools.mk
>  create mode 100755
> support/testing/tests/package/sample_bmap_tools.sh
>  create mode 100644 support/testing/tests/package/test_bmap_tools.py
> 
> --
> 2.31.1
>
Thomas Petazzoni June 9, 2021, 7:50 a.m. UTC | #2 
Hello Nicolas,

On Wed, 09 Jun 2021 09:35:38 +0200
Nicolas Carrier <nicolas.carrier@orolia.com> wrote:

> * the target package works fine
> * so does the host package
> * the tests TestPy3BmapTools and TestPy2BmapTools are failing at run
> time.
> 
> The first error in the run log is:
> 
> qemu-system-arm: Invalid SD card size: 60 MiB
> SD card size has to be a power of 2, e.g. 64 MiB.
> 
> So I set a fixed image size of 64M.
> With this modification, the python 2 test passes, but with that, the
> python 3 one, fails with a timeout issue:
> 
> ...
>   File "/usr/lib/python3/dist-packages/pexpect/expect.py", line 181, in
> expect_loop
>     return self.timeout(e)
>   File "/usr/lib/python3/dist-packages/pexpect/expect.py", line 144, in
> timeout
>     raise exc
> pexpect.exceptions.TIMEOUT: Timeout exceeded.
> ...
> 
> By increasing the timeout to 60 seconds, I could have all the tests
> passing.
> 
> I attached my diff for reference.

Thanks a lot for the testing! I'll double check, but I believe the
tests were working here. But of course, I've done quite a bit of back
and forth, so let's double check :)

> And I have a question, is keeping python 2 compatibility still
> required? Shouldn't we get rid of this test? It'd save quite an amount
> of CPU time :)

python2 is already marked deprecated in BR, and we're more and more
thinking of getting rid of it. I think this may happen in the near
future.

Best regards,

Thomas
Nicolas Carrier June 9, 2021, 8:31 a.m. UTC | #3 
> Thanks a lot for the testing! I'll double check, but I believe the
> tests were working here. But of course, I've done quite a bit of back
> and forth, so let's double check :)
> 

For the 64M thing, maybe a difference in qemu versions?
I'm using debian bullseye (11) and my qemu version is:
$ qemu-system-arm  --version
QEMU emulator version 5.2.0 (Debian 1:5.2+dfsg-10)
Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers

About the timeout, I bet it was due to the work load, because I can't
reproduce it today, when the activity on my machine is low.
That said, the execution times range between 13s to 16s which makes me
say that the orinigal 20s limit was maybe too optimistic.
Maybe 60 seconds is not strictly required, but at least something like
40s would be safer.

> > And I have a question, is keeping python 2 compatibility still
> > required? Shouldn't we get rid of this test? It'd save quite an
> > amount
> > of CPU time :)
> 
> python2 is already marked deprecated in BR, and we're more and more
> thinking of getting rid of it. I think this may happen in the near
> future.
> 
> Best regards,
> 
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Thomas Petazzoni June 9, 2021, 6:52 p.m. UTC | #4 
Hello Nicolas,

On Wed, 09 Jun 2021 09:35:38 +0200
Nicolas Carrier <nicolas.carrier@orolia.com> wrote:

> The first error in the run log is:
> 
> qemu-system-arm: Invalid SD card size: 60 MiB
> SD card size has to be a power of 2, e.g. 64 MiB.
> 
> So I set a fixed image size of 64M.

This is weird, because all the ext2/3/4 test cases in our test
infrastructure also do not specify a size, and so they get the default
of 60 MiB.

Could you try:

./support/testing/run-tests tests.fs.test_ext.TestExt4

and see if it works for you ?

Which version of Qemu are you using? I'm using 4.2.1.

Best regards,

Thomas
Romain Naour June 9, 2021, 7:16 p.m. UTC | #5 
Hi Thomas, Nicolas,

Le 09/06/2021 à 20:52, Thomas Petazzoni a écrit :
> Hello Nicolas,
> 
> On Wed, 09 Jun 2021 09:35:38 +0200
> Nicolas Carrier <nicolas.carrier@orolia.com> wrote:
> 
>> The first error in the run log is:
>>
>> qemu-system-arm: Invalid SD card size: 60 MiB
>> SD card size has to be a power of 2, e.g. 64 MiB.
>>
>> So I set a fixed image size of 64M.
> 
> This is weird, because all the ext2/3/4 test cases in our test
> infrastructure also do not specify a size, and so they get the default
> of 60 MiB.
> 
> Could you try:
> 
> ./support/testing/run-tests tests.fs.test_ext.TestExt4
> 
> and see if it works for you ?
> 
> Which version of Qemu are you using? I'm using 4.2.1.

We had the same problem after a qemu version bump to 5.1:

https://git.buildroot.net/buildroot/commit/?id=4bd2a317a7d6c3b3b19396a9459816da76b2c796

Best regards,
Romain


> 
> Best regards,
> 
> Thomas
>
Nicolas Carrier June 10, 2021, 7:39 a.m. UTC | #6 
Sure,

My qemu version is:
$ qemu-system-arm  --version
QEMU emulator version 5.2.0 (Debian 1:5.2+dfsg-10)

The Ext4 test passes:
$ ./support/testing/run-tests tests.fs.test_ext.TestExt4 -o tests -d dl
09:03:31 TestExt4                                 Starting
09:03:34 TestExt4                                 Building
ah09:08:12 TestExt4                                 Building done
Downloading to /home/ncarrier/workspace/buildroot/dl/tmp_43sbo0r
Renaming from /home/ncarrier/workspace/buildroot/dl/tmp_43sbo0r to
/home/ncarrier/workspace/buildroot/dl/kernel-vexpress-5.10.7
Downloading to /home/ncarrier/workspace/buildroot/dl/tmpnj5e2uiw
Renaming from /home/ncarrier/workspace/buildroot/dl/tmpnj5e2uiw to
/home/ncarrier/workspace/buildroot/dl/vexpress-v2p-ca9-5.10.7.dtb
09:08:20 TestExt4                                 Cleaning up
.
----------------------------------------------------------------------
Ran 1 test in 289.374s

OK

But its file system image has a size of 16M which _is_ a power of two:

BR2_TARGET_ROOTFS_EXT2_SIZE="16384"

On Wed, 2021-06-09 at 21:16 +0200, Romain Naour wrote:
> CAUTION: This email originated from outside of the organization.
> Do not click links or open attachments unless you recognize the
> sender and know the content is safe.
> 
> Hi Thomas, Nicolas,
> 
> Le 09/06/2021 à 20:52, Thomas Petazzoni a écrit :
> > Hello Nicolas,
> > 
> > On Wed, 09 Jun 2021 09:35:38 +0200
> > Nicolas Carrier <nicolas.carrier@orolia.com> wrote:
> > 
> > > The first error in the run log is:
> > > 
> > > qemu-system-arm: Invalid SD card size: 60 MiB
> > > SD card size has to be a power of 2, e.g. 64 MiB.
> > > 
> > > So I set a fixed image size of 64M.
> > 
> > This is weird, because all the ext2/3/4 test cases in our test
> > infrastructure also do not specify a size, and so they get the
> > default
> > of 60 MiB.
> > 
> > Could you try:
> > 
> > ./support/testing/run-tests tests.fs.test_ext.TestExt4
> > 
> > and see if it works for you ?
> > 
> > Which version of Qemu are you using? I'm using 4.2.1.
> 
> We had the same problem after a qemu version bump to 5.1:
> 
> https://git.buildroot.net/buildroot/commit/?id=4bd2a317a7d6c3b3b19396a9459816da76b2c796
> 
> Best regards,
> Romain
> 
> 
> > 
> > Best regards,
> > 
> > Thomas
> > 
>
Thomas Petazzoni June 10, 2021, 12:31 p.m. UTC | #7 
Hello Nicolas,

On Thu, 10 Jun 2021 09:39:43 +0200
Nicolas Carrier <nicolas.carrier@orolia.com> wrote:

> Sure,
> 
> My qemu version is:
> $ qemu-system-arm  --version
> QEMU emulator version 5.2.0 (Debian 1:5.2+dfsg-10)
> 
> The Ext4 test passes:
> $ ./support/testing/run-tests tests.fs.test_ext.TestExt4 -o tests -d dl
> 09:03:31 TestExt4                                 Starting
> 09:03:34 TestExt4                                 Building
> ah09:08:12 TestExt4                                 Building done
> Downloading to /home/ncarrier/workspace/buildroot/dl/tmp_43sbo0r
> Renaming from /home/ncarrier/workspace/buildroot/dl/tmp_43sbo0r to
> /home/ncarrier/workspace/buildroot/dl/kernel-vexpress-5.10.7
> Downloading to /home/ncarrier/workspace/buildroot/dl/tmpnj5e2uiw
> Renaming from /home/ncarrier/workspace/buildroot/dl/tmpnj5e2uiw to
> /home/ncarrier/workspace/buildroot/dl/vexpress-v2p-ca9-5.10.7.dtb
> 09:08:20 TestExt4                                 Cleaning up
> .
> ----------------------------------------------------------------------
> Ran 1 test in 289.374s
> 
> OK
> 
> But its file system image has a size of 16M which _is_ a power of two:

Sorry, I picked the wrong test. Could you try the Ext3 tests, i.e
tests.fs.test_ext.TestExt3 ? This one doesn't specify a size, so it
should fallback to the default, which is 60MB.

Best regards,

Thomas
Nicolas Carrier June 10, 2021, 1:15 p.m. UTC | #8 
Ok,
So I ran the test and this time, it failed as expected :)

buildroot$ tail -f tests/TestExt3-run.log
> starting qemu with 'qemu-system-arm -serial stdio -display none -m
256 -drive
file=/home/ncarrier/workspace/buildroot/tests/TestExt3/images/rootfs.ex
t3,if=sd -dtb /home/ncarrier/workspace/buildroot/dl/vexpress-v2p-ca9-
5.10.7.dtb -M vexpress-a9 -kernel
/home/ncarrier/workspace/buildroot/dl/kernel-vexpress-5.10.7 -append
root=/dev/mmcblk0 rootfstype=ext3 console=ttyAMA0'
WARNING: Image format was not specified for
'/home/ncarrier/workspace/buildroot/tests/TestExt3/images/rootfs.ext3'
and probing guessed raw.
         Automatically detecting the format is dangerous for raw
images, write operations on block 0 will be restricted.
         Specify the 'raw' format explicitly to remove the
restrictions.
qemu-system-arm: Invalid SD card size: 60 MiB
SD card size has to be a power of 2, e.g. 64 MiB.
You can resize disk images with 'qemu-img resize <imagefile> <new-
size>'
(note that this will lose data if you make the image smaller than it
currently is).


On Thu, 2021-06-10 at 14:31 +0200, Thomas Petazzoni wrote:
> CAUTION: This email originated from outside of the organization.
> Do not click links or open attachments unless you recognize the
> sender and know the content is safe.
> 
> Hello Nicolas,
> 
> On Thu, 10 Jun 2021 09:39:43 +0200
> Nicolas Carrier <nicolas.carrier@orolia.com> wrote:
> 
> > Sure,
> > 
> > My qemu version is:
> > $ qemu-system-arm  --version
> > QEMU emulator version 5.2.0 (Debian 1:5.2+dfsg-10)
> > 
> > The Ext4 test passes:
> > $ ./support/testing/run-tests tests.fs.test_ext.TestExt4 -o tests -
> > d dl
> > 09:03:31 TestExt4                                 Starting
> > 09:03:34 TestExt4                                 Building
> > ah09:08:12 TestExt4                                 Building done
> > Downloading to /home/ncarrier/workspace/buildroot/dl/tmp_43sbo0r
> > Renaming from /home/ncarrier/workspace/buildroot/dl/tmp_43sbo0r to
> > /home/ncarrier/workspace/buildroot/dl/kernel-vexpress-5.10.7
> > Downloading to /home/ncarrier/workspace/buildroot/dl/tmpnj5e2uiw
> > Renaming from /home/ncarrier/workspace/buildroot/dl/tmpnj5e2uiw to
> > /home/ncarrier/workspace/buildroot/dl/vexpress-v2p-ca9-5.10.7.dtb
> > 09:08:20 TestExt4                                 Cleaning up
> > .
> > -------------------------------------------------------------------
> > ---
> > Ran 1 test in 289.374s
> > 
> > OK
> > 
> > But its file system image has a size of 16M which _is_ a power of
> > two:
> 
> Sorry, I picked the wrong test. Could you try the Ext3 tests, i.e
> tests.fs.test_ext.TestExt3 ? This one doesn't specify a size, so it
> should fallback to the default, which is 60MB.
> 
> Best regards,
> 
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com