| Message ID | 1542150146-12188-3-git-send-email-matthew.weber@rockwellcollins.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/5] package/compiler-rt: new package | expand |
Hello, + Thomas On Tue, Nov 13, 2018 at 09:02 PM, Matt Weber wrote: > - Add the condition under the -kernel assignment for aarch64 > - Tested with a defconfig simliar to qemu_aarch64_virt_defconfig s/simliar/similar > > Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com> > --- > support/testing/infra/emulator.py | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py > index 802e89d..8e5a7e9 100644 > --- a/support/testing/infra/emulator.py > +++ b/support/testing/infra/emulator.py > @@ -62,7 +62,9 @@ class Emulator(object): > kernel = infra.download(self.downloaddir, > "kernel-versatile") > qemu_cmd += ["-M", "versatilepb"] > - > + elif arch == "aarch64": > + kernel_cmdline.append("console=ttyAMA0") > + qemu_cmd += ["-M", "virt", "-cpu", "cortex-a53"] We don't really need this code here. You could do this in the test case: self.emulator.boot(arch="aarch64", kernel=kern, kernel_cmdline=["console=ttyAMA0"], options=["-M", "virt", "-cpu", "cortex-a53", "-m", "512", "-initrd", img]) If we think more and more test cases would need/want this we could add this here, preferably as a "builtin" pre-compiled kernel for aarch64. But for now, I suggest to move this code to the test case. Regards, Ricardo
Ricardo, On Fri, Nov 16, 2018 at 4:13 PM Ricardo Martincoski <ricardo.martincoski@gmail.com> wrote: > > Hello, > > + Thomas > > On Tue, Nov 13, 2018 at 09:02 PM, Matt Weber wrote: > > > - Add the condition under the -kernel assignment for aarch64 > > - Tested with a defconfig simliar to qemu_aarch64_virt_defconfig > > s/simliar/similar Noted. > > > > > Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com> > > --- > > support/testing/infra/emulator.py | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py > > index 802e89d..8e5a7e9 100644 > > --- a/support/testing/infra/emulator.py > > +++ b/support/testing/infra/emulator.py > > @@ -62,7 +62,9 @@ class Emulator(object): > > kernel = infra.download(self.downloaddir, > > "kernel-versatile") > > qemu_cmd += ["-M", "versatilepb"] > > - > > + elif arch == "aarch64": > > + kernel_cmdline.append("console=ttyAMA0") > > + qemu_cmd += ["-M", "virt", "-cpu", "cortex-a53"] > > We don't really need this code here. > You could do this in the test case: > > self.emulator.boot(arch="aarch64", > kernel=kern, > kernel_cmdline=["console=ttyAMA0"], > options=["-M", "virt", "-cpu", "cortex-a53", "-m", "512", "-initrd", img]) > > If we think more and more test cases would need/want this we could add this > here, preferably as a "builtin" pre-compiled kernel for aarch64. > > But for now, I suggest to move this code to the test case. I think I'm good either way and asked a bit about this during the last developers meeting. I just need to know if the preference is to provide a kernel or build it each time. If we start using more pre-builts, probably need a way to version control those and streamline submission? For this test it is quite time consuming already so the kernel build is in the noise. Plus your point is valid that we don't have a least two examples where we would have this in common (yet). I'll consolidate this in with the test in my next version. Thanks for the review! Matt
Hello, I renamed the mail thread (much easier to search for, a few months from now): -[PATCH 3/5] support/testing/infra/emulator.py: support aarch64 +Builtin kernel images for the testing infra Warning: a lot of opinions from my side. I certainly missed something. On Fri, Nov 16, 2018 at 10:57 PM, Matthew Weber wrote: > On Fri, Nov 16, 2018 at 4:13 PM Ricardo Martincoski > <ricardo.martincoski@gmail.com> wrote: [snip] >> If we think more and more test cases would need/want this we could add this >> here, preferably as a "builtin" pre-compiled kernel for aarch64. >> >> But for now, I suggest to move this code to the test case. > > I think I'm good either way and asked a bit about this during the last > developers meeting. I just need to know if the preference is to > provide a kernel or build it each time. If we start using more > pre-builts, probably need a way to version control those and > streamline submission? As I see it, these are the steps: 1 - prefer to reuse the pre-built kernel images that already exist whenever possible; 2 - if that is not enough, build one, preferably based on an existing qemu...defconfig; 3 - when many tests need to build a kernel, check the commonalities between them and provide a new pre-built that fits many tests; 4 - back to 1; And we are currently in step 2, near to go to step 3. But that is only my opinion, of course. Concerning new builtin images, if you want to add a new one, I suppose you could do this: - generate the new image and store it in a public temporary url; - add the support in the test infra for this new image and use it in at least one test case; - test your test case by placing the image in the test-dl or your usual BR2_DL_DIR folder. It works even on GitLab CI!, see [1]; - send the patch for review adding a note below "---" with the url for the image to be stored in the current artifacts url; Since you mentioned version control... Maybe, in the long term, we could create a separate repo, i.e. buildroot-runtime-tests-binaries in GitHub and accept merges. I think people in the list would not like to receive patches with 2-5 MB. A .txt file along each binary stating how the image was generated (i.e. the sha1 of buildroot and defconfig name, and even the url for a build in GitLab CI) would be nice IMO. Just an idea/example. Two more questions came to mind when thinking about adding more builtin images: What is the naming to use? I am happy with the current one: kernel-versatile, and similar. And when we need to generate a new one for the same arch (actually a machine that qemu supports) with newer kernel version, the kernel version is a good suffix IMO. What kernel config to use? Should we trim down to have a small image? Should we add stuff to allow maximum reuse? IMO, using the default kernel intree defconfig selected by our qemu defconfigs seems a good compromise. Looking at the images generated at [2]: versatile = 2.6 MB vexpress = 4.1 MB aarch64 = 6.7 MB And comparing to the images currently used by the test infra: versatile = 2.0 MB vexpress = 3.3 MB Those sizes seem acceptable to me. Notice I did not tested using such images with the test infra. [1] https://gitlab.com/RicardoMartincoski/buildroot/commit/e226e5a0a1918670fd688cb268176da589ca2fae [2] https://gitlab.com/buildroot.org/buildroot/pipelines/36081029 Regards, Ricardo
Hello, On Mon, 19 Nov 2018 00:22:24 -0200, Ricardo Martincoski wrote: > As I see it, these are the steps: > 1 - prefer to reuse the pre-built kernel images that already exist whenever > possible; > 2 - if that is not enough, build one, preferably based on an existing > qemu...defconfig; > 3 - when many tests need to build a kernel, check the commonalities between > them and provide a new pre-built that fits many tests; > 4 - back to 1; > And we are currently in step 2, near to go to step 3. > But that is only my opinion, of course. Yes, fully agreed. The idea of those pre-built kernel images is to save time. For a purely user-space test such as the Python test cases, or Lua test cases, it already takes quite some time to rebuild the Python interpreter for each test case, it would be annoying to rebuild the kernel as well. So it makes sense to provide a few pre-built kernel images for a variety of architectures, with some "sane" kernel configuration. Of course, this doesn't prevent from adding other test cases that will build the Linux kernel: - To test the Buildroot Linux kernel packaging - Because we want to build some Buildroot packages that provide kernel modules, or need some very special kernel configuration options to be set > Concerning new builtin images, if you want to add a new one, I suppose you > could do this: > - generate the new image and store it in a public temporary url; > - add the support in the test infra for this new image and use it in at least > one test case; > - test your test case by placing the image in the test-dl or your usual > BR2_DL_DIR folder. It works even on GitLab CI!, see [1]; > - send the patch for review adding a note below "---" with the url for the > image to be stored in the current artifacts url; Absolutely. > Since you mentioned version control... > Maybe, in the long term, we could create a separate repo, i.e. > buildroot-runtime-tests-binaries in GitHub and accept merges. > I think people in the list would not like to receive patches with 2-5 MB. > A .txt file along each binary stating how the image was generated (i.e. the > sha1 of buildroot and defconfig name, and even the url for a build in GitLab CI) > would be nice IMO. > Just an idea/example. The way we handle those artifacts right now is very basic, and there are certainly ways of improving that over time. > Should we trim down to have a small image? Should we add stuff to allow maximum > reuse? IMO, using the default kernel intree defconfig selected by our qemu > defconfigs seems a good compromise. Agree. Best regards, Thomas
On 19/11/2018 09:29, Thomas Petazzoni wrote: > Hello, > > On Mon, 19 Nov 2018 00:22:24 -0200, Ricardo Martincoski wrote: > >> As I see it, these are the steps: >> 1 - prefer to reuse the pre-built kernel images that already exist whenever >> possible; >> 2 - if that is not enough, build one, preferably based on an existing >> qemu...defconfig; >> 3 - when many tests need to build a kernel, check the commonalities between >> them and provide a new pre-built that fits many tests; >> 4 - back to 1; >> And we are currently in step 2, near to go to step 3. >> But that is only my opinion, of course. > > Yes, fully agreed. The idea of those pre-built kernel images is to save > time. For a purely user-space test such as the Python test cases, or > Lua test cases, it already takes quite some time to rebuild the Python > interpreter for each test case, it would be annoying to rebuild the > kernel as well. > > So it makes sense to provide a few pre-built kernel images for a > variety of architectures, with some "sane" kernel configuration. > > Of course, this doesn't prevent from adding other test cases that will > build the Linux kernel: > > - To test the Buildroot Linux kernel packaging > - Because we want to build some Buildroot packages that provide kernel > modules, or need some very special kernel configuration options to > be set > >> Concerning new builtin images, if you want to add a new one, I suppose you >> could do this: >> - generate the new image and store it in a public temporary url; >> - add the support in the test infra for this new image and use it in at least >> one test case; >> - test your test case by placing the image in the test-dl or your usual >> BR2_DL_DIR folder. It works even on GitLab CI!, see [1]; >> - send the patch for review adding a note below "---" with the url for the >> image to be stored in the current artifacts url; > > Absolutely. > >> Since you mentioned version control... >> Maybe, in the long term, we could create a separate repo, i.e. >> buildroot-runtime-tests-binaries in GitHub and accept merges. >> I think people in the list would not like to receive patches with 2-5 MB. >> A .txt file along each binary stating how the image was generated (i.e. the >> sha1 of buildroot and defconfig name, and even the url for a build in GitLab CI) >> would be nice IMO. >> Just an idea/example. > > The way we handle those artifacts right now is very basic, and there > are certainly ways of improving that over time. We could use gitlab pipelines to handle this. Define gitlab jobs to generate the kernel (or whatever), and add dependencies on those. The test should then check if the kernel is available, and if not, print instructions on where to get them. Regards, Arnout > >> Should we trim down to have a small image? Should we add stuff to allow maximum >> reuse? IMO, using the default kernel intree defconfig selected by our qemu >> defconfigs seems a good compromise. > > Agree. > > Best regards, > > Thomas >
Arnout, On Thu, Nov 22, 2018 at 5:02 PM Arnout Vandecappelle <arnout@mind.be> wrote: > > > > On 19/11/2018 09:29, Thomas Petazzoni wrote: > > Hello, > > > > On Mon, 19 Nov 2018 00:22:24 -0200, Ricardo Martincoski wrote: > > > >> As I see it, these are the steps: > >> 1 - prefer to reuse the pre-built kernel images that already exist whenever > >> possible; > >> 2 - if that is not enough, build one, preferably based on an existing > >> qemu...defconfig; > >> 3 - when many tests need to build a kernel, check the commonalities between > >> them and provide a new pre-built that fits many tests; > >> 4 - back to 1; > >> And we are currently in step 2, near to go to step 3. > >> But that is only my opinion, of course. > > > > Yes, fully agreed. The idea of those pre-built kernel images is to save > > time. For a purely user-space test such as the Python test cases, or > > Lua test cases, it already takes quite some time to rebuild the Python > > interpreter for each test case, it would be annoying to rebuild the > > kernel as well. > > > > So it makes sense to provide a few pre-built kernel images for a > > variety of architectures, with some "sane" kernel configuration. > > > > Of course, this doesn't prevent from adding other test cases that will > > build the Linux kernel: > > > > - To test the Buildroot Linux kernel packaging > > - Because we want to build some Buildroot packages that provide kernel > > modules, or need some very special kernel configuration options to > > be set > > > >> Concerning new builtin images, if you want to add a new one, I suppose you > >> could do this: > >> - generate the new image and store it in a public temporary url; > >> - add the support in the test infra for this new image and use it in at least > >> one test case; > >> - test your test case by placing the image in the test-dl or your usual > >> BR2_DL_DIR folder. It works even on GitLab CI!, see [1]; > >> - send the patch for review adding a note below "---" with the url for the > >> image to be stored in the current artifacts url; > > > > Absolutely. > > > >> Since you mentioned version control... > >> Maybe, in the long term, we could create a separate repo, i.e. > >> buildroot-runtime-tests-binaries in GitHub and accept merges. > >> I think people in the list would not like to receive patches with 2-5 MB. > >> A .txt file along each binary stating how the image was generated (i.e. the > >> sha1 of buildroot and defconfig name, and even the url for a build in GitLab CI) > >> would be nice IMO. > >> Just an idea/example. > > > > The way we handle those artifacts right now is very basic, and there > > are certainly ways of improving that over time. > > We could use gitlab pipelines to handle this. Define gitlab jobs to generate > the kernel (or whatever), and add dependencies on those. > > The test should then check if the kernel is available, and if not, print > instructions on where to get them. > Maybe this is an opportunity to use the images we're building as part of the other defconfig test builds? I'm sure a few kernels would need some options enabled for virtualization but generally I bet the images/artifacts would just need to be archived off in a fashion we could pull from during the run-time tests. It would give some coverage of it those images work. Matt
diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py index 802e89d..8e5a7e9 100644 --- a/support/testing/infra/emulator.py +++ b/support/testing/infra/emulator.py @@ -62,7 +62,9 @@ class Emulator(object): kernel = infra.download(self.downloaddir, "kernel-versatile") qemu_cmd += ["-M", "versatilepb"] - + elif arch == "aarch64": + kernel_cmdline.append("console=ttyAMA0") + qemu_cmd += ["-M", "virt", "-cpu", "cortex-a53"] qemu_cmd += ["-kernel", kernel] if kernel_cmdline:
- Add the condition under the -kernel assignment for aarch64 - Tested with a defconfig simliar to qemu_aarch64_virt_defconfig Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com> --- support/testing/infra/emulator.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)