[PATCH v1] automation: provide example for downloading an existing container

[PATCH v1] automation: provide example for downloading an existing container

[Olaf Hering]

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 automation/build/README.md | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/automation/build/README.md b/automation/build/README.md
index 2d07cafe0e..8ad89a259a 100644
--- a/automation/build/README.md
+++ b/automation/build/README.md
@@ -12,6 +12,12 @@ can be pulled with Docker from the following path:
 docker pull registry.gitlab.com/xen-project/xen/DISTRO:VERSION
 ```
 
+This example shows how to pull the existing container for Tumbleweed:
+
+```
+docker pull registry.gitlab.com/xen-project/xen/suse:opensuse-tumbleweed
+```
+
 To see the list of available containers run `make` in this
 directory. You will have to replace the `/` with a `:` to use
 them.

Re: [PATCH v1] automation: provide example for downloading an existing container

[Stefano Stabellini]

On Tue, 2 May 2023, Olaf Hering wrote:
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

Given that opensuse-tumbleweed is still broken (doesn't complete the Xen
build successfully) even after these patches, I suggest we use a
different example?


> ---
>  automation/build/README.md | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/automation/build/README.md b/automation/build/README.md
> index 2d07cafe0e..8ad89a259a 100644
> --- a/automation/build/README.md
> +++ b/automation/build/README.md
> @@ -12,6 +12,12 @@ can be pulled with Docker from the following path:
>  docker pull registry.gitlab.com/xen-project/xen/DISTRO:VERSION
>  ```
>  
> +This example shows how to pull the existing container for Tumbleweed:
> +
> +```
> +docker pull registry.gitlab.com/xen-project/xen/suse:opensuse-tumbleweed
> +```
> +
>  To see the list of available containers run `make` in this
>  directory. You will have to replace the `/` with a `:` to use
>  them.
> 

Re: [PATCH v1] automation: provide example for downloading an existing container

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 373 bytes --]

Am Mon, 15 May 2023 16:03:01 -0700 (PDT)
schrieb Stefano Stabellini <sstabellini@kernel.org>:

> Given that opensuse-tumbleweed is still broken (doesn't complete the
> Xen build successfully) even after these patches, I suggest we use a
> different example?

For some reason it succeeded for me locally.
Does it fail for you as well if you run it locally?

Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH v1] automation: provide example for downloading an existing container

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]

Am Mon, 15 May 2023 16:03:01 -0700 (PDT)
schrieb Stefano Stabellini <sstabellini@kernel.org>:

> Given that opensuse-tumbleweed is still broken (doesn't complete the
> Xen build successfully) even after these patches, I suggest we use a
> different example?

I think the example in automation/build/README.md needs to be fixed.
Right now it does something different than the Gitlab CI.

The CI runs automation/scripts/build with some environment variables
set, the example runs automation/scripts/containerize.


For me qemu-xen builds, I assume it is supposed to be 'master ==
"8c51cd9705 (HEAD -> dummy, origin/staging, origin/master, origin/HEAD,
master) hw/xen/xen_pt: fix uninitialized variable", but we do not know
what GI tests, because scripts/git-checkout.sh does not show what HEAD
actually is. I think it needs to run "$GIT --no-pager log --oneline
-n1" at the end, so everyone knows what 'dummy' actually is.


I think it is perfectly fine that both examples refer to Tumbleweed,
because one may need to fix future build errors, not test on something
from which we already know that it works.


Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH v1] automation: provide example for downloading an existing container

[Stefano Stabellini]

On Tue, 16 May 2023, Olaf Hering wrote:
> Am Mon, 15 May 2023 16:03:01 -0700 (PDT)
> schrieb Stefano Stabellini <sstabellini@kernel.org>:
> 
> > Given that opensuse-tumbleweed is still broken (doesn't complete the
> > Xen build successfully) even after these patches, I suggest we use a
> > different example?
> 
> I think the example in automation/build/README.md needs to be fixed.
> Right now it does something different than the Gitlab CI.
> 
> The CI runs automation/scripts/build with some environment variables
> set, the example runs automation/scripts/containerize.
 
I think you have a point that automation/build/README.md should also
describe how to do what gitlab-ci does but locally (i.e. call
automation/scripts/build). It should not only describe
automation/scripts/containerize.

 
> For me qemu-xen builds, I assume it is supposed to be 'master ==
> "8c51cd9705 (HEAD -> dummy, origin/staging, origin/master, origin/HEAD,
> master) hw/xen/xen_pt: fix uninitialized variable", but we do not know
> what GI tests, because scripts/git-checkout.sh does not show what HEAD
> actually is. I think it needs to run "$GIT --no-pager log --oneline
> -n1" at the end, so everyone knows what 'dummy' actually is.

Gitlab-ci only runs automation/scripts/build which builds QEMU as part
of a regular Xen build.

If you want to see details of a failure:
https://gitlab.com/xen-project/xen/-/jobs/4284741849

---
In file included from /builds/xen-project/xen/tools/qemu-xen-dir-remote/include/qemu/coroutine.h:18,
                 from /builds/xen-project/xen/tools/qemu-xen-dir-remote/include/block/aio.h:20,
                 from ../qemu-xen-dir-remote/util/async.c:28:
../qemu-xen-dir-remote/util/async.c: In function 'aio_bh_poll':
/builds/xen-project/xen/tools/qemu-xen-dir-remote/include/qemu/queue.h:303:22: error: storing the address of local variable 'slice' in '*ctx.bh_slice_list.sqh_last' [-Werror=dangling-pointer=]
  303 |     (head)->sqh_last = &(elm)->field.sqe_next;                          \
      |     ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
../qemu-xen-dir-remote/util/async.c:161:5: note: in expansion of macro 'QSIMPLEQ_INSERT_TAIL'
  161 |     QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
      |     ^~~~~~~~~~~~~~~~~~~~
../qemu-xen-dir-remote/util/async.c:156:17: note: 'slice' declared here
  156 |     BHListSlice slice;
      |                 ^~~~~
../qemu-xen-dir-remote/util/async.c:154:29: note: 'ctx' declared here
  154 | int aio_bh_poll(AioContext *ctx)
      |                 ~~~~~~~~~~~~^~~
cc1: all warnings being treated as errors
---

> I think it is perfectly fine that both examples refer to Tumbleweed,
> because one may need to fix future build errors, not test on something
> from which we already know that it works.

Sure I see your point. On the other hand Tumbleweed jobs are the ones
and only with "allow_failure". So among all the possible choices as
example, do we really need to pick the one and only that has been
failing for months? :-)

Re: [PATCH v1] automation: provide example for downloading an existing container

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 1657 bytes --]

Am Tue, 16 May 2023 11:46:00 -0700 (PDT)
schrieb Stefano Stabellini <sstabellini@kernel.org>:

> I think you have a point that automation/build/README.md should also
> describe how to do what gitlab-ci does but locally (i.e. call
> automation/scripts/build). It should not only describe
> automation/scripts/containerize.

Meanwhile I have figured this out, additional variables must be set. I
already sent a patch for the example. That way I was able to
understand and reproduce the error seen in the CI build.

> https://gitlab.com/xen-project/xen/-/jobs/4284741849

It turned out this bug in qemu is triggered by debug=y vs. debug=n in
the build environment. I have not checked which commit exactly fixed it
in upstream qemu.git, it should probably be backported. Or qemu should
be moved forward to v8.x at some point. I think I have not seen this
specific failure in my own qemu.git builds.

The reason is: --enable-debug will disable _FORTIFY_SOURCE, so the build
succeeds. Without that flag, configure will enable _FORTIFY_SOURCE.

> Sure I see your point. On the other hand Tumbleweed jobs are the ones
> and only with "allow_failure". So among all the possible choices as
> example, do we really need to pick the one and only that has been
> failing for months? :-)

Yeah, this is exactly the point, to give copy&paste commands so that
contributors can investigate such failures locally.

I did not follow the state of the openSUSE builds in the past months. I
think Tumbleweed did succeed a few weeks ago because the last snapshot
was meanwhile one year old, and all gcc12 bugs are already fixed by now.


Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH v1] automation: provide example for downloading an existing container

[Stefano Stabellini]

On Tue, 16 May 2023, Olaf Hering wrote:
> Am Tue, 16 May 2023 11:46:00 -0700 (PDT)
> schrieb Stefano Stabellini <sstabellini@kernel.org>:
> 
> > I think you have a point that automation/build/README.md should also
> > describe how to do what gitlab-ci does but locally (i.e. call
> > automation/scripts/build). It should not only describe
> > automation/scripts/containerize.
> 
> Meanwhile I have figured this out, additional variables must be set. I
> already sent a patch for the example. That way I was able to
> understand and reproduce the error seen in the CI build.

Thanks!


> > https://gitlab.com/xen-project/xen/-/jobs/4284741849
> 
> It turned out this bug in qemu is triggered by debug=y vs. debug=n in
> the build environment. I have not checked which commit exactly fixed it
> in upstream qemu.git, it should probably be backported. Or qemu should
> be moved forward to v8.x at some point. I think I have not seen this
> specific failure in my own qemu.git builds.
> 
> The reason is: --enable-debug will disable _FORTIFY_SOURCE, so the build
> succeeds. Without that flag, configure will enable _FORTIFY_SOURCE.

This is very interesting. Thank you for investigating the problem.

I would prefer a proper backported fix to QEMU, or wholesale QEMU
upgrade. But if neither are possible we could also add a way to add
--enable-debug or disable _FORTIFY_SOURCE in another way for Tumbleweed.
See for instance EXTRA_XEN_CONFIG as a way to pass special parameters
from automation/gitlab-ci/build.yaml to automation/script/build.