kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/9] Alpine Linux build fix and CI pipeline
@ 2021-01-18  6:37 Jiaxun Yang
  2021-01-18  6:38 ` [PATCH v2 1/9] configure: Add sys/timex.h to probe clock_adjtime Jiaxun Yang
                   ` (10 more replies)
  0 siblings, 11 replies; 31+ messages in thread
From: Jiaxun Yang @ 2021-01-18  6:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Thomas Huth, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf, Jiaxun Yang

Alpine Linux is a security-oriented, lightweight Linux distribution
based on musl libc and busybox.

It it popular among Docker guests and embedded applications.

Adding it to test against different libc.

Patches pending review at v2 are: 7, 8, 9

Tree avilable at: https://gitlab.com/FlyGoat/qemu/-/commits/alpine_linux_v2
CI All green: https://gitlab.com/FlyGoat/qemu/-/pipelines/242003288

It is known to have checkpatch complains about identation but they're
all pre-existing issues as I'm only doing string replacement. 

v2:
 - Reoreder patches (Wainer)
 - Add shadow to dockerfile (Wainer)
 - Pickup proper signal.h fix (PMM)
 - Correct clock_adjtime title (Thomas Huth)
 - Collect review tags

Jiaxun Yang (8):
  configure: Add sys/timex.h to probe clock_adjtime
  libvhost-user: Include poll.h instead of sys/poll.h
  hw/block/nand: Rename PAGE_SIZE to NAND_PAGE_SIZE
  elf2dmp: Rename PAGE_SIZE to ELF2DMP_PAGE_SIZE
  tests: Rename PAGE_SIZE definitions
  accel/kvm: avoid using predefined PAGE_SIZE
  tests/docker: Add dockerfile for Alpine Linux
  gitlab-ci: Add alpine to pipeline

Michael Forney (1):
  osdep.h: Remove <sys/signal.h> include

 configure                                 |  1 +
 meson.build                               |  1 -
 contrib/elf2dmp/addrspace.h               |  6 +-
 include/qemu/osdep.h                      |  4 --
 subprojects/libvhost-user/libvhost-user.h |  2 +-
 accel/kvm/kvm-all.c                       |  3 +
 contrib/elf2dmp/addrspace.c               |  4 +-
 contrib/elf2dmp/main.c                    | 18 +++---
 hw/block/nand.c                           | 40 ++++++-------
 tests/migration/stress.c                  | 10 ++--
 tests/qtest/libqos/malloc-pc.c            |  4 +-
 tests/qtest/libqos/malloc-spapr.c         |  4 +-
 tests/qtest/m25p80-test.c                 | 54 ++++++++---------
 tests/tcg/multiarch/system/memory.c       |  6 +-
 tests/test-xbzrle.c                       | 70 +++++++++++------------
 .gitlab-ci.d/containers.yml               |  5 ++
 .gitlab-ci.yml                            | 23 ++++++++
 tests/docker/dockerfiles/alpine.docker    | 57 ++++++++++++++++++
 18 files changed, 198 insertions(+), 114 deletions(-)
 create mode 100644 tests/docker/dockerfiles/alpine.docker

-- 
2.30.0


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 1/9] configure: Add sys/timex.h to probe clock_adjtime
  2021-01-18  6:37 [PATCH v2 0/9] Alpine Linux build fix and CI pipeline Jiaxun Yang
@ 2021-01-18  6:38 ` Jiaxun Yang
  2021-01-18  6:38 ` [PATCH v2 2/9] libvhost-user: Include poll.h instead of sys/poll.h Jiaxun Yang
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 31+ messages in thread
From: Jiaxun Yang @ 2021-01-18  6:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Thomas Huth, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf, Jiaxun Yang

It is not a part of standard time.h. Glibc put it under
time.h however musl treat it as a sys timex extension.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 configure | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index 155dda124c..1a9e1afa39 100755
--- a/configure
+++ b/configure
@@ -4039,6 +4039,7 @@ fi
 clock_adjtime=no
 cat > $TMPC <<EOF
 #include <time.h>
+#include <sys/timex.h>
 
 int main(void)
 {
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v2 2/9] libvhost-user: Include poll.h instead of sys/poll.h
  2021-01-18  6:37 [PATCH v2 0/9] Alpine Linux build fix and CI pipeline Jiaxun Yang
  2021-01-18  6:38 ` [PATCH v2 1/9] configure: Add sys/timex.h to probe clock_adjtime Jiaxun Yang
@ 2021-01-18  6:38 ` Jiaxun Yang
  2021-01-18  9:41   ` Philippe Mathieu-Daudé
  2021-01-18  6:38 ` [PATCH v2 3/9] osdep.h: Remove <sys/signal.h> include Jiaxun Yang
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 31+ messages in thread
From: Jiaxun Yang @ 2021-01-18  6:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Thomas Huth, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf, Jiaxun Yang

Musl libc complains about it's wrong usage.

In file included from ../subprojects/libvhost-user/libvhost-user.h:20,
                 from ../subprojects/libvhost-user/libvhost-user-glib.h:19,
                 from ../subprojects/libvhost-user/libvhost-user-glib.c:15:
/usr/include/sys/poll.h:1:2: error: #warning redirecting incorrect #include <sys/poll.h> to <poll.h> [-Werror=cpp]
    1 | #warning redirecting incorrect #include <sys/poll.h> to <poll.h>
      |  ^~~~~~~

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 subprojects/libvhost-user/libvhost-user.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h
index 7d47f1364a..3d13dfadde 100644
--- a/subprojects/libvhost-user/libvhost-user.h
+++ b/subprojects/libvhost-user/libvhost-user.h
@@ -17,7 +17,7 @@
 #include <stdint.h>
 #include <stdbool.h>
 #include <stddef.h>
-#include <sys/poll.h>
+#include <poll.h>
 #include <linux/vhost.h>
 #include <pthread.h>
 #include "standard-headers/linux/virtio_ring.h"
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v2 3/9] osdep.h: Remove <sys/signal.h> include
  2021-01-18  6:37 [PATCH v2 0/9] Alpine Linux build fix and CI pipeline Jiaxun Yang
  2021-01-18  6:38 ` [PATCH v2 1/9] configure: Add sys/timex.h to probe clock_adjtime Jiaxun Yang
  2021-01-18  6:38 ` [PATCH v2 2/9] libvhost-user: Include poll.h instead of sys/poll.h Jiaxun Yang
@ 2021-01-18  6:38 ` Jiaxun Yang
  2021-01-18  9:41   ` Philippe Mathieu-Daudé
  2021-01-18  6:38 ` [PATCH v2 4/9] hw/block/nand: Rename PAGE_SIZE to NAND_PAGE_SIZE Jiaxun Yang
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 31+ messages in thread
From: Jiaxun Yang @ 2021-01-18  6:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Thomas Huth, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf, Michael Forney,
	Eric Blake, Jiaxun Yang

From: Michael Forney <mforney@mforney.org>

Prior to 2a4b472c3c, sys/signal.h was only included on OpenBSD
(apart from two .c files). The POSIX standard location for this
header is just <signal.h> and in fact, OpenBSD's signal.h includes
sys/signal.h itself.

Unconditionally including <sys/signal.h> on musl causes warnings
for just about every source file:

  /usr/include/sys/signal.h:1:2: warning: #warning redirecting incorrect #include <sys/signal.h> to <signal.h> [-Wcpp]
      1 | #warning redirecting incorrect #include <sys/signal.h> to <signal.h>
        |  ^~~~~~~

Since there don't seem to be any platforms which require including
<sys/signal.h> in addition to <signal.h>, and some platforms like
Haiku lack it completely, just remove it.

Tested building on OpenBSD after removing this include.

Signed-off-by: Michael Forney <mforney@mforney.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
[jiaxun.yang@flygoat.com: Move to meson]
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 meson.build          | 1 -
 include/qemu/osdep.h | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/meson.build b/meson.build
index 3d889857a0..af2bc89741 100644
--- a/meson.build
+++ b/meson.build
@@ -1113,7 +1113,6 @@ config_host_data.set('HAVE_DRM_H', cc.has_header('libdrm/drm.h'))
 config_host_data.set('HAVE_PTY_H', cc.has_header('pty.h'))
 config_host_data.set('HAVE_SYS_IOCCOM_H', cc.has_header('sys/ioccom.h'))
 config_host_data.set('HAVE_SYS_KCOV_H', cc.has_header('sys/kcov.h'))
-config_host_data.set('HAVE_SYS_SIGNAL_H', cc.has_header('sys/signal.h'))
 
 ignored = ['CONFIG_QEMU_INTERP_PREFIX'] # actually per-target
 arrays = ['CONFIG_AUDIO_DRIVERS', 'CONFIG_BDRV_RW_WHITELIST', 'CONFIG_BDRV_RO_WHITELIST']
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index f9ec8c84e9..a434382c58 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -104,10 +104,6 @@ extern int daemon(int, int);
 #include <setjmp.h>
 #include <signal.h>
 
-#ifdef HAVE_SYS_SIGNAL_H
-#include <sys/signal.h>
-#endif
-
 #ifndef _WIN32
 #include <sys/wait.h>
 #else
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v2 4/9] hw/block/nand: Rename PAGE_SIZE to NAND_PAGE_SIZE
  2021-01-18  6:37 [PATCH v2 0/9] Alpine Linux build fix and CI pipeline Jiaxun Yang
                   ` (2 preceding siblings ...)
  2021-01-18  6:38 ` [PATCH v2 3/9] osdep.h: Remove <sys/signal.h> include Jiaxun Yang
@ 2021-01-18  6:38 ` Jiaxun Yang
  2021-01-18  9:44   ` Philippe Mathieu-Daudé
  2021-01-18  6:38 ` [PATCH v2 5/9] elf2dmp: Rename PAGE_SIZE to ELF2DMP_PAGE_SIZE Jiaxun Yang
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 31+ messages in thread
From: Jiaxun Yang @ 2021-01-18  6:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Thomas Huth, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf, Jiaxun Yang

As per POSIX specification of limits.h [1], OS libc may define
PAGE_SIZE in limits.h.

To prevent collosion of definition, we rename PAGE_SIZE here.

[1]: https://pubs.opengroup.org/onlinepubs/7908799/xsh/limits.h.html

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 hw/block/nand.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/hw/block/nand.c b/hw/block/nand.c
index 123020aebf..cd26a14d57 100644
--- a/hw/block/nand.c
+++ b/hw/block/nand.c
@@ -115,24 +115,24 @@ static void mem_and(uint8_t *dest, const uint8_t *src, size_t n)
 # define NAND_IO
 
 # define PAGE(addr)		((addr) >> ADDR_SHIFT)
-# define PAGE_START(page)	(PAGE(page) * (PAGE_SIZE + OOB_SIZE))
+# define PAGE_START(page)	(PAGE(page) * (NAND_PAGE_SIZE + OOB_SIZE))
 # define PAGE_MASK		((1 << ADDR_SHIFT) - 1)
 # define OOB_SHIFT		(PAGE_SHIFT - 5)
 # define OOB_SIZE		(1 << OOB_SHIFT)
 # define SECTOR(addr)		((addr) >> (9 + ADDR_SHIFT - PAGE_SHIFT))
 # define SECTOR_OFFSET(addr)	((addr) & ((511 >> PAGE_SHIFT) << 8))
 
-# define PAGE_SIZE		256
+# define NAND_PAGE_SIZE     256
 # define PAGE_SHIFT		8
 # define PAGE_SECTORS		1
 # define ADDR_SHIFT		8
 # include "nand.c"
-# define PAGE_SIZE		512
+# define NAND_PAGE_SIZE     512
 # define PAGE_SHIFT		9
 # define PAGE_SECTORS		1
 # define ADDR_SHIFT		8
 # include "nand.c"
-# define PAGE_SIZE		2048
+# define NAND_PAGE_SIZE		2048
 # define PAGE_SHIFT		11
 # define PAGE_SECTORS		4
 # define ADDR_SHIFT		16
@@ -652,7 +652,7 @@ type_init(nand_register_types)
 #else
 
 /* Program a single page */
-static void glue(nand_blk_write_, PAGE_SIZE)(NANDFlashState *s)
+static void glue(nand_blk_write_, NAND_PAGE_SIZE)(NANDFlashState *s)
 {
     uint64_t off, page, sector, soff;
     uint8_t iobuf[(PAGE_SECTORS + 2) * 0x200];
@@ -672,11 +672,11 @@ static void glue(nand_blk_write_, PAGE_SIZE)(NANDFlashState *s)
             return;
         }
 
-        mem_and(iobuf + (soff | off), s->io, MIN(s->iolen, PAGE_SIZE - off));
-        if (off + s->iolen > PAGE_SIZE) {
+        mem_and(iobuf + (soff | off), s->io, MIN(s->iolen, NAND_PAGE_SIZE - off));
+        if (off + s->iolen > NAND_PAGE_SIZE) {
             page = PAGE(s->addr);
-            mem_and(s->storage + (page << OOB_SHIFT), s->io + PAGE_SIZE - off,
-                            MIN(OOB_SIZE, off + s->iolen - PAGE_SIZE));
+            mem_and(s->storage + (page << OOB_SHIFT), s->io + NAND_PAGE_SIZE - off,
+                            MIN(OOB_SIZE, off + s->iolen - NAND_PAGE_SIZE));
         }
 
         if (blk_pwrite(s->blk, sector << BDRV_SECTOR_BITS, iobuf,
@@ -704,7 +704,7 @@ static void glue(nand_blk_write_, PAGE_SIZE)(NANDFlashState *s)
 }
 
 /* Erase a single block */
-static void glue(nand_blk_erase_, PAGE_SIZE)(NANDFlashState *s)
+static void glue(nand_blk_erase_, NAND_PAGE_SIZE)(NANDFlashState *s)
 {
     uint64_t i, page, addr;
     uint8_t iobuf[0x200] = { [0 ... 0x1ff] = 0xff, };
@@ -716,7 +716,7 @@ static void glue(nand_blk_erase_, PAGE_SIZE)(NANDFlashState *s)
 
     if (!s->blk) {
         memset(s->storage + PAGE_START(addr),
-                        0xff, (PAGE_SIZE + OOB_SIZE) << s->erase_shift);
+                        0xff, (NAND_PAGE_SIZE + OOB_SIZE) << s->erase_shift);
     } else if (s->mem_oob) {
         memset(s->storage + (PAGE(addr) << OOB_SHIFT),
                         0xff, OOB_SIZE << s->erase_shift);
@@ -742,7 +742,7 @@ static void glue(nand_blk_erase_, PAGE_SIZE)(NANDFlashState *s)
 
         memset(iobuf, 0xff, 0x200);
         i = (addr & ~0x1ff) + 0x200;
-        for (addr += ((PAGE_SIZE + OOB_SIZE) << s->erase_shift) - 0x200;
+        for (addr += ((NAND_PAGE_SIZE + OOB_SIZE) << s->erase_shift) - 0x200;
                         i < addr; i += 0x200) {
             if (blk_pwrite(s->blk, i, iobuf, BDRV_SECTOR_SIZE, 0) < 0) {
                 printf("%s: write error in sector %" PRIu64 "\n",
@@ -763,7 +763,7 @@ static void glue(nand_blk_erase_, PAGE_SIZE)(NANDFlashState *s)
     }
 }
 
-static void glue(nand_blk_load_, PAGE_SIZE)(NANDFlashState *s,
+static void glue(nand_blk_load_, NAND_PAGE_SIZE)(NANDFlashState *s,
                 uint64_t addr, int offset)
 {
     if (PAGE(addr) >= s->pages) {
@@ -777,7 +777,7 @@ static void glue(nand_blk_load_, PAGE_SIZE)(NANDFlashState *s,
                 printf("%s: read error in sector %" PRIu64 "\n",
                                 __func__, SECTOR(addr));
             }
-            memcpy(s->io + SECTOR_OFFSET(s->addr) + PAGE_SIZE,
+            memcpy(s->io + SECTOR_OFFSET(s->addr) + NAND_PAGE_SIZE,
                             s->storage + (PAGE(s->addr) << OOB_SHIFT),
                             OOB_SIZE);
             s->ioaddr = s->io + SECTOR_OFFSET(s->addr) + offset;
@@ -791,23 +791,23 @@ static void glue(nand_blk_load_, PAGE_SIZE)(NANDFlashState *s,
         }
     } else {
         memcpy(s->io, s->storage + PAGE_START(s->addr) +
-                        offset, PAGE_SIZE + OOB_SIZE - offset);
+                        offset, NAND_PAGE_SIZE + OOB_SIZE - offset);
         s->ioaddr = s->io;
     }
 }
 
-static void glue(nand_init_, PAGE_SIZE)(NANDFlashState *s)
+static void glue(nand_init_, NAND_PAGE_SIZE)(NANDFlashState *s)
 {
     s->oob_shift = PAGE_SHIFT - 5;
     s->pages = s->size >> PAGE_SHIFT;
     s->addr_shift = ADDR_SHIFT;
 
-    s->blk_erase = glue(nand_blk_erase_, PAGE_SIZE);
-    s->blk_write = glue(nand_blk_write_, PAGE_SIZE);
-    s->blk_load = glue(nand_blk_load_, PAGE_SIZE);
+    s->blk_erase = glue(nand_blk_erase_, NAND_PAGE_SIZE);
+    s->blk_write = glue(nand_blk_write_, NAND_PAGE_SIZE);
+    s->blk_load = glue(nand_blk_load_, NAND_PAGE_SIZE);
 }
 
-# undef PAGE_SIZE
+# undef NAND_PAGE_SIZE
 # undef PAGE_SHIFT
 # undef PAGE_SECTORS
 # undef ADDR_SHIFT
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v2 5/9] elf2dmp: Rename PAGE_SIZE to ELF2DMP_PAGE_SIZE
  2021-01-18  6:37 [PATCH v2 0/9] Alpine Linux build fix and CI pipeline Jiaxun Yang
                   ` (3 preceding siblings ...)
  2021-01-18  6:38 ` [PATCH v2 4/9] hw/block/nand: Rename PAGE_SIZE to NAND_PAGE_SIZE Jiaxun Yang
@ 2021-01-18  6:38 ` Jiaxun Yang
  2021-01-18  9:44   ` Philippe Mathieu-Daudé
  2021-01-18  6:38 ` [PATCH v2 6/9] tests: Rename PAGE_SIZE definitions Jiaxun Yang
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 31+ messages in thread
From: Jiaxun Yang @ 2021-01-18  6:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Thomas Huth, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf, Jiaxun Yang

As per POSIX specification of limits.h [1], OS libc may define
PAGE_SIZE in limits.h.

To prevent collosion of definition, we rename PAGE_SIZE here.

[1]: https://pubs.opengroup.org/onlinepubs/7908799/xsh/limits.h.html

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 contrib/elf2dmp/addrspace.h |  6 +++---
 contrib/elf2dmp/addrspace.c |  4 ++--
 contrib/elf2dmp/main.c      | 18 +++++++++---------
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/contrib/elf2dmp/addrspace.h b/contrib/elf2dmp/addrspace.h
index d87f6a18c6..00b44c1218 100644
--- a/contrib/elf2dmp/addrspace.h
+++ b/contrib/elf2dmp/addrspace.h
@@ -10,9 +10,9 @@
 
 #include "qemu_elf.h"
 
-#define PAGE_BITS 12
-#define PAGE_SIZE (1ULL << PAGE_BITS)
-#define PFN_MASK (~(PAGE_SIZE - 1))
+#define ELF2DMP_PAGE_BITS 12
+#define ELF2DMP_PAGE_SIZE (1ULL << ELF2DMP_PAGE_BITS)
+#define ELF2DMP_PFN_MASK (~(ELF2DMP_PAGE_SIZE - 1))
 
 #define INVALID_PA  UINT64_MAX
 
diff --git a/contrib/elf2dmp/addrspace.c b/contrib/elf2dmp/addrspace.c
index 8a76069cb5..53ded17061 100644
--- a/contrib/elf2dmp/addrspace.c
+++ b/contrib/elf2dmp/addrspace.c
@@ -207,8 +207,8 @@ int va_space_rw(struct va_space *vs, uint64_t addr,
         void *buf, size_t size, int is_write)
 {
     while (size) {
-        uint64_t page = addr & PFN_MASK;
-        size_t s = (page + PAGE_SIZE) - addr;
+        uint64_t page = addr & ELF2DMP_PFN_MASK;
+        size_t s = (page + ELF2DMP_PAGE_SIZE) - addr;
         void *ptr;
 
         s = (s > size) ? size : s;
diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index ac746e49e0..20b477d582 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -244,8 +244,8 @@ static int fill_header(WinDumpHeader64 *hdr, struct pa_space *ps,
     WinDumpHeader64 h;
     size_t i;
 
-    QEMU_BUILD_BUG_ON(KUSD_OFFSET_SUITE_MASK >= PAGE_SIZE);
-    QEMU_BUILD_BUG_ON(KUSD_OFFSET_PRODUCT_TYPE >= PAGE_SIZE);
+    QEMU_BUILD_BUG_ON(KUSD_OFFSET_SUITE_MASK >= ELF2DMP_PAGE_SIZE);
+    QEMU_BUILD_BUG_ON(KUSD_OFFSET_PRODUCT_TYPE >= ELF2DMP_PAGE_SIZE);
 
     if (!suite_mask || !product_type) {
         return 1;
@@ -281,14 +281,14 @@ static int fill_header(WinDumpHeader64 *hdr, struct pa_space *ps,
     };
 
     for (i = 0; i < ps->block_nr; i++) {
-        h.PhysicalMemoryBlock.NumberOfPages += ps->block[i].size / PAGE_SIZE;
+        h.PhysicalMemoryBlock.NumberOfPages += ps->block[i].size / ELF2DMP_PAGE_SIZE;
         h.PhysicalMemoryBlock.Run[i] = (WinDumpPhyMemRun64) {
-            .BasePage = ps->block[i].paddr / PAGE_SIZE,
-            .PageCount = ps->block[i].size / PAGE_SIZE,
+            .BasePage = ps->block[i].paddr / ELF2DMP_PAGE_SIZE,
+            .PageCount = ps->block[i].size / ELF2DMP_PAGE_SIZE,
         };
     }
 
-    h.RequiredDumpSpace += h.PhysicalMemoryBlock.NumberOfPages << PAGE_BITS;
+    h.RequiredDumpSpace += h.PhysicalMemoryBlock.NumberOfPages << ELF2DMP_PAGE_BITS;
 
     *hdr = h;
 
@@ -379,7 +379,7 @@ static int pe_get_pdb_symstore_hash(uint64_t base, void *start_addr,
     size_t pdb_name_sz;
     size_t i;
 
-    QEMU_BUILD_BUG_ON(sizeof(*dos_hdr) >= PAGE_SIZE);
+    QEMU_BUILD_BUG_ON(sizeof(*dos_hdr) >= ELF2DMP_PAGE_SIZE);
 
     if (memcmp(&dos_hdr->e_magic, e_magic, sizeof(e_magic))) {
         return 1;
@@ -509,10 +509,10 @@ int main(int argc, char *argv[])
     }
     printf("CPU #0 IDT[0] -> 0x%016"PRIx64"\n", idt_desc_addr(first_idt_desc));
 
-    KernBase = idt_desc_addr(first_idt_desc) & ~(PAGE_SIZE - 1);
+    KernBase = idt_desc_addr(first_idt_desc) & ~(ELF2DMP_PAGE_SIZE - 1);
     printf("Searching kernel downwards from 0x%016"PRIx64"...\n", KernBase);
 
-    for (; KernBase >= 0xfffff78000000000; KernBase -= PAGE_SIZE) {
+    for (; KernBase >= 0xfffff78000000000; KernBase -= ELF2DMP_PAGE_SIZE) {
         nt_start_addr = va_space_resolve(&vs, KernBase);
         if (!nt_start_addr) {
             continue;
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v2 6/9] tests: Rename PAGE_SIZE definitions
  2021-01-18  6:37 [PATCH v2 0/9] Alpine Linux build fix and CI pipeline Jiaxun Yang
                   ` (4 preceding siblings ...)
  2021-01-18  6:38 ` [PATCH v2 5/9] elf2dmp: Rename PAGE_SIZE to ELF2DMP_PAGE_SIZE Jiaxun Yang
@ 2021-01-18  6:38 ` Jiaxun Yang
  2021-01-18  9:47   ` Philippe Mathieu-Daudé
  2021-01-18  6:38 ` [PATCH v2 7/9] accel/kvm: avoid using predefined PAGE_SIZE Jiaxun Yang
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 31+ messages in thread
From: Jiaxun Yang @ 2021-01-18  6:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Thomas Huth, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf, Jiaxun Yang

As per POSIX specification of limits.h [1], OS libc may define
PAGE_SIZE in limits.h.

Self defined PAGE_SIZE is frequently used in tests, to prevent
collosion of definition, we give PAGE_SIZE definitons reasonable
prefixs.

[1]: https://pubs.opengroup.org/onlinepubs/7908799/xsh/limits.h.html

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 tests/migration/stress.c            | 10 ++---
 tests/qtest/libqos/malloc-pc.c      |  4 +-
 tests/qtest/libqos/malloc-spapr.c   |  4 +-
 tests/qtest/m25p80-test.c           | 54 +++++++++++-----------
 tests/tcg/multiarch/system/memory.c |  6 +--
 tests/test-xbzrle.c                 | 70 ++++++++++++++---------------
 6 files changed, 74 insertions(+), 74 deletions(-)

diff --git a/tests/migration/stress.c b/tests/migration/stress.c
index de45e8e490..b7240a15c8 100644
--- a/tests/migration/stress.c
+++ b/tests/migration/stress.c
@@ -27,7 +27,7 @@
 
 const char *argv0;
 
-#define PAGE_SIZE 4096
+#define RAM_PAGE_SIZE 4096
 
 #ifndef CONFIG_GETTID
 static int gettid(void)
@@ -158,11 +158,11 @@ static unsigned long long now(void)
 
 static void stressone(unsigned long long ramsizeMB)
 {
-    size_t pagesPerMB = 1024 * 1024 / PAGE_SIZE;
+    size_t pagesPerMB = 1024 * 1024 / RAM_PAGE_SIZE;
     g_autofree char *ram = g_malloc(ramsizeMB * 1024 * 1024);
     char *ramptr;
     size_t i, j, k;
-    g_autofree char *data = g_malloc(PAGE_SIZE);
+    g_autofree char *data = g_malloc(RAM_PAGE_SIZE);
     char *dataptr;
     size_t nMB = 0;
     unsigned long long before, after;
@@ -174,7 +174,7 @@ static void stressone(unsigned long long ramsizeMB)
      * calloc instead :-) */
     memset(ram, 0xfe, ramsizeMB * 1024 * 1024);
 
-    if (random_bytes(data, PAGE_SIZE) < 0) {
+    if (random_bytes(data, RAM_PAGE_SIZE) < 0) {
         return;
     }
 
@@ -186,7 +186,7 @@ static void stressone(unsigned long long ramsizeMB)
         for (i = 0; i < ramsizeMB; i++, nMB++) {
             for (j = 0; j < pagesPerMB; j++) {
                 dataptr = data;
-                for (k = 0; k < PAGE_SIZE; k += sizeof(long long)) {
+                for (k = 0; k < RAM_PAGE_SIZE; k += sizeof(long long)) {
                     ramptr += sizeof(long long);
                     dataptr += sizeof(long long);
                     *(unsigned long long *)ramptr ^= *(unsigned long long *)dataptr;
diff --git a/tests/qtest/libqos/malloc-pc.c b/tests/qtest/libqos/malloc-pc.c
index 16ff9609cc..f1e3b392a5 100644
--- a/tests/qtest/libqos/malloc-pc.c
+++ b/tests/qtest/libqos/malloc-pc.c
@@ -18,7 +18,7 @@
 
 #include "qemu-common.h"
 
-#define PAGE_SIZE (4096)
+#define ALLOC_PAGE_SIZE (4096)
 
 void pc_alloc_init(QGuestAllocator *s, QTestState *qts, QAllocOpts flags)
 {
@@ -26,7 +26,7 @@ void pc_alloc_init(QGuestAllocator *s, QTestState *qts, QAllocOpts flags)
     QFWCFG *fw_cfg = pc_fw_cfg_init(qts);
 
     ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE);
-    alloc_init(s, flags, 1 << 20, MIN(ram_size, 0xE0000000), PAGE_SIZE);
+    alloc_init(s, flags, 1 << 20, MIN(ram_size, 0xE0000000), ALLOC_PAGE_SIZE);
 
     /* clean-up */
     pc_fw_cfg_uninit(fw_cfg);
diff --git a/tests/qtest/libqos/malloc-spapr.c b/tests/qtest/libqos/malloc-spapr.c
index 84862e4876..05b306c191 100644
--- a/tests/qtest/libqos/malloc-spapr.c
+++ b/tests/qtest/libqos/malloc-spapr.c
@@ -10,7 +10,7 @@
 
 #include "qemu-common.h"
 
-#define PAGE_SIZE 4096
+#define SPAPR_PAGE_SIZE 4096
 
 /* Memory must be a multiple of 256 MB,
  * so we have at least 256MB
@@ -19,5 +19,5 @@
 
 void spapr_alloc_init(QGuestAllocator *s, QTestState *qts, QAllocOpts flags)
 {
-    alloc_init(s, flags, 1 << 20, SPAPR_MIN_SIZE, PAGE_SIZE);
+    alloc_init(s, flags, 1 << 20, SPAPR_MIN_SIZE, SPAPR_PAGE_SIZE);
 }
diff --git a/tests/qtest/m25p80-test.c b/tests/qtest/m25p80-test.c
index 50c6b79fb3..f860cef5f0 100644
--- a/tests/qtest/m25p80-test.c
+++ b/tests/qtest/m25p80-test.c
@@ -62,7 +62,7 @@ enum {
 #define FLASH_JEDEC         0x20ba19  /* n25q256a */
 #define FLASH_SIZE          (32 * 1024 * 1024)
 
-#define PAGE_SIZE           256
+#define FLASH_PAGE_SIZE           256
 
 /*
  * Use an explicit bswap for the values read/wrote to the flash region
@@ -165,7 +165,7 @@ static void read_page(uint32_t addr, uint32_t *page)
     writel(ASPEED_FLASH_BASE, make_be32(addr));
 
     /* Continuous read are supported */
-    for (i = 0; i < PAGE_SIZE / 4; i++) {
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
         page[i] = make_be32(readl(ASPEED_FLASH_BASE));
     }
     spi_ctrl_stop_user();
@@ -178,15 +178,15 @@ static void read_page_mem(uint32_t addr, uint32_t *page)
     /* move out USER mode to use direct reads from the AHB bus */
     spi_ctrl_setmode(CTRL_READMODE, READ);
 
-    for (i = 0; i < PAGE_SIZE / 4; i++) {
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
         page[i] = make_be32(readl(ASPEED_FLASH_BASE + addr + i * 4));
     }
 }
 
 static void test_erase_sector(void)
 {
-    uint32_t some_page_addr = 0x600 * PAGE_SIZE;
-    uint32_t page[PAGE_SIZE / 4];
+    uint32_t some_page_addr = 0x600 * FLASH_PAGE_SIZE;
+    uint32_t page[FLASH_PAGE_SIZE / 4];
     int i;
 
     spi_conf(CONF_ENABLE_W0);
@@ -200,14 +200,14 @@ static void test_erase_sector(void)
 
     /* Previous page should be full of zeroes as backend is not
      * initialized */
-    read_page(some_page_addr - PAGE_SIZE, page);
-    for (i = 0; i < PAGE_SIZE / 4; i++) {
+    read_page(some_page_addr - FLASH_PAGE_SIZE, page);
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
         g_assert_cmphex(page[i], ==, 0x0);
     }
 
     /* But this one was erased */
     read_page(some_page_addr, page);
-    for (i = 0; i < PAGE_SIZE / 4; i++) {
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
         g_assert_cmphex(page[i], ==, 0xffffffff);
     }
 
@@ -216,8 +216,8 @@ static void test_erase_sector(void)
 
 static void test_erase_all(void)
 {
-    uint32_t some_page_addr = 0x15000 * PAGE_SIZE;
-    uint32_t page[PAGE_SIZE / 4];
+    uint32_t some_page_addr = 0x15000 * FLASH_PAGE_SIZE;
+    uint32_t page[FLASH_PAGE_SIZE / 4];
     int i;
 
     spi_conf(CONF_ENABLE_W0);
@@ -225,7 +225,7 @@ static void test_erase_all(void)
     /* Check some random page. Should be full of zeroes as backend is
      * not initialized */
     read_page(some_page_addr, page);
-    for (i = 0; i < PAGE_SIZE / 4; i++) {
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
         g_assert_cmphex(page[i], ==, 0x0);
     }
 
@@ -236,7 +236,7 @@ static void test_erase_all(void)
 
     /* Recheck that some random page */
     read_page(some_page_addr, page);
-    for (i = 0; i < PAGE_SIZE / 4; i++) {
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
         g_assert_cmphex(page[i], ==, 0xffffffff);
     }
 
@@ -245,9 +245,9 @@ static void test_erase_all(void)
 
 static void test_write_page(void)
 {
-    uint32_t my_page_addr = 0x14000 * PAGE_SIZE; /* beyond 16MB */
-    uint32_t some_page_addr = 0x15000 * PAGE_SIZE;
-    uint32_t page[PAGE_SIZE / 4];
+    uint32_t my_page_addr = 0x14000 * FLASH_PAGE_SIZE; /* beyond 16MB */
+    uint32_t some_page_addr = 0x15000 * FLASH_PAGE_SIZE;
+    uint32_t page[FLASH_PAGE_SIZE / 4];
     int i;
 
     spi_conf(CONF_ENABLE_W0);
@@ -259,20 +259,20 @@ static void test_write_page(void)
     writel(ASPEED_FLASH_BASE, make_be32(my_page_addr));
 
     /* Fill the page with its own addresses */
-    for (i = 0; i < PAGE_SIZE / 4; i++) {
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
         writel(ASPEED_FLASH_BASE, make_be32(my_page_addr + i * 4));
     }
     spi_ctrl_stop_user();
 
     /* Check what was written */
     read_page(my_page_addr, page);
-    for (i = 0; i < PAGE_SIZE / 4; i++) {
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
         g_assert_cmphex(page[i], ==, my_page_addr + i * 4);
     }
 
     /* Check some other page. It should be full of 0xff */
     read_page(some_page_addr, page);
-    for (i = 0; i < PAGE_SIZE / 4; i++) {
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
         g_assert_cmphex(page[i], ==, 0xffffffff);
     }
 
@@ -281,9 +281,9 @@ static void test_write_page(void)
 
 static void test_read_page_mem(void)
 {
-    uint32_t my_page_addr = 0x14000 * PAGE_SIZE; /* beyond 16MB */
-    uint32_t some_page_addr = 0x15000 * PAGE_SIZE;
-    uint32_t page[PAGE_SIZE / 4];
+    uint32_t my_page_addr = 0x14000 * FLASH_PAGE_SIZE; /* beyond 16MB */
+    uint32_t some_page_addr = 0x15000 * FLASH_PAGE_SIZE;
+    uint32_t page[FLASH_PAGE_SIZE / 4];
     int i;
 
     /* Enable 4BYTE mode for controller. This is should be strapped by
@@ -300,13 +300,13 @@ static void test_read_page_mem(void)
 
     /* Check what was written */
     read_page_mem(my_page_addr, page);
-    for (i = 0; i < PAGE_SIZE / 4; i++) {
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
         g_assert_cmphex(page[i], ==, my_page_addr + i * 4);
     }
 
     /* Check some other page. It should be full of 0xff */
     read_page_mem(some_page_addr, page);
-    for (i = 0; i < PAGE_SIZE / 4; i++) {
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
         g_assert_cmphex(page[i], ==, 0xffffffff);
     }
 
@@ -315,8 +315,8 @@ static void test_read_page_mem(void)
 
 static void test_write_page_mem(void)
 {
-    uint32_t my_page_addr = 0x15000 * PAGE_SIZE;
-    uint32_t page[PAGE_SIZE / 4];
+    uint32_t my_page_addr = 0x15000 * FLASH_PAGE_SIZE;
+    uint32_t page[FLASH_PAGE_SIZE / 4];
     int i;
 
     /* Enable 4BYTE mode for controller. This is should be strapped by
@@ -334,14 +334,14 @@ static void test_write_page_mem(void)
     /* move out USER mode to use direct writes to the AHB bus */
     spi_ctrl_setmode(CTRL_WRITEMODE, PP);
 
-    for (i = 0; i < PAGE_SIZE / 4; i++) {
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
         writel(ASPEED_FLASH_BASE + my_page_addr + i * 4,
                make_be32(my_page_addr + i * 4));
     }
 
     /* Check what was written */
     read_page_mem(my_page_addr, page);
-    for (i = 0; i < PAGE_SIZE / 4; i++) {
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
         g_assert_cmphex(page[i], ==, my_page_addr + i * 4);
     }
 
diff --git a/tests/tcg/multiarch/system/memory.c b/tests/tcg/multiarch/system/memory.c
index d124502d73..eb0ec6f8eb 100644
--- a/tests/tcg/multiarch/system/memory.c
+++ b/tests/tcg/multiarch/system/memory.c
@@ -20,12 +20,12 @@
 # error "Target does not specify CHECK_UNALIGNED"
 #endif
 
-#define PAGE_SIZE 4096             /* nominal 4k "pages" */
-#define TEST_SIZE (PAGE_SIZE * 4)  /* 4 pages */
+#define MEM_PAGE_SIZE 4096             /* nominal 4k "pages" */
+#define TEST_SIZE (MEM_PAGE_SIZE * 4)  /* 4 pages */
 
 #define ARRAY_SIZE(x) ((sizeof(x) / sizeof((x)[0])))
 
-__attribute__((aligned(PAGE_SIZE)))
+__attribute__((aligned(MEM_PAGE_SIZE)))
 static uint8_t test_data[TEST_SIZE];
 
 typedef void (*init_ufn) (int offset);
diff --git a/tests/test-xbzrle.c b/tests/test-xbzrle.c
index f5e08de91e..795d6f1cba 100644
--- a/tests/test-xbzrle.c
+++ b/tests/test-xbzrle.c
@@ -15,7 +15,7 @@
 #include "qemu/cutils.h"
 #include "../migration/xbzrle.h"
 
-#define PAGE_SIZE 4096
+#define XBZRLE_PAGE_SIZE 4096
 
 static void test_uleb(void)
 {
@@ -41,11 +41,11 @@ static void test_uleb(void)
 
 static void test_encode_decode_zero(void)
 {
-    uint8_t *buffer = g_malloc0(PAGE_SIZE);
-    uint8_t *compressed = g_malloc0(PAGE_SIZE);
+    uint8_t *buffer = g_malloc0(XBZRLE_PAGE_SIZE);
+    uint8_t *compressed = g_malloc0(XBZRLE_PAGE_SIZE);
     int i = 0;
     int dlen = 0;
-    int diff_len = g_test_rand_int_range(0, PAGE_SIZE - 1006);
+    int diff_len = g_test_rand_int_range(0, XBZRLE_PAGE_SIZE - 1006);
 
     for (i = diff_len; i > 0; i--) {
         buffer[1000 + i] = i;
@@ -55,8 +55,8 @@ static void test_encode_decode_zero(void)
     buffer[1000 + diff_len + 5] = 105;
 
     /* encode zero page */
-    dlen = xbzrle_encode_buffer(buffer, buffer, PAGE_SIZE, compressed,
-                       PAGE_SIZE);
+    dlen = xbzrle_encode_buffer(buffer, buffer, XBZRLE_PAGE_SIZE, compressed,
+                       XBZRLE_PAGE_SIZE);
     g_assert(dlen == 0);
 
     g_free(buffer);
@@ -65,11 +65,11 @@ static void test_encode_decode_zero(void)
 
 static void test_encode_decode_unchanged(void)
 {
-    uint8_t *compressed = g_malloc0(PAGE_SIZE);
-    uint8_t *test = g_malloc0(PAGE_SIZE);
+    uint8_t *compressed = g_malloc0(XBZRLE_PAGE_SIZE);
+    uint8_t *test = g_malloc0(XBZRLE_PAGE_SIZE);
     int i = 0;
     int dlen = 0;
-    int diff_len = g_test_rand_int_range(0, PAGE_SIZE - 1006);
+    int diff_len = g_test_rand_int_range(0, XBZRLE_PAGE_SIZE - 1006);
 
     for (i = diff_len; i > 0; i--) {
         test[1000 + i] = i + 4;
@@ -79,8 +79,8 @@ static void test_encode_decode_unchanged(void)
     test[1000 + diff_len + 5] = 109;
 
     /* test unchanged buffer */
-    dlen = xbzrle_encode_buffer(test, test, PAGE_SIZE, compressed,
-                                PAGE_SIZE);
+    dlen = xbzrle_encode_buffer(test, test, XBZRLE_PAGE_SIZE, compressed,
+                                XBZRLE_PAGE_SIZE);
     g_assert(dlen == 0);
 
     g_free(test);
@@ -89,21 +89,21 @@ static void test_encode_decode_unchanged(void)
 
 static void test_encode_decode_1_byte(void)
 {
-    uint8_t *buffer = g_malloc0(PAGE_SIZE);
-    uint8_t *test = g_malloc0(PAGE_SIZE);
-    uint8_t *compressed = g_malloc(PAGE_SIZE);
+    uint8_t *buffer = g_malloc0(XBZRLE_PAGE_SIZE);
+    uint8_t *test = g_malloc0(XBZRLE_PAGE_SIZE);
+    uint8_t *compressed = g_malloc(XBZRLE_PAGE_SIZE);
     int dlen = 0, rc = 0;
     uint8_t buf[2];
 
-    test[PAGE_SIZE - 1] = 1;
+    test[XBZRLE_PAGE_SIZE - 1] = 1;
 
-    dlen = xbzrle_encode_buffer(buffer, test, PAGE_SIZE, compressed,
-                       PAGE_SIZE);
+    dlen = xbzrle_encode_buffer(buffer, test, XBZRLE_PAGE_SIZE, compressed,
+                       XBZRLE_PAGE_SIZE);
     g_assert(dlen == (uleb128_encode_small(&buf[0], 4095) + 2));
 
-    rc = xbzrle_decode_buffer(compressed, dlen, buffer, PAGE_SIZE);
-    g_assert(rc == PAGE_SIZE);
-    g_assert(memcmp(test, buffer, PAGE_SIZE) == 0);
+    rc = xbzrle_decode_buffer(compressed, dlen, buffer, XBZRLE_PAGE_SIZE);
+    g_assert(rc == XBZRLE_PAGE_SIZE);
+    g_assert(memcmp(test, buffer, XBZRLE_PAGE_SIZE) == 0);
 
     g_free(buffer);
     g_free(compressed);
@@ -112,18 +112,18 @@ static void test_encode_decode_1_byte(void)
 
 static void test_encode_decode_overflow(void)
 {
-    uint8_t *compressed = g_malloc0(PAGE_SIZE);
-    uint8_t *test = g_malloc0(PAGE_SIZE);
-    uint8_t *buffer = g_malloc0(PAGE_SIZE);
+    uint8_t *compressed = g_malloc0(XBZRLE_PAGE_SIZE);
+    uint8_t *test = g_malloc0(XBZRLE_PAGE_SIZE);
+    uint8_t *buffer = g_malloc0(XBZRLE_PAGE_SIZE);
     int i = 0, rc = 0;
 
-    for (i = 0; i < PAGE_SIZE / 2 - 1; i++) {
+    for (i = 0; i < XBZRLE_PAGE_SIZE / 2 - 1; i++) {
         test[i * 2] = 1;
     }
 
     /* encode overflow */
-    rc = xbzrle_encode_buffer(buffer, test, PAGE_SIZE, compressed,
-                              PAGE_SIZE);
+    rc = xbzrle_encode_buffer(buffer, test, XBZRLE_PAGE_SIZE, compressed,
+                              XBZRLE_PAGE_SIZE);
     g_assert(rc == -1);
 
     g_free(buffer);
@@ -133,13 +133,13 @@ static void test_encode_decode_overflow(void)
 
 static void encode_decode_range(void)
 {
-    uint8_t *buffer = g_malloc0(PAGE_SIZE);
-    uint8_t *compressed = g_malloc(PAGE_SIZE);
-    uint8_t *test = g_malloc0(PAGE_SIZE);
+    uint8_t *buffer = g_malloc0(XBZRLE_PAGE_SIZE);
+    uint8_t *compressed = g_malloc(XBZRLE_PAGE_SIZE);
+    uint8_t *test = g_malloc0(XBZRLE_PAGE_SIZE);
     int i = 0, rc = 0;
     int dlen = 0;
 
-    int diff_len = g_test_rand_int_range(0, PAGE_SIZE - 1006);
+    int diff_len = g_test_rand_int_range(0, XBZRLE_PAGE_SIZE - 1006);
 
     for (i = diff_len; i > 0; i--) {
         buffer[1000 + i] = i;
@@ -153,12 +153,12 @@ static void encode_decode_range(void)
     test[1000 + diff_len + 5] = 109;
 
     /* test encode/decode */
-    dlen = xbzrle_encode_buffer(test, buffer, PAGE_SIZE, compressed,
-                                PAGE_SIZE);
+    dlen = xbzrle_encode_buffer(test, buffer, XBZRLE_PAGE_SIZE, compressed,
+                                XBZRLE_PAGE_SIZE);
 
-    rc = xbzrle_decode_buffer(compressed, dlen, test, PAGE_SIZE);
-    g_assert(rc < PAGE_SIZE);
-    g_assert(memcmp(test, buffer, PAGE_SIZE) == 0);
+    rc = xbzrle_decode_buffer(compressed, dlen, test, XBZRLE_PAGE_SIZE);
+    g_assert(rc < XBZRLE_PAGE_SIZE);
+    g_assert(memcmp(test, buffer, XBZRLE_PAGE_SIZE) == 0);
 
     g_free(buffer);
     g_free(compressed);
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v2 7/9] accel/kvm: avoid using predefined PAGE_SIZE
  2021-01-18  6:37 [PATCH v2 0/9] Alpine Linux build fix and CI pipeline Jiaxun Yang
                   ` (5 preceding siblings ...)
  2021-01-18  6:38 ` [PATCH v2 6/9] tests: Rename PAGE_SIZE definitions Jiaxun Yang
@ 2021-01-18  6:38 ` Jiaxun Yang
  2021-01-18  6:38 ` [PATCH v2 8/9] tests/docker: Add dockerfile for Alpine Linux Jiaxun Yang
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 31+ messages in thread
From: Jiaxun Yang @ 2021-01-18  6:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Thomas Huth, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf, Jiaxun Yang

As per POSIX specification of limits.h [1], OS libc may define
PAGE_SIZE in limits.h.

PAGE_SIZE is used in included kernel uapi headers.

To prevent collosion of definition, we discard PAGE_SIZE from
defined by libc and take QEMU's variable.

[1]: https://pubs.opengroup.org/onlinepubs/7908799/xsh/limits.h.html

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 accel/kvm/kvm-all.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 389eaace72..3feb17d965 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -58,6 +58,9 @@
 /* KVM uses PAGE_SIZE in its definition of KVM_COALESCED_MMIO_MAX. We
  * need to use the real host PAGE_SIZE, as that's what KVM will use.
  */
+#ifdef PAGE_SIZE
+#undef PAGE_SIZE
+#endif
 #define PAGE_SIZE qemu_real_host_page_size
 
 //#define DEBUG_KVM
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v2 8/9] tests/docker: Add dockerfile for Alpine Linux
  2021-01-18  6:37 [PATCH v2 0/9] Alpine Linux build fix and CI pipeline Jiaxun Yang
                   ` (6 preceding siblings ...)
  2021-01-18  6:38 ` [PATCH v2 7/9] accel/kvm: avoid using predefined PAGE_SIZE Jiaxun Yang
@ 2021-01-18  6:38 ` Jiaxun Yang
  2021-01-18 10:33   ` Daniel P. Berrangé
  2021-01-18  6:38 ` [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline Jiaxun Yang
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 31+ messages in thread
From: Jiaxun Yang @ 2021-01-18  6:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Thomas Huth, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf, Jiaxun Yang

Alpine Linux[1] is a security-oriented, lightweight Linux distribution
based on musl libc and busybox.

It it popular among Docker guests and embedded applications.

Adding it to test against different libc.

[1]: https://alpinelinux.org/

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 tests/docker/dockerfiles/alpine.docker | 57 ++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 tests/docker/dockerfiles/alpine.docker

diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker
new file mode 100644
index 0000000000..5be5198d00
--- /dev/null
+++ b/tests/docker/dockerfiles/alpine.docker
@@ -0,0 +1,57 @@
+
+FROM alpine:edge
+
+RUN apk update
+RUN apk upgrade
+
+# Please keep this list sorted alphabetically
+ENV PACKAGES \
+	alsa-lib-dev \
+	bash \
+	bison \
+	build-base \
+	coreutils \
+	curl-dev \
+	flex \
+	git \
+	glib-dev \
+	glib-static \
+	gnutls-dev \
+	gtk+3.0-dev \
+	libaio-dev \
+	libcap-dev \
+	libcap-ng-dev \
+	libjpeg-turbo-dev \
+	libnfs-dev \
+	libpng-dev \
+	libseccomp-dev \
+	libssh-dev \
+	libusb-dev \
+	libxml2-dev \
+	linux-headers \
+	lzo-dev \
+	mesa-dev \
+	mesa-egl \
+	mesa-gbm \
+	meson \
+	ncurses-dev \
+	ninja \
+	paxmark \
+	perl \
+	pulseaudio-dev \
+	python3 \
+	py3-sphinx \
+	shadow \
+	snappy-dev \
+	spice-dev \
+	texinfo \
+	usbredir-dev \
+	util-linux-dev \
+	vde2-dev \
+	virglrenderer-dev \
+	vte3-dev \
+	xfsprogs-dev \
+	zlib-dev \
+	zlib-static
+
+RUN apk add $PACKAGES
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline
  2021-01-18  6:37 [PATCH v2 0/9] Alpine Linux build fix and CI pipeline Jiaxun Yang
                   ` (7 preceding siblings ...)
  2021-01-18  6:38 ` [PATCH v2 8/9] tests/docker: Add dockerfile for Alpine Linux Jiaxun Yang
@ 2021-01-18  6:38 ` Jiaxun Yang
  2021-01-18 10:11   ` Daniel P. Berrangé
  2021-01-18  7:06 ` [PATCH v2 0/9] Alpine Linux build fix and CI pipeline no-reply
  2021-01-18  8:02 ` Thomas Huth
  10 siblings, 1 reply; 31+ messages in thread
From: Jiaxun Yang @ 2021-01-18  6:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Thomas Huth, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf, Jiaxun Yang

We only run build test and check-acceptance as their are too many
failures in checks due to minor string mismatch.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 .gitlab-ci.d/containers.yml |  5 +++++
 .gitlab-ci.yml              | 23 +++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/.gitlab-ci.d/containers.yml b/.gitlab-ci.d/containers.yml
index 910754a699..90fac85ce4 100644
--- a/.gitlab-ci.d/containers.yml
+++ b/.gitlab-ci.d/containers.yml
@@ -28,6 +28,11 @@
     - if: '$CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH'
     - if: '$CI_COMMIT_REF_NAME == "testing/next"'
 
+amd64-alpine-container:
+  <<: *container_job_definition
+  variables:
+    NAME: alpine
+
 amd64-centos7-container:
   <<: *container_job_definition
   variables:
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 4532f1718a..6cc922aedb 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -72,6 +72,29 @@ include:
     - cd build
     - du -chs ${CI_PROJECT_DIR}/avocado-cache
 
+build-system-alpine:
+  <<: *native_build_job_definition
+  variables:
+    IMAGE: alpine
+    TARGETS: aarch64-softmmu alpha-softmmu cris-softmmu hppa-softmmu
+      moxie-softmmu microblazeel-softmmu mips64el-softmmu
+    MAKE_CHECK_ARGS: check-build
+    CONFIGURE_ARGS: --enable-docs
+  artifacts:
+    expire_in: 2 days
+    paths:
+      - build
+
+acceptance-system-alpine:
+  <<: *native_test_job_definition
+  needs:
+    - job: build-system-alpine
+      artifacts: true
+  variables:
+    IMAGE: alpine
+    MAKE_CHECK_ARGS: check-acceptance
+  <<: *acceptance_definition
+
 build-system-ubuntu:
   <<: *native_build_job_definition
   variables:
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 0/9] Alpine Linux build fix and CI pipeline
  2021-01-18  6:37 [PATCH v2 0/9] Alpine Linux build fix and CI pipeline Jiaxun Yang
                   ` (8 preceding siblings ...)
  2021-01-18  6:38 ` [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline Jiaxun Yang
@ 2021-01-18  7:06 ` no-reply
  2021-01-18  8:02 ` Thomas Huth
  10 siblings, 0 replies; 31+ messages in thread
From: no-reply @ 2021-01-18  7:06 UTC (permalink / raw)
  To: jiaxun.yang
  Cc: qemu-devel, fam, lvivier, thuth, viktor.prutyanov, kvm,
	alex.bennee, alistair, groug, wainersm, mreitz, qemu-ppc, kwolf,
	pbonzini, qemu-block, philmd, david

Patchew URL: https://patchew.org/QEMU/20210118063808.12471-1-jiaxun.yang@flygoat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210118063808.12471-1-jiaxun.yang@flygoat.com
Subject: [PATCH v2 0/9] Alpine Linux build fix and CI pipeline

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210118063808.12471-1-jiaxun.yang@flygoat.com -> patchew/20210118063808.12471-1-jiaxun.yang@flygoat.com
 * [new tag]         patchew/20210118065627.79903-1-ganqixin@huawei.com -> patchew/20210118065627.79903-1-ganqixin@huawei.com
Switched to a new branch 'test'
eeef33a gitlab-ci: Add alpine to pipeline
85ea588 tests/docker: Add dockerfile for Alpine Linux
9f5e3ca accel/kvm: avoid using predefined PAGE_SIZE
0292db5 tests: Rename PAGE_SIZE definitions
fdcacb9 elf2dmp: Rename PAGE_SIZE to ELF2DMP_PAGE_SIZE
9ff36c5 hw/block/nand: Rename PAGE_SIZE to NAND_PAGE_SIZE
b096605 osdep.h: Remove <sys/signal.h> include
9e5e67d libvhost-user: Include poll.h instead of sys/poll.h
c20bc72 configure: Add sys/timex.h to probe clock_adjtime

=== OUTPUT BEGIN ===
1/9 Checking commit c20bc7253f10 (configure: Add sys/timex.h to probe clock_adjtime)
2/9 Checking commit 9e5e67d01f91 (libvhost-user: Include poll.h instead of sys/poll.h)
3/9 Checking commit b0966056d77e (osdep.h: Remove <sys/signal.h> include)
4/9 Checking commit 9ff36c58081b (hw/block/nand: Rename PAGE_SIZE to NAND_PAGE_SIZE)
ERROR: code indent should never use tabs
#27: FILE: hw/block/nand.c:118:
+# define PAGE_START(page)^I(PAGE(page) * (NAND_PAGE_SIZE + OOB_SIZE))$

ERROR: code indent should never use tabs
#47: FILE: hw/block/nand.c:135:
+# define NAND_PAGE_SIZE^I^I2048$

WARNING: line over 80 characters
#66: FILE: hw/block/nand.c:675:
+        mem_and(iobuf + (soff | off), s->io, MIN(s->iolen, NAND_PAGE_SIZE - off));

WARNING: line over 80 characters
#71: FILE: hw/block/nand.c:678:
+            mem_and(s->storage + (page << OOB_SHIFT), s->io + NAND_PAGE_SIZE - off,

total: 2 errors, 2 warnings, 120 lines checked

Patch 4/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/9 Checking commit fdcacb94e921 (elf2dmp: Rename PAGE_SIZE to ELF2DMP_PAGE_SIZE)
WARNING: line over 80 characters
#70: FILE: contrib/elf2dmp/main.c:284:
+        h.PhysicalMemoryBlock.NumberOfPages += ps->block[i].size / ELF2DMP_PAGE_SIZE;

WARNING: line over 80 characters
#80: FILE: contrib/elf2dmp/main.c:291:
+    h.RequiredDumpSpace += h.PhysicalMemoryBlock.NumberOfPages << ELF2DMP_PAGE_BITS;

total: 0 errors, 2 warnings, 70 lines checked

Patch 5/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/9 Checking commit 0292db5a571a (tests: Rename PAGE_SIZE definitions)
7/9 Checking commit 9f5e3ca96943 (accel/kvm: avoid using predefined PAGE_SIZE)
8/9 Checking commit 85ea5884f78d (tests/docker: Add dockerfile for Alpine Linux)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#20: 
new file mode 100644

total: 0 errors, 1 warnings, 57 lines checked

Patch 8/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/9 Checking commit eeef33a77edf (gitlab-ci: Add alpine to pipeline)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210118063808.12471-1-jiaxun.yang@flygoat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 0/9] Alpine Linux build fix and CI pipeline
  2021-01-18  6:37 [PATCH v2 0/9] Alpine Linux build fix and CI pipeline Jiaxun Yang
                   ` (9 preceding siblings ...)
  2021-01-18  7:06 ` [PATCH v2 0/9] Alpine Linux build fix and CI pipeline no-reply
@ 2021-01-18  8:02 ` Thomas Huth
  10 siblings, 0 replies; 31+ messages in thread
From: Thomas Huth @ 2021-01-18  8:02 UTC (permalink / raw)
  To: Jiaxun Yang, qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf

On 18/01/2021 07.37, Jiaxun Yang wrote:
> Alpine Linux is a security-oriented, lightweight Linux distribution
> based on musl libc and busybox.
> 
> It it popular among Docker guests and embedded applications.
> 
> Adding it to test against different libc.
> 
> Patches pending review at v2 are: 7, 8, 9
> 
> Tree avilable at: https://gitlab.com/FlyGoat/qemu/-/commits/alpine_linux_v2
> CI All green: https://gitlab.com/FlyGoat/qemu/-/pipelines/242003288
> 
> It is known to have checkpatch complains about identation but they're
> all pre-existing issues as I'm only doing string replacement.
> 
> v2:
>   - Reoreder patches (Wainer)
>   - Add shadow to dockerfile (Wainer)
>   - Pickup proper signal.h fix (PMM)
>   - Correct clock_adjtime title (Thomas Huth)
>   - Collect review tags
> 
> Jiaxun Yang (8):
>    configure: Add sys/timex.h to probe clock_adjtime
>    libvhost-user: Include poll.h instead of sys/poll.h
>    hw/block/nand: Rename PAGE_SIZE to NAND_PAGE_SIZE
>    elf2dmp: Rename PAGE_SIZE to ELF2DMP_PAGE_SIZE
>    tests: Rename PAGE_SIZE definitions
>    accel/kvm: avoid using predefined PAGE_SIZE
>    tests/docker: Add dockerfile for Alpine Linux
>    gitlab-ci: Add alpine to pipeline
> 
> Michael Forney (1):
>    osdep.h: Remove <sys/signal.h> include
> 
>   configure                                 |  1 +
>   meson.build                               |  1 -
>   contrib/elf2dmp/addrspace.h               |  6 +-
>   include/qemu/osdep.h                      |  4 --
>   subprojects/libvhost-user/libvhost-user.h |  2 +-
>   accel/kvm/kvm-all.c                       |  3 +
>   contrib/elf2dmp/addrspace.c               |  4 +-
>   contrib/elf2dmp/main.c                    | 18 +++---
>   hw/block/nand.c                           | 40 ++++++-------
>   tests/migration/stress.c                  | 10 ++--
>   tests/qtest/libqos/malloc-pc.c            |  4 +-
>   tests/qtest/libqos/malloc-spapr.c         |  4 +-
>   tests/qtest/m25p80-test.c                 | 54 ++++++++---------
>   tests/tcg/multiarch/system/memory.c       |  6 +-
>   tests/test-xbzrle.c                       | 70 +++++++++++------------
>   .gitlab-ci.d/containers.yml               |  5 ++
>   .gitlab-ci.yml                            | 23 ++++++++
>   tests/docker/dockerfiles/alpine.docker    | 57 ++++++++++++++++++
>   18 files changed, 198 insertions(+), 114 deletions(-)
>   create mode 100644 tests/docker/dockerfiles/alpine.docker
> 

Thanks! I'll take this through my testing-next branch:

  https://gitlab.com/huth/qemu/-/commits/testing-next/

  Thomas


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 2/9] libvhost-user: Include poll.h instead of sys/poll.h
  2021-01-18  6:38 ` [PATCH v2 2/9] libvhost-user: Include poll.h instead of sys/poll.h Jiaxun Yang
@ 2021-01-18  9:41   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-18  9:41 UTC (permalink / raw)
  To: Jiaxun Yang, qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Thomas Huth, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf

On 1/18/21 7:38 AM, Jiaxun Yang wrote:
> Musl libc complains about it's wrong usage.
> 
> In file included from ../subprojects/libvhost-user/libvhost-user.h:20,
>                  from ../subprojects/libvhost-user/libvhost-user-glib.h:19,
>                  from ../subprojects/libvhost-user/libvhost-user-glib.c:15:
> /usr/include/sys/poll.h:1:2: error: #warning redirecting incorrect #include <sys/poll.h> to <poll.h> [-Werror=cpp]
>     1 | #warning redirecting incorrect #include <sys/poll.h> to <poll.h>
>       |  ^~~~~~~
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>  subprojects/libvhost-user/libvhost-user.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 3/9] osdep.h: Remove <sys/signal.h> include
  2021-01-18  6:38 ` [PATCH v2 3/9] osdep.h: Remove <sys/signal.h> include Jiaxun Yang
@ 2021-01-18  9:41   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-18  9:41 UTC (permalink / raw)
  To: Jiaxun Yang, qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Thomas Huth, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf, Michael Forney,
	Eric Blake

On 1/18/21 7:38 AM, Jiaxun Yang wrote:
> From: Michael Forney <mforney@mforney.org>
> 
> Prior to 2a4b472c3c, sys/signal.h was only included on OpenBSD
> (apart from two .c files). The POSIX standard location for this
> header is just <signal.h> and in fact, OpenBSD's signal.h includes
> sys/signal.h itself.
> 
> Unconditionally including <sys/signal.h> on musl causes warnings
> for just about every source file:
> 
>   /usr/include/sys/signal.h:1:2: warning: #warning redirecting incorrect #include <sys/signal.h> to <signal.h> [-Wcpp]
>       1 | #warning redirecting incorrect #include <sys/signal.h> to <signal.h>
>         |  ^~~~~~~
> 
> Since there don't seem to be any platforms which require including
> <sys/signal.h> in addition to <signal.h>, and some platforms like
> Haiku lack it completely, just remove it.
> 
> Tested building on OpenBSD after removing this include.
> 
> Signed-off-by: Michael Forney <mforney@mforney.org>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> [jiaxun.yang@flygoat.com: Move to meson]
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  meson.build          | 1 -
>  include/qemu/osdep.h | 4 ----
>  2 files changed, 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 4/9] hw/block/nand: Rename PAGE_SIZE to NAND_PAGE_SIZE
  2021-01-18  6:38 ` [PATCH v2 4/9] hw/block/nand: Rename PAGE_SIZE to NAND_PAGE_SIZE Jiaxun Yang
@ 2021-01-18  9:44   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-18  9:44 UTC (permalink / raw)
  To: Jiaxun Yang, qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Thomas Huth, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf

On 1/18/21 7:38 AM, Jiaxun Yang wrote:
> As per POSIX specification of limits.h [1], OS libc may define
> PAGE_SIZE in limits.h.
> 
> To prevent collosion of definition, we rename PAGE_SIZE here.
> 
> [1]: https://pubs.opengroup.org/onlinepubs/7908799/xsh/limits.h.html
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/block/nand.c | 40 ++++++++++++++++++++--------------------
>  1 file changed, 20 insertions(+), 20 deletions(-)
...

> -# define PAGE_SIZE		256
> +# define NAND_PAGE_SIZE     256
>  # define PAGE_SHIFT		8
>  # define PAGE_SECTORS		1
>  # define ADDR_SHIFT		8
>  # include "nand.c"

Why not rename all SIZE/SHIFT/SECTORS at once, to avoid
having half NAND and half generic names?


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 5/9] elf2dmp: Rename PAGE_SIZE to ELF2DMP_PAGE_SIZE
  2021-01-18  6:38 ` [PATCH v2 5/9] elf2dmp: Rename PAGE_SIZE to ELF2DMP_PAGE_SIZE Jiaxun Yang
@ 2021-01-18  9:44   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-18  9:44 UTC (permalink / raw)
  To: Jiaxun Yang, qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Thomas Huth, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf

On 1/18/21 7:38 AM, Jiaxun Yang wrote:
> As per POSIX specification of limits.h [1], OS libc may define
> PAGE_SIZE in limits.h.
> 
> To prevent collosion of definition, we rename PAGE_SIZE here.
> 
> [1]: https://pubs.opengroup.org/onlinepubs/7908799/xsh/limits.h.html
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>  contrib/elf2dmp/addrspace.h |  6 +++---
>  contrib/elf2dmp/addrspace.c |  4 ++--
>  contrib/elf2dmp/main.c      | 18 +++++++++---------
>  3 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/contrib/elf2dmp/addrspace.h b/contrib/elf2dmp/addrspace.h
> index d87f6a18c6..00b44c1218 100644
> --- a/contrib/elf2dmp/addrspace.h
> +++ b/contrib/elf2dmp/addrspace.h
> @@ -10,9 +10,9 @@
>  
>  #include "qemu_elf.h"
>  
> -#define PAGE_BITS 12
> -#define PAGE_SIZE (1ULL << PAGE_BITS)
> -#define PFN_MASK (~(PAGE_SIZE - 1))
> +#define ELF2DMP_PAGE_BITS 12
> +#define ELF2DMP_PAGE_SIZE (1ULL << ELF2DMP_PAGE_BITS)
> +#define ELF2DMP_PFN_MASK (~(ELF2DMP_PAGE_SIZE - 1))

Here you renamed all definitions, better.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 6/9] tests: Rename PAGE_SIZE definitions
  2021-01-18  6:38 ` [PATCH v2 6/9] tests: Rename PAGE_SIZE definitions Jiaxun Yang
@ 2021-01-18  9:47   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-18  9:47 UTC (permalink / raw)
  To: Jiaxun Yang, qemu-devel
  Cc: David Gibson, qemu-ppc, Greg Kurz, Max Reitz, kvm,
	Wainer dos Santos Moschetta, Paolo Bonzini, Fam Zheng,
	Viktor Prutyanov, Alistair Francis, Thomas Huth, Laurent Vivier,
	Alex Bennée, qemu-block, Kevin Wolf

On 1/18/21 7:38 AM, Jiaxun Yang wrote:
> As per POSIX specification of limits.h [1], OS libc may define
> PAGE_SIZE in limits.h.
> 
> Self defined PAGE_SIZE is frequently used in tests, to prevent
> collosion of definition, we give PAGE_SIZE definitons reasonable
> prefixs.
> 
> [1]: https://pubs.opengroup.org/onlinepubs/7908799/xsh/limits.h.html
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/migration/stress.c            | 10 ++---
>  tests/qtest/libqos/malloc-pc.c      |  4 +-
>  tests/qtest/libqos/malloc-spapr.c   |  4 +-
>  tests/qtest/m25p80-test.c           | 54 +++++++++++-----------
>  tests/tcg/multiarch/system/memory.c |  6 +--
>  tests/test-xbzrle.c                 | 70 ++++++++++++++---------------
>  6 files changed, 74 insertions(+), 74 deletions(-)
...

> diff --git a/tests/tcg/multiarch/system/memory.c b/tests/tcg/multiarch/system/memory.c
> index d124502d73..eb0ec6f8eb 100644
> --- a/tests/tcg/multiarch/system/memory.c
> +++ b/tests/tcg/multiarch/system/memory.c
> @@ -20,12 +20,12 @@
>  # error "Target does not specify CHECK_UNALIGNED"
>  #endif
>  
> -#define PAGE_SIZE 4096             /* nominal 4k "pages" */
> -#define TEST_SIZE (PAGE_SIZE * 4)  /* 4 pages */
> +#define MEM_PAGE_SIZE 4096             /* nominal 4k "pages" */
> +#define TEST_SIZE (MEM_PAGE_SIZE * 4)  /* 4 pages */

Clearer renaming TEST_PAGE_SIZE and TEST_MEM_SIZE.

If possible using TEST_PAGE_SIZE / TEST_MEM_SIZE:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>  
>  #define ARRAY_SIZE(x) ((sizeof(x) / sizeof((x)[0])))
>  
> -__attribute__((aligned(PAGE_SIZE)))
> +__attribute__((aligned(MEM_PAGE_SIZE)))
>  static uint8_t test_data[TEST_SIZE];


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline
  2021-01-18  6:38 ` [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline Jiaxun Yang
@ 2021-01-18 10:11   ` Daniel P. Berrangé
  2021-01-18 10:22     ` Thomas Huth
  2021-01-18 13:37     ` Jiaxun Yang
  0 siblings, 2 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2021-01-18 10:11 UTC (permalink / raw)
  To: Jiaxun Yang
  Cc: qemu-devel, Fam Zheng, Laurent Vivier, Thomas Huth,
	Viktor Prutyanov, kvm, Alex Bennée, Alistair Francis,
	Greg Kurz, Wainer dos Santos Moschetta, Max Reitz, qemu-ppc,
	Kevin Wolf, Paolo Bonzini, qemu-block,
	Philippe Mathieu-Daudé,
	David Gibson

On Mon, Jan 18, 2021 at 02:38:08PM +0800, Jiaxun Yang wrote:
> We only run build test and check-acceptance as their are too many
> failures in checks due to minor string mismatch.

Can you give real examples of what's broken here, as that sounds
rather suspicious, and I'm not convinced it should be ignored.

> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  .gitlab-ci.d/containers.yml |  5 +++++
>  .gitlab-ci.yml              | 23 +++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/.gitlab-ci.d/containers.yml b/.gitlab-ci.d/containers.yml
> index 910754a699..90fac85ce4 100644
> --- a/.gitlab-ci.d/containers.yml
> +++ b/.gitlab-ci.d/containers.yml
> @@ -28,6 +28,11 @@
>      - if: '$CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH'
>      - if: '$CI_COMMIT_REF_NAME == "testing/next"'
>  
> +amd64-alpine-container:
> +  <<: *container_job_definition
> +  variables:
> +    NAME: alpine
> +
>  amd64-centos7-container:
>    <<: *container_job_definition
>    variables:
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 4532f1718a..6cc922aedb 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -72,6 +72,29 @@ include:
>      - cd build
>      - du -chs ${CI_PROJECT_DIR}/avocado-cache
>  
> +build-system-alpine:
> +  <<: *native_build_job_definition
> +  variables:
> +    IMAGE: alpine
> +    TARGETS: aarch64-softmmu alpha-softmmu cris-softmmu hppa-softmmu
> +      moxie-softmmu microblazeel-softmmu mips64el-softmmu
> +    MAKE_CHECK_ARGS: check-build
> +    CONFIGURE_ARGS: --enable-docs
> +  artifacts:
> +    expire_in: 2 days
> +    paths:
> +      - build
> +
> +acceptance-system-alpine:
> +  <<: *native_test_job_definition
> +  needs:
> +    - job: build-system-alpine
> +      artifacts: true
> +  variables:
> +    IMAGE: alpine
> +    MAKE_CHECK_ARGS: check-acceptance
> +  <<: *acceptance_definition
> +
>  build-system-ubuntu:
>    <<: *native_build_job_definition
>    variables:
> -- 
> 2.30.0
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline
  2021-01-18 10:11   ` Daniel P. Berrangé
@ 2021-01-18 10:22     ` Thomas Huth
  2021-01-18 10:26       ` Daniel P. Berrangé
  2021-01-18 13:37     ` Jiaxun Yang
  1 sibling, 1 reply; 31+ messages in thread
From: Thomas Huth @ 2021-01-18 10:22 UTC (permalink / raw)
  To: Daniel P. Berrangé, Jiaxun Yang
  Cc: qemu-devel, Fam Zheng, Laurent Vivier, Viktor Prutyanov, kvm,
	Alex Bennée, Alistair Francis, Greg Kurz,
	Wainer dos Santos Moschetta, Max Reitz, qemu-ppc, Kevin Wolf,
	Paolo Bonzini, qemu-block, Philippe Mathieu-Daudé,
	David Gibson

On 18/01/2021 11.11, Daniel P. Berrangé wrote:
> On Mon, Jan 18, 2021 at 02:38:08PM +0800, Jiaxun Yang wrote:
>> We only run build test and check-acceptance as their are too many
>> failures in checks due to minor string mismatch.
> 
> Can you give real examples of what's broken here, as that sounds
> rather suspicious, and I'm not convinced it should be ignored.

I haven't tried, but I guess it's the "check-block" iotests that are likely 
failing with a different libc, since they do string comparison on the 
textual output of the tests. If that's the case, it would maybe be still ok 
to run "check-qtest" and "check-union" on Alpine instead of the whole 
"check" test suite.

  Thomas


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline
  2021-01-18 10:22     ` Thomas Huth
@ 2021-01-18 10:26       ` Daniel P. Berrangé
  0 siblings, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2021-01-18 10:26 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Jiaxun Yang, Fam Zheng, Laurent Vivier, qemu-block, kvm,
	Viktor Prutyanov, Philippe Mathieu-Daudé,
	Alistair Francis, qemu-devel, Wainer dos Santos Moschetta,
	Greg Kurz, qemu-ppc, Paolo Bonzini, Kevin Wolf, Max Reitz,
	Alex Bennée, David Gibson

On Mon, Jan 18, 2021 at 11:22:47AM +0100, Thomas Huth wrote:
> On 18/01/2021 11.11, Daniel P. Berrangé wrote:
> > On Mon, Jan 18, 2021 at 02:38:08PM +0800, Jiaxun Yang wrote:
> > > We only run build test and check-acceptance as their are too many
> > > failures in checks due to minor string mismatch.
> > 
> > Can you give real examples of what's broken here, as that sounds
> > rather suspicious, and I'm not convinced it should be ignored.
> 
> I haven't tried, but I guess it's the "check-block" iotests that are likely
> failing with a different libc, since they do string comparison on the
> textual output of the tests. If that's the case, it would maybe be still ok
> to run "check-qtest" and "check-union" on Alpine instead of the whole
> "check" test suite.

Perhaps errno strings are different due to libc hitting block tests. I
would expect this to be explained in the commit message though with
example, so we have a clear record of why we needed to disable this.

It might indicate a need to change our test suite to be more robust
in this area, because that would also suggest the tests might fail
on  non-Linux.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 8/9] tests/docker: Add dockerfile for Alpine Linux
  2021-01-18  6:38 ` [PATCH v2 8/9] tests/docker: Add dockerfile for Alpine Linux Jiaxun Yang
@ 2021-01-18 10:33   ` Daniel P. Berrangé
  2021-01-19 13:41     ` Thomas Huth
  0 siblings, 1 reply; 31+ messages in thread
From: Daniel P. Berrangé @ 2021-01-18 10:33 UTC (permalink / raw)
  To: Jiaxun Yang
  Cc: qemu-devel, Fam Zheng, Laurent Vivier, Thomas Huth,
	Viktor Prutyanov, kvm, Alex Bennée, Alistair Francis,
	Greg Kurz, Wainer dos Santos Moschetta, Max Reitz, qemu-ppc,
	Kevin Wolf, Paolo Bonzini, qemu-block,
	Philippe Mathieu-Daudé,
	David Gibson

On Mon, Jan 18, 2021 at 02:38:07PM +0800, Jiaxun Yang wrote:
> Alpine Linux[1] is a security-oriented, lightweight Linux distribution
> based on musl libc and busybox.
> 
> It it popular among Docker guests and embedded applications.
> 
> Adding it to test against different libc.
> 
> [1]: https://alpinelinux.org/
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  tests/docker/dockerfiles/alpine.docker | 57 ++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
>  create mode 100644 tests/docker/dockerfiles/alpine.docker
> 
> diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker
> new file mode 100644
> index 0000000000..5be5198d00
> --- /dev/null
> +++ b/tests/docker/dockerfiles/alpine.docker
> @@ -0,0 +1,57 @@
> +
> +FROM alpine:edge
> +
> +RUN apk update
> +RUN apk upgrade
> +
> +# Please keep this list sorted alphabetically
> +ENV PACKAGES \
> +	alsa-lib-dev \
> +	bash \
> +	bison \

This shouldn't be required.

> +	build-base \

This seems to be a meta packae that pulls in other
misc toolchain packages. Please list the pieces we
need explicitly instead.

> +	coreutils \
> +	curl-dev \
> +	flex \

This shouldn't be needed.

> +	git \
> +	glib-dev \
> +	glib-static \
> +	gnutls-dev \
> +	gtk+3.0-dev \
> +	libaio-dev \
> +	libcap-dev \

Should not be required, as we use cap-ng.

> +	libcap-ng-dev \
> +	libjpeg-turbo-dev \
> +	libnfs-dev \
> +	libpng-dev \
> +	libseccomp-dev \
> +	libssh-dev \
> +	libusb-dev \
> +	libxml2-dev \
> +	linux-headers \

Is this really needed ? We don't install kernel-headers on other
distros AFAICT.

> +	lzo-dev \
> +	mesa-dev \
> +	mesa-egl \
> +	mesa-gbm \
> +	meson \
> +	ncurses-dev \
> +	ninja \
> +	paxmark \

What is this needed for ?

> +	perl \
> +	pulseaudio-dev \
> +	python3 \
> +	py3-sphinx \
> +	shadow \

Is this really needed ?

> +	snappy-dev \
> +	spice-dev \
> +	texinfo \
> +	usbredir-dev \
> +	util-linux-dev \
> +	vde2-dev \
> +	virglrenderer-dev \
> +	vte3-dev \
> +	xfsprogs-dev \
> +	zlib-dev \
> +	zlib-static
> +
> +RUN apk add $PACKAGES

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline
  2021-01-18 10:11   ` Daniel P. Berrangé
  2021-01-18 10:22     ` Thomas Huth
@ 2021-01-18 13:37     ` Jiaxun Yang
  2021-01-18 14:44       ` Thomas Huth
  1 sibling, 1 reply; 31+ messages in thread
From: Jiaxun Yang @ 2021-01-18 13:37 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: BALATON Zoltan via, Fam Zheng, Laurent Vivier, Thomas Huth,
	Viktor Prutyanov, kvm, Alex Bennée, Alistair Francis,
	Greg Kurz, Wainer dos Santos Moschetta, Max Reitz, qemu-ppc,
	Kevin Wolf, Paolo Bonzini, qemu-block,
	Philippe Mathieu-Daudé,
	David Gibson



On Mon, Jan 18, 2021, at 6:11 PM, Daniel P. Berrangé wrote:
> On Mon, Jan 18, 2021 at 02:38:08PM +0800, Jiaxun Yang wrote:
> > We only run build test and check-acceptance as their are too many
> > failures in checks due to minor string mismatch.
> 
> Can you give real examples of what's broken here, as that sounds
> rather suspicious, and I'm not convinced it should be ignored.

Mostly Input/Output error vs I/O Error.

- Jiaxun

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline
  2021-01-18 13:37     ` Jiaxun Yang
@ 2021-01-18 14:44       ` Thomas Huth
  2021-01-18 14:50         ` Daniel P. Berrangé
  0 siblings, 1 reply; 31+ messages in thread
From: Thomas Huth @ 2021-01-18 14:44 UTC (permalink / raw)
  To: Jiaxun Yang, Daniel P. Berrangé
  Cc: BALATON Zoltan via, Fam Zheng, Laurent Vivier, Viktor Prutyanov,
	kvm, Alex Bennée, Alistair Francis, Greg Kurz,
	Wainer dos Santos Moschetta, Max Reitz, qemu-ppc, Kevin Wolf,
	Paolo Bonzini, qemu-block, Philippe Mathieu-Daudé,
	David Gibson

On 18/01/2021 14.37, Jiaxun Yang wrote:
> 
> 
> On Mon, Jan 18, 2021, at 6:11 PM, Daniel P. Berrangé wrote:
>> On Mon, Jan 18, 2021 at 02:38:08PM +0800, Jiaxun Yang wrote:
>>> We only run build test and check-acceptance as their are too many
>>> failures in checks due to minor string mismatch.
>>
>> Can you give real examples of what's broken here, as that sounds
>> rather suspicious, and I'm not convinced it should be ignored.
> 
> Mostly Input/Output error vs I/O Error.

Right, out of curiosity, I also gave it a try:

  https://gitlab.com/huth/qemu/-/jobs/969225330

Apart from the "I/O Error" vs. "Input/Output Error" difference, there also 
seems to be a problem with "sed" in some of the tests.

Jiaxun, I think you could simply add a job like this instead:

check-system-alpine:
   <<: *native_test_job_definition
   needs:
     - job: build-system-alpine
       artifacts: true
   variables:
     IMAGE: alpine
     MAKE_CHECK_ARGS: check-qtest check-unit check-qapi-schema check-softfloat

That will run all the other tests, without the problematic check-block part.

  Thomas


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline
  2021-01-18 14:44       ` Thomas Huth
@ 2021-01-18 14:50         ` Daniel P. Berrangé
  2021-01-18 15:12           ` Thomas Huth
  0 siblings, 1 reply; 31+ messages in thread
From: Daniel P. Berrangé @ 2021-01-18 14:50 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Jiaxun Yang, BALATON Zoltan via, Fam Zheng, Laurent Vivier,
	Viktor Prutyanov, kvm, Alex Bennée, Alistair Francis,
	Greg Kurz, Wainer dos Santos Moschetta, Max Reitz, qemu-ppc,
	Kevin Wolf, Paolo Bonzini, qemu-block,
	Philippe Mathieu-Daudé,
	David Gibson

On Mon, Jan 18, 2021 at 03:44:49PM +0100, Thomas Huth wrote:
> On 18/01/2021 14.37, Jiaxun Yang wrote:
> > 
> > 
> > On Mon, Jan 18, 2021, at 6:11 PM, Daniel P. Berrangé wrote:
> > > On Mon, Jan 18, 2021 at 02:38:08PM +0800, Jiaxun Yang wrote:
> > > > We only run build test and check-acceptance as their are too many
> > > > failures in checks due to minor string mismatch.
> > > 
> > > Can you give real examples of what's broken here, as that sounds
> > > rather suspicious, and I'm not convinced it should be ignored.
> > 
> > Mostly Input/Output error vs I/O Error.
> 
> Right, out of curiosity, I also gave it a try:
> 
>  https://gitlab.com/huth/qemu/-/jobs/969225330
> 
> Apart from the "I/O Error" vs. "Input/Output Error" difference, there also
> seems to be a problem with "sed" in some of the tests.

The "sed" thing sounds like something that ought to be investigated
from a portability POV rather than ignored.

More worrying is the fact that there's a segv in there too when
running qemu-img, which does not give me confidence in use of
it with musl.



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline
  2021-01-18 14:50         ` Daniel P. Berrangé
@ 2021-01-18 15:12           ` Thomas Huth
  2021-01-19 11:49             ` Thomas Huth
  0 siblings, 1 reply; 31+ messages in thread
From: Thomas Huth @ 2021-01-18 15:12 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Fam Zheng, Laurent Vivier, qemu-block, Viktor Prutyanov, kvm,
	Philippe Mathieu-Daudé,
	Alistair Francis, BALATON Zoltan via, Greg Kurz, qemu-ppc,
	Wainer dos Santos Moschetta, Paolo Bonzini, Kevin Wolf,
	Max Reitz, Alex Bennée, David Gibson

On 18/01/2021 15.50, Daniel P. Berrangé wrote:
> On Mon, Jan 18, 2021 at 03:44:49PM +0100, Thomas Huth wrote:
>> On 18/01/2021 14.37, Jiaxun Yang wrote:
>>>
>>>
>>> On Mon, Jan 18, 2021, at 6:11 PM, Daniel P. Berrangé wrote:
>>>> On Mon, Jan 18, 2021 at 02:38:08PM +0800, Jiaxun Yang wrote:
>>>>> We only run build test and check-acceptance as their are too many
>>>>> failures in checks due to minor string mismatch.
>>>>
>>>> Can you give real examples of what's broken here, as that sounds
>>>> rather suspicious, and I'm not convinced it should be ignored.
>>>
>>> Mostly Input/Output error vs I/O Error.
>>
>> Right, out of curiosity, I also gave it a try:
>>
>>   https://gitlab.com/huth/qemu/-/jobs/969225330
>>
>> Apart from the "I/O Error" vs. "Input/Output Error" difference, there also
>> seems to be a problem with "sed" in some of the tests.
> 
> The "sed" thing sounds like something that ought to be investigated
> from a portability POV rather than ignored.

The weird thing is that we explicitly test for GNU sed in 
tests/check-block.sh and skip the iotests if it's not available... so I'm a 
little bit surprised that the iotests are run here with an apparently 
different version of sed...?

  Thomas


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline
  2021-01-18 15:12           ` Thomas Huth
@ 2021-01-19 11:49             ` Thomas Huth
  0 siblings, 0 replies; 31+ messages in thread
From: Thomas Huth @ 2021-01-19 11:49 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Fam Zheng, Laurent Vivier, qemu-block, Viktor Prutyanov,
	Alex Bennée, Alistair Francis, BALATON Zoltan via,
	Wainer dos Santos Moschetta, Greg Kurz, Max Reitz, qemu-ppc, kvm,
	Paolo Bonzini, Kevin Wolf, Philippe Mathieu-Daudé,
	David Gibson

On 18/01/2021 16.12, Thomas Huth wrote:
> On 18/01/2021 15.50, Daniel P. Berrangé wrote:
>> On Mon, Jan 18, 2021 at 03:44:49PM +0100, Thomas Huth wrote:
>>> On 18/01/2021 14.37, Jiaxun Yang wrote:
>>>>
>>>>
>>>> On Mon, Jan 18, 2021, at 6:11 PM, Daniel P. Berrangé wrote:
>>>>> On Mon, Jan 18, 2021 at 02:38:08PM +0800, Jiaxun Yang wrote:
>>>>>> We only run build test and check-acceptance as their are too many
>>>>>> failures in checks due to minor string mismatch.
>>>>>
>>>>> Can you give real examples of what's broken here, as that sounds
>>>>> rather suspicious, and I'm not convinced it should be ignored.
>>>>
>>>> Mostly Input/Output error vs I/O Error.
>>>
>>> Right, out of curiosity, I also gave it a try:
>>>
>>>   https://gitlab.com/huth/qemu/-/jobs/969225330
>>>
>>> Apart from the "I/O Error" vs. "Input/Output Error" difference, there also
>>> seems to be a problem with "sed" in some of the tests.
>>
>> The "sed" thing sounds like something that ought to be investigated
>> from a portability POV rather than ignored.
> 
> The weird thing is that we explicitly test for GNU sed in 
> tests/check-block.sh and skip the iotests if it's not available... so I'm a 
> little bit surprised that the iotests are run here with an apparently 
> different version of sed...?

Oh, well, I've fired up a bootable ISO image of Alpine, and ran "sed 
--version" and it says:

  This is not GNU sed version 4.0

Ouch. But I guess we could add a check for that to our tests/check-block.sh 
script, too...

  Thomas


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 8/9] tests/docker: Add dockerfile for Alpine Linux
  2021-01-18 10:33   ` Daniel P. Berrangé
@ 2021-01-19 13:41     ` Thomas Huth
  2021-01-19 14:18       ` Daniel P. Berrangé
  2021-01-26 21:38       ` John Snow
  0 siblings, 2 replies; 31+ messages in thread
From: Thomas Huth @ 2021-01-19 13:41 UTC (permalink / raw)
  To: Daniel P. Berrangé, Jiaxun Yang
  Cc: Fam Zheng, Laurent Vivier, kvm, Viktor Prutyanov, qemu-block,
	Philippe Mathieu-Daudé,
	Alistair Francis, qemu-devel, Wainer dos Santos Moschetta,
	Greg Kurz, qemu-ppc, Paolo Bonzini, Kevin Wolf, Max Reitz,
	Alex Bennée, David Gibson

On 18/01/2021 11.33, Daniel P. Berrangé wrote:
> On Mon, Jan 18, 2021 at 02:38:07PM +0800, Jiaxun Yang wrote:
>> Alpine Linux[1] is a security-oriented, lightweight Linux distribution
>> based on musl libc and busybox.
>>
>> It it popular among Docker guests and embedded applications.
>>
>> Adding it to test against different libc.
>>
>> [1]: https://alpinelinux.org/
>>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>   tests/docker/dockerfiles/alpine.docker | 57 ++++++++++++++++++++++++++
>>   1 file changed, 57 insertions(+)
>>   create mode 100644 tests/docker/dockerfiles/alpine.docker
>>
>> diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker
>> new file mode 100644
>> index 0000000000..5be5198d00
>> --- /dev/null
>> +++ b/tests/docker/dockerfiles/alpine.docker
>> @@ -0,0 +1,57 @@
>> +
>> +FROM alpine:edge
>> +
>> +RUN apk update
>> +RUN apk upgrade
>> +
>> +# Please keep this list sorted alphabetically
>> +ENV PACKAGES \
>> +	alsa-lib-dev \
>> +	bash \
>> +	bison \
> 
> This shouldn't be required.

bison and flex were required to avoid some warnings in the past while 
compiling the dtc submodule ... but I thought we got rid of the problem at 
one point in time, so this can be removed now, indeed.

>> +	build-base \
> 
> This seems to be a meta packae that pulls in other
> misc toolchain packages. Please list the pieces we
> need explicitly instead.

Looking at the "Depends" list on 
https://pkgs.alpinelinux.org/package/v3.3/main/x86/build-base there are only 
6 dependencies and we need most of those for QEMU anyway, so I think it is 
ok to keep build-base here.

>> +	coreutils \
>> +	curl-dev \
>> +	flex \
> 
> This shouldn't be needed.
> 
>> +	git \
>> +	glib-dev \
>> +	glib-static \
>> +	gnutls-dev \
>> +	gtk+3.0-dev \
>> +	libaio-dev \
>> +	libcap-dev \
> 
> Should not be required, as we use cap-ng.

Right.

>> +	libcap-ng-dev \
>> +	libjpeg-turbo-dev \
>> +	libnfs-dev \
>> +	libpng-dev \
>> +	libseccomp-dev \
>> +	libssh-dev \
>> +	libusb-dev \
>> +	libxml2-dev \
>> +	linux-headers \
> 
> Is this really needed ? We don't install kernel-headers on other
> distros AFAICT.

I tried a build without this package, and it works fine indeed.

>> +	lzo-dev \
>> +	mesa-dev \
>> +	mesa-egl \
>> +	mesa-gbm \
>> +	meson \
>> +	ncurses-dev \
>> +	ninja \
>> +	paxmark \
> 
> What is this needed for ?

Seems like it also can be dropped.

>> +	perl \
>> +	pulseaudio-dev \
>> +	python3 \
>> +	py3-sphinx \
>> +	shadow \
> 
> Is this really needed ?

See:
https://www.spinics.net/lists/kvm/msg231556.html

I can remove the superfluous packages when picking up the patch, no need to 
respin just because of this.

  Thomas


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 8/9] tests/docker: Add dockerfile for Alpine Linux
  2021-01-19 13:41     ` Thomas Huth
@ 2021-01-19 14:18       ` Daniel P. Berrangé
  2021-01-26 21:38       ` John Snow
  1 sibling, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2021-01-19 14:18 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Jiaxun Yang, Fam Zheng, Laurent Vivier, kvm, Viktor Prutyanov,
	qemu-block, Philippe Mathieu-Daudé,
	Alistair Francis, qemu-devel, Wainer dos Santos Moschetta,
	Greg Kurz, qemu-ppc, Paolo Bonzini, Kevin Wolf, Max Reitz,
	Alex Bennée, David Gibson

On Tue, Jan 19, 2021 at 02:41:47PM +0100, Thomas Huth wrote:
> On 18/01/2021 11.33, Daniel P. Berrangé wrote:
> > On Mon, Jan 18, 2021 at 02:38:07PM +0800, Jiaxun Yang wrote:
> > > Alpine Linux[1] is a security-oriented, lightweight Linux distribution
> > > based on musl libc and busybox.
> > > 
> > > It it popular among Docker guests and embedded applications.
> > > 
> > > Adding it to test against different libc.
> > > 
> > > [1]: https://alpinelinux.org/
> > > 
> > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> > > ---
> > >   tests/docker/dockerfiles/alpine.docker | 57 ++++++++++++++++++++++++++
> > >   1 file changed, 57 insertions(+)
> > >   create mode 100644 tests/docker/dockerfiles/alpine.docker
> > > 
> > > diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker
> > > new file mode 100644
> > > index 0000000000..5be5198d00
> > > --- /dev/null
> > > +++ b/tests/docker/dockerfiles/alpine.docker
> > > @@ -0,0 +1,57 @@
> > > +
> > > +FROM alpine:edge
> > > +
> > > +RUN apk update
> > > +RUN apk upgrade
> > > +
> > > +# Please keep this list sorted alphabetically
> > > +ENV PACKAGES \
> > > +	alsa-lib-dev \
> > > +	bash \
> > > +	bison \
> > 
> > This shouldn't be required.
> 
> bison and flex were required to avoid some warnings in the past while
> compiling the dtc submodule ... but I thought we got rid of the problem at
> one point in time, so this can be removed now, indeed.
> 
> > > +	build-base \
> > 
> > This seems to be a meta packae that pulls in other
> > misc toolchain packages. Please list the pieces we
> > need explicitly instead.
> 
> Looking at the "Depends" list on
> https://pkgs.alpinelinux.org/package/v3.3/main/x86/build-base there are only
> 6 dependencies and we need most of those for QEMU anyway, so I think it is
> ok to keep build-base here.

I would like to keep them all explicit, as it makes it easier for us to
ensure that we have provided the same set of dependancies across all our
dockerfiles. Ideally we'll add Alpiine to libvirt-ci, so that we can then
auto-generate this dockerfile in future.


> > > +	perl \
> > > +	pulseaudio-dev \
> > > +	python3 \
> > > +	py3-sphinx \
> > > +	shadow \
> > 
> > Is this really needed ?
> 
> See:
> https://www.spinics.net/lists/kvm/msg231556.html

Ok, so this is required by the docker.py commands running extra tools.

The other dockerfiles are working just by luck, and we should make this
package expicit on all of them too


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 8/9] tests/docker: Add dockerfile for Alpine Linux
  2021-01-19 13:41     ` Thomas Huth
  2021-01-19 14:18       ` Daniel P. Berrangé
@ 2021-01-26 21:38       ` John Snow
  2021-01-27  9:58         ` Daniel P. Berrangé
  1 sibling, 1 reply; 31+ messages in thread
From: John Snow @ 2021-01-26 21:38 UTC (permalink / raw)
  To: Thomas Huth, Daniel P. Berrangé, Jiaxun Yang
  Cc: Fam Zheng, Laurent Vivier, Viktor Prutyanov, qemu-block,
	Alex Bennée, Alistair Francis, qemu-devel,
	Wainer dos Santos Moschetta, Greg Kurz, Max Reitz, qemu-ppc, kvm,
	Paolo Bonzini, Kevin Wolf, Philippe Mathieu-Daudé,
	David Gibson

On 1/19/21 8:41 AM, Thomas Huth wrote:
> On 18/01/2021 11.33, Daniel P. Berrangé wrote:
>> On Mon, Jan 18, 2021 at 02:38:07PM +0800, Jiaxun Yang wrote:
>>> Alpine Linux[1] is a security-oriented, lightweight Linux distribution
>>> based on musl libc and busybox.
>>>
>>> It it popular among Docker guests and embedded applications.
>>>
>>> Adding it to test against different libc.
>>>
>>> [1]: https://alpinelinux.org/
>>>
>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>> ---
>>>   tests/docker/dockerfiles/alpine.docker | 57 ++++++++++++++++++++++++++
>>>   1 file changed, 57 insertions(+)
>>>   create mode 100644 tests/docker/dockerfiles/alpine.docker
>>>
>>> diff --git a/tests/docker/dockerfiles/alpine.docker 
>>> b/tests/docker/dockerfiles/alpine.docker
>>> new file mode 100644
>>> index 0000000000..5be5198d00
>>> --- /dev/null
>>> +++ b/tests/docker/dockerfiles/alpine.docker
>>> @@ -0,0 +1,57 @@
>>> +
>>> +FROM alpine:edge
>>> +
>>> +RUN apk update
>>> +RUN apk upgrade
>>> +
>>> +# Please keep this list sorted alphabetically
>>> +ENV PACKAGES \
>>> +    alsa-lib-dev \
>>> +    bash \
>>> +    bison \
>>
>> This shouldn't be required.
> 
> bison and flex were required to avoid some warnings in the past while 
> compiling the dtc submodule ... but I thought we got rid of the problem 
> at one point in time, so this can be removed now, indeed.
> 
>>> +    build-base \
>>
>> This seems to be a meta packae that pulls in other
>> misc toolchain packages. Please list the pieces we
>> need explicitly instead.
> 
> Looking at the "Depends" list on 
> https://pkgs.alpinelinux.org/package/v3.3/main/x86/build-base there are 
> only 6 dependencies and we need most of those for QEMU anyway, so I 
> think it is ok to keep build-base here.
> 
>>> +    coreutils \
>>> +    curl-dev \
>>> +    flex \
>>
>> This shouldn't be needed.
>>
>>> +    git \
>>> +    glib-dev \
>>> +    glib-static \
>>> +    gnutls-dev \
>>> +    gtk+3.0-dev \
>>> +    libaio-dev \
>>> +    libcap-dev \
>>
>> Should not be required, as we use cap-ng.
> 
> Right.
> 
>>> +    libcap-ng-dev \
>>> +    libjpeg-turbo-dev \
>>> +    libnfs-dev \
>>> +    libpng-dev \
>>> +    libseccomp-dev \
>>> +    libssh-dev \
>>> +    libusb-dev \
>>> +    libxml2-dev \
>>> +    linux-headers \
>>
>> Is this really needed ? We don't install kernel-headers on other
>> distros AFAICT.
> 
> I tried a build without this package, and it works fine indeed.
> 
>>> +    lzo-dev \
>>> +    mesa-dev \
>>> +    mesa-egl \
>>> +    mesa-gbm \
>>> +    meson \
>>> +    ncurses-dev \
>>> +    ninja \
>>> +    paxmark \
>>
>> What is this needed for ?
> 
> Seems like it also can be dropped.
> 
>>> +    perl \
>>> +    pulseaudio-dev \
>>> +    python3 \
>>> +    py3-sphinx \
>>> +    shadow \
>>
>> Is this really needed ?
> 
> See:
> https://www.spinics.net/lists/kvm/msg231556.html
> 
> I can remove the superfluous packages when picking up the patch, no need 
> to respin just because of this.
> 
>   Thomas
> 
> 

You can refer to my post earlier this January for a "minimal" Alpine 
Linux build, if you wish.

My goal was to find the smallest set of packages possible without 
passing any explicit configure flags.

I wonder if it's worth having layered "core build" and "test build" 
images so that we can smoke test the minimalistic build from time to 
time -- I seem to recall Dan posting information about a dependency 
management tool for Dockerfiles, but I admit I didn't look too closely 
at what problem that solves, exactly.

--js


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 8/9] tests/docker: Add dockerfile for Alpine Linux
  2021-01-26 21:38       ` John Snow
@ 2021-01-27  9:58         ` Daniel P. Berrangé
  2021-01-27 19:22           ` John Snow
  0 siblings, 1 reply; 31+ messages in thread
From: Daniel P. Berrangé @ 2021-01-27  9:58 UTC (permalink / raw)
  To: John Snow
  Cc: Thomas Huth, Jiaxun Yang, Fam Zheng, Laurent Vivier,
	Viktor Prutyanov, qemu-block, Alex Bennée, Alistair Francis,
	qemu-devel, Wainer dos Santos Moschetta, Greg Kurz, Max Reitz,
	qemu-ppc, kvm, Paolo Bonzini, Kevin Wolf,
	Philippe Mathieu-Daudé,
	David Gibson

On Tue, Jan 26, 2021 at 04:38:57PM -0500, John Snow wrote:
> On 1/19/21 8:41 AM, Thomas Huth wrote:
> > On 18/01/2021 11.33, Daniel P. Berrangé wrote:
> > > On Mon, Jan 18, 2021 at 02:38:07PM +0800, Jiaxun Yang wrote:
> > > > Alpine Linux[1] is a security-oriented, lightweight Linux distribution
> > > > based on musl libc and busybox.
> > > > 
> > > > It it popular among Docker guests and embedded applications.
> > > > 
> > > > Adding it to test against different libc.
> > > > 
> > > > [1]: https://alpinelinux.org/
> > > > 
> > > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> > > > ---
> > > >   tests/docker/dockerfiles/alpine.docker | 57 ++++++++++++++++++++++++++
> > > >   1 file changed, 57 insertions(+)
> > > >   create mode 100644 tests/docker/dockerfiles/alpine.docker
> > > > 
> > > > diff --git a/tests/docker/dockerfiles/alpine.docker
> > > > b/tests/docker/dockerfiles/alpine.docker
> > > > new file mode 100644
> > > > index 0000000000..5be5198d00
> > > > --- /dev/null
> > > > +++ b/tests/docker/dockerfiles/alpine.docker
> > > > @@ -0,0 +1,57 @@
> > > > +
> > > > +FROM alpine:edge
> > > > +
> > > > +RUN apk update
> > > > +RUN apk upgrade
> > > > +
> > > > +# Please keep this list sorted alphabetically
> > > > +ENV PACKAGES \
> > > > +    alsa-lib-dev \
> > > > +    bash \
> > > > +    bison \
> > > 
> > > This shouldn't be required.
> > 
> > bison and flex were required to avoid some warnings in the past while
> > compiling the dtc submodule ... but I thought we got rid of the problem
> > at one point in time, so this can be removed now, indeed.
> > 
> > > > +    build-base \
> > > 
> > > This seems to be a meta packae that pulls in other
> > > misc toolchain packages. Please list the pieces we
> > > need explicitly instead.
> > 
> > Looking at the "Depends" list on
> > https://pkgs.alpinelinux.org/package/v3.3/main/x86/build-base there are
> > only 6 dependencies and we need most of those for QEMU anyway, so I
> > think it is ok to keep build-base here.
> > 
> > > > +    coreutils \
> > > > +    curl-dev \
> > > > +    flex \
> > > 
> > > This shouldn't be needed.
> > > 
> > > > +    git \
> > > > +    glib-dev \
> > > > +    glib-static \
> > > > +    gnutls-dev \
> > > > +    gtk+3.0-dev \
> > > > +    libaio-dev \
> > > > +    libcap-dev \
> > > 
> > > Should not be required, as we use cap-ng.
> > 
> > Right.
> > 
> > > > +    libcap-ng-dev \
> > > > +    libjpeg-turbo-dev \
> > > > +    libnfs-dev \
> > > > +    libpng-dev \
> > > > +    libseccomp-dev \
> > > > +    libssh-dev \
> > > > +    libusb-dev \
> > > > +    libxml2-dev \
> > > > +    linux-headers \
> > > 
> > > Is this really needed ? We don't install kernel-headers on other
> > > distros AFAICT.
> > 
> > I tried a build without this package, and it works fine indeed.
> > 
> > > > +    lzo-dev \
> > > > +    mesa-dev \
> > > > +    mesa-egl \
> > > > +    mesa-gbm \
> > > > +    meson \
> > > > +    ncurses-dev \
> > > > +    ninja \
> > > > +    paxmark \
> > > 
> > > What is this needed for ?
> > 
> > Seems like it also can be dropped.
> > 
> > > > +    perl \
> > > > +    pulseaudio-dev \
> > > > +    python3 \
> > > > +    py3-sphinx \
> > > > +    shadow \
> > > 
> > > Is this really needed ?
> > 
> > See:
> > https://www.spinics.net/lists/kvm/msg231556.html
> > 
> > I can remove the superfluous packages when picking up the patch, no need
> > to respin just because of this.
> > 
> >   Thomas
> > 
> > 
> 
> You can refer to my post earlier this January for a "minimal" Alpine Linux
> build, if you wish.
> 
> My goal was to find the smallest set of packages possible without passing
> any explicit configure flags.
> 
> I wonder if it's worth having layered "core build" and "test build" images
> so that we can smoke test the minimalistic build from time to time -- I seem
> to recall Dan posting information about a dependency management tool for
> Dockerfiles, but I admit I didn't look too closely at what problem that
> solves, exactly.

I'd rather we avoid layered images entirely as it creates extra stages
in the gitlab pipeline, and also makes it more complex to auto-generate.

Once this initial alpine image is merged, then I intend to add it to the
set which are auto-generated from my other patch series.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 8/9] tests/docker: Add dockerfile for Alpine Linux
  2021-01-27  9:58         ` Daniel P. Berrangé
@ 2021-01-27 19:22           ` John Snow
  0 siblings, 0 replies; 31+ messages in thread
From: John Snow @ 2021-01-27 19:22 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Thomas Huth, Jiaxun Yang, Fam Zheng, Laurent Vivier,
	Viktor Prutyanov, qemu-block, Alex Bennée, Alistair Francis,
	qemu-devel, Wainer dos Santos Moschetta, Greg Kurz, Max Reitz,
	qemu-ppc, kvm, Paolo Bonzini, Kevin Wolf,
	Philippe Mathieu-Daudé,
	David Gibson

On 1/27/21 4:58 AM, Daniel P. Berrangé wrote:
> On Tue, Jan 26, 2021 at 04:38:57PM -0500, John Snow wrote:
>> On 1/19/21 8:41 AM, Thomas Huth wrote:
>>> On 18/01/2021 11.33, Daniel P. Berrangé wrote:
>>>> On Mon, Jan 18, 2021 at 02:38:07PM +0800, Jiaxun Yang wrote:
>>>>> Alpine Linux[1] is a security-oriented, lightweight Linux distribution
>>>>> based on musl libc and busybox.
>>>>>
>>>>> It it popular among Docker guests and embedded applications.
>>>>>
>>>>> Adding it to test against different libc.
>>>>>
>>>>> [1]: https://alpinelinux.org/
>>>>>
>>>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>>>> ---
>>>>>    tests/docker/dockerfiles/alpine.docker | 57 ++++++++++++++++++++++++++
>>>>>    1 file changed, 57 insertions(+)
>>>>>    create mode 100644 tests/docker/dockerfiles/alpine.docker
>>>>>
>>>>> diff --git a/tests/docker/dockerfiles/alpine.docker
>>>>> b/tests/docker/dockerfiles/alpine.docker
>>>>> new file mode 100644
>>>>> index 0000000000..5be5198d00
>>>>> --- /dev/null
>>>>> +++ b/tests/docker/dockerfiles/alpine.docker
>>>>> @@ -0,0 +1,57 @@
>>>>> +
>>>>> +FROM alpine:edge
>>>>> +
>>>>> +RUN apk update
>>>>> +RUN apk upgrade
>>>>> +
>>>>> +# Please keep this list sorted alphabetically
>>>>> +ENV PACKAGES \
>>>>> +    alsa-lib-dev \
>>>>> +    bash \
>>>>> +    bison \
>>>>
>>>> This shouldn't be required.
>>>
>>> bison and flex were required to avoid some warnings in the past while
>>> compiling the dtc submodule ... but I thought we got rid of the problem
>>> at one point in time, so this can be removed now, indeed.
>>>
>>>>> +    build-base \
>>>>
>>>> This seems to be a meta packae that pulls in other
>>>> misc toolchain packages. Please list the pieces we
>>>> need explicitly instead.
>>>
>>> Looking at the "Depends" list on
>>> https://pkgs.alpinelinux.org/package/v3.3/main/x86/build-base there are
>>> only 6 dependencies and we need most of those for QEMU anyway, so I
>>> think it is ok to keep build-base here.
>>>
>>>>> +    coreutils \
>>>>> +    curl-dev \
>>>>> +    flex \
>>>>
>>>> This shouldn't be needed.
>>>>
>>>>> +    git \
>>>>> +    glib-dev \
>>>>> +    glib-static \
>>>>> +    gnutls-dev \
>>>>> +    gtk+3.0-dev \
>>>>> +    libaio-dev \
>>>>> +    libcap-dev \
>>>>
>>>> Should not be required, as we use cap-ng.
>>>
>>> Right.
>>>
>>>>> +    libcap-ng-dev \
>>>>> +    libjpeg-turbo-dev \
>>>>> +    libnfs-dev \
>>>>> +    libpng-dev \
>>>>> +    libseccomp-dev \
>>>>> +    libssh-dev \
>>>>> +    libusb-dev \
>>>>> +    libxml2-dev \
>>>>> +    linux-headers \
>>>>
>>>> Is this really needed ? We don't install kernel-headers on other
>>>> distros AFAICT.
>>>
>>> I tried a build without this package, and it works fine indeed.
>>>
>>>>> +    lzo-dev \
>>>>> +    mesa-dev \
>>>>> +    mesa-egl \
>>>>> +    mesa-gbm \
>>>>> +    meson \
>>>>> +    ncurses-dev \
>>>>> +    ninja \
>>>>> +    paxmark \
>>>>
>>>> What is this needed for ?
>>>
>>> Seems like it also can be dropped.
>>>
>>>>> +    perl \
>>>>> +    pulseaudio-dev \
>>>>> +    python3 \
>>>>> +    py3-sphinx \
>>>>> +    shadow \
>>>>
>>>> Is this really needed ?
>>>
>>> See:
>>> https://www.spinics.net/lists/kvm/msg231556.html
>>>
>>> I can remove the superfluous packages when picking up the patch, no need
>>> to respin just because of this.
>>>
>>>    Thomas
>>>
>>>
>>
>> You can refer to my post earlier this January for a "minimal" Alpine Linux
>> build, if you wish.
>>
>> My goal was to find the smallest set of packages possible without passing
>> any explicit configure flags.
>>
>> I wonder if it's worth having layered "core build" and "test build" images
>> so that we can smoke test the minimalistic build from time to time -- I seem
>> to recall Dan posting information about a dependency management tool for
>> Dockerfiles, but I admit I didn't look too closely at what problem that
>> solves, exactly.
> 
> I'd rather we avoid layered images entirely as it creates extra stages
> in the gitlab pipeline, and also makes it more complex to auto-generate.
> 
> Once this initial alpine image is merged, then I intend to add it to the
> set which are auto-generated from my other patch series.
> 

OK, happy to defer to you here for best practices.

--js


^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2021-01-27 19:25 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-18  6:37 [PATCH v2 0/9] Alpine Linux build fix and CI pipeline Jiaxun Yang
2021-01-18  6:38 ` [PATCH v2 1/9] configure: Add sys/timex.h to probe clock_adjtime Jiaxun Yang
2021-01-18  6:38 ` [PATCH v2 2/9] libvhost-user: Include poll.h instead of sys/poll.h Jiaxun Yang
2021-01-18  9:41   ` Philippe Mathieu-Daudé
2021-01-18  6:38 ` [PATCH v2 3/9] osdep.h: Remove <sys/signal.h> include Jiaxun Yang
2021-01-18  9:41   ` Philippe Mathieu-Daudé
2021-01-18  6:38 ` [PATCH v2 4/9] hw/block/nand: Rename PAGE_SIZE to NAND_PAGE_SIZE Jiaxun Yang
2021-01-18  9:44   ` Philippe Mathieu-Daudé
2021-01-18  6:38 ` [PATCH v2 5/9] elf2dmp: Rename PAGE_SIZE to ELF2DMP_PAGE_SIZE Jiaxun Yang
2021-01-18  9:44   ` Philippe Mathieu-Daudé
2021-01-18  6:38 ` [PATCH v2 6/9] tests: Rename PAGE_SIZE definitions Jiaxun Yang
2021-01-18  9:47   ` Philippe Mathieu-Daudé
2021-01-18  6:38 ` [PATCH v2 7/9] accel/kvm: avoid using predefined PAGE_SIZE Jiaxun Yang
2021-01-18  6:38 ` [PATCH v2 8/9] tests/docker: Add dockerfile for Alpine Linux Jiaxun Yang
2021-01-18 10:33   ` Daniel P. Berrangé
2021-01-19 13:41     ` Thomas Huth
2021-01-19 14:18       ` Daniel P. Berrangé
2021-01-26 21:38       ` John Snow
2021-01-27  9:58         ` Daniel P. Berrangé
2021-01-27 19:22           ` John Snow
2021-01-18  6:38 ` [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline Jiaxun Yang
2021-01-18 10:11   ` Daniel P. Berrangé
2021-01-18 10:22     ` Thomas Huth
2021-01-18 10:26       ` Daniel P. Berrangé
2021-01-18 13:37     ` Jiaxun Yang
2021-01-18 14:44       ` Thomas Huth
2021-01-18 14:50         ` Daniel P. Berrangé
2021-01-18 15:12           ` Thomas Huth
2021-01-19 11:49             ` Thomas Huth
2021-01-18  7:06 ` [PATCH v2 0/9] Alpine Linux build fix and CI pipeline no-reply
2021-01-18  8:02 ` Thomas Huth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).