Re: [Qemu-devel] [PATCH 03/14] libqos: create libqos.c

[Paolo Bonzini]

On 13/01/2015 04:34, John Snow wrote:
> The intent of this file is to serve as a misc. utilities file to be
> shared amongst tests that are utilizing libqos facilities.
> 
> In a later patch, migration test helpers will be added to libqos.c that
> will allow simplified testing of migration cases where libqos is
> "Just Enough OS" for migrations testing.
> 
> The addition of the AHCIQState structure will also allow us to eliminate
> global variables inside of qtests to manage allocators and test instances
> in a better, more functional way.
> 
> libqos.c:
>         - Add qtest_boot
>         - Add qtest_shutdown
> 
> libqos.h:
>         - Create QOSState structure for allocator and QTestState.
> 
> ahci-test.c:
>         - Move qtest_boot and qtest_shutdown to libqos.c/h
>         - Create AHCIQState to interface with new qtest_boot/shutdown 
> prototypes
>         - Modify tests slightly to use new types.
> 
> For now, the new object file is only linked to ahci-test, because it still
> relies on pc architecture specific code in libqos. The next two patches will
> reorganize the code to be more general.
> 
> Signed-off-by: John Snow <address@hidden>
> ---
>  tests/Makefile        |  2 +-
>  tests/ahci-test.c     | 95 
> +++++++++++++++++++++------------------------------
>  tests/libqos/ahci.h   |  5 +++
>  tests/libqos/libqos.c | 48 ++++++++++++++++++++++++++
>  tests/libqos/libqos.h | 26 ++++++++++++++
>  5 files changed, 119 insertions(+), 57 deletions(-)
>  create mode 100644 tests/libqos/libqos.c
>  create mode 100644 tests/libqos/libqos.h
> 
> diff --git a/tests/Makefile b/tests/Makefile
> index 77f995d..c5b3b21 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -314,7 +314,7 @@ tests/endianness-test$(EXESUF): tests/endianness-test.o
>  tests/spapr-phb-test$(EXESUF): tests/spapr-phb-test.o $(libqos-obj-y)
>  tests/fdc-test$(EXESUF): tests/fdc-test.o
>  tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y)
> -tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y)
> +tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y) 
> tests/libqos/libqos.o
>  tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o
>  tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
>  tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o $(libqos-obj-y)
> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> index fe903dd..a498cbb 100644
> --- a/tests/ahci-test.c
> +++ b/tests/ahci-test.c
> @@ -29,6 +29,7 @@
>  #include <glib.h>
>  
>  #include "libqtest.h"
> +#include "libqos/libqos.h"
>  #include "libqos/ahci.h"
>  #include "libqos/pci-pc.h"
>  #include "libqos/malloc-pc.h"
> @@ -136,58 +137,40 @@ static void free_ahci_device(QPCIDevice *ahci)
>  /*** Test Setup & Teardown ***/
>  
>  /**
> - * Launch QEMU with the given command line,
> - * and then set up interrupts and our guest malloc interface.
> - */
> -static void qtest_boot(const char *cmdline_fmt, ...)
> -{
> -    va_list ap;
> -    char *cmdline;
> -
> -    va_start(ap, cmdline_fmt);
> -    cmdline = g_strdup_vprintf(cmdline_fmt, ap);
> -    va_end(ap);
> -
> -    qtest_start(cmdline);
> -    qtest_irq_intercept_in(global_qtest, "ioapic");
> -    guest_malloc = pc_alloc_init();
> -
> -    g_free(cmdline);
> -}
> -
> -/**
> - * Tear down the QEMU instance.
> - */
> -static void qtest_shutdown(void)
> -{
> -    g_free(guest_malloc);
> -    guest_malloc = NULL;
> -    qtest_end();
> -}
> -
> -/**
>   * Start a Q35 machine and bookmark a handle to the AHCI device.
>   */
> -static QPCIDevice *ahci_boot(void)
> +static AHCIQState *ahci_boot(void)
>  {
> -    qtest_boot("-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s,"
> -               "format=raw"
> -               " -M q35 "
> -               "-device ide-hd,drive=drive0 "
> -               "-global ide-hd.ver=%s",
> -               tmp_path, "testdisk", "version");
> +    AHCIQState *s;
> +    const char *cli;
> +
> +    s = g_malloc0(sizeof(AHCIQState));
> +
> +    cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
> +        ",format=raw"
> +        " -M q35 "
> +        "-device ide-hd,drive=drive0 "
> +        "-global ide-hd.ver=%s";
> +    s->parent = qtest_boot(cli, tmp_path, "testdisk", "version");
>  
>      /* Verify that we have an AHCI device present. */
> -    return get_ahci_device();
> +    s->dev = get_ahci_device();
> +
> +    /* Stopgap: Copy the allocator reference */
> +    guest_malloc = s->parent->alloc;
> +
> +    return s;
>  }
>  
>  /**
>   * Clean up the PCI device, then terminate the QEMU instance.
>   */
> -static void ahci_shutdown(QPCIDevice *ahci)
> +static void ahci_shutdown(AHCIQState *ahci)
>  {
> -    free_ahci_device(ahci);
> -    qtest_shutdown();
> +    QOSState *qs = ahci->parent;
> +    free_ahci_device(ahci->dev);
> +    g_free(ahci);
> +    qtest_shutdown(qs);
>  }
>  
>  /*** Logical Device Initialization ***/
> @@ -1104,7 +1087,7 @@ static void ahci_test_identify(QPCIDevice *ahci, void 
> *hba_base)
>   */
>  static void test_sanity(void)
>  {
> -    QPCIDevice *ahci;
> +    AHCIQState *ahci;
>      ahci = ahci_boot();
>      ahci_shutdown(ahci);
>  }
> @@ -1115,9 +1098,9 @@ static void test_sanity(void)
>   */
>  static void test_pci_spec(void)
>  {
> -    QPCIDevice *ahci;
> +    AHCIQState *ahci;
>      ahci = ahci_boot();
> -    ahci_test_pci_spec(ahci);
> +    ahci_test_pci_spec(ahci->dev);
>      ahci_shutdown(ahci);
>  }
>  
> @@ -1127,10 +1110,10 @@ static void test_pci_spec(void)
>   */
>  static void test_pci_enable(void)
>  {
> -    QPCIDevice *ahci;
> +    AHCIQState *ahci;
>      void *hba_base;
>      ahci = ahci_boot();
> -    ahci_pci_enable(ahci, &hba_base);
> +    ahci_pci_enable(ahci->dev, &hba_base);
>      ahci_shutdown(ahci);
>  }
>  
> @@ -1140,12 +1123,12 @@ static void test_pci_enable(void)
>   */
>  static void test_hba_spec(void)
>  {
> -    QPCIDevice *ahci;
> +    AHCIQState *ahci;
>      void *hba_base;
>  
>      ahci = ahci_boot();
> -    ahci_pci_enable(ahci, &hba_base);
> -    ahci_test_hba_spec(ahci, hba_base);
> +    ahci_pci_enable(ahci->dev, &hba_base);
> +    ahci_test_hba_spec(ahci->dev, hba_base);
>      ahci_shutdown(ahci);
>  }
>  
> @@ -1155,12 +1138,12 @@ static void test_hba_spec(void)
>   */
>  static void test_hba_enable(void)
>  {
> -    QPCIDevice *ahci;
> +    AHCIQState *ahci;
>      void *hba_base;
>  
>      ahci = ahci_boot();
> -    ahci_pci_enable(ahci, &hba_base);
> -    ahci_hba_enable(ahci, hba_base);
> +    ahci_pci_enable(ahci->dev, &hba_base);
> +    ahci_hba_enable(ahci->dev, hba_base);
>      ahci_shutdown(ahci);
>  }
>  
> @@ -1170,13 +1153,13 @@ static void test_hba_enable(void)
>   */
>  static void test_identify(void)
>  {
> -    QPCIDevice *ahci;
> +    AHCIQState *ahci;
>      void *hba_base;
>  
>      ahci = ahci_boot();
> -    ahci_pci_enable(ahci, &hba_base);
> -    ahci_hba_enable(ahci, hba_base);
> -    ahci_test_identify(ahci, hba_base);
> +    ahci_pci_enable(ahci->dev, &hba_base);
> +    ahci_hba_enable(ahci->dev, hba_base);
> +    ahci_test_identify(ahci->dev, hba_base);
>      ahci_shutdown(ahci);
>  }
>  
> diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
> index 6564c5a..bc5f45d 100644
> --- a/tests/libqos/ahci.h
> +++ b/tests/libqos/ahci.h
> @@ -245,6 +245,11 @@
>  
>  /*** Structures ***/
>  
> +typedef struct AHCIQState {
> +    QOSState *parent;
> +    QPCIDevice *dev;
> +} AHCIQState;
> +
>  /**
>   * Generic FIS structure.
>   */
> diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
> new file mode 100644
> index 0000000..c478bc9
> --- /dev/null
> +++ b/tests/libqos/libqos.c
> @@ -0,0 +1,48 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <glib.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <sys/wait.h>
> +
> +#include "libqtest.h"
> +#include "libqos/libqos.h"
> +#include "libqos/pci.h"
> +#include "libqos/malloc-pc.h"
> +
> +/*** Test Setup & Teardown ***/
> +
> +/**
> + * Launch QEMU with the given command line,
> + * and then set up interrupts and our guest malloc interface.
> + */
> +QOSState *qtest_boot(const char *cmdline_fmt, ...)
> +{
> +    QOSState *qs = g_malloc(sizeof(QOSState));
> +    char *cmdline;
> +    va_list ap;
> +
> +    va_start(ap, cmdline_fmt);
> +    cmdline = g_strdup_vprintf(cmdline_fmt, ap);
> +    va_end(ap);
> +
> +    qs->qts = qtest_start(cmdline);
> +    qtest_irq_intercept_in(global_qtest, "ioapic");
> +    qs->alloc = pc_alloc_init();
> +
> +    g_free(cmdline);
> +    return qs;
> +}
> +
> +/**
> + * Tear down the QEMU instance.
> + */
> +void qtest_shutdown(QOSState *qs)
> +{
> +    if (qs->alloc) {
> +        pc_alloc_uninit(qs->alloc);
> +        qs->alloc = NULL;
> +    }
> +    qtest_quit(qs->qts);
> +    g_free(qs);
> +}
> diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
> new file mode 100644
> index 0000000..7a106f2
> --- /dev/null
> +++ b/tests/libqos/libqos.h
> @@ -0,0 +1,26 @@
> +#ifndef __libqos_h
> +#define __libqos_h
> +
> +#include "libqtest.h"
> +#include "libqos/pci.h"
> +#include "libqos/malloc-pc.h"
> +
> +typedef struct QOSState {
> +    QTestState *qts;
> +    QGuestAllocator *alloc;
> +} QOSState;
> +
> +QOSState *qtest_boot(const char *cmdline_fmt, ...);
> +void qtest_shutdown(QOSState *qs);
> +
> +static inline uint64_t qmalloc(QOSState *q, size_t bytes)
> +{
> +    return guest_alloc(q->alloc, bytes);
> +}
> +
> +static inline void qfree(QOSState *q, uint64_t addr)
> +{
> +    guest_free(q->alloc, addr);
> +}
> +
> +#endif
> 

Reviewed-by: Paolo Bonzini <address@hidden>