On Fri, 30 Sep 2016 11:19:39 -0400 John Snow <js...@redhat.com> wrote:
> On 09/30/2016 06:30 AM, Laurent Vivier wrote: > > > > > > On 30/09/2016 12:18, Greg Kurz wrote: > >> On Thu, 29 Sep 2016 19:15:07 +0200 > >> Laurent Vivier <lviv...@redhat.com> wrote: > >> > >>> but disable MSI-X tests on SPAPR as we can't check the result > >>> (the memory region used on PC is not readable on SPAPR). > >>> > >>> Signed-off-by: Laurent Vivier <lviv...@redhat.com> > >>> --- > >>> tests/Makefile.include | 3 ++- > >>> tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++-- > >>> tests/virtio-9p-test.c | 11 ++++++++++- > >>> tests/virtio-blk-test.c | 22 +++++++++++++++++----- > >>> tests/virtio-net-test.c | 17 +++++++++++++++-- > >>> tests/virtio-rng-test.c | 7 ++++++- > >>> tests/virtio-scsi-test.c | 10 +++++++++- > >>> 7 files changed, 79 insertions(+), 13 deletions(-) > > ... > >>> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c > >>> index 28d7f5b..a73bccb 100644 > >>> --- a/tests/virtio-9p-test.c > >>> +++ b/tests/virtio-9p-test.c > >>> @@ -11,6 +11,7 @@ > >>> #include "libqtest.h" > >>> #include "qemu-common.h" > >>> #include "libqos/libqos-pc.h" > >>> +#include "libqos/libqos-spapr.h" > >>> #include "libqos/virtio.h" > >>> #include "libqos/virtio-pci.h" > >>> #include "standard-headers/linux/virtio_ids.h" > >>> @@ -22,12 +23,20 @@ static char *test_share; > >>> > >>> static QOSState *qvirtio_9p_start(void) > >>> { > >>> + const char *arch = qtest_get_arch(); > >>> + QOSState *qs = NULL; > >>> test_share = g_strdup("/tmp/qtest.XXXXXX"); > >>> g_assert_nonnull(mkdtemp(test_share)); > >>> const char *cmd = "-fsdev > >>> local,id=fsdev0,security_model=none,path=%s " > >>> "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s"; > >>> > >>> - return qtest_pc_boot(cmd, test_share, mount_tag); > >>> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > >>> + qs = qtest_pc_boot(cmd, test_share, mount_tag); > >>> + } else if (strcmp(arch, "ppc64") == 0) { > >>> + qs = qtest_spapr_boot(cmd, test_share, mount_tag); > >>> + } > >>> + > >> > >> What about introducing a qtest_arch_boot() helper that does ^^ and > >> > >> } else { > >> g_printerr("qtest_arch_boot() not supported for arch %s\n", > >> qtest_get_arch()); > >> exit(EXIT_FAILURE); > >> } > >> > > > > The problem with adding a function like that is it will pull > > $(libqos-pc-obj-y) and $(libqos-spapr-obj-y) for every tests using it, > > and for the moment we are pulling pc or spapr objects only if we need > > them for the given test. > > > > I think it explains why qtest_pc_boot() calls qtest_vboot() and we don't > > have a generic qtest_boot() calling the architecture specific function. > > > > I cc: John Snow as he has written the initial code for this. > > ("90e5add libqos: add pc specific interface") > > > > Laurent > > > > > > This was a while ago for me (and I was brand new to QEMU!), but that > sounds about right. I wasn't able to reason about requirements for other > architectures, so we made the PC-specific frontend to do the > configuration for us. libqos.o does not pull in any of the PC-specific > requirements as a result. Neither does the allocator. > > I didn't necessarily design it to be like this, just a path of least > resistance type of thing. > > You probably could make an ArchOps callback structure if you wanted and > pass that along to a generic bootup function to avoid the linking issues > if you wanted a one-size-fits-all initialization function. > > --js It looks like libqos may be divided in 3 families: - base libqos for tests that don't need platform specific support (basically what we currently have in libqos-y) - platfrom specific libqos for platform specific tests (libqos-pc, libqos-spapr) - one-size-fits-all libqos for tests that should run on several platforms Makes sense ? -- Greg