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
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
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
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
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
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
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
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
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
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
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
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
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>
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>
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?
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>
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];
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 :|
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
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 :|
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 :|
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
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
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 :|
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
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
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
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 :|
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
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 :|
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