[Qemu-devel] [PATCH v2 0/8] ahci: add more IO tests

[Qemu-devel] [PATCH v2 0/8] ahci: add more IO tests

[John Snow]

This series is based on top of my ahci DMA test series, which is in
turn based on the ahci preliminary refactoring series. Both are currently
pending on stefanha/block.

This series adds many variations that expand on the existing trivial
DMA I/O case. These pathways check different PRDT configurations,
different I/O commands for PIO and DMA, different address scheme
combinations for LBA28 and LBA48, and different sector offsets.

John Snow (8):
  libqos/ahci: Zero-fill AHCI headers
  qtest/ahci: Add a macro bootup routine
  libqos/ahci: add ahci command helpers
  qtest/ahci: Add DMA test variants
  qtest/ahci: Add PIO and LBA48 tests
  qtest/ahci: add fragmented dma test
  qtest/ahci: add qcow2 support to ahci-test
  qtest/ahci: test different disk sectors

 tests/Makefile        |   1 +
 tests/ahci-test.c     | 296 +++++++++++++++++++++++++++++++++++++++++++++-----
 tests/libqos/ahci.c   |  54 ++++++++-
 tests/libqos/ahci.h   |   9 +-
 tests/libqos/libqos.c |  37 +++++++
 tests/libqos/libqos.h |   2 +
 6 files changed, 365 insertions(+), 34 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v2 1/8] libqos/ahci: Zero-fill AHCI headers

[John Snow]

Even though it's just the reserved space, make sure they're zeroes.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/libqos/ahci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index a6105c7..9dc505c 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -487,7 +487,7 @@ void ahci_get_command_header(AHCIQState *ahci, uint8_t port,
 void ahci_set_command_header(AHCIQState *ahci, uint8_t port,
                              uint8_t slot, AHCICommandHeader *cmd)
 {
-    AHCICommandHeader tmp;
+    AHCICommandHeader tmp = { .flags = 0 };
     uint64_t ba = ahci->port[port].clb;
     ba += slot * sizeof(AHCICommandHeader);
 
-- 
1.9.3

[Qemu-devel] [PATCH v2 2/8] qtest/ahci: Add a macro bootup routine

[John Snow]

Add a routine that can be used to engage the AHCI
device at a not-granular level so that bringing up
the functionality of the HBA is easy in future tests
that are not concerned with testing the bring-up process.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 53fd068..9fe9fb5 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -107,6 +107,21 @@ static void ahci_shutdown(AHCIQState *ahci)
     qtest_shutdown(qs);
 }
 
+/**
+ * Boot and fully enable the HBA device.
+ * @see ahci_boot, ahci_pci_enable and ahci_hba_enable.
+ */
+static AHCIQState *ahci_boot_and_enable(void)
+{
+    AHCIQState *ahci;
+    ahci = ahci_boot();
+
+    ahci_pci_enable(ahci);
+    ahci_hba_enable(ahci);
+
+    return ahci;
+}
+
 /*** Specification Adherence Tests ***/
 
 /**
@@ -831,9 +846,7 @@ static void test_identify(void)
 {
     AHCIQState *ahci;
 
-    ahci = ahci_boot();
-    ahci_pci_enable(ahci);
-    ahci_hba_enable(ahci);
+    ahci = ahci_boot_and_enable();
     ahci_test_identify(ahci);
     ahci_shutdown(ahci);
 }
@@ -845,9 +858,7 @@ static void test_dma_rw_simple(void)
 {
     AHCIQState *ahci;
 
-    ahci = ahci_boot();
-    ahci_pci_enable(ahci);
-    ahci_hba_enable(ahci);
+    ahci = ahci_boot_and_enable();
     ahci_test_dma_rw_simple(ahci);
     ahci_shutdown(ahci);
 }
-- 
1.9.3

[Qemu-devel] [PATCH v2 3/8] libqos/ahci: add ahci command helpers

[John Snow]

ahci_command_set_flags:  Set additional flags in the command header.
ahci_command_clr_flags:  Clear flags from the command header.
ahci_command_set_offset: Change the IO sector from 0.
ahci_command_adjust:     Adjust many values simultaneously.

To be used to adjust the command header if the default values/guesses
were incorrect or undesirable.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/libqos/ahci.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/ahci.h |  5 +++++
 2 files changed, 47 insertions(+)

diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 9dc505c..14651ff 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -713,6 +713,40 @@ void ahci_command_free(AHCICommand *cmd)
     g_free(cmd);
 }
 
+void ahci_command_set_flags(AHCICommand *cmd, uint16_t cmdh_flags)
+{
+    cmd->header.flags |= cmdh_flags;
+}
+
+void ahci_command_clr_flags(AHCICommand *cmd, uint16_t cmdh_flags)
+{
+    cmd->header.flags &= ~cmdh_flags;
+}
+
+void ahci_command_set_offset(AHCICommand *cmd, uint64_t lba_sect)
+{
+    RegH2DFIS *fis = &(cmd->fis);
+    if (cmd->props->lba28) {
+        g_assert_cmphex(lba_sect, <=, 0xFFFFFFF);
+    } else if (cmd->props->lba48) {
+        g_assert_cmphex(lba_sect, <=, 0xFFFFFFFFFFFF);
+    } else {
+        /* Can't set offset if we don't know the format. */
+        g_assert_not_reached();
+    }
+
+    /* LBA28 uses the low nibble of the device/control register for LBA24:27 */
+    fis->lba_lo[0] = (lba_sect & 0xFF);
+    fis->lba_lo[1] = (lba_sect >> 8) & 0xFF;
+    fis->lba_lo[2] = (lba_sect >> 16) & 0xFF;
+    if (cmd->props->lba28) {
+        fis->device = (fis->device & 0xF0) || (lba_sect >> 24) & 0x0F;
+    }
+    fis->lba_hi[0] = (lba_sect >> 24) & 0xFF;
+    fis->lba_hi[1] = (lba_sect >> 32) & 0xFF;
+    fis->lba_hi[2] = (lba_sect >> 40) & 0xFF;
+}
+
 void ahci_command_set_buffer(AHCICommand *cmd, uint64_t buffer)
 {
     cmd->buffer = buffer;
@@ -740,6 +774,14 @@ void ahci_command_set_prd_size(AHCICommand *cmd, unsigned prd_size)
     ahci_command_set_sizes(cmd, cmd->xbytes, prd_size);
 }
 
+void ahci_command_adjust(AHCICommand *cmd, uint64_t offset, uint64_t buffer,
+                         size_t xbytes, unsigned prd_size)
+{
+    ahci_command_set_sizes(cmd, xbytes, prd_size);
+    ahci_command_set_buffer(cmd, buffer);
+    ahci_command_set_offset(cmd, offset);
+}
+
 void ahci_command_commit(AHCIQState *ahci, AHCICommand *cmd, uint8_t port)
 {
     uint16_t i, prdtl;
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 39b99d3..888545d 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -537,11 +537,16 @@ void ahci_command_verify(AHCIQState *ahci, AHCICommand *cmd);
 void ahci_command_free(AHCICommand *cmd);
 
 /* Command adjustments */
+void ahci_command_set_flags(AHCICommand *cmd, uint16_t cmdh_flags);
+void ahci_command_clr_flags(AHCICommand *cmd, uint16_t cmdh_flags);
+void ahci_command_set_offset(AHCICommand *cmd, uint64_t lba_sect);
 void ahci_command_set_buffer(AHCICommand *cmd, uint64_t buffer);
 void ahci_command_set_size(AHCICommand *cmd, uint64_t xbytes);
 void ahci_command_set_prd_size(AHCICommand *cmd, unsigned prd_size);
 void ahci_command_set_sizes(AHCICommand *cmd, uint64_t xbytes,
                             unsigned prd_size);
+void ahci_command_adjust(AHCICommand *cmd, uint64_t lba_sect, uint64_t gbuffer,
+                         uint64_t xbytes, unsigned prd_size);
 
 /* Command Misc */
 uint8_t ahci_command_slot(AHCICommand *cmd);
-- 
1.9.3

[Qemu-devel] [PATCH v2 4/8] qtest/ahci: Add DMA test variants

[John Snow]

These test a few different pathways in the AHCI code.

short:  Test the minimum transfer size, exactly one sector.
simple: Test a transfer using a single PRD, in this case, 4K.
double: Test transferring 8K, which we will split up as two PRDs.
long:   Test transferring a lot of data using many PRDs, 256K.
Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c | 38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 9fe9fb5..9394d85 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -731,12 +731,11 @@ static void ahci_test_identify(AHCIQState *ahci)
     g_assert_cmphex(sect_size, ==, 0x200);
 }
 
-static void ahci_test_dma_rw_simple(AHCIQState *ahci)
+static void ahci_test_dma_rw_simple(AHCIQState *ahci, unsigned bufsize)
 {
     uint64_t ptr;
     uint8_t port;
     unsigned i;
-    const unsigned bufsize = 4096;
     unsigned char *tx = g_malloc(bufsize);
     unsigned char *rx = g_malloc0(bufsize);
 
@@ -751,7 +750,7 @@ static void ahci_test_dma_rw_simple(AHCIQState *ahci)
     ptr = ahci_alloc(ahci, bufsize);
     g_assert(ptr);
 
-    /* Write some indicative pattern to our 4K buffer. */
+    /* Write some indicative pattern to our buffer. */
     for (i = 0; i < bufsize; i++) {
         tx[i] = (bufsize - i);
     }
@@ -852,15 +851,35 @@ static void test_identify(void)
 }
 
 /**
- * Perform a simple DMA R/W test, using a single PRD and non-NCQ commands.
+ * Perform a simple DMA R/W test using non-NCQ commands.
  */
+static void test_dma_rw_interface(unsigned bufsize)
+{
+    AHCIQState *ahci;
+
+    ahci = ahci_boot_and_enable();
+    ahci_test_dma_rw_simple(ahci, bufsize);
+    ahci_shutdown(ahci);
+}
+
 static void test_dma_rw_simple(void)
 {
-    AHCIQState *ahci;
+    test_dma_rw_interface(4096);
+}
 
-    ahci = ahci_boot_and_enable();
-    ahci_test_dma_rw_simple(ahci);
-    ahci_shutdown(ahci);
+static void test_dma_rw_double(void)
+{
+    test_dma_rw_interface(8192);
+}
+
+static void test_dma_rw_long(void)
+{
+    test_dma_rw_interface(4096 * 64);
+}
+
+static void test_dma_rw_short(void)
+{
+    test_dma_rw_interface(512);
 }
 
 /******************************************************************************/
@@ -919,6 +938,9 @@ int main(int argc, char **argv)
     qtest_add_func("/ahci/hba_enable", test_hba_enable);
     qtest_add_func("/ahci/identify",   test_identify);
     qtest_add_func("/ahci/dma/simple", test_dma_rw_simple);
+    qtest_add_func("/ahci/dma/double", test_dma_rw_double);
+    qtest_add_func("/ahci/dma/long",   test_dma_rw_long);
+    qtest_add_func("/ahci/dma/short",  test_dma_rw_short);
 
     ret = g_test_run();
 
-- 
1.9.3

[Qemu-devel] [PATCH v2 5/8] qtest/ahci: Add PIO and LBA48 tests

[John Snow]

In addition to DMA tests, test PIO and LBA48 command pathways in AHCI.
To accomplish this, a primitive multiplexer for gtest is added.

Though guests may prefer not to issue PIO commands directly except
for single sector cases during early boot and shutdown, these pathways
are still used for the transfer of ATAPI commands as well, and should
be behaving well.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 134 insertions(+), 22 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 9394d85..21f20f7 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -731,7 +731,8 @@ static void ahci_test_identify(AHCIQState *ahci)
     g_assert_cmphex(sect_size, ==, 0x200);
 }
 
-static void ahci_test_dma_rw_simple(AHCIQState *ahci, unsigned bufsize)
+static void ahci_test_io_rw_simple(AHCIQState *ahci, unsigned bufsize,
+                                   uint8_t read_cmd, uint8_t write_cmd)
 {
     uint64_t ptr;
     uint8_t port;
@@ -757,9 +758,9 @@ static void ahci_test_dma_rw_simple(AHCIQState *ahci, unsigned bufsize)
     memwrite(ptr, tx, bufsize);
 
     /* Write this buffer to disk, then read it back to the DMA buffer. */
-    ahci_guest_io(ahci, port, CMD_WRITE_DMA, ptr, bufsize);
+    ahci_guest_io(ahci, port, write_cmd, ptr, bufsize);
     qmemset(ptr, 0x00, bufsize);
-    ahci_guest_io(ahci, port, CMD_READ_DMA, ptr, bufsize);
+    ahci_guest_io(ahci, port, read_cmd, ptr, bufsize);
 
     /*** Read back the Data ***/
     memread(ptr, rx, bufsize);
@@ -850,36 +851,142 @@ static void test_identify(void)
     ahci_shutdown(ahci);
 }
 
+/******************************************************************************/
+/* AHCI I/O Test Matrix Definitions                                           */
+
+enum BuffLen {
+    LEN_BEGIN = 0,
+    LEN_SIMPLE = LEN_BEGIN,
+    LEN_DOUBLE,
+    LEN_LONG,
+    LEN_SHORT,
+    NUM_LENGTHS
+};
+
+static const char *buff_len_str[NUM_LENGTHS] = { "simple", "double",
+                                                 "long", "short" };
+
+enum AddrMode {
+    ADDR_MODE_BEGIN = 0,
+    ADDR_MODE_LBA28 = ADDR_MODE_BEGIN,
+    ADDR_MODE_LBA48,
+    NUM_ADDR_MODES
+};
+
+static const char *addr_mode_str[NUM_ADDR_MODES] = { "lba28", "lba48" };
+
+enum IOMode {
+    MODE_BEGIN = 0,
+    MODE_PIO = MODE_BEGIN,
+    MODE_DMA,
+    NUM_MODES
+};
+
+static const char *io_mode_str[NUM_MODES] = { "pio", "dma" };
+
+enum IOOps {
+    IO_BEGIN = 0,
+    IO_READ = IO_BEGIN,
+    IO_WRITE,
+    NUM_IO_OPS
+};
+
+typedef struct AHCIIOTestOptions {
+    enum BuffLen length;
+    enum AddrMode address_type;
+    enum IOMode io_type;
+} AHCIIOTestOptions;
+
 /**
- * Perform a simple DMA R/W test using non-NCQ commands.
+ * Table of possible I/O ATA commands given a set of enumerations.
  */
-static void test_dma_rw_interface(unsigned bufsize)
+static const uint8_t io_cmds[NUM_MODES][NUM_ADDR_MODES][NUM_IO_OPS] = {
+    [MODE_PIO] = {
+        [ADDR_MODE_LBA28] = {
+            [IO_READ] = CMD_READ_PIO,
+            [IO_WRITE] = CMD_WRITE_PIO },
+        [ADDR_MODE_LBA48] = {
+            [IO_READ] = CMD_READ_PIO_EXT,
+            [IO_WRITE] = CMD_WRITE_PIO_EXT }
+    },
+    [MODE_DMA] = {
+        [ADDR_MODE_LBA28] = {
+            [IO_READ] = CMD_READ_DMA,
+            [IO_WRITE] = CMD_WRITE_DMA },
+        [ADDR_MODE_LBA48] = {
+            [IO_READ] = CMD_READ_DMA_EXT,
+            [IO_WRITE] = CMD_WRITE_DMA_EXT }
+    }
+};
+
+/**
+ * Test a Read/Write pattern using various commands, addressing modes,
+ * transfer modes, and buffer sizes.
+ */
+static void test_io_rw_interface(enum AddrMode lba48, enum IOMode dma,
+                                 unsigned bufsize)
 {
     AHCIQState *ahci;
 
     ahci = ahci_boot_and_enable();
-    ahci_test_dma_rw_simple(ahci, bufsize);
+    ahci_test_io_rw_simple(ahci, bufsize,
+                           io_cmds[dma][lba48][IO_READ],
+                           io_cmds[dma][lba48][IO_WRITE]);
     ahci_shutdown(ahci);
 }
 
-static void test_dma_rw_simple(void)
+/**
+ * Demultiplex the test data and invoke the actual test routine.
+ */
+static void test_io_interface(gconstpointer opaque)
 {
-    test_dma_rw_interface(4096);
-}
+    AHCIIOTestOptions *opts = (AHCIIOTestOptions *)opaque;
+    unsigned bufsize;
 
-static void test_dma_rw_double(void)
-{
-    test_dma_rw_interface(8192);
-}
+    switch (opts->length) {
+    case LEN_SIMPLE:
+        bufsize = 4096;
+        break;
+    case LEN_DOUBLE:
+        bufsize = 8192;
+        break;
+    case LEN_LONG:
+        bufsize = 4096 * 64;
+        break;
+    case LEN_SHORT:
+        bufsize = 512;
+        break;
+    default:
+        g_assert_not_reached();
+    }
 
-static void test_dma_rw_long(void)
-{
-    test_dma_rw_interface(4096 * 64);
+    test_io_rw_interface(opts->address_type, opts->io_type, bufsize);
+    g_free(opts);
+    return;
 }
 
-static void test_dma_rw_short(void)
+static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
+                                enum BuffLen len)
 {
-    test_dma_rw_interface(512);
+    static const char *arch;
+    char *name;
+    AHCIIOTestOptions *opts = g_malloc(sizeof(AHCIIOTestOptions));
+
+    opts->length = len;
+    opts->address_type = addr;
+    opts->io_type = type;
+
+    if (!arch) {
+        arch = qtest_get_arch();
+    }
+
+    name = g_strdup_printf("/%s/ahci/io/%s/%s/%s", arch,
+                           io_mode_str[type],
+                           addr_mode_str[addr],
+                           buff_len_str[len]);
+
+    g_test_add_data_func(name, opts, test_io_interface);
+    g_free(name);
 }
 
 /******************************************************************************/
@@ -890,6 +997,7 @@ int main(int argc, char **argv)
     int fd;
     int ret;
     int c;
+    int i, j, k;
 
     static struct option long_options[] = {
         {"pedantic", no_argument, 0, 'p' },
@@ -937,10 +1045,14 @@ int main(int argc, char **argv)
     qtest_add_func("/ahci/hba_spec",   test_hba_spec);
     qtest_add_func("/ahci/hba_enable", test_hba_enable);
     qtest_add_func("/ahci/identify",   test_identify);
-    qtest_add_func("/ahci/dma/simple", test_dma_rw_simple);
-    qtest_add_func("/ahci/dma/double", test_dma_rw_double);
-    qtest_add_func("/ahci/dma/long",   test_dma_rw_long);
-    qtest_add_func("/ahci/dma/short",  test_dma_rw_short);
+
+    for (i = MODE_BEGIN; i < NUM_MODES; i++) {
+        for (j = ADDR_MODE_BEGIN; j < NUM_ADDR_MODES; j++) {
+            for (k = LEN_BEGIN; k < NUM_LENGTHS; k++) {
+                create_ahci_io_test(i, j, k);
+            }
+        }
+    }
 
     ret = g_test_run();
 
-- 
1.9.3

[Qemu-devel] [PATCH v2 6/8] qtest/ahci: add fragmented dma test

[John Snow]

Test what happens when we try to use extremely short PRDTs
to accomplish a small data transfer.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 21f20f7..cf0b98b 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -851,6 +851,63 @@ static void test_identify(void)
     ahci_shutdown(ahci);
 }
 
+/**
+ * Fragmented DMA test: Perform a standard 4K DMA read/write
+ * test, but make sure the physical regions are fragmented to
+ * be very small, each just 32 bytes, to see how AHCI performs
+ * with chunks defined to be much less than a sector.
+ */
+static void test_dma_fragmented(void)
+{
+    AHCIQState *ahci;
+    AHCICommand *cmd;
+    uint8_t px;
+    size_t bufsize = 4096;
+    unsigned char *tx = g_malloc(bufsize);
+    unsigned char *rx = g_malloc0(bufsize);
+    unsigned i;
+    uint64_t ptr;
+
+    ahci = ahci_boot_and_enable();
+    px = ahci_port_select(ahci);
+    ahci_port_clear(ahci, px);
+
+    /* create pattern */
+    for (i = 0; i < bufsize; i++) {
+        tx[i] = (bufsize - i);
+    }
+
+    /* Create a DMA buffer in guest memory, and write our pattern to it. */
+    ptr = guest_alloc(ahci->parent->alloc, bufsize);
+    g_assert(ptr);
+    memwrite(ptr, tx, bufsize);
+
+    cmd = ahci_command_create(CMD_WRITE_DMA);
+    ahci_command_adjust(cmd, 0, ptr, bufsize, 32);
+    ahci_command_commit(ahci, cmd, px);
+    ahci_command_issue(ahci, cmd);
+    ahci_command_verify(ahci, cmd);
+    g_free(cmd);
+
+    cmd = ahci_command_create(CMD_READ_DMA);
+    ahci_command_adjust(cmd, 0, ptr, bufsize, 32);
+    ahci_command_commit(ahci, cmd, px);
+    ahci_command_issue(ahci, cmd);
+    ahci_command_verify(ahci, cmd);
+    g_free(cmd);
+
+    /* Read back the guest's receive buffer into local memory */
+    memread(ptr, rx, bufsize);
+    guest_free(ahci->parent->alloc, ptr);
+
+    g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0);
+
+    ahci_shutdown(ahci);
+
+    g_free(rx);
+    g_free(tx);
+}
+
 /******************************************************************************/
 /* AHCI I/O Test Matrix Definitions                                           */
 
@@ -1054,6 +1111,8 @@ int main(int argc, char **argv)
         }
     }
 
+    qtest_add_func("/ahci/io/dma/lba28/fragmented", test_dma_fragmented);
+
     ret = g_test_run();
 
     /* Cleanup */
-- 
1.9.3

[Qemu-devel] [PATCH v2 7/8] qtest/ahci: add qcow2 support to ahci-test

[John Snow]

This will enable the testing of high offsets without
wasting a lot of disk space, and does not impact the
previous tests.

mkimg and mkqcow2 are added to libqos for other tests.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/Makefile        |  1 +
 tests/ahci-test.c     | 16 ++++++----------
 tests/libqos/libqos.c | 37 +++++++++++++++++++++++++++++++++++++
 tests/libqos/libqos.h |  2 ++
 4 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index 307035c..09ecb66 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -413,6 +413,7 @@ GCOV_OPTIONS = -n $(if $(V),-f,)
 $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
 	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
 	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
+		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
 		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
 		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y),"GTESTER $@")
 	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \
diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index cf0b98b..3f93c15 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -39,8 +39,8 @@
 #include "hw/pci/pci_ids.h"
 #include "hw/pci/pci_regs.h"
 
-/* Test-specific defines. */
-#define TEST_IMAGE_SIZE    (64 * 1024 * 1024)
+/* Test-specific defines -- in MiB */
+#define TEST_IMAGE_SIZE_MB (200 * 1024)
 
 /*** Globals ***/
 static char tmp_path[] = "/tmp/qtest.XXXXXX";
@@ -81,7 +81,7 @@ static AHCIQState *ahci_boot(void)
     s = g_malloc0(sizeof(AHCIQState));
 
     cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
-        ",format=raw"
+        ",format=qcow2"
         " -M q35 "
         "-device ide-hd,drive=drive0 "
         "-global ide-hd.ver=%s";
@@ -1051,7 +1051,6 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
 int main(int argc, char **argv)
 {
     const char *arch;
-    int fd;
     int ret;
     int c;
     int i, j, k;
@@ -1088,12 +1087,9 @@ int main(int argc, char **argv)
         return 0;
     }
 
-    /* Create a temporary raw image */
-    fd = mkstemp(tmp_path);
-    g_assert(fd >= 0);
-    ret = ftruncate(fd, TEST_IMAGE_SIZE);
-    g_assert(ret == 0);
-    close(fd);
+    /* Create a temporary qcow2 image */
+    close(mkstemp(tmp_path));
+    mkqcow2(tmp_path, TEST_IMAGE_SIZE_MB);
 
     /* Run the tests */
     qtest_add_func("/ahci/sanity",     test_sanity);
diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
index bc8beb2..c825486 100644
--- a/tests/libqos/libqos.c
+++ b/tests/libqos/libqos.c
@@ -61,3 +61,40 @@ void qtest_shutdown(QOSState *qs)
     qtest_quit(qs->qts);
     g_free(qs);
 }
+
+void mkimg(const char *file, const char *fmt, unsigned size_mb)
+{
+    gchar *cli;
+    bool ret;
+    int rc;
+    GError *err = NULL;
+    char *qemu_img_path;
+    gchar *out, *out2;
+
+    qemu_img_path = getenv("QTEST_QEMU_IMG");
+    assert(qemu_img_path);
+
+    cli = g_strdup_printf("./%s create -f %s %s %uM", qemu_img_path,
+                          fmt, file, size_mb);
+    ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err);
+    if (err) {
+        fprintf(stderr, "%s\n", err->message);
+        g_error_free(err);
+    }
+    g_assert(ret && !err);
+
+    ret = g_spawn_check_exit_status(rc, &err);
+    if (err) {
+        fprintf(stderr, "%s\n", err->message);
+    }
+    g_assert(ret && !err);
+
+    g_free(out);
+    g_free(out2);
+    g_free(cli);
+}
+
+void mkqcow2(const char *file, unsigned size_mb)
+{
+    return mkimg(file, "qcow2", size_mb);
+}
diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
index 612d41e..8cb2987 100644
--- a/tests/libqos/libqos.h
+++ b/tests/libqos/libqos.h
@@ -19,6 +19,8 @@ typedef struct QOSState {
 QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap);
 QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt, ...);
 void qtest_shutdown(QOSState *qs);
+void mkimg(const char *file, const char *fmt, unsigned size_mb);
+void mkqcow2(const char *file, unsigned size_mb);
 
 static inline uint64_t qmalloc(QOSState *q, size_t bytes)
 {
-- 
1.9.3

[Qemu-devel] [PATCH v2 8/8] qtest/ahci: test different disk sectors

[John Snow]

Test sector offset 0, 1, and the last sector(s)
in LBA28 and LBA48 modes.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c   | 68 +++++++++++++++++++++++++++++++++++++++++++----------
 tests/libqos/ahci.c | 10 ++++----
 tests/libqos/ahci.h |  4 ++--
 3 files changed, 63 insertions(+), 19 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 3f93c15..fb4739f 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -41,6 +41,8 @@
 
 /* Test-specific defines -- in MiB */
 #define TEST_IMAGE_SIZE_MB (200 * 1024)
+#define TEST_IMAGE_SECTORS ((TEST_IMAGE_SIZE_MB / AHCI_SECTOR_SIZE)     \
+                            * 1024 * 1024)
 
 /*** Globals ***/
 static char tmp_path[] = "/tmp/qtest.XXXXXX";
@@ -712,7 +714,7 @@ static void ahci_test_identify(AHCIQState *ahci)
     ahci_port_clear(ahci, px);
 
     /* "Read" 512 bytes using CMD_IDENTIFY into the host buffer. */
-    ahci_io(ahci, px, CMD_IDENTIFY, &buff, buffsize);
+    ahci_io(ahci, px, CMD_IDENTIFY, &buff, buffsize, 0);
 
     /* Check serial number/version in the buffer */
     /* NB: IDENTIFY strings are packed in 16bit little endian chunks.
@@ -728,11 +730,12 @@ static void ahci_test_identify(AHCIQState *ahci)
     g_assert_cmphex(rc, ==, 0);
 
     sect_size = le16_to_cpu(*((uint16_t *)(&buff[5])));
-    g_assert_cmphex(sect_size, ==, 0x200);
+    g_assert_cmphex(sect_size, ==, AHCI_SECTOR_SIZE);
 }
 
 static void ahci_test_io_rw_simple(AHCIQState *ahci, unsigned bufsize,
-                                   uint8_t read_cmd, uint8_t write_cmd)
+                                   uint64_t sector, uint8_t read_cmd,
+                                   uint8_t write_cmd)
 {
     uint64_t ptr;
     uint8_t port;
@@ -758,9 +761,9 @@ static void ahci_test_io_rw_simple(AHCIQState *ahci, unsigned bufsize,
     memwrite(ptr, tx, bufsize);
 
     /* Write this buffer to disk, then read it back to the DMA buffer. */
-    ahci_guest_io(ahci, port, write_cmd, ptr, bufsize);
+    ahci_guest_io(ahci, port, write_cmd, ptr, bufsize, sector);
     qmemset(ptr, 0x00, bufsize);
-    ahci_guest_io(ahci, port, read_cmd, ptr, bufsize);
+    ahci_guest_io(ahci, port, read_cmd, ptr, bufsize, sector);
 
     /*** Read back the Data ***/
     memread(ptr, rx, bufsize);
@@ -948,12 +951,45 @@ enum IOOps {
     NUM_IO_OPS
 };
 
+enum OffsetType {
+    OFFSET_BEGIN = 0,
+    OFFSET_ZERO = OFFSET_BEGIN,
+    OFFSET_LOW,
+    OFFSET_HIGH,
+    NUM_OFFSETS
+};
+
+static const char *offset_str[NUM_OFFSETS] = { "zero", "low", "high" };
+
 typedef struct AHCIIOTestOptions {
     enum BuffLen length;
     enum AddrMode address_type;
     enum IOMode io_type;
+    enum OffsetType offset;
 } AHCIIOTestOptions;
 
+static uint64_t offset_sector(enum OffsetType ofst,
+                              enum AddrMode addr_type,
+                              uint64_t buffsize)
+{
+    uint64_t ceil;
+    uint64_t nsectors;
+
+    switch (ofst) {
+    case OFFSET_ZERO:
+        return 0;
+    case OFFSET_LOW:
+        return 1;
+    case OFFSET_HIGH:
+        ceil = (addr_type == ADDR_MODE_LBA28) ? 0xfffffff : 0xffffffffffff;
+        ceil = MIN(ceil, TEST_IMAGE_SECTORS - 1);
+        nsectors = buffsize / AHCI_SECTOR_SIZE;
+        return ceil - nsectors + 1;
+    default:
+        g_assert_not_reached();
+    }
+}
+
 /**
  * Table of possible I/O ATA commands given a set of enumerations.
  */
@@ -981,12 +1017,12 @@ static const uint8_t io_cmds[NUM_MODES][NUM_ADDR_MODES][NUM_IO_OPS] = {
  * transfer modes, and buffer sizes.
  */
 static void test_io_rw_interface(enum AddrMode lba48, enum IOMode dma,
-                                 unsigned bufsize)
+                                 unsigned bufsize, uint64_t sector)
 {
     AHCIQState *ahci;
 
     ahci = ahci_boot_and_enable();
-    ahci_test_io_rw_simple(ahci, bufsize,
+    ahci_test_io_rw_simple(ahci, bufsize, sector,
                            io_cmds[dma][lba48][IO_READ],
                            io_cmds[dma][lba48][IO_WRITE]);
     ahci_shutdown(ahci);
@@ -999,6 +1035,7 @@ static void test_io_interface(gconstpointer opaque)
 {
     AHCIIOTestOptions *opts = (AHCIIOTestOptions *)opaque;
     unsigned bufsize;
+    uint64_t sector;
 
     switch (opts->length) {
     case LEN_SIMPLE:
@@ -1017,13 +1054,14 @@ static void test_io_interface(gconstpointer opaque)
         g_assert_not_reached();
     }
 
-    test_io_rw_interface(opts->address_type, opts->io_type, bufsize);
+    sector = offset_sector(opts->offset, opts->address_type, bufsize);
+    test_io_rw_interface(opts->address_type, opts->io_type, bufsize, sector);
     g_free(opts);
     return;
 }
 
 static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
-                                enum BuffLen len)
+                                enum BuffLen len, enum OffsetType offset)
 {
     static const char *arch;
     char *name;
@@ -1032,15 +1070,17 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
     opts->length = len;
     opts->address_type = addr;
     opts->io_type = type;
+    opts->offset = offset;
 
     if (!arch) {
         arch = qtest_get_arch();
     }
 
-    name = g_strdup_printf("/%s/ahci/io/%s/%s/%s", arch,
+    name = g_strdup_printf("/%s/ahci/io/%s/%s/%s/%s", arch,
                            io_mode_str[type],
                            addr_mode_str[addr],
-                           buff_len_str[len]);
+                           buff_len_str[len],
+                           offset_str[offset]);
 
     g_test_add_data_func(name, opts, test_io_interface);
     g_free(name);
@@ -1053,7 +1093,7 @@ int main(int argc, char **argv)
     const char *arch;
     int ret;
     int c;
-    int i, j, k;
+    int i, j, k, m;
 
     static struct option long_options[] = {
         {"pedantic", no_argument, 0, 'p' },
@@ -1102,7 +1142,9 @@ int main(int argc, char **argv)
     for (i = MODE_BEGIN; i < NUM_MODES; i++) {
         for (j = ADDR_MODE_BEGIN; j < NUM_ADDR_MODES; j++) {
             for (k = LEN_BEGIN; k < NUM_LENGTHS; k++) {
-                create_ahci_io_test(i, j, k);
+                for (m = OFFSET_BEGIN; m < NUM_OFFSETS; m++) {
+                    create_ahci_io_test(i, j, k, m);
+                }
             }
         }
     }
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 14651ff..dbd69b0 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -568,13 +568,15 @@ inline unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd)
 
 /* Given a guest buffer address, perform an IO operation */
 void ahci_guest_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd,
-                   uint64_t buffer, size_t bufsize)
+                   uint64_t buffer, size_t bufsize, uint64_t sector)
 {
     AHCICommand *cmd;
-
     cmd = ahci_command_create(ide_cmd);
     ahci_command_set_buffer(cmd, buffer);
     ahci_command_set_size(cmd, bufsize);
+    if (sector) {
+        ahci_command_set_offset(cmd, sector);
+    }
     ahci_command_commit(ahci, cmd, port);
     ahci_command_issue(ahci, cmd);
     ahci_command_verify(ahci, cmd);
@@ -612,7 +614,7 @@ static AHCICommandProp *ahci_command_find(uint8_t command_name)
 
 /* Given a HOST buffer, create a buffer address and perform an IO operation. */
 void ahci_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd,
-             void *buffer, size_t bufsize)
+             void *buffer, size_t bufsize, uint64_t sector)
 {
     uint64_t ptr;
     AHCICommandProp *props;
@@ -626,7 +628,7 @@ void ahci_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd,
         memwrite(ptr, buffer, bufsize);
     }
 
-    ahci_guest_io(ahci, port, ide_cmd, ptr, bufsize);
+    ahci_guest_io(ahci, port, ide_cmd, ptr, bufsize, sector);
 
     if (props->read) {
         memread(ptr, buffer, bufsize);
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 888545d..40e8ca4 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -523,9 +523,9 @@ void ahci_write_fis(AHCIQState *ahci, RegH2DFIS *fis, uint64_t addr);
 unsigned ahci_pick_cmd(AHCIQState *ahci, uint8_t port);
 unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd);
 void ahci_guest_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd,
-                   uint64_t gbuffer, size_t size);
+                   uint64_t gbuffer, size_t size, uint64_t sector);
 void ahci_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd,
-             void *buffer, size_t bufsize);
+             void *buffer, size_t bufsize, uint64_t sector);
 
 /* Command Lifecycle */
 AHCICommand *ahci_command_create(uint8_t command_name);
-- 
1.9.3

Re: [Qemu-devel] [PATCH v2 0/8] ahci: add more IO tests

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1333 bytes --]

On Wed, Feb 25, 2015 at 06:06:34PM -0500, John Snow wrote:
> This series is based on top of my ahci DMA test series, which is in
> turn based on the ahci preliminary refactoring series. Both are currently
> pending on stefanha/block.
> 
> This series adds many variations that expand on the existing trivial
> DMA I/O case. These pathways check different PRDT configurations,
> different I/O commands for PIO and DMA, different address scheme
> combinations for LBA28 and LBA48, and different sector offsets.
> 
> John Snow (8):
>   libqos/ahci: Zero-fill AHCI headers
>   qtest/ahci: Add a macro bootup routine
>   libqos/ahci: add ahci command helpers
>   qtest/ahci: Add DMA test variants
>   qtest/ahci: Add PIO and LBA48 tests
>   qtest/ahci: add fragmented dma test
>   qtest/ahci: add qcow2 support to ahci-test
>   qtest/ahci: test different disk sectors
> 
>  tests/Makefile        |   1 +
>  tests/ahci-test.c     | 296 +++++++++++++++++++++++++++++++++++++++++++++-----
>  tests/libqos/ahci.c   |  54 ++++++++-
>  tests/libqos/ahci.h   |   9 +-
>  tests/libqos/libqos.c |  37 +++++++
>  tests/libqos/libqos.h |   2 +
>  6 files changed, 365 insertions(+), 34 deletions(-)
> 
> -- 
> 1.9.3
> 
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v2 7/8] qtest/ahci: add qcow2 support to ahci-test

[Kevin Wolf]

Am 26.02.2015 um 00:06 hat John Snow geschrieben:
> This will enable the testing of high offsets without
> wasting a lot of disk space, and does not impact the
> previous tests.
> 
> mkimg and mkqcow2 are added to libqos for other tests.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/Makefile        |  1 +
>  tests/ahci-test.c     | 16 ++++++----------
>  tests/libqos/libqos.c | 37 +++++++++++++++++++++++++++++++++++++
>  tests/libqos/libqos.h |  2 ++
>  4 files changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/Makefile b/tests/Makefile
> index 307035c..09ecb66 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -413,6 +413,7 @@ GCOV_OPTIONS = -n $(if $(V),-f,)
>  $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
>  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
>  	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
> +		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
>  		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
>  		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y),"GTESTER $@")
>  	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \
> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> index cf0b98b..3f93c15 100644
> --- a/tests/ahci-test.c
> +++ b/tests/ahci-test.c
> @@ -39,8 +39,8 @@
>  #include "hw/pci/pci_ids.h"
>  #include "hw/pci/pci_regs.h"
>  
> -/* Test-specific defines. */
> -#define TEST_IMAGE_SIZE    (64 * 1024 * 1024)
> +/* Test-specific defines -- in MiB */
> +#define TEST_IMAGE_SIZE_MB (200 * 1024)
>  
>  /*** Globals ***/
>  static char tmp_path[] = "/tmp/qtest.XXXXXX";
> @@ -81,7 +81,7 @@ static AHCIQState *ahci_boot(void)
>      s = g_malloc0(sizeof(AHCIQState));
>  
>      cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
> -        ",format=raw"
> +        ",format=qcow2"
>          " -M q35 "
>          "-device ide-hd,drive=drive0 "
>          "-global ide-hd.ver=%s";
> @@ -1051,7 +1051,6 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
>  int main(int argc, char **argv)
>  {
>      const char *arch;
> -    int fd;
>      int ret;
>      int c;
>      int i, j, k;
> @@ -1088,12 +1087,9 @@ int main(int argc, char **argv)
>          return 0;
>      }
>  
> -    /* Create a temporary raw image */
> -    fd = mkstemp(tmp_path);
> -    g_assert(fd >= 0);
> -    ret = ftruncate(fd, TEST_IMAGE_SIZE);
> -    g_assert(ret == 0);
> -    close(fd);
> +    /* Create a temporary qcow2 image */
> +    close(mkstemp(tmp_path));
> +    mkqcow2(tmp_path, TEST_IMAGE_SIZE_MB);
>  
>      /* Run the tests */
>      qtest_add_func("/ahci/sanity",     test_sanity);
> diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
> index bc8beb2..c825486 100644
> --- a/tests/libqos/libqos.c
> +++ b/tests/libqos/libqos.c
> @@ -61,3 +61,40 @@ void qtest_shutdown(QOSState *qs)
>      qtest_quit(qs->qts);
>      g_free(qs);
>  }
> +
> +void mkimg(const char *file, const char *fmt, unsigned size_mb)
> +{
> +    gchar *cli;
> +    bool ret;
> +    int rc;
> +    GError *err = NULL;
> +    char *qemu_img_path;
> +    gchar *out, *out2;
> +
> +    qemu_img_path = getenv("QTEST_QEMU_IMG");
> +    assert(qemu_img_path);
> +
> +    cli = g_strdup_printf("./%s create -f %s %s %uM", qemu_img_path,
> +                          fmt, file, size_mb);
> +    ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err);
> +    if (err) {
> +        fprintf(stderr, "%s\n", err->message);
> +        g_error_free(err);
> +    }
> +    g_assert(ret && !err);
> +
> +    ret = g_spawn_check_exit_status(rc, &err);

This function only exists since glib 2.34. Dropping the following
patches from the queue:

pick 23134a5 qtest/ahci: add qcow2 support to ahci-test
pick 6ca5609 qtest/ahci: test different disk sectors
pick e2f0dee qtest/ahci: Add simple flush test
pick 396491b qtest/ahci: Allow override of default CLI options
pick eb8c8bd libqtest: add qmp_eventwait
pick 398bfc3 libqtest: add qmp_async
pick d3f77d1 libqos: add blkdebug_prepare_script
pick d628e51 qtest/ahci: add flush retry test

This is patch 7 and 8 from this series and the complete series "ahci:
rerror/werror=stop resume tests", which seems to depend on them.

Kevin

Re: [Qemu-devel] [PATCH v2 7/8] qtest/ahci: add qcow2 support to ahci-test

[John Snow]

On 03/09/2015 10:27 AM, Kevin Wolf wrote:
> Am 26.02.2015 um 00:06 hat John Snow geschrieben:
>> This will enable the testing of high offsets without
>> wasting a lot of disk space, and does not impact the
>> previous tests.
>>
>> mkimg and mkqcow2 are added to libqos for other tests.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   tests/Makefile        |  1 +
>>   tests/ahci-test.c     | 16 ++++++----------
>>   tests/libqos/libqos.c | 37 +++++++++++++++++++++++++++++++++++++
>>   tests/libqos/libqos.h |  2 ++
>>   4 files changed, 46 insertions(+), 10 deletions(-)
>>
>> diff --git a/tests/Makefile b/tests/Makefile
>> index 307035c..09ecb66 100644
>> --- a/tests/Makefile
>> +++ b/tests/Makefile
>> @@ -413,6 +413,7 @@ GCOV_OPTIONS = -n $(if $(V),-f,)
>>   $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
>>   	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
>>   	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
>> +		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
>>   		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
>>   		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y),"GTESTER $@")
>>   	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \
>> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
>> index cf0b98b..3f93c15 100644
>> --- a/tests/ahci-test.c
>> +++ b/tests/ahci-test.c
>> @@ -39,8 +39,8 @@
>>   #include "hw/pci/pci_ids.h"
>>   #include "hw/pci/pci_regs.h"
>>
>> -/* Test-specific defines. */
>> -#define TEST_IMAGE_SIZE    (64 * 1024 * 1024)
>> +/* Test-specific defines -- in MiB */
>> +#define TEST_IMAGE_SIZE_MB (200 * 1024)
>>
>>   /*** Globals ***/
>>   static char tmp_path[] = "/tmp/qtest.XXXXXX";
>> @@ -81,7 +81,7 @@ static AHCIQState *ahci_boot(void)
>>       s = g_malloc0(sizeof(AHCIQState));
>>
>>       cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
>> -        ",format=raw"
>> +        ",format=qcow2"
>>           " -M q35 "
>>           "-device ide-hd,drive=drive0 "
>>           "-global ide-hd.ver=%s";
>> @@ -1051,7 +1051,6 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
>>   int main(int argc, char **argv)
>>   {
>>       const char *arch;
>> -    int fd;
>>       int ret;
>>       int c;
>>       int i, j, k;
>> @@ -1088,12 +1087,9 @@ int main(int argc, char **argv)
>>           return 0;
>>       }
>>
>> -    /* Create a temporary raw image */
>> -    fd = mkstemp(tmp_path);
>> -    g_assert(fd >= 0);
>> -    ret = ftruncate(fd, TEST_IMAGE_SIZE);
>> -    g_assert(ret == 0);
>> -    close(fd);
>> +    /* Create a temporary qcow2 image */
>> +    close(mkstemp(tmp_path));
>> +    mkqcow2(tmp_path, TEST_IMAGE_SIZE_MB);
>>
>>       /* Run the tests */
>>       qtest_add_func("/ahci/sanity",     test_sanity);
>> diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
>> index bc8beb2..c825486 100644
>> --- a/tests/libqos/libqos.c
>> +++ b/tests/libqos/libqos.c
>> @@ -61,3 +61,40 @@ void qtest_shutdown(QOSState *qs)
>>       qtest_quit(qs->qts);
>>       g_free(qs);
>>   }
>> +
>> +void mkimg(const char *file, const char *fmt, unsigned size_mb)
>> +{
>> +    gchar *cli;
>> +    bool ret;
>> +    int rc;
>> +    GError *err = NULL;
>> +    char *qemu_img_path;
>> +    gchar *out, *out2;
>> +
>> +    qemu_img_path = getenv("QTEST_QEMU_IMG");
>> +    assert(qemu_img_path);
>> +
>> +    cli = g_strdup_printf("./%s create -f %s %s %uM", qemu_img_path,
>> +                          fmt, file, size_mb);
>> +    ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err);
>> +    if (err) {
>> +        fprintf(stderr, "%s\n", err->message);
>> +        g_error_free(err);
>> +    }
>> +    g_assert(ret && !err);
>> +
>> +    ret = g_spawn_check_exit_status(rc, &err);
>
> This function only exists since glib 2.34. Dropping the following
> patches from the queue:
>
> pick 23134a5 qtest/ahci: add qcow2 support to ahci-test
> pick 6ca5609 qtest/ahci: test different disk sectors
> pick e2f0dee qtest/ahci: Add simple flush test
> pick 396491b qtest/ahci: Allow override of default CLI options
> pick eb8c8bd libqtest: add qmp_eventwait
> pick 398bfc3 libqtest: add qmp_async
> pick d3f77d1 libqos: add blkdebug_prepare_script
> pick d628e51 qtest/ahci: add flush retry test
>
> This is patch 7 and 8 from this series and the complete series "ahci:
> rerror/werror=stop resume tests", which seems to depend on them.
>
> Kevin
>

Do we have a policy on glib version for qtests? I know we require a 
specific version for within QEMU itself, but there are many instances of 
functions newer than that being using in qtests already.

Re: [Qemu-devel] [PATCH v2 7/8] qtest/ahci: add qcow2 support to ahci-test

[Kevin Wolf]

Am 09.03.2015 um 17:03 hat John Snow geschrieben:
> On 03/09/2015 10:27 AM, Kevin Wolf wrote:
> >Am 26.02.2015 um 00:06 hat John Snow geschrieben:
> >>+void mkimg(const char *file, const char *fmt, unsigned size_mb)
> >>+{
> >>+    gchar *cli;
> >>+    bool ret;
> >>+    int rc;
> >>+    GError *err = NULL;
> >>+    char *qemu_img_path;
> >>+    gchar *out, *out2;
> >>+
> >>+    qemu_img_path = getenv("QTEST_QEMU_IMG");
> >>+    assert(qemu_img_path);
> >>+
> >>+    cli = g_strdup_printf("./%s create -f %s %s %uM", qemu_img_path,
> >>+                          fmt, file, size_mb);
> >>+    ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err);
> >>+    if (err) {
> >>+        fprintf(stderr, "%s\n", err->message);
> >>+        g_error_free(err);
> >>+    }
> >>+    g_assert(ret && !err);
> >>+
> >>+    ret = g_spawn_check_exit_status(rc, &err);
> >
> >This function only exists since glib 2.34. Dropping the following
> >patches from the queue:
> >
> >pick 23134a5 qtest/ahci: add qcow2 support to ahci-test
> >pick 6ca5609 qtest/ahci: test different disk sectors
> >pick e2f0dee qtest/ahci: Add simple flush test
> >pick 396491b qtest/ahci: Allow override of default CLI options
> >pick eb8c8bd libqtest: add qmp_eventwait
> >pick 398bfc3 libqtest: add qmp_async
> >pick d3f77d1 libqos: add blkdebug_prepare_script
> >pick d628e51 qtest/ahci: add flush retry test
> >
> >This is patch 7 and 8 from this series and the complete series "ahci:
> >rerror/werror=stop resume tests", which seems to depend on them.
> >
> >Kevin
> >
> 
> Do we have a policy on glib version for qtests? I know we require a
> specific version for within QEMU itself, but there are many
> instances of functions newer than that being using in qtests
> already.

I would have assumed that it's the same requirement and make check
should work whereever you can build qemu. In practice, however, I'm
running qtests on a RHEL 6 host (without this series that still works
fine) and complaining only if things break there.

Kevin

Re: [Qemu-devel] [PATCH v2 7/8] qtest/ahci: add qcow2 support to ahci-test

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On 03/09/2015 10:27 AM, Kevin Wolf wrote:
>> Am 26.02.2015 um 00:06 hat John Snow geschrieben:
>>> This will enable the testing of high offsets without
>>> wasting a lot of disk space, and does not impact the
>>> previous tests.
>>>
>>> mkimg and mkqcow2 are added to libqos for other tests.
>>>
>>> Signed-off-by: John Snow <jsnow@redhat.com>
[...]
>> This function only exists since glib 2.34. Dropping the following
>> patches from the queue:
>>
>> pick 23134a5 qtest/ahci: add qcow2 support to ahci-test
>> pick 6ca5609 qtest/ahci: test different disk sectors
>> pick e2f0dee qtest/ahci: Add simple flush test
>> pick 396491b qtest/ahci: Allow override of default CLI options
>> pick eb8c8bd libqtest: add qmp_eventwait
>> pick 398bfc3 libqtest: add qmp_async
>> pick d3f77d1 libqos: add blkdebug_prepare_script
>> pick d628e51 qtest/ahci: add flush retry test
>>
>> This is patch 7 and 8 from this series and the complete series "ahci:
>> rerror/werror=stop resume tests", which seems to depend on them.
>>
>> Kevin
>>
>
> Do we have a policy on glib version for qtests? I know we require a
> specific version for within QEMU itself, but there are many instances
> of functions newer than that being using in qtests already.

We should have one.  Peter?

Re: [Qemu-devel] [PATCH v2 7/8] qtest/ahci: add qcow2 support to ahci-test

[John Snow]

On 03/09/2015 12:27 PM, Kevin Wolf wrote:
> Am 09.03.2015 um 17:03 hat John Snow geschrieben:
>> On 03/09/2015 10:27 AM, Kevin Wolf wrote:
>>> Am 26.02.2015 um 00:06 hat John Snow geschrieben:
>>>> +void mkimg(const char *file, const char *fmt, unsigned size_mb)
>>>> +{
>>>> +    gchar *cli;
>>>> +    bool ret;
>>>> +    int rc;
>>>> +    GError *err = NULL;
>>>> +    char *qemu_img_path;
>>>> +    gchar *out, *out2;
>>>> +
>>>> +    qemu_img_path = getenv("QTEST_QEMU_IMG");
>>>> +    assert(qemu_img_path);
>>>> +
>>>> +    cli = g_strdup_printf("./%s create -f %s %s %uM", qemu_img_path,
>>>> +                          fmt, file, size_mb);
>>>> +    ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err);
>>>> +    if (err) {
>>>> +        fprintf(stderr, "%s\n", err->message);
>>>> +        g_error_free(err);
>>>> +    }
>>>> +    g_assert(ret && !err);
>>>> +
>>>> +    ret = g_spawn_check_exit_status(rc, &err);
>>>
>>> This function only exists since glib 2.34. Dropping the following
>>> patches from the queue:
>>>
>>> pick 23134a5 qtest/ahci: add qcow2 support to ahci-test
>>> pick 6ca5609 qtest/ahci: test different disk sectors
>>> pick e2f0dee qtest/ahci: Add simple flush test
>>> pick 396491b qtest/ahci: Allow override of default CLI options
>>> pick eb8c8bd libqtest: add qmp_eventwait
>>> pick 398bfc3 libqtest: add qmp_async
>>> pick d3f77d1 libqos: add blkdebug_prepare_script
>>> pick d628e51 qtest/ahci: add flush retry test
>>>
>>> This is patch 7 and 8 from this series and the complete series "ahci:
>>> rerror/werror=stop resume tests", which seems to depend on them.
>>>
>>> Kevin
>>>
>>
>> Do we have a policy on glib version for qtests? I know we require a
>> specific version for within QEMU itself, but there are many
>> instances of functions newer than that being using in qtests
>> already.
>
> I would have assumed that it's the same requirement and make check
> should work whereever you can build qemu. In practice, however, I'm
> running qtests on a RHEL 6 host (without this series that still works
> fine) and complaining only if things break there.
>
> Kevin
>

It looks like we currently require 2.12 for the basic builds, but pretty 
much the entirety of the gtest framework itself was introduced in 2.16, 
hence the ask.

I saw some discussion recently of bumping the glib version required, but 
I think MST circumvented the need to do so for the time-being.

Still, we should at least bump this up to 2.16 to have some consistency 
with the testing framework... Or acknowledge that the testing framework 
may have a different set of requirements.

--js

Re: [Qemu-devel] [PATCH v2 7/8] qtest/ahci: add qcow2 support to ahci-test

[John Snow]

On 03/09/2015 10:27 AM, Kevin Wolf wrote:
> Am 26.02.2015 um 00:06 hat John Snow geschrieben:
>> This will enable the testing of high offsets without
>> wasting a lot of disk space, and does not impact the
>> previous tests.
>>
>> mkimg and mkqcow2 are added to libqos for other tests.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   tests/Makefile        |  1 +
>>   tests/ahci-test.c     | 16 ++++++----------
>>   tests/libqos/libqos.c | 37 +++++++++++++++++++++++++++++++++++++
>>   tests/libqos/libqos.h |  2 ++
>>   4 files changed, 46 insertions(+), 10 deletions(-)
>>
>> diff --git a/tests/Makefile b/tests/Makefile
>> index 307035c..09ecb66 100644
>> --- a/tests/Makefile
>> +++ b/tests/Makefile
>> @@ -413,6 +413,7 @@ GCOV_OPTIONS = -n $(if $(V),-f,)
>>   $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
>>   	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
>>   	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
>> +		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
>>   		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
>>   		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y),"GTESTER $@")
>>   	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \
>> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
>> index cf0b98b..3f93c15 100644
>> --- a/tests/ahci-test.c
>> +++ b/tests/ahci-test.c
>> @@ -39,8 +39,8 @@
>>   #include "hw/pci/pci_ids.h"
>>   #include "hw/pci/pci_regs.h"
>>
>> -/* Test-specific defines. */
>> -#define TEST_IMAGE_SIZE    (64 * 1024 * 1024)
>> +/* Test-specific defines -- in MiB */
>> +#define TEST_IMAGE_SIZE_MB (200 * 1024)
>>
>>   /*** Globals ***/
>>   static char tmp_path[] = "/tmp/qtest.XXXXXX";
>> @@ -81,7 +81,7 @@ static AHCIQState *ahci_boot(void)
>>       s = g_malloc0(sizeof(AHCIQState));
>>
>>       cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
>> -        ",format=raw"
>> +        ",format=qcow2"
>>           " -M q35 "
>>           "-device ide-hd,drive=drive0 "
>>           "-global ide-hd.ver=%s";
>> @@ -1051,7 +1051,6 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
>>   int main(int argc, char **argv)
>>   {
>>       const char *arch;
>> -    int fd;
>>       int ret;
>>       int c;
>>       int i, j, k;
>> @@ -1088,12 +1087,9 @@ int main(int argc, char **argv)
>>           return 0;
>>       }
>>
>> -    /* Create a temporary raw image */
>> -    fd = mkstemp(tmp_path);
>> -    g_assert(fd >= 0);
>> -    ret = ftruncate(fd, TEST_IMAGE_SIZE);
>> -    g_assert(ret == 0);
>> -    close(fd);
>> +    /* Create a temporary qcow2 image */
>> +    close(mkstemp(tmp_path));
>> +    mkqcow2(tmp_path, TEST_IMAGE_SIZE_MB);
>>
>>       /* Run the tests */
>>       qtest_add_func("/ahci/sanity",     test_sanity);
>> diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
>> index bc8beb2..c825486 100644
>> --- a/tests/libqos/libqos.c
>> +++ b/tests/libqos/libqos.c
>> @@ -61,3 +61,40 @@ void qtest_shutdown(QOSState *qs)
>>       qtest_quit(qs->qts);
>>       g_free(qs);
>>   }
>> +
>> +void mkimg(const char *file, const char *fmt, unsigned size_mb)
>> +{
>> +    gchar *cli;
>> +    bool ret;
>> +    int rc;
>> +    GError *err = NULL;
>> +    char *qemu_img_path;
>> +    gchar *out, *out2;
>> +
>> +    qemu_img_path = getenv("QTEST_QEMU_IMG");
>> +    assert(qemu_img_path);
>> +
>> +    cli = g_strdup_printf("./%s create -f %s %s %uM", qemu_img_path,
>> +                          fmt, file, size_mb);
>> +    ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err);
>> +    if (err) {
>> +        fprintf(stderr, "%s\n", err->message);
>> +        g_error_free(err);
>> +    }
>> +    g_assert(ret && !err);
>> +
>> +    ret = g_spawn_check_exit_status(rc, &err);
>
> This function only exists since glib 2.34. Dropping the following
> patches from the queue:
>

I'm looking at this some more. The glib code basically does this:

If windows: Set an error if rc is nonzero.
If linux: use the WIFEXITED, WIFSIGNALED or WIFSTOPPED macros to 
determine the domain of the error code. If WIFEXITED, check for nonzero 
status. if WIFSIGNALED, WIFSTOPPED or !WIFEXITED, return an error.

I *think* I can just generalize this all, if I am not interested in 
*why* we failed, to just checking rc to be nonzero. That should be 
adequately multiplatform.

Agree?

> pick 23134a5 qtest/ahci: add qcow2 support to ahci-test
> pick 6ca5609 qtest/ahci: test different disk sectors
> pick e2f0dee qtest/ahci: Add simple flush test
> pick 396491b qtest/ahci: Allow override of default CLI options
> pick eb8c8bd libqtest: add qmp_eventwait
> pick 398bfc3 libqtest: add qmp_async
> pick d3f77d1 libqos: add blkdebug_prepare_script
> pick d628e51 qtest/ahci: add flush retry test
>
> This is patch 7 and 8 from this series and the complete series "ahci:
> rerror/werror=stop resume tests", which seems to depend on them.
>
> Kevin
>

Re: [Qemu-devel] [PATCH v2 7/8] qtest/ahci: add qcow2 support to ahci-test

[Kevin Wolf]

Am 09.03.2015 um 21:34 hat John Snow geschrieben:
> 
> 
> On 03/09/2015 10:27 AM, Kevin Wolf wrote:
> >Am 26.02.2015 um 00:06 hat John Snow geschrieben:
> >>This will enable the testing of high offsets without
> >>wasting a lot of disk space, and does not impact the
> >>previous tests.
> >>
> >>mkimg and mkqcow2 are added to libqos for other tests.
> >>
> >>Signed-off-by: John Snow <jsnow@redhat.com>
> >>---
> >>  tests/Makefile        |  1 +
> >>  tests/ahci-test.c     | 16 ++++++----------
> >>  tests/libqos/libqos.c | 37 +++++++++++++++++++++++++++++++++++++
> >>  tests/libqos/libqos.h |  2 ++
> >>  4 files changed, 46 insertions(+), 10 deletions(-)
> >>
> >>diff --git a/tests/Makefile b/tests/Makefile
> >>index 307035c..09ecb66 100644
> >>--- a/tests/Makefile
> >>+++ b/tests/Makefile
> >>@@ -413,6 +413,7 @@ GCOV_OPTIONS = -n $(if $(V),-f,)
> >>  $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
> >>  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
> >>  	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
> >>+		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
> >>  		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
> >>  		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y),"GTESTER $@")
> >>  	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \
> >>diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> >>index cf0b98b..3f93c15 100644
> >>--- a/tests/ahci-test.c
> >>+++ b/tests/ahci-test.c
> >>@@ -39,8 +39,8 @@
> >>  #include "hw/pci/pci_ids.h"
> >>  #include "hw/pci/pci_regs.h"
> >>
> >>-/* Test-specific defines. */
> >>-#define TEST_IMAGE_SIZE    (64 * 1024 * 1024)
> >>+/* Test-specific defines -- in MiB */
> >>+#define TEST_IMAGE_SIZE_MB (200 * 1024)
> >>
> >>  /*** Globals ***/
> >>  static char tmp_path[] = "/tmp/qtest.XXXXXX";
> >>@@ -81,7 +81,7 @@ static AHCIQState *ahci_boot(void)
> >>      s = g_malloc0(sizeof(AHCIQState));
> >>
> >>      cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
> >>-        ",format=raw"
> >>+        ",format=qcow2"
> >>          " -M q35 "
> >>          "-device ide-hd,drive=drive0 "
> >>          "-global ide-hd.ver=%s";
> >>@@ -1051,7 +1051,6 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
> >>  int main(int argc, char **argv)
> >>  {
> >>      const char *arch;
> >>-    int fd;
> >>      int ret;
> >>      int c;
> >>      int i, j, k;
> >>@@ -1088,12 +1087,9 @@ int main(int argc, char **argv)
> >>          return 0;
> >>      }
> >>
> >>-    /* Create a temporary raw image */
> >>-    fd = mkstemp(tmp_path);
> >>-    g_assert(fd >= 0);
> >>-    ret = ftruncate(fd, TEST_IMAGE_SIZE);
> >>-    g_assert(ret == 0);
> >>-    close(fd);
> >>+    /* Create a temporary qcow2 image */
> >>+    close(mkstemp(tmp_path));
> >>+    mkqcow2(tmp_path, TEST_IMAGE_SIZE_MB);
> >>
> >>      /* Run the tests */
> >>      qtest_add_func("/ahci/sanity",     test_sanity);
> >>diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
> >>index bc8beb2..c825486 100644
> >>--- a/tests/libqos/libqos.c
> >>+++ b/tests/libqos/libqos.c
> >>@@ -61,3 +61,40 @@ void qtest_shutdown(QOSState *qs)
> >>      qtest_quit(qs->qts);
> >>      g_free(qs);
> >>  }
> >>+
> >>+void mkimg(const char *file, const char *fmt, unsigned size_mb)
> >>+{
> >>+    gchar *cli;
> >>+    bool ret;
> >>+    int rc;
> >>+    GError *err = NULL;
> >>+    char *qemu_img_path;
> >>+    gchar *out, *out2;
> >>+
> >>+    qemu_img_path = getenv("QTEST_QEMU_IMG");
> >>+    assert(qemu_img_path);
> >>+
> >>+    cli = g_strdup_printf("./%s create -f %s %s %uM", qemu_img_path,
> >>+                          fmt, file, size_mb);
> >>+    ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err);
> >>+    if (err) {
> >>+        fprintf(stderr, "%s\n", err->message);
> >>+        g_error_free(err);
> >>+    }
> >>+    g_assert(ret && !err);
> >>+
> >>+    ret = g_spawn_check_exit_status(rc, &err);
> >
> >This function only exists since glib 2.34. Dropping the following
> >patches from the queue:
> >
> 
> I'm looking at this some more. The glib code basically does this:
> 
> If windows: Set an error if rc is nonzero.
> If linux: use the WIFEXITED, WIFSIGNALED or WIFSTOPPED macros to
> determine the domain of the error code. If WIFEXITED, check for
> nonzero status. if WIFSIGNALED, WIFSTOPPED or !WIFEXITED, return an
> error.
> 
> I *think* I can just generalize this all, if I am not interested in
> *why* we failed, to just checking rc to be nonzero. That should be
> adequately multiplatform.
> 
> Agree?

I would think so, yes.

Kevin