Re: [Qemu-devel] [PATCH v2 3/6] libqos: use generic qtest_shutdown()

[Greg Kurz <groug@kaod.org>]

On Tue, 27 Sep 2016 20:55:56 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> Machine specific shutdown function can be registered by
> the machine specific qtest_XXX_boot() if needed.
> 
> So we will not have to test twice the architecture (on boot and on
> shutdown) if the test can be run on several architectures.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---

Minor remarks below, but anyway:

Reviewed-by: Greg Kurz <groug@kaod.org>

Cheers.

--
Greg

>  tests/libqos/libqos-pc.c    |  3 ++-
>  tests/libqos/libqos-spapr.c |  5 +++--
>  tests/libqos/libqos.c       | 11 ++++++++++-
>  tests/libqos/libqos.h       |  8 ++++++--
>  tests/rtas-test.c           |  2 +-
>  5 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/libqos/libqos-pc.c b/tests/libqos/libqos-pc.c
> index aa17c98..219824d 100644
> --- a/tests/libqos/libqos-pc.c
> +++ b/tests/libqos/libqos-pc.c
> @@ -8,6 +8,7 @@ static QOSOps qos_ops = {
>      .uninit_allocator = pc_alloc_uninit,
>      .qpci_init = qpci_init_pc,
>      .qpci_free = qpci_free_pc,
> +    .shutdown = qtest_pc_shutdown

Trailing comma ?

>  };
>  
>  QOSState *qtest_pc_vboot(const char *cmdline_fmt, va_list ap)
> @@ -31,5 +32,5 @@ QOSState *qtest_pc_boot(const char *cmdline_fmt, ...)
>  
>  void qtest_pc_shutdown(QOSState *qs)
>  {
> -    return qtest_shutdown(qs);
> +    return qtest_common_shutdown(qs);
>  }
> diff --git a/tests/libqos/libqos-spapr.c b/tests/libqos/libqos-spapr.c
> index 125c6b3..8f7fc14 100644
> --- a/tests/libqos/libqos-spapr.c
> +++ b/tests/libqos/libqos-spapr.c
> @@ -7,7 +7,8 @@ static QOSOps qos_ops = {
>      .init_allocator = spapr_alloc_init_flags,
>      .uninit_allocator = spapr_alloc_uninit,
>      .qpci_init = qpci_init_spapr,
> -    .qpci_free = qpci_free_spapr
> +    .qpci_free = qpci_free_spapr,
> +    .shutdown = qtest_spapr_shutdown

Ditto.

>  };
>  
>  QOSState *qtest_spapr_vboot(const char *cmdline_fmt, va_list ap)
> @@ -29,5 +30,5 @@ QOSState *qtest_spapr_boot(const char *cmdline_fmt, ...)
>  
>  void qtest_spapr_shutdown(QOSState *qs)
>  {
> -    return qtest_shutdown(qs);
> +    return qtest_common_shutdown(qs);
>  }
> diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
> index d842bf5..7abb482 100644
> --- a/tests/libqos/libqos.c
> +++ b/tests/libqos/libqos.c
> @@ -52,7 +52,7 @@ QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt, ...)
>  /**
>   * Tear down the QEMU instance.
>   */
> -void qtest_shutdown(QOSState *qs)
> +void qtest_common_shutdown(QOSState *qs)
>  {
>      if (qs->ops) {
>          if (qs->pcibus && qs->ops->qpci_free) {
> @@ -68,6 +68,15 @@ void qtest_shutdown(QOSState *qs)
>      g_free(qs);
>  }
>  
> +void qtest_shutdown(QOSState *qs)
> +{
> +    if (qs->ops && qs->ops->shutdown) {
> +        qs->ops->shutdown(qs);
> +    } else {
> +        qtest_common_shutdown(qs);
> +    }
> +}
> +
>  void set_context(QOSState *s)
>  {
>      global_qtest = s->qts;
> diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
> index a9f6990..2319697 100644
> --- a/tests/libqos/libqos.h
> +++ b/tests/libqos/libqos.h
> @@ -5,22 +5,26 @@
>  #include "libqos/pci.h"
>  #include "libqos/malloc-pc.h"
>  
> +typedef struct QOSState QOSState;
> +
>  typedef struct QOSOps {
>      QGuestAllocator *(*init_allocator)(QAllocOpts);
>      void (*uninit_allocator)(QGuestAllocator *);
>      QPCIBus *(*qpci_init)(QGuestAllocator *alloc);
>      void (*qpci_free)(QPCIBus *bus);
> +    void (*shutdown)(QOSState *);
>  } QOSOps;
>  
> -typedef struct QOSState {
> +struct QOSState {
>      QTestState *qts;
>      QGuestAllocator *alloc;
>      QPCIBus *pcibus;
>      QOSOps *ops;
> -} QOSState;
> +};
>  
>  QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap);
>  QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt, ...);
> +void qtest_common_shutdown(QOSState *qs);
>  void qtest_shutdown(QOSState *qs);
>  bool have_qemu_img(void);
>  void mkimg(const char *file, const char *fmt, unsigned size_mb);
> diff --git a/tests/rtas-test.c b/tests/rtas-test.c
> index 73c7803..ba0867a 100644
> --- a/tests/rtas-test.c
> +++ b/tests/rtas-test.c
> @@ -22,7 +22,7 @@ static void test_rtas_get_time_of_day(void)
>      t2 = mktimegm(&tm);
>      g_assert(t2 - t1 < 5); /* 5 sec max to run the test */
>  
> -    qtest_spapr_shutdown(qs);
> +    qtest_shutdown(qs);
>  }
>  
>  int main(int argc, char *argv[])