| Message ID | 95f6965c48dbf2f754818e2bec1f2d3891015475.1500398733.git.yann.morin.1998@free.fr |
|---|---|
| State | Changes Requested |
| Headers | show |
I think this patch should go in right away, even before the skeleton/systemd fixes. Yes, that may make some tests fail, but that's fine. On 18-07-17 19:25, Yann E. MORIN wrote: > The "builtin" kernel does not boot a systemd-based system, so > we resort to building the same one as currently used by our > qemu_arm_vexpress_defconfig. > > We test the 11 following combinations: > > - busybox, read-only, without network > - busybox, read-only, with network > - busybox, read-write, without network > - busybox, read-write, with network > > - basic systemd, read-only, network w/ ifupdown > - basic systemd, read-only, network w/ networkd > - basic systemd, read-write, network w/ ifupdown > - basic systemd, read-write, network w/ networkd > > - full systemd, read-only, network w/ networkd > - full systemd, read-write, network w/ networkd > > - no init system, read-only, without network Given that we're testing init here, it would be nice to also add tests with the cpio filesystem, which runs a shell script before starting /sbin/init. It's probably sufficient to just do the maximal test with this scenario. Oh, and a test with sysvinit would also be nice... > The tests just verify what the /sbin/init binary is, and that we were > able to grab an IP address. More tests can be added later, for example > to check each systemd features (journal, tmpfiles...) > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> I have a bunch of comments, but nothing critical so Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> [snip] > + def startEmulator(self, fs_type, kernel=None, dtb=None, init=None): PEP8 says snake-case for functions. I'm not sure if the rest of the testing infra satisfies this, but let's try to not make things worse. > + img = os.path.join(self.builddir, "images", "rootfs.{}".format(fs_type)) > + subprocess.call(["truncate", "-s", "%1M", img]) It's a bit weird to do that here, but I see no better way either. > + > + options = ["-drive", > + "file={},if=sd,format=raw".format(img), > + "-M", "vexpress-a9"] > + > + if kernel is None: > + kernel = "builtin" > + else: > + kernel = os.path.join(self.builddir, "images", kernel) This condition should probably be refactored into emulator.boot itself - although admittedly that doesn't have a reference to builddir at the moment. > + options.extend(["-dtb", os.path.join(self.builddir, "images", > + "{}.dtb".format(dtb))]) This should only be done if dtb is not None. > + > + kernel_cmdline = ["root=/dev/mmcblk0", > + "rootfstype={}".format(fs_type), > + "rootwait", > + "ro", > + "console=ttyAMA0"] > + > + if not init is None: > + kernel_cmdline.extend(["init={}".format(init)]) Just a single element, so append() instead of extend(). > + > + self.emulator.boot(arch="armv7", > + kernel=kernel, > + kernel_cmdline=kernel_cmdline, > + options=options) > + > + if init is None: > + self.emulator.login() This looks weird so warrants a comment: # The init argument is passed when there is no /sbin/init and we instead # directly run an executable. So when init is None, we have to log in. > + > + def checkInit(self, path): > + cmd = "cmp /proc/1/exe {}".format(path) > + _, exit_code = self.emulator.run(cmd) > + self.assertEqual(exit_code, 0) > + > + def checkNetwork(self, interface, exitCode=0): > + cmd = "ip addr show {} |grep inet".format(interface) > + _, exit_code = self.emulator.run(cmd) > + self.assertEqual(exit_code, exitCode) These you could also run automatically, and request the arguments from the instance: def test_run(self): self.startEmulator(self.fs_type()) self.checkInit(self.init_path()) if self.have_network(): self.checkNetwork("eth0") else: self.checkNetwork("eth0", 1) Ideally you'd then make this an ABC class and declare those callbacks as @abstractmethod, but perhaps that's going too far for you :-) [snip] > +class TestInitSystemNone(InitSystemBase): > + config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \ > + """ > + BR2_INIT_NONE=y > + # BR2_TARGET_ROOTFS_TAR is not set > + BR2_TARGET_ROOTFS_SQUASHFS=y > + """ > + > + def test_run(self): > + self.startEmulator(fs_type="squashfs", init="/bin/sh") > + index = self.emulator.qemu.expect(["/bin/sh: can't access tty; job control turned off", pexpect.TIMEOUT], timeout=60) > + if index != 0: > + self.emulator.logfile.write("==> System does not boot") > + raise SystemError("System does not boot") Why do you need to raise an exception here? Why not if index != 0: self.fail(==> System does not boot") return Regards, Arnout [snip]
Arnout, All, On 2017-07-22 16:32 +0200, Arnout Vandecappelle spake thusly: > I think this patch should go in right away, even before the skeleton/systemd > fixes. Yes, that may make some tests fail, but that's fine. I am not so sure. We don;t know how long it will take this series to get in in its entirety. OTOH, ahving the tests in right now would be quite an incentive to get the rest in. ;-] > On 18-07-17 19:25, Yann E. MORIN wrote: > > The "builtin" kernel does not boot a systemd-based system, so > > we resort to building the same one as currently used by our > > qemu_arm_vexpress_defconfig. > > > > We test the 11 following combinations: > > > > - busybox, read-only, without network > > - busybox, read-only, with network > > - busybox, read-write, without network > > - busybox, read-write, with network > > > > - basic systemd, read-only, network w/ ifupdown > > - basic systemd, read-only, network w/ networkd > > - basic systemd, read-write, network w/ ifupdown > > - basic systemd, read-write, network w/ networkd > > > > - full systemd, read-only, network w/ networkd > > - full systemd, read-write, network w/ networkd > > > > - no init system, read-only, without network > > Given that we're testing init here, it would be nice to also add tests with the > cpio filesystem, which runs a shell script before starting /sbin/init. As I said, we can add more tests in the future. I already have a few ideas of what we could/should add, but I'll refrain to do so for now, to concentrate on doing the requested cleanups on this series. ;-) > It's probably sufficient to just do the maximal test with this scenario. > Oh, and a test with sysvinit would also be nice... > > > The tests just verify what the /sbin/init binary is, and that we were > > able to grab an IP address. More tests can be added later, for example > > to check each systemd features (journal, tmpfiles...) > > > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > > I have a bunch of comments, but nothing critical so > > Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> > > [snip] > > + def startEmulator(self, fs_type, kernel=None, dtb=None, init=None): > > PEP8 says snake-case for functions. I'm not sure if the rest of the testing > infra satisfies this, but let's try to not make things worse. We already have BRTest.setUp() and BRTest.tearDown(), so I mimicked that. And I also applied Thomas suggestion to go camel-case with functions (reviewed on IRC). > > + img = os.path.join(self.builddir, "images", "rootfs.{}".format(fs_type)) > > + subprocess.call(["truncate", "-s", "%1M", img]) > > It's a bit weird to do that here, but I see no better way either. > > > + > > + options = ["-drive", > > + "file={},if=sd,format=raw".format(img), > > + "-M", "vexpress-a9"] > > + > > + if kernel is None: > > + kernel = "builtin" > > + else: > > + kernel = os.path.join(self.builddir, "images", kernel) > > This condition should probably be refactored into emulator.boot itself - > although admittedly that doesn't have a reference to builddir at the moment. I was planning on reworking the test infra as you suggested, because I also found it did make sense, but again, that is out-of-scope for this series. > > + options.extend(["-dtb", os.path.join(self.builddir, "images", > > + "{}.dtb".format(dtb))]) > > This should only be done if dtb is not None. Right. > > + > > + kernel_cmdline = ["root=/dev/mmcblk0", > > + "rootfstype={}".format(fs_type), > > + "rootwait", > > + "ro", > > + "console=ttyAMA0"] > > + > > + if not init is None: > > + kernel_cmdline.extend(["init={}".format(init)]) > > Just a single element, so append() instead of extend(). Right. > > + > > + self.emulator.boot(arch="armv7", > > + kernel=kernel, > > + kernel_cmdline=kernel_cmdline, > > + options=options) > > + > > + if init is None: > > + self.emulator.login() > > This looks weird so warrants a comment: > > # The init argument is passed when there is no /sbin/init and we instead > # directly run an executable. So when init is None, we have to log in. Ack. > > + > > + def checkInit(self, path): > > + cmd = "cmp /proc/1/exe {}".format(path) > > + _, exit_code = self.emulator.run(cmd) > > + self.assertEqual(exit_code, 0) > > + > > + def checkNetwork(self, interface, exitCode=0): > > + cmd = "ip addr show {} |grep inet".format(interface) > > + _, exit_code = self.emulator.run(cmd) > > + self.assertEqual(exit_code, exitCode) > > These you could also run automatically, and request the arguments from the > instance: > > def test_run(self): > self.startEmulator(self.fs_type()) > self.checkInit(self.init_path()) > if self.have_network(): > self.checkNetwork("eth0") > else: > self.checkNetwork("eth0", 1) > > Ideally you'd then make this an ABC class and declare those callbacks as > @abstractmethod, but perhaps that's going too far for you :-) Is it really necessary that we go this route? It is much more complex, for a dubious gain, if at all. I'd prefer we keep with simple, plain code, so that everyone can contribute more easily... > [snip] > > +class TestInitSystemNone(InitSystemBase): > > + config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \ > > + """ > > + BR2_INIT_NONE=y > > + # BR2_TARGET_ROOTFS_TAR is not set > > + BR2_TARGET_ROOTFS_SQUASHFS=y > > + """ > > + > > + def test_run(self): > > + self.startEmulator(fs_type="squashfs", init="/bin/sh") > > + index = self.emulator.qemu.expect(["/bin/sh: can't access tty; job control turned off", pexpect.TIMEOUT], timeout=60) > > + if index != 0: > > + self.emulator.logfile.write("==> System does not boot") > > + raise SystemError("System does not boot") > > Why do you need to raise an exception here? Why not > > if index != 0: > self.fail(==> System does not boot") > return I did exactly as is currently done in Emulator.login(): index = self.qemu.expect(["buildroot login:", pexpect.TIMEOUT], timeout=60) if index != 0: self.logfile.write("==> System does not boot") raise SystemError("System does not boot") Regards, Yann E. MORIN. > > Regards, > Arnout > > [snip] > -- > Arnout Vandecappelle arnout at mind be > Senior Embedded Software Architect +32-16-286500 > Essensium/Mind http://www.mind.be > G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven > LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle > GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
Hello, On Sat, 22 Jul 2017 16:45:52 +0200, Yann E. MORIN wrote: > > PEP8 says snake-case for functions. I'm not sure if the rest of the testing > > infra satisfies this, but let's try to not make things worse. > > We already have BRTest.setUp() and BRTest.tearDown(), so I mimicked > that. And I also applied Thomas suggestion to go camel-case with > functions (reviewed on IRC). I don't think I said to use camel case for functions. What I said is to not use ThisIsAFunction for functions/methods, because that's normally used for classes. > > def test_run(self): > > self.startEmulator(self.fs_type()) > > self.checkInit(self.init_path()) > > if self.have_network(): > > self.checkNetwork("eth0") > > else: > > self.checkNetwork("eth0", 1) > > > > Ideally you'd then make this an ABC class and declare those callbacks as > > @abstractmethod, but perhaps that's going too far for you :-) > > Is it really necessary that we go this route? It is much more complex, > for a dubious gain, if at all. > > I'd prefer we keep with simple, plain code, so that everyone can > contribute more easily... Fully agreed with Yann. I know there are lots of funky but useful constructs / features in Python, but I'd like to keep the testing infrastructure to a usage of Python that can reasonably be understood by the average C programmer. So, a good indicator is: do Yann and Thomas understand what it does? If the answer is no, the construct shouldn't be used :-) Best regards, Thomas
On 22-07-17 16:45, Yann E. MORIN wrote: > Arnout, All, > > On 2017-07-22 16:32 +0200, Arnout Vandecappelle spake thusly: >> I think this patch should go in right away, even before the skeleton/systemd >> fixes. Yes, that may make some tests fail, but that's fine. > > I am not so sure. We don;t know how long it will take this series to get > in in its entirety. > > OTOH, ahving the tests in right now would be quite an incentive to get > the rest in. ;-] Well, we already have two tests failing and nobody seems to fix them... [snip] >>> + def startEmulator(self, fs_type, kernel=None, dtb=None, init=None): >> >> PEP8 says snake-case for functions. I'm not sure if the rest of the testing >> infra satisfies this, but let's try to not make things worse. > > We already have BRTest.setUp() and BRTest.tearDown(), so I mimicked > that. Well, that's because python unittest predates PEP8 and it already had these names, so they couldn't be changed. There are two tools, pep8 and flake8, that help checking PEP8 compliance. [snip] >> These you could also run automatically, and request the arguments from the >> instance: >> >> def test_run(self): >> self.startEmulator(self.fs_type()) >> self.checkInit(self.init_path()) >> if self.have_network(): >> self.checkNetwork("eth0") >> else: >> self.checkNetwork("eth0", 1) >> >> Ideally you'd then make this an ABC class and declare those callbacks as >> @abstractmethod, but perhaps that's going too far for you :-) > > Is it really necessary that we go this route? It is much more complex, > for a dubious gain, if at all. That's why I said "ideally". The callbacks (or they could be class members as well, like we have with config) are indeed useful I think. > I'd prefer we keep with simple, plain code, so that everyone can > contribute more easily... Fair enough. Regards, Arnout [snip]
Hello, On Sat, 22 Jul 2017 21:53:38 +0200, Arnout Vandecappelle wrote: > > OTOH, ahving the tests in right now would be quite an incentive to get > > the rest in. ;-] > > Well, we already have two tests failing and nobody seems to fix them... The patches fixing them are here: http://patchwork.ozlabs.org/patch/783135/ http://patchwork.ozlabs.org/patch/783137/ http://patchwork.ozlabs.org/patch/783136/ and nobody reviews them. (Actually only the first patch of the series is needed to fix the tests) I made those on July 1st, pretty much the day we started running the tests in Gitlab CI. So it's a bit hard to now read your complaint that nobody cares about the failing tests... Best regards, Thomas
Thomas, All, On 2017-07-22 22:10 +0200, Thomas Petazzoni spake thusly: > On Sat, 22 Jul 2017 21:53:38 +0200, Arnout Vandecappelle wrote: > > > > OTOH, ahving the tests in right now would be quite an incentive to get > > > the rest in. ;-] > > > > Well, we already have two tests failing and nobody seems to fix them... > > The patches fixing them are here: > > http://patchwork.ozlabs.org/patch/783135/ > http://patchwork.ozlabs.org/patch/783137/ > http://patchwork.ozlabs.org/patch/783136/ > > and nobody reviews them. I gave them a spin here, and they indeed fix the build, but the tests are still broken because qemu can't boot the resulting system... I'll investigate that tomorrow... Regards, Yann E. MORIN. > (Actually only the first patch of the series is needed to fix the > tests) > > I made those on July 1st, pretty much the day we started running the > tests in Gitlab CI. So it's a bit hard to now read your complaint that > nobody cares about the failing tests... > > Best regards, > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com
Hello, On Sun, 23 Jul 2017 00:50:16 +0200, Yann E. MORIN wrote: > > The patches fixing them are here: > > > > http://patchwork.ozlabs.org/patch/783135/ > > http://patchwork.ozlabs.org/patch/783137/ > > http://patchwork.ozlabs.org/patch/783136/ > > > > and nobody reviews them. > > I gave them a spin here, and they indeed fix the build, but the tests > are still broken because qemu can't boot the resulting system... > > I'll investigate that tomorrow... Hum, weird, because I'm pretty sure I tested the test cases with those patches. I'll test again, maybe I missed something. Best regards, Thomas
Thomas, All, On 2017-07-23 09:58 +0200, Thomas Petazzoni spake thusly: > On Sun, 23 Jul 2017 00:50:16 +0200, Yann E. MORIN wrote: > > > The patches fixing them are here: > > > > > > http://patchwork.ozlabs.org/patch/783135/ > > > http://patchwork.ozlabs.org/patch/783137/ > > > http://patchwork.ozlabs.org/patch/783136/ > > > > > > and nobody reviews them. > > > > I gave them a spin here, and they indeed fix the build, but the tests > > are still broken because qemu can't boot the resulting system... > > > > I'll investigate that tomorrow... > > Hum, weird, because I'm pretty sure I tested the test cases with those > patches. I'll test again, maybe I missed something. So, to rule out any sleep-deprivation issue while testign yesterday evening, I re-spawn the tests here this morning, and they still do not run. Maybe it is my qemu version? I'll try and bisect them between July the 1st and now. Regards, Yann E. MORIN.
Hello, I renamed the e-mail thread from: [PATCH 20/20] support/testing: add runtime testing for init systems On Sun, Jul 23, 2017 at 06:26 AM, Yann E. MORIN wrote: > On 2017-07-23 09:58 +0200, Thomas Petazzoni spake thusly: >> On Sun, 23 Jul 2017 00:50:16 +0200, Yann E. MORIN wrote: >> > > The patches fixing them are here: >> > > >> > > http://patchwork.ozlabs.org/patch/783135/ >> > > http://patchwork.ozlabs.org/patch/783137/ >> > > http://patchwork.ozlabs.org/patch/783136/ >> > > >> > > and nobody reviews them. >> > >> > I gave them a spin here, and they indeed fix the build, but the tests >> > are still broken because qemu can't boot the resulting system... >> > >> > I'll investigate that tomorrow... >> >> Hum, weird, because I'm pretty sure I tested the test cases with those >> patches. I'll test again, maybe I missed something. > > So, to rule out any sleep-deprivation issue while testign yesterday > evening, I re-spawn the tests here this morning, and they still do not > run. > > Maybe it is my qemu version? > > I'll try and bisect them between July the 1st and now. Since you are investigating this, let me provide some data I hope is useful. My local computer has: binutils 2.26.1 qemu 2.5.0 Using my local computer the tests TestIso9660GrubExternal and TestIso9660GrubInternal always pass, in current master and also with the 3 grub patches applied. Using the image buildroot/base I can reproduce the problems: https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/23825707 GRUB requires a working absolute objcopy; upgrade your binutils https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/23831270 ==> System does not boot If you need me to check versions for other tools in my local computer (Ubuntu 16.04.2, not plain vanilla), feel free to ask. Regards, Ricardo
Ricardo, All, On 2017-07-23 15:13 -0300, Ricardo Martincoski spake thusly: > On Sun, Jul 23, 2017 at 06:26 AM, Yann E. MORIN wrote: > > On 2017-07-23 09:58 +0200, Thomas Petazzoni spake thusly: > >> On Sun, 23 Jul 2017 00:50:16 +0200, Yann E. MORIN wrote: > >> > > The patches fixing them are here: > >> > > > >> > > http://patchwork.ozlabs.org/patch/783135/ > >> > > http://patchwork.ozlabs.org/patch/783137/ > >> > > http://patchwork.ozlabs.org/patch/783136/ > >> > > > >> > > and nobody reviews them. > >> > > >> > I gave them a spin here, and they indeed fix the build, but the tests > >> > are still broken because qemu can't boot the resulting system... > >> > > >> > I'll investigate that tomorrow... > >> > >> Hum, weird, because I'm pretty sure I tested the test cases with those > >> patches. I'll test again, maybe I missed something. > > > > So, to rule out any sleep-deprivation issue while testign yesterday > > evening, I re-spawn the tests here this morning, and they still do not > > run. > > > > Maybe it is my qemu version? > > > > I'll try and bisect them between July the 1st and now. > > Since you are investigating this, let me provide some data I hope is useful. Thanks for the feedback! :-) So I was not even capable of running it on a snashot of July (at the time the patches were made). It already failed to boot. > My local computer has: > binutils 2.26.1 > qemu 2.5.0 I have Ubuntu 17.04: GNU ar (GNU Binutils for Ubuntu) 2.28 QEMU emulator version 2.8.0(Debian 1:2.8+dfsg-3ubuntu2.3) > Using my local computer the tests TestIso9660GrubExternal and > TestIso9660GrubInternal always pass, in current master and also with the 3 grub > patches applied. It is ex[ected that the build suceeds without the patches on your machine, because it is binutils 2.28 that broke the build. However, that you were ablt to run is interesting. I'll spawn that on my server, which is a 16.04 as well. > Using the image buildroot/base I can reproduce the problems: > https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/23825707 > GRUB requires a working absolute objcopy; upgrade your binutils > https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/23831270 > ==> System does not boot So, it would look like it is either: - a qemu issue, - a binutils regression we have not identified (maybe another Debian patch is required?), - a grub regression. > If you need me to check versions for other tools in my local computer (Ubuntu > 16.04.2, not plain vanilla), feel free to ask. I plan on doign those tests; - build on 16.04, run on 16.04 - build on 16.04, run on 17.04 - build on 17.04, run on 16.04 This should identify if qemu is the culprit (the second would not work), or if binutils is the culprit (the third would not work). Regards, Yann E. MORIN.
Ricardo, All, On 2017-07-23 22:02 +0200, Yann E. MORIN spake thusly: > On 2017-07-23 15:13 -0300, Ricardo Martincoski spake thusly: > > On Sun, Jul 23, 2017 at 06:26 AM, Yann E. MORIN wrote: > > > On 2017-07-23 09:58 +0200, Thomas Petazzoni spake thusly: > > >> On Sun, 23 Jul 2017 00:50:16 +0200, Yann E. MORIN wrote: > > >> > > The patches fixing them are here: > > >> > > > > >> > > http://patchwork.ozlabs.org/patch/783135/ > > >> > > http://patchwork.ozlabs.org/patch/783137/ > > >> > > http://patchwork.ozlabs.org/patch/783136/ > > >> > > > > >> > > and nobody reviews them. > > >> > > > >> > I gave them a spin here, and they indeed fix the build, but the tests > > >> > are still broken because qemu can't boot the resulting system... > > >> > > > >> > I'll investigate that tomorrow... > > >> > > >> Hum, weird, because I'm pretty sure I tested the test cases with those > > >> patches. I'll test again, maybe I missed something. > > > > > > So, to rule out any sleep-deprivation issue while testign yesterday > > > evening, I re-spawn the tests here this morning, and they still do not > > > run. > > > > > > Maybe it is my qemu version? > > > > > > I'll try and bisect them between July the 1st and now. > > > > Since you are investigating this, let me provide some data I hope is useful. [--SNIP--] > I plan on doign those tests; > > - build on 16.04, run on 16.04 > - build on 16.04, run on 17.04 > - build on 17.04, run on 16.04 So, even before the tests are all done, I already noticed an issue here: on my machine, the iso images are about 1GiB large, while on my server, they are only a bit less than 5MiB. So, it looks like this is a binutils issue, with objdump being the culprit (it very much look like older similar issues). I'll continue investigating... Thanks! Regards, Yann E. MORIN.
Ricardo, All,
On 2017-07-23 22:02 +0200, Yann E. MORIN spake thusly:
[--SNIP--]
> I plan on doign those tests;
- build on 16.04, run on 16.04
==> builds and runs OK
- build on 16.04, run on 17.04
==> builds and runs OK
- build on 17.04, run on 16.04
==> builds OK, does not run.
So, I think this is realyl a regression with newer binutils:
$ ls -l fail/build/grub-0.97/stage1/stage1
-rwxr-xr-x 1 ymorin ymorin 129M juil. 23 22:10 stage1
$ ls -l works/build/grub-0.97/stage1/stage1
-rwxr-xr-x 1 ymorin ymorin 512 juil. 23 22:09 stage1
Similarly with stage2 and above, with file reaching over 256MiB...
So, definitely something fishy with binutils-2.28...
Regards,
Yann E. MORIN.
Hello, On Sun, 23 Jul 2017 15:13:41 -0300, Ricardo Martincoski wrote: > My local computer has: > binutils 2.26.1 > qemu 2.5.0 > > Using my local computer the tests TestIso9660GrubExternal and > TestIso9660GrubInternal always pass, in current master and also with the 3 grub > patches applied. > > Using the image buildroot/base I can reproduce the problems: > https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/23825707 > GRUB requires a working absolute objcopy; upgrade your binutils > https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/23831270 > ==> System does not boot And my patches did not fix this problem ? Best regards, Thomas Petazoni
Hello, ----- Original Message ----- > From: "Thomas Petazzoni" > Sent: Monday, July 24, 2017 4:13:21 AM > On Sun, 23 Jul 2017 15:13:41 -0300, Ricardo Martincoski wrote: > >> My local computer has: >> binutils 2.26.1 >> qemu 2.5.0 >> >> Using my local computer the tests TestIso9660GrubExternal and >> TestIso9660GrubInternal always pass, in current master and also with the 3 grub >> patches applied. >> >> Using the image buildroot/base I can reproduce the problems: >> https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/23825707 >> GRUB requires a working absolute objcopy; upgrade your binutils >> https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/23831270 >> ==> System does not boot > > And my patches did not fix this problem ? Your patches fix the build problem with newer binutils. But the runtime problem remains, according to Yann's analysis related to binutils-2.28 installed in the host machine. The second url above (ending in 270) are the tests running with your 3 patches applied. Regards, -- Ricardo Martincoski DATACOM Ethernet Switches Rua América, 1000 - Eldorado do Sul, RS - 92990-000 - Brasil +55 51 3933 3000 - Ramal 3307 ricardo.martincoski@datacom.ind.br www.datacom.ind.br
Thomas, All, On 2017-07-24 09:13 +0200, Thomas Petazzoni spake thusly: > On Sun, 23 Jul 2017 15:13:41 -0300, Ricardo Martincoski wrote: > > > My local computer has: > > binutils 2.26.1 > > qemu 2.5.0 > > > > Using my local computer the tests TestIso9660GrubExternal and > > TestIso9660GrubInternal always pass, in current master and also with the 3 grub > > patches applied. > > > > Using the image buildroot/base I can reproduce the problems: > > https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/23825707 > > GRUB requires a working absolute objcopy; upgrade your binutils > > https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/23831270 > > ==> System does not boot > > And my patches did not fix this problem ? Not really, no. Without your patches, the build breaks, because objdump fails, and we catch a build failure. With your patches, the build seems to succeed, because it does not fail. However, the generated iso images are about 1GiB in size (instead of less than 5MiB), and they do not boot. See the summary I've written in: http://lists.busybox.net/pipermail/buildroot/2017-July/198940.html So no, your patches are not sufficient to solve the run-time test. Regards, Yann E. MORIN.
On Tue, Jul 18, 2017 at 10:25 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > The "builtin" kernel does not boot a systemd-based system, so > we resort to building the same one as currently used by our > qemu_arm_vexpress_defconfig. > > We test the 11 following combinations: > > - busybox, read-only, without network > - busybox, read-only, with network > - busybox, read-write, without network > - busybox, read-write, with network > > - basic systemd, read-only, network w/ ifupdown > - basic systemd, read-only, network w/ networkd > - basic systemd, read-write, network w/ ifupdown > - basic systemd, read-write, network w/ networkd > > - full systemd, read-only, network w/ networkd > - full systemd, read-write, network w/ networkd > > - no init system, read-only, without network > > The tests just verify what the /sbin/init binary is, and that we were > able to grab an IP address. More tests can be added later, for example > to check each systemd features (journal, tmpfiles...) > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > support/testing/tests/init/__init__.py | 0 > support/testing/tests/init/base.py | 47 ++++++++++ > support/testing/tests/init/test_busybox.py | 67 +++++++++++++ > support/testing/tests/init/test_none.py | 32 +++++++ > support/testing/tests/init/test_systemd.py | 145 +++++++++++++++++++++++++++++ > 5 files changed, 291 insertions(+) > create mode 100644 support/testing/tests/init/__init__.py > create mode 100644 support/testing/tests/init/base.py > create mode 100644 support/testing/tests/init/test_busybox.py > create mode 100644 support/testing/tests/init/test_none.py > create mode 100644 support/testing/tests/init/test_systemd.py > > diff --git a/support/testing/tests/init/__init__.py b/support/testing/tests/init/__init__.py > new file mode 100644 > index 0000000000..e69de29bb2 > diff --git a/support/testing/tests/init/base.py b/support/testing/tests/init/base.py > new file mode 100644 > index 0000000000..a261d7dd46 > --- /dev/null > +++ b/support/testing/tests/init/base.py > @@ -0,0 +1,47 @@ > +import os > +import subprocess > +import infra.basetest > + > +class InitSystemBase(infra.basetest.BRTest): > + > + def startEmulator(self, fs_type, kernel=None, dtb=None, init=None): > + img = os.path.join(self.builddir, "images", "rootfs.{}".format(fs_type)) > + subprocess.call(["truncate", "-s", "%1M", img]) > + > + options = ["-drive", > + "file={},if=sd,format=raw".format(img), > + "-M", "vexpress-a9"] > + > + if kernel is None: > + kernel = "builtin" > + else: > + kernel = os.path.join(self.builddir, "images", kernel) > + options.extend(["-dtb", os.path.join(self.builddir, "images", > + "{}.dtb".format(dtb))]) > + > + kernel_cmdline = ["root=/dev/mmcblk0", > + "rootfstype={}".format(fs_type), > + "rootwait", > + "ro", > + "console=ttyAMA0"] > + > + if not init is None: > + kernel_cmdline.extend(["init={}".format(init)]) > + > + self.emulator.boot(arch="armv7", > + kernel=kernel, > + kernel_cmdline=kernel_cmdline, > + options=options) > + > + if init is None: > + self.emulator.login() > + > + def checkInit(self, path): > + cmd = "cmp /proc/1/exe {}".format(path) > + _, exit_code = self.emulator.run(cmd) > + self.assertEqual(exit_code, 0) > + > + def checkNetwork(self, interface, exitCode=0): > + cmd = "ip addr show {} |grep inet".format(interface) > + _, exit_code = self.emulator.run(cmd) > + self.assertEqual(exit_code, exitCode) > diff --git a/support/testing/tests/init/test_busybox.py b/support/testing/tests/init/test_busybox.py > new file mode 100644 > index 0000000000..c3e425bf5d > --- /dev/null > +++ b/support/testing/tests/init/test_busybox.py > @@ -0,0 +1,67 @@ > +import infra.basetest > +from tests.init.base import InitSystemBase as InitSystemBase > + > +class InitSystemBusyboxBase(InitSystemBase): > + config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \ > + """ > + # BR2_TARGET_ROOTFS_TAR is not set > + """ > + > + def checkInit(self): > + super(InitSystemBusyboxBase, self).checkInit("/bin/busybox") > + Just as a suggestion, you can probably get away without needing to override that function by defining a class variable "init_path" that SystemBusyboxBase and SystemSystemdBase will override to some appropriate value. See, for example, the code I wrote for Python and IPython related tests. https://git.buildroot.net/buildroot/tree/support/testing/tests/package/test_python.py#n11 https://git.buildroot.net/buildroot/tree/support/testing/tests/package/test_ipython.py#n18 Thanks, Andrey Smirnov
Andrey, All, On 2017-07-24 08:20 -0700, Andrey Smirnov spake thusly: > On Tue, Jul 18, 2017 at 10:25 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > The "builtin" kernel does not boot a systemd-based system, so > > we resort to building the same one as currently used by our > > qemu_arm_vexpress_defconfig. [--SNIP--] > > diff --git a/support/testing/tests/init/test_busybox.py b/support/testing/tests/init/test_busybox.py > > new file mode 100644 > > index 0000000000..c3e425bf5d > > --- /dev/null > > +++ b/support/testing/tests/init/test_busybox.py > > @@ -0,0 +1,67 @@ > > +import infra.basetest > > +from tests.init.base import InitSystemBase as InitSystemBase > > + > > +class InitSystemBusyboxBase(InitSystemBase): > > + config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \ > > + """ > > + # BR2_TARGET_ROOTFS_TAR is not set > > + """ > > + > > + def checkInit(self): > > + super(InitSystemBusyboxBase, self).checkInit("/bin/busybox") > > + > > Just as a suggestion, you can probably get away without needing to > override that function by defining a class variable "init_path" that > SystemBusyboxBase and SystemSystemdBase will override to some > appropriate value. See, for example, the code I wrote for Python and > IPython related tests. > > https://git.buildroot.net/buildroot/tree/support/testing/tests/package/test_python.py#n11 > https://git.buildroot.net/buildroot/tree/support/testing/tests/package/test_ipython.py#n18 Hmm, I see what you mean. I'll see if that helps with the code. OTOH, calling super() is not too complex either. ;-) Thanks for the suggestion! :-) Regards, Yann E. MORIN.
diff --git a/support/testing/tests/init/__init__.py b/support/testing/tests/init/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/support/testing/tests/init/base.py b/support/testing/tests/init/base.py new file mode 100644 index 0000000000..a261d7dd46 --- /dev/null +++ b/support/testing/tests/init/base.py @@ -0,0 +1,47 @@ +import os +import subprocess +import infra.basetest + +class InitSystemBase(infra.basetest.BRTest): + + def startEmulator(self, fs_type, kernel=None, dtb=None, init=None): + img = os.path.join(self.builddir, "images", "rootfs.{}".format(fs_type)) + subprocess.call(["truncate", "-s", "%1M", img]) + + options = ["-drive", + "file={},if=sd,format=raw".format(img), + "-M", "vexpress-a9"] + + if kernel is None: + kernel = "builtin" + else: + kernel = os.path.join(self.builddir, "images", kernel) + options.extend(["-dtb", os.path.join(self.builddir, "images", + "{}.dtb".format(dtb))]) + + kernel_cmdline = ["root=/dev/mmcblk0", + "rootfstype={}".format(fs_type), + "rootwait", + "ro", + "console=ttyAMA0"] + + if not init is None: + kernel_cmdline.extend(["init={}".format(init)]) + + self.emulator.boot(arch="armv7", + kernel=kernel, + kernel_cmdline=kernel_cmdline, + options=options) + + if init is None: + self.emulator.login() + + def checkInit(self, path): + cmd = "cmp /proc/1/exe {}".format(path) + _, exit_code = self.emulator.run(cmd) + self.assertEqual(exit_code, 0) + + def checkNetwork(self, interface, exitCode=0): + cmd = "ip addr show {} |grep inet".format(interface) + _, exit_code = self.emulator.run(cmd) + self.assertEqual(exit_code, exitCode) diff --git a/support/testing/tests/init/test_busybox.py b/support/testing/tests/init/test_busybox.py new file mode 100644 index 0000000000..c3e425bf5d --- /dev/null +++ b/support/testing/tests/init/test_busybox.py @@ -0,0 +1,67 @@ +import infra.basetest +from tests.init.base import InitSystemBase as InitSystemBase + +class InitSystemBusyboxBase(InitSystemBase): + config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \ + """ + # BR2_TARGET_ROOTFS_TAR is not set + """ + + def checkInit(self): + super(InitSystemBusyboxBase, self).checkInit("/bin/busybox") + + +#------------------------------------------------------------------------------- +class TestInitSystemBusyboxRo(InitSystemBusyboxBase): + config = InitSystemBusyboxBase.config + \ + """ + # BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is not set + BR2_TARGET_ROOTFS_SQUASHFS=y + """ + + def test_run(self): + self.startEmulator("squashfs") + self.checkInit() + self.checkNetwork("eth0", 1) + + +#------------------------------------------------------------------------------- +class TestInitSystemBusyboxRw(InitSystemBusyboxBase): + config = InitSystemBusyboxBase.config + \ + """ + BR2_TARGET_ROOTFS_EXT2=y + """ + + def test_run(self): + self.startEmulator("ext2") + self.checkInit() + self.checkNetwork("eth0", 1) + + +#------------------------------------------------------------------------------- +class TestInitSystemBusyboxRoNet(InitSystemBusyboxBase): + config = InitSystemBusyboxBase.config + \ + """ + BR2_SYSTEM_DHCP="eth0" + # BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is not set + BR2_TARGET_ROOTFS_SQUASHFS=y + """ + + def test_run(self): + self.startEmulator("squashfs") + self.checkInit() + self.checkNetwork("eth0") + + +#------------------------------------------------------------------------------- +class TestInitSystemBusyboxRwNet(InitSystemBusyboxBase): + config = InitSystemBusyboxBase.config + \ + """ + BR2_SYSTEM_DHCP="eth0" + BR2_TARGET_ROOTFS_EXT2=y + """ + + def test_run(self): + self.startEmulator("ext2") + self.checkInit() + self.checkNetwork("eth0") diff --git a/support/testing/tests/init/test_none.py b/support/testing/tests/init/test_none.py new file mode 100644 index 0000000000..f55db5d3e0 --- /dev/null +++ b/support/testing/tests/init/test_none.py @@ -0,0 +1,32 @@ +import pexpect + +import infra.basetest +from tests.init.base import InitSystemBase as InitSystemBase + +class TestInitSystemNone(InitSystemBase): + config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \ + """ + BR2_INIT_NONE=y + # BR2_TARGET_ROOTFS_TAR is not set + BR2_TARGET_ROOTFS_SQUASHFS=y + """ + + def test_run(self): + self.startEmulator(fs_type="squashfs", init="/bin/sh") + index = self.emulator.qemu.expect(["/bin/sh: can't access tty; job control turned off", pexpect.TIMEOUT], timeout=60) + if index != 0: + self.emulator.logfile.write("==> System does not boot") + raise SystemError("System does not boot") + index = self.emulator.qemu.expect(["#", pexpect.TIMEOUT], timeout=60) + if index != 0: + self.emulator.logfile.write("==> System does not boot") + raise SystemError("System does not boot") + + out, exit_code = self.emulator.run("sh -c 'echo $PPID'") + self.assertEqual(exit_code, 0) + self.assertEqual(out[0], "1") + + _, exit_code = self.emulator.run("mount -t proc none /proc") + self.assertEqual(exit_code, 0) + + self.checkInit("/bin/sh") diff --git a/support/testing/tests/init/test_systemd.py b/support/testing/tests/init/test_systemd.py new file mode 100644 index 0000000000..010e1f6514 --- /dev/null +++ b/support/testing/tests/init/test_systemd.py @@ -0,0 +1,145 @@ +import infra.basetest +from tests.init.base import InitSystemBase as InitSystemBase + +class InitSystemSystemdBase(InitSystemBase): + config = \ + """ + BR2_arm=y + BR2_TOOLCHAIN_EXTERNAL=y + BR2_INIT_SYSTEMD=y + BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0" + BR2_LINUX_KERNEL=y + BR2_LINUX_KERNEL_CUSTOM_VERSION=y + BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.11.3" + BR2_LINUX_KERNEL_DEFCONFIG="vexpress" + BR2_LINUX_KERNEL_DTS_SUPPORT=y + BR2_LINUX_KERNEL_INTREE_DTS_NAME="vexpress-v2p-ca9" + # BR2_TARGET_ROOTFS_TAR is not set + """ + + def checkInit(self): + super(InitSystemSystemdBase, self).checkInit("/lib/systemd/systemd") + + +#------------------------------------------------------------------------------- +class TestInitSystemSystemdRoNetworkd(InitSystemSystemdBase): + config = InitSystemSystemdBase.config + \ + """ + BR2_SYSTEM_DHCP="eth0" + # BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is not set + BR2_TARGET_ROOTFS_SQUASHFS=y + """ + + def test_run(self): + self.startEmulator("squashfs", "zImage", "vexpress-v2p-ca9") + self.checkInit() + self.checkNetwork("eth0") + + +#------------------------------------------------------------------------------- +class TestInitSystemSystemdRwNetworkd(InitSystemSystemdBase): + config = InitSystemSystemdBase.config + \ + """ + BR2_SYSTEM_DHCP="eth0" + BR2_TARGET_ROOTFS_EXT2=y + """ + + def test_run(self): + self.startEmulator("ext2", "zImage", "vexpress-v2p-ca9") + self.checkInit() + self.checkNetwork("eth0") + + +#------------------------------------------------------------------------------- +class TestInitSystemSystemdRoIfupdown(InitSystemSystemdBase): + config = InitSystemSystemdBase.config + \ + """ + BR2_SYSTEM_DHCP="eth0" + # BR2_PACKAGE_SYSTEMD_NETWORKD is not set + # BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is not set + BR2_TARGET_ROOTFS_SQUASHFS=y + """ + + def test_run(self): + self.startEmulator("squashfs", "zImage", "vexpress-v2p-ca9") + self.checkInit() + self.checkNetwork("eth0") + + +#------------------------------------------------------------------------------- +class TestInitSystemSystemdRwIfupdown(InitSystemSystemdBase): + config = InitSystemSystemdBase.config + \ + """ + BR2_SYSTEM_DHCP="eth0" + # BR2_PACKAGE_SYSTEMD_NETWORKD is not set + # BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is not set + BR2_TARGET_ROOTFS_EXT2=y + """ + + def test_run(self): + self.startEmulator("ext2", "zImage", "vexpress-v2p-ca9") + self.checkInit() + self.checkNetwork("eth0") + + +#------------------------------------------------------------------------------- +class TestInitSystemSystemdRoFull(InitSystemSystemdBase): + config = InitSystemSystemdBase.config + \ + """ + BR2_SYSTEM_DHCP="eth0" + # BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is not set + BR2_PACKAGE_SYSTEMD_JOURNAL_GATEWAY=y + BR2_PACKAGE_SYSTEMD_BACKLIGHT=y + BR2_PACKAGE_SYSTEMD_BINFMT=y + BR2_PACKAGE_SYSTEMD_COREDUMP=y + BR2_PACKAGE_SYSTEMD_FIRSTBOOT=y + BR2_PACKAGE_SYSTEMD_HIBERNATE=y + BR2_PACKAGE_SYSTEMD_IMPORTD=y + BR2_PACKAGE_SYSTEMD_LOCALED=y + BR2_PACKAGE_SYSTEMD_LOGIND=y + BR2_PACKAGE_SYSTEMD_MACHINED=y + BR2_PACKAGE_SYSTEMD_POLKIT=y + BR2_PACKAGE_SYSTEMD_QUOTACHECK=y + BR2_PACKAGE_SYSTEMD_RANDOMSEED=y + BR2_PACKAGE_SYSTEMD_RFKILL=y + BR2_PACKAGE_SYSTEMD_SMACK_SUPPORT=y + BR2_PACKAGE_SYSTEMD_SYSUSERS=y + BR2_PACKAGE_SYSTEMD_VCONSOLE=y + BR2_TARGET_ROOTFS_SQUASHFS=y + """ + + def test_run(self): + self.startEmulator("squashfs", "zImage", "vexpress-v2p-ca9") + self.checkInit() + self.checkNetwork("eth0") + + +#------------------------------------------------------------------------------- +class TestInitSystemSystemdRwFull(InitSystemSystemdBase): + config = InitSystemSystemdBase.config + \ + """ + BR2_SYSTEM_DHCP="eth0" + BR2_PACKAGE_SYSTEMD_JOURNAL_GATEWAY=y + BR2_PACKAGE_SYSTEMD_BACKLIGHT=y + BR2_PACKAGE_SYSTEMD_BINFMT=y + BR2_PACKAGE_SYSTEMD_COREDUMP=y + BR2_PACKAGE_SYSTEMD_FIRSTBOOT=y + BR2_PACKAGE_SYSTEMD_HIBERNATE=y + BR2_PACKAGE_SYSTEMD_IMPORTD=y + BR2_PACKAGE_SYSTEMD_LOCALED=y + BR2_PACKAGE_SYSTEMD_LOGIND=y + BR2_PACKAGE_SYSTEMD_MACHINED=y + BR2_PACKAGE_SYSTEMD_POLKIT=y + BR2_PACKAGE_SYSTEMD_QUOTACHECK=y + BR2_PACKAGE_SYSTEMD_RANDOMSEED=y + BR2_PACKAGE_SYSTEMD_RFKILL=y + BR2_PACKAGE_SYSTEMD_SMACK_SUPPORT=y + BR2_PACKAGE_SYSTEMD_SYSUSERS=y + BR2_PACKAGE_SYSTEMD_VCONSOLE=y + BR2_TARGET_ROOTFS_EXT2=y + """ + + def test_run(self): + self.startEmulator("ext2", "zImage", "vexpress-v2p-ca9") + self.checkInit() + self.checkNetwork("eth0")
The "builtin" kernel does not boot a systemd-based system, so we resort to building the same one as currently used by our qemu_arm_vexpress_defconfig. We test the 11 following combinations: - busybox, read-only, without network - busybox, read-only, with network - busybox, read-write, without network - busybox, read-write, with network - basic systemd, read-only, network w/ ifupdown - basic systemd, read-only, network w/ networkd - basic systemd, read-write, network w/ ifupdown - basic systemd, read-write, network w/ networkd - full systemd, read-only, network w/ networkd - full systemd, read-write, network w/ networkd - no init system, read-only, without network The tests just verify what the /sbin/init binary is, and that we were able to grab an IP address. More tests can be added later, for example to check each systemd features (journal, tmpfiles...) Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- support/testing/tests/init/__init__.py | 0 support/testing/tests/init/base.py | 47 ++++++++++ support/testing/tests/init/test_busybox.py | 67 +++++++++++++ support/testing/tests/init/test_none.py | 32 +++++++ support/testing/tests/init/test_systemd.py | 145 +++++++++++++++++++++++++++++ 5 files changed, 291 insertions(+) create mode 100644 support/testing/tests/init/__init__.py create mode 100644 support/testing/tests/init/base.py create mode 100644 support/testing/tests/init/test_busybox.py create mode 100644 support/testing/tests/init/test_none.py create mode 100644 support/testing/tests/init/test_systemd.py