1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] tests: Disable some tests on 32 bit systems

Michal Privoznik posted 1 patch 5 years ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/b0c7425b477879d739fad3435cacc95af706e038.1553772560.git.mprivozn@redhat.com
tests/qemufirmwaretest.c | 7 +++++++
tests/qemuxml2argvtest.c | 6 ++++++
2 files changed, 13 insertions(+)
[libvirt] [PATCH] tests: Disable some tests on 32 bit systems
Posted by Michal Privoznik 5 years ago 
On 32 bit systems there are two tests failing currently:
qemufirmwaretest and qemuxml2argvtest (not every test case in
them is failing, only some of them). There are several problems:

1) in qemufirmwaretest it's 'QEMU FW precedence test' which is
failing. This is because the code tests
qemuFirmwareFetchConfigs() which tries to compile a list of some
paths from the system and if one of the paths is an empty file it
should not appear on the returned list. The code uses
virFileLength() to query the file size which uses stat() under
the hood. The test uses virfilewrapper to redirect stat() into
qemufirmwaredata/ but for reasons beyond me real_stat() returns
mangled buffer making the code see a file with a real size even
for empty files. I've track this one to a problem in
virfilewrapper.c which will call real___xstat() which points to a
function in libvirt.so (!) which returns stat with 32bit members
even though we're compiling with LARGE_FILE and therefore expect
64bit members in the stat struct.

2) qemuxml2argvtest has two tests failing:
2a) [aarch64-]os-firmware-* - These fail because of the same
reason as described in 1)

2b) pseries-hostdevs-* - These fail because they again rely on
some stat (lstat to be precise). When building the cmd line for
these test cases the code does some search in /sys/bus/pci/...
and since we have virpcimock everything works just fine. Except
when virfilewrapper.c is linked in because then lstat() is taken
from there and when it initializes itself it will lookup
real_lstat() which will now point not to virpcimock but to glibc.
Again, reasons beyond me.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---

Quite frankly, I hate this patch. But the problem is in our mocking, not
in the actual code its testing. And I've tried everything I was able to
come up with on how to fix this. So if you have any idea, I'm all ears.

 tests/qemufirmwaretest.c | 7 +++++++
 tests/qemuxml2argvtest.c | 6 ++++++
 2 files changed, 13 insertions(+)

diff --git a/tests/qemufirmwaretest.c b/tests/qemufirmwaretest.c
index 2b5cbf649b..3332bf0a34 100644
--- a/tests/qemufirmwaretest.c
+++ b/tests/qemufirmwaretest.c
@@ -52,6 +52,8 @@ testParseFormatFW(const void *opaque)
 }
 
 
+#if defined(__x86_64__) || defined(__amd64__) || defined(__aarch64__)
+/* XXX Dirty hack, but mocking stat on 32bits is above my skills */
 static int
 testFWPrecedence(const void *opaque ATTRIBUTE_UNUSED)
 {
@@ -97,6 +99,7 @@ testFWPrecedence(const void *opaque ATTRIBUTE_UNUSED)
 
     return 0;
 }
+#endif
 
 
 static int
@@ -124,8 +127,12 @@ mymain(void)
     DO_PARSE_TEST("usr/share/qemu/firmware/61-ovmf.json");
     DO_PARSE_TEST("usr/share/qemu/firmware/70-aavmf.json");
 
+
+#if defined(__x86_64__) || defined(__amd64__) || defined(__aarch64__)
+    /* XXX Dirty hack, but mocking stat on 32bits is above my skills */
     if (virTestRun("QEMU FW precedence test", testFWPrecedence, NULL) < 0)
         ret = -1;
+#endif
 
     virFileWrapperClearPrefixes();
 
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0c0dcae197..364792d24d 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2038,6 +2038,8 @@ mymain(void)
     DO_TEST("pseries-many-buses-2",
             QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_VIRTIO_SCSI);
+# if defined(__x86_64__) || defined(__amd64__) || defined(__aarch64__)
+    /* XXX Dirty hack, but mocking stat on 32bits is above my skills */
     DO_TEST("pseries-hostdevs-1",
             QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_VIRTIO_SCSI,
@@ -2050,6 +2052,7 @@ mymain(void)
             QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_VIRTIO_SCSI,
             QEMU_CAPS_DEVICE_VFIO_PCI);
+# endif
 
     DO_TEST("pseries-features",
             QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
@@ -3157,10 +3160,13 @@ mymain(void)
     DO_TEST_CAPS_ARCH_LATEST("x86_64-pc-graphics", "x86_64");
     DO_TEST_CAPS_ARCH_LATEST("x86_64-q35-graphics", "x86_64");
 
+# if defined(__x86_64__) || defined(__amd64__) || defined(__aarch64__)
+    /* XXX Dirty hack, but mocking stat on 32bits is above my skills */
     DO_TEST_CAPS_LATEST("os-firmware-bios");
     DO_TEST_CAPS_LATEST("os-firmware-efi");
     DO_TEST_CAPS_LATEST("os-firmware-efi-secboot");
     DO_TEST_CAPS_ARCH_LATEST("aarch64-os-firmware-efi", "aarch64");
+# endif
 
     if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
         virFileDeleteTree(fakerootdir);
-- 
2.19.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Disable some tests on 32 bit systems
Posted by Daniel P. Berrangé 5 years ago 
On Thu, Mar 28, 2019 at 12:30:30PM +0100, Michal Privoznik wrote:
> On 32 bit systems there are two tests failing currently:
> qemufirmwaretest and qemuxml2argvtest (not every test case in
> them is failing, only some of them). There are several problems:
> 
> 1) in qemufirmwaretest it's 'QEMU FW precedence test' which is
> failing. This is because the code tests
> qemuFirmwareFetchConfigs() which tries to compile a list of some
> paths from the system and if one of the paths is an empty file it
> should not appear on the returned list. The code uses
> virFileLength() to query the file size which uses stat() under
> the hood. The test uses virfilewrapper to redirect stat() into
> qemufirmwaredata/ but for reasons beyond me real_stat() returns
> mangled buffer making the code see a file with a real size even
> for empty files. I've track this one to a problem in
> virfilewrapper.c which will call real___xstat() which points to a
> function in libvirt.so (!) which returns stat with 32bit members
> even though we're compiling with LARGE_FILE and therefore expect
> 64bit members in the stat struct.
> 
> 2) qemuxml2argvtest has two tests failing:
> 2a) [aarch64-]os-firmware-* - These fail because of the same
> reason as described in 1)
> 
> 2b) pseries-hostdevs-* - These fail because they again rely on
> some stat (lstat to be precise). When building the cmd line for
> these test cases the code does some search in /sys/bus/pci/...
> and since we have virpcimock everything works just fine. Except
> when virfilewrapper.c is linked in because then lstat() is taken
> from there and when it initializes itself it will lookup
> real_lstat() which will now point not to virpcimock but to glibc.
> Again, reasons beyond me.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
> 
> Quite frankly, I hate this patch. But the problem is in our mocking, not
> in the actual code its testing. And I've tried everything I was able to
> come up with on how to fix this. So if you have any idea, I'm all ears.

I'm happy to take a look at this.  I wouldn't be surprised if there is
an entirely different syscall API that needs mocking on 32bit to deal
with the largefile stuff.


>  tests/qemufirmwaretest.c | 7 +++++++
>  tests/qemuxml2argvtest.c | 6 ++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/tests/qemufirmwaretest.c b/tests/qemufirmwaretest.c
> index 2b5cbf649b..3332bf0a34 100644
> --- a/tests/qemufirmwaretest.c
> +++ b/tests/qemufirmwaretest.c
> @@ -52,6 +52,8 @@ testParseFormatFW(const void *opaque)
>  }
>  
>  
> +#if defined(__x86_64__) || defined(__amd64__) || defined(__aarch64__)
> +/* XXX Dirty hack, but mocking stat on 32bits is above my skills */
>  static int
>  testFWPrecedence(const void *opaque ATTRIBUTE_UNUSED)
>  {
> @@ -97,6 +99,7 @@ testFWPrecedence(const void *opaque ATTRIBUTE_UNUSED)
>  
>      return 0;
>  }
> +#endif
>  
>  
>  static int
> @@ -124,8 +127,12 @@ mymain(void)
>      DO_PARSE_TEST("usr/share/qemu/firmware/61-ovmf.json");
>      DO_PARSE_TEST("usr/share/qemu/firmware/70-aavmf.json");
>  
> +
> +#if defined(__x86_64__) || defined(__amd64__) || defined(__aarch64__)
> +    /* XXX Dirty hack, but mocking stat on 32bits is above my skills */
>      if (virTestRun("QEMU FW precedence test", testFWPrecedence, NULL) < 0)
>          ret = -1;
> +#endif
>  
>      virFileWrapperClearPrefixes();
>  
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 0c0dcae197..364792d24d 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -2038,6 +2038,8 @@ mymain(void)
>      DO_TEST("pseries-many-buses-2",
>              QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
>              QEMU_CAPS_VIRTIO_SCSI);
> +# if defined(__x86_64__) || defined(__amd64__) || defined(__aarch64__)
> +    /* XXX Dirty hack, but mocking stat on 32bits is above my skills */
>      DO_TEST("pseries-hostdevs-1",
>              QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
>              QEMU_CAPS_VIRTIO_SCSI,
> @@ -2050,6 +2052,7 @@ mymain(void)
>              QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
>              QEMU_CAPS_VIRTIO_SCSI,
>              QEMU_CAPS_DEVICE_VFIO_PCI);
> +# endif
>  
>      DO_TEST("pseries-features",
>              QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
> @@ -3157,10 +3160,13 @@ mymain(void)
>      DO_TEST_CAPS_ARCH_LATEST("x86_64-pc-graphics", "x86_64");
>      DO_TEST_CAPS_ARCH_LATEST("x86_64-q35-graphics", "x86_64");
>  
> +# if defined(__x86_64__) || defined(__amd64__) || defined(__aarch64__)
> +    /* XXX Dirty hack, but mocking stat on 32bits is above my skills */
>      DO_TEST_CAPS_LATEST("os-firmware-bios");
>      DO_TEST_CAPS_LATEST("os-firmware-efi");
>      DO_TEST_CAPS_LATEST("os-firmware-efi-secboot");
>      DO_TEST_CAPS_ARCH_LATEST("aarch64-os-firmware-efi", "aarch64");
> +# endif
>  
>      if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
>          virFileDeleteTree(fakerootdir);
> -- 
> 2.19.2
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Disable some tests on 32 bit systems
Posted by Andrea Bolognani 5 years ago 
On Thu, 2019-03-28 at 11:53 +0000, Daniel P. Berrangé wrote:
> On Thu, Mar 28, 2019 at 12:30:30PM +0100, Michal Privoznik wrote:
> > Quite frankly, I hate this patch. But the problem is in our mocking, not
> > in the actual code its testing. And I've tried everything I was able to
> > come up with on how to fix this. So if you have any idea, I'm all ears.
> 
> I'm happy to take a look at this.  I wouldn't be surprised if there is
> an entirely different syscall API that needs mocking on 32bit to deal
> with the largefile stuff.

Thanks in advance for looking at this :)

Just one more note:

> > +#if defined(__x86_64__) || defined(__amd64__) || defined(__aarch64__)
> > +/* XXX Dirty hack, but mocking stat on 32bits is above my skills */

If we actually end up doing something like this after all, we should
use a blacklist approach, excluding architectures where we know the
test fails, rather than a whitelist approach such as the one
presented here.

Case in point, the above excludes a bunch of architectures where the
tests work fine on account of their 64-bitness: ppc64, riscv64 and
s390x.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.