[Qemu-devel] [PATCH v3 0/9] ahci: enable migration

[Qemu-devel] [PATCH v3 0/9] ahci: enable migration

[John Snow]

The day we all feared is here, and I am proposing we allow the migration
of the AHCI device tentatively for the 2.4 development window.

There are some more NCQ migration tests are needed, but I felt that it was
important to get migration enabled as close to the start of the 2.4
development window as possible.

If the NCQ patches don't pan out by the time the 2.4 freeze occurs, we can
revert the migration boolean and add a conditional around the ahci tests
that rely on the migration feature being enabled.

I am justifying this checkin based on a series of ping-pong
migration tests I ran under heavy load (using google's stressapptest)
and saw over 300 successful migrations without a single failure.

This series does a few things:
(1) Add migration facilities to libqos
(2) Enable AHCI and ICH9 migration
(3) Add a series of migration tests to ahci-test

v3:
 - Rebase and resend for 2.4.
 - Minor style guide fix.

v2:
 - Added a URI parameter to the migrate() helper
 - Adjust ahci_shutdown to set qtest context for itself
 - Make sure verify() is part of ahci_migrate() and redundant
   calls are eliminated
 - Add new helpers to make tests with blkdebug injections more
   succint
 - Change the flush migrate test to not load the blkdebug rule
   on the destination host
 - Modify the migrate() function so that it does not poll the
   VM for migration status if it can rely on RESUME events.
 - New patch: Repair the ahci_command_set_offset helper.
 - New test: Test DMA halt and resume.
 - New test: Test DMA halt, migrate, and resume.

==
For convenience, this branch is available at:
https://github.com/jnsnow/qemu.git branch ahci-migration-test
https://github.com/jnsnow/qemu/tree/ahci-migration-test

This version is tagged ahci-migration-test-v3:
https://github.com/jnsnow/qemu/releases/tag/ahci-migration-test-v3
==

John Snow (9):
  libqos/ahci: Add halted command helpers
  libqos/ahci: Fix sector set method
  libqos: Add migration helpers
  ich9/ahci: Enable Migration
  qtest/ahci: Add migration test
  qtest/ahci: add migrate dma test
  qtest/ahci: add flush migrate test
  qtest/ahci: add halted dma test
  qtest/ahci: add migrate halted dma test

 hw/ide/ahci.c         |   1 -
 hw/ide/ich.c          |   1 -
 tests/ahci-test.c     | 318 +++++++++++++++++++++++++++++++++++++++++++++++++-
 tests/libqos/ahci.c   |  34 +++++-
 tests/libqos/ahci.h   |   3 +
 tests/libqos/libqos.c |  84 +++++++++++++
 tests/libqos/libqos.h |   2 +
 tests/libqos/malloc.c |  74 +++++++++---
 tests/libqos/malloc.h |   1 +
 9 files changed, 496 insertions(+), 22 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v3 1/9] libqos/ahci: Add halted command helpers

[John Snow]

Sometimes we want a command to halt the VM instead
of complete successfully, so it'd be nice to let the
libqos/ahci functions cope with such scenarios.

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

diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index a18c12b..05dd04d 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -566,6 +566,33 @@ inline unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd)
     return (bytes + bytes_per_prd - 1) / bytes_per_prd;
 }
 
+/* Issue a command, expecting it to fail and STOP the VM */
+AHCICommand *ahci_guest_io_halt(AHCIQState *ahci, uint8_t port,
+                                uint8_t ide_cmd, uint64_t buffer,
+                                size_t bufsize, uint64_t sector)
+{
+    AHCICommand *cmd;
+
+    cmd = ahci_command_create(ide_cmd);
+    ahci_command_adjust(cmd, sector, buffer, bufsize, 0);
+    ahci_command_commit(ahci, cmd, port);
+    ahci_command_issue_async(ahci, cmd);
+    qmp_eventwait("STOP");
+
+    return cmd;
+}
+
+/* Resume a previously failed command and verify/finalize */
+void ahci_guest_io_resume(AHCIQState *ahci, AHCICommand *cmd)
+{
+    /* Complete the command */
+    qmp_async("{'execute':'cont' }");
+    qmp_eventwait("RESUME");
+    ahci_command_wait(ahci, cmd);
+    ahci_command_verify(ahci, cmd);
+    ahci_command_free(cmd);
+}
+
 /* 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 sector)
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 40e8ca4..779e812 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -524,6 +524,9 @@ 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 sector);
+AHCICommand *ahci_guest_io_halt(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd,
+                                uint64_t gbuffer, size_t size, uint64_t sector);
+void ahci_guest_io_resume(AHCIQState *ahci, AHCICommand *cmd);
 void ahci_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd,
              void *buffer, size_t bufsize, uint64_t sector);
 
-- 
2.1.0

[Qemu-devel] [PATCH v3 2/9] libqos/ahci: Fix sector set method

[John Snow]

|| probably does not mean the same thing as |.

Additionally, allow users to submit a prd_size of 0
to indicate that they'd like to continue using the default.

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

diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 05dd04d..29e12f9 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -769,7 +769,7 @@ void ahci_command_set_offset(AHCICommand *cmd, uint64_t lba_sect)
     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->device = (fis->device & 0xF0) | ((lba_sect >> 24) & 0x0F);
     }
     fis->lba_hi[0] = (lba_sect >> 24) & 0xFF;
     fis->lba_hi[1] = (lba_sect >> 32) & 0xFF;
@@ -787,7 +787,9 @@ void ahci_command_set_sizes(AHCICommand *cmd, uint64_t xbytes,
     /* Each PRD can describe up to 4MiB, and must not be odd. */
     g_assert_cmphex(prd_size, <=, 4096 * 1024);
     g_assert_cmphex(prd_size & 0x01, ==, 0x00);
-    cmd->prd_size = prd_size;
+    if (prd_size) {
+        cmd->prd_size = prd_size;
+    }
     cmd->xbytes = xbytes;
     cmd->fis.count = (cmd->xbytes / AHCI_SECTOR_SIZE);
     cmd->header.prdtl = size_to_prdtl(cmd->xbytes, cmd->prd_size);
-- 
2.1.0

[Qemu-devel] [PATCH v3 3/9] libqos: Add migration helpers

[John Snow]

libqos.c:
    -set_context for addressing which commands go where
    -migrate performs the actual migration

malloc.c:
    - Structure of the allocator is adjusted slightly with
      a second-tier malloc to make swapping around the allocators
      easy when we "migrate" the lists from the source to the destination.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/libqos/libqos.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/libqos.h |  2 ++
 tests/libqos/malloc.c | 74 ++++++++++++++++++++++++++++++++++-----------
 tests/libqos/malloc.h |  1 +
 4 files changed, 144 insertions(+), 17 deletions(-)

diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
index 7e72078..ac1bae1 100644
--- a/tests/libqos/libqos.c
+++ b/tests/libqos/libqos.c
@@ -1,5 +1,6 @@
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <glib.h>
 #include <unistd.h>
 #include <fcntl.h>
@@ -62,6 +63,89 @@ void qtest_shutdown(QOSState *qs)
     g_free(qs);
 }
 
+void set_context(QOSState *s)
+{
+    global_qtest = s->qts;
+}
+
+static QDict *qmp_execute(const char *command)
+{
+    char *fmt;
+    QDict *rsp;
+
+    fmt = g_strdup_printf("{ 'execute': '%s' }", command);
+    rsp = qmp(fmt);
+    g_free(fmt);
+
+    return rsp;
+}
+
+void migrate(QOSState *from, QOSState *to, const char *uri)
+{
+    const char *st;
+    char *s;
+    QDict *rsp, *sub;
+    bool running;
+
+    set_context(from);
+
+    /* Is the machine currently running? */
+    rsp = qmp_execute("query-status");
+    g_assert(qdict_haskey(rsp, "return"));
+    sub = qdict_get_qdict(rsp, "return");
+    g_assert(qdict_haskey(sub, "running"));
+    running = qdict_get_bool(sub, "running");
+    QDECREF(rsp);
+
+    /* Issue the migrate command. */
+    s = g_strdup_printf("{ 'execute': 'migrate',"
+                        "'arguments': { 'uri': '%s' } }",
+                        uri);
+    rsp = qmp(s);
+    g_free(s);
+    g_assert(qdict_haskey(rsp, "return"));
+    QDECREF(rsp);
+
+    /* Wait for STOP event, but only if we were running: */
+    if (running) {
+        qmp_eventwait("STOP");
+    }
+
+    /* If we were running, we can wait for an event. */
+    if (running) {
+        migrate_allocator(from->alloc, to->alloc);
+        set_context(to);
+        qmp_eventwait("RESUME");
+        return;
+    }
+
+    /* Otherwise, we need to wait: poll until migration is completed. */
+    while (1) {
+        rsp = qmp_execute("query-migrate");
+        g_assert(qdict_haskey(rsp, "return"));
+        sub = qdict_get_qdict(rsp, "return");
+        g_assert(qdict_haskey(sub, "status"));
+        st = qdict_get_str(sub, "status");
+
+        /* "setup", "active", "completed", "failed", "cancelled" */
+        if (strcmp(st, "completed") == 0) {
+            QDECREF(rsp);
+            break;
+        }
+
+        if ((strcmp(st, "setup") == 0) || (strcmp(st, "active") == 0)) {
+            QDECREF(rsp);
+            continue;
+        }
+
+        fprintf(stderr, "Migration did not complete, status: %s\n", st);
+        g_assert_not_reached();
+    }
+
+    migrate_allocator(from->alloc, to->alloc);
+    set_context(to);
+}
+
 void mkimg(const char *file, const char *fmt, unsigned size_mb)
 {
     gchar *cli;
diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
index f57362b..e1f14ea 100644
--- a/tests/libqos/libqos.h
+++ b/tests/libqos/libqos.h
@@ -21,6 +21,8 @@ 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);
+void set_context(QOSState *s);
+void migrate(QOSState *from, QOSState *to, const char *uri);
 void prepare_blkdebug_script(const char *debug_fn, const char *event);
 
 static inline uint64_t qmalloc(QOSState *q, size_t bytes)
diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
index 67f3190..8276130 100644
--- a/tests/libqos/malloc.c
+++ b/tests/libqos/malloc.c
@@ -30,8 +30,8 @@ struct QGuestAllocator {
     uint64_t end;
     uint32_t page_size;
 
-    MemList used;
-    MemList free;
+    MemList *used;
+    MemList *free;
 };
 
 #define DEFAULT_PAGE_SIZE 4096
@@ -150,7 +150,7 @@ static uint64_t mlist_fulfill(QGuestAllocator *s, MemBlock *freenode,
     addr = freenode->addr;
     if (freenode->size == size) {
         /* re-use this freenode as our used node */
-        QTAILQ_REMOVE(&s->free, freenode, MLIST_ENTNAME);
+        QTAILQ_REMOVE(s->free, freenode, MLIST_ENTNAME);
         usednode = freenode;
     } else {
         /* adjust the free node and create a new used node */
@@ -159,7 +159,7 @@ static uint64_t mlist_fulfill(QGuestAllocator *s, MemBlock *freenode,
         usednode = mlist_new(addr, size);
     }
 
-    mlist_sort_insert(&s->used, usednode);
+    mlist_sort_insert(s->used, usednode);
     return addr;
 }
 
@@ -171,7 +171,7 @@ static void mlist_check(QGuestAllocator *s)
     uint64_t addr = s->start > 0 ? s->start - 1 : 0;
     uint64_t next = s->start;
 
-    QTAILQ_FOREACH(node, &s->free, MLIST_ENTNAME) {
+    QTAILQ_FOREACH(node, s->free, MLIST_ENTNAME) {
         g_assert_cmpint(node->addr, >, addr);
         g_assert_cmpint(node->addr, >=, next);
         addr = node->addr;
@@ -180,7 +180,7 @@ static void mlist_check(QGuestAllocator *s)
 
     addr = s->start > 0 ? s->start - 1 : 0;
     next = s->start;
-    QTAILQ_FOREACH(node, &s->used, MLIST_ENTNAME) {
+    QTAILQ_FOREACH(node, s->used, MLIST_ENTNAME) {
         g_assert_cmpint(node->addr, >, addr);
         g_assert_cmpint(node->addr, >=, next);
         addr = node->addr;
@@ -192,7 +192,7 @@ static uint64_t mlist_alloc(QGuestAllocator *s, uint64_t size)
 {
     MemBlock *node;
 
-    node = mlist_find_space(&s->free, size);
+    node = mlist_find_space(s->free, size);
     if (!node) {
         fprintf(stderr, "Out of guest memory.\n");
         g_assert_not_reached();
@@ -208,7 +208,7 @@ static void mlist_free(QGuestAllocator *s, uint64_t addr)
         return;
     }
 
-    node = mlist_find_key(&s->used, addr);
+    node = mlist_find_key(s->used, addr);
     if (!node) {
         fprintf(stderr, "Error: no record found for an allocation at "
                 "0x%016" PRIx64 ".\n",
@@ -217,9 +217,9 @@ static void mlist_free(QGuestAllocator *s, uint64_t addr)
     }
 
     /* Rip it out of the used list and re-insert back into the free list. */
-    QTAILQ_REMOVE(&s->used, node, MLIST_ENTNAME);
-    mlist_sort_insert(&s->free, node);
-    mlist_coalesce(&s->free, node);
+    QTAILQ_REMOVE(s->used, node, MLIST_ENTNAME);
+    mlist_sort_insert(s->free, node);
+    mlist_coalesce(s->free, node);
 }
 
 /*
@@ -233,7 +233,7 @@ void alloc_uninit(QGuestAllocator *allocator)
     QAllocOpts mask;
 
     /* Check for guest leaks, and destroy the list. */
-    QTAILQ_FOREACH_SAFE(node, &allocator->used, MLIST_ENTNAME, tmp) {
+    QTAILQ_FOREACH_SAFE(node, allocator->used, MLIST_ENTNAME, tmp) {
         if (allocator->opts & (ALLOC_LEAK_WARN | ALLOC_LEAK_ASSERT)) {
             fprintf(stderr, "guest malloc leak @ 0x%016" PRIx64 "; "
                     "size 0x%016" PRIx64 ".\n",
@@ -248,7 +248,7 @@ void alloc_uninit(QGuestAllocator *allocator)
     /* If we have previously asserted that there are no leaks, then there
      * should be only one node here with a specific address and size. */
     mask = ALLOC_LEAK_ASSERT | ALLOC_PARANOID;
-    QTAILQ_FOREACH_SAFE(node, &allocator->free, MLIST_ENTNAME, tmp) {
+    QTAILQ_FOREACH_SAFE(node, allocator->free, MLIST_ENTNAME, tmp) {
         if ((allocator->opts & mask) == mask) {
             if ((node->addr != allocator->start) ||
                 (node->size != allocator->end - allocator->start)) {
@@ -260,6 +260,8 @@ void alloc_uninit(QGuestAllocator *allocator)
         g_free(node);
     }
 
+    g_free(allocator->used);
+    g_free(allocator->free);
     g_free(allocator);
 }
 
@@ -297,11 +299,13 @@ QGuestAllocator *alloc_init(uint64_t start, uint64_t end)
     s->start = start;
     s->end = end;
 
-    QTAILQ_INIT(&s->used);
-    QTAILQ_INIT(&s->free);
+    s->used = g_malloc(sizeof(MemList));
+    s->free = g_malloc(sizeof(MemList));
+    QTAILQ_INIT(s->used);
+    QTAILQ_INIT(s->free);
 
     node = mlist_new(s->start, s->end - s->start);
-    QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME);
+    QTAILQ_INSERT_HEAD(s->free, node, MLIST_ENTNAME);
 
     s->page_size = DEFAULT_PAGE_SIZE;
 
@@ -319,7 +323,7 @@ QGuestAllocator *alloc_init_flags(QAllocOpts opts,
 void alloc_set_page_size(QGuestAllocator *allocator, size_t page_size)
 {
     /* Can't alter the page_size for an allocator in-use */
-    g_assert(QTAILQ_EMPTY(&allocator->used));
+    g_assert(QTAILQ_EMPTY(allocator->used));
 
     g_assert(is_power_of_2(page_size));
     allocator->page_size = page_size;
@@ -329,3 +333,39 @@ void alloc_set_flags(QGuestAllocator *allocator, QAllocOpts opts)
 {
     allocator->opts |= opts;
 }
+
+void migrate_allocator(QGuestAllocator *src,
+                       QGuestAllocator *dst)
+{
+    MemBlock *node, *tmp;
+    MemList *tmpused, *tmpfree;
+
+    /* The general memory layout should be equivalent,
+     * though opts can differ. */
+    g_assert_cmphex(src->start, ==, dst->start);
+    g_assert_cmphex(src->end, ==, dst->end);
+
+    /* Destroy (silently, regardless of options) the dest-list: */
+    QTAILQ_FOREACH_SAFE(node, dst->used, MLIST_ENTNAME, tmp) {
+        g_free(node);
+    }
+    QTAILQ_FOREACH_SAFE(node, dst->free, MLIST_ENTNAME, tmp) {
+        g_free(node);
+    }
+
+    tmpused = dst->used;
+    tmpfree = dst->free;
+
+    /* Inherit the lists of the source allocator: */
+    dst->used = src->used;
+    dst->free = src->free;
+
+    /* Source is now re-initialized, the source memory is 'invalid' now: */
+    src->used = tmpused;
+    src->free = tmpfree;
+    QTAILQ_INIT(src->used);
+    QTAILQ_INIT(src->free);
+    node = mlist_new(src->start, src->end - src->start);
+    QTAILQ_INSERT_HEAD(src->free, node, MLIST_ENTNAME);
+    return;
+}
diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
index 71ac407..0c6c9b7 100644
--- a/tests/libqos/malloc.h
+++ b/tests/libqos/malloc.h
@@ -31,6 +31,7 @@ void alloc_uninit(QGuestAllocator *allocator);
 /* Always returns page aligned values */
 uint64_t guest_alloc(QGuestAllocator *allocator, size_t size);
 void guest_free(QGuestAllocator *allocator, uint64_t addr);
+void migrate_allocator(QGuestAllocator *src, QGuestAllocator *dst);
 
 QGuestAllocator *alloc_init(uint64_t start, uint64_t end);
 QGuestAllocator *alloc_init_flags(QAllocOpts flags,
-- 
2.1.0

[Qemu-devel] [PATCH v3 4/9] ich9/ahci: Enable Migration

[John Snow]

Lift the flag preventing the migration of the ICH9/AHCI devices.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/ahci.c | 1 -
 hw/ide/ich.c  | 1 -
 2 files changed, 2 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 833fd45..8e36dec 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1461,7 +1461,6 @@ typedef struct SysbusAHCIState {
 
 static const VMStateDescription vmstate_sysbus_ahci = {
     .name = "sysbus-ahci",
-    .unmigratable = 1, /* Still buggy under I/O load */
     .fields = (VMStateField[]) {
         VMSTATE_AHCI(ahci, SysbusAHCIState),
         VMSTATE_END_OF_LIST()
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index b1d8874..350c7f1 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -82,7 +82,6 @@
 
 static const VMStateDescription vmstate_ich9_ahci = {
     .name = "ich9_ahci",
-    .unmigratable = 1, /* Still buggy under I/O load */
     .version_id = 1,
     .fields = (VMStateField[]) {
         VMSTATE_PCI_DEVICE(parent_obj, AHCIPCIState),
-- 
2.1.0

[Qemu-devel] [PATCH v3 5/9] qtest/ahci: Add migration test

[John Snow]

Notes:

 * The migration is performed on QOSState objects.

 * The migration is performed in such a way that it does not assume
   consistency between the allocators attached to each. That is to say,
   you can use each QOSState object completely independently and then at
   an arbitrary point decide to migrate, and the destination object will
   now be consistent with the memory within the source guest. The source
   object that was migrated from will have a completely blank allocator.

ahci-test.c:
 - verify_state is added
 - ahci_migrate is added as a frontend to migrate
 - test_migrate_sanity test case is added.

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

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 7c23bb2..2656b37 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -97,6 +97,70 @@ static void generate_pattern(void *buffer, size_t len, size_t cycle_len)
     }
 }
 
+/**
+ * Verify that the transfer did not corrupt our state at all.
+ */
+static void verify_state(AHCIQState *ahci)
+{
+    int i, j;
+    uint32_t ahci_fingerprint;
+    uint64_t hba_base;
+    AHCICommandHeader cmd;
+
+    ahci_fingerprint = qpci_config_readl(ahci->dev, PCI_VENDOR_ID);
+    g_assert_cmphex(ahci_fingerprint, ==, ahci->fingerprint);
+
+    /* If we haven't initialized, this is as much as can be validated. */
+    if (!ahci->hba_base) {
+        return;
+    }
+
+    hba_base = qpci_config_readl(ahci->dev, PCI_BASE_ADDRESS_5);
+    g_assert_cmphex(hba_base, ==, (uint64_t)ahci->hba_base);
+
+    g_assert_cmphex(ahci_rreg(ahci, AHCI_CAP), ==, ahci->cap);
+    g_assert_cmphex(ahci_rreg(ahci, AHCI_CAP2), ==, ahci->cap2);
+
+    for (i = 0; i < 32; i++) {
+        g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_FB), ==,
+                        ahci->port[i].fb);
+        g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_CLB), ==,
+                        ahci->port[i].clb);
+        for (j = 0; j < 32; j++) {
+            ahci_get_command_header(ahci, i, j, &cmd);
+            g_assert_cmphex(cmd.prdtl, ==, ahci->port[i].prdtl[j]);
+            g_assert_cmphex(cmd.ctba, ==, ahci->port[i].ctba[j]);
+        }
+    }
+}
+
+static void ahci_migrate(AHCIQState *from, AHCIQState *to, const char *uri)
+{
+    QOSState *tmp = to->parent;
+    QPCIDevice *dev = to->dev;
+    if (uri == NULL) {
+        uri = "tcp:127.0.0.1:1234";
+    }
+
+    /* context will be 'to' after completion. */
+    migrate(from->parent, to->parent, uri);
+
+    /* We'd like for the AHCIState objects to still point
+     * to information specific to its specific parent
+     * instance, but otherwise just inherit the new data. */
+    memcpy(to, from, sizeof(AHCIQState));
+    to->parent = tmp;
+    to->dev = dev;
+
+    tmp = from->parent;
+    dev = from->dev;
+    memset(from, 0x00, sizeof(AHCIQState));
+    from->parent = tmp;
+    from->dev = dev;
+
+    verify_state(to);
+}
+
 /*** Test Setup & Teardown ***/
 
 /**
@@ -146,6 +210,8 @@ static AHCIQState *ahci_boot(const char *cli, ...)
 static void ahci_shutdown(AHCIQState *ahci)
 {
     QOSState *qs = ahci->parent;
+
+    set_context(qs);
     ahci_clean_mem(ahci);
     free_ahci_device(ahci->dev);
     g_free(ahci);
@@ -1022,6 +1088,26 @@ static void test_flush_retry(void)
     ahci_shutdown(ahci);
 }
 
+/**
+ * Basic sanity test to boot a machine, find an AHCI device, and shutdown.
+ */
+static void test_migrate_sanity(void)
+{
+    AHCIQState *src, *dst;
+    const char *uri = "tcp:127.0.0.1:1234";
+
+    src = ahci_boot("-m 1024 -M q35 "
+                    "-hda %s ", tmp_path);
+    dst = ahci_boot("-m 1024 -M q35 "
+                    "-hda %s "
+                    "-incoming %s", tmp_path, uri);
+
+    ahci_migrate(src, dst, uri);
+
+    ahci_shutdown(src);
+    ahci_shutdown(dst);
+}
+
 /******************************************************************************/
 /* AHCI I/O Test Matrix Definitions                                           */
 
@@ -1271,6 +1357,8 @@ int main(int argc, char **argv)
     qtest_add_func("/ahci/flush/simple", test_flush);
     qtest_add_func("/ahci/flush/retry", test_flush_retry);
 
+    qtest_add_func("/ahci/migrate/sanity", test_migrate_sanity);
+
     ret = g_test_run();
 
     /* Cleanup */
-- 
2.1.0

[Qemu-devel] [PATCH v3 6/9] qtest/ahci: add migrate dma test

[John Snow]

Write to one guest, migrate, and then read from the other.
adjust ahci_io to clear any buffers it creates, so that we
can use ahci_io safely on both guests knowing we are using
empty buffers and not accidentally re-using data.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c   | 45 +++++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/ahci.c |  1 +
 2 files changed, 46 insertions(+)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 2656b37..90f631c 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1108,6 +1108,50 @@ static void test_migrate_sanity(void)
     ahci_shutdown(dst);
 }
 
+/**
+ * DMA Migration test: Write a pattern, migrate, then read.
+ */
+static void test_migrate_dma(void)
+{
+    AHCIQState *src, *dst;
+    uint8_t px;
+    size_t bufsize = 4096;
+    unsigned char *tx = g_malloc(bufsize);
+    unsigned char *rx = g_malloc0(bufsize);
+    unsigned i;
+    const char *uri = "tcp:127.0.0.1:1234";
+
+    src = ahci_boot_and_enable("-m 1024 -M q35 "
+                               "-hda %s ", tmp_path);
+    dst = ahci_boot("-m 1024 -M q35 "
+                    "-hda %s "
+                    "-incoming %s", tmp_path, uri);
+
+    set_context(src->parent);
+
+    /* initialize */
+    px = ahci_port_select(src);
+    ahci_port_clear(src, px);
+
+    /* create pattern */
+    for (i = 0; i < bufsize; i++) {
+        tx[i] = (bufsize - i);
+    }
+
+    /* Write, migrate, then read. */
+    ahci_io(src, px, CMD_WRITE_DMA, tx, bufsize, 0);
+    ahci_migrate(src, dst, uri);
+    ahci_io(dst, px, CMD_READ_DMA, rx, bufsize, 0);
+
+    /* Verify pattern */
+    g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0);
+
+    ahci_shutdown(src);
+    ahci_shutdown(dst);
+    g_free(rx);
+    g_free(tx);
+}
+
 /******************************************************************************/
 /* AHCI I/O Test Matrix Definitions                                           */
 
@@ -1358,6 +1402,7 @@ int main(int argc, char **argv)
     qtest_add_func("/ahci/flush/retry", test_flush_retry);
 
     qtest_add_func("/ahci/migrate/sanity", test_migrate_sanity);
+    qtest_add_func("/ahci/migrate/dma", test_migrate_dma);
 
     ret = g_test_run();
 
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 29e12f9..e2ac0d7 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -650,6 +650,7 @@ void ahci_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd,
     g_assert(props);
     ptr = ahci_alloc(ahci, bufsize);
     g_assert(ptr);
+    qmemset(ptr, 0x00, bufsize);
 
     if (props->write) {
         memwrite(ptr, buffer, bufsize);
-- 
2.1.0

[Qemu-devel] [PATCH v3 7/9] qtest/ahci: add flush migrate test

[John Snow]

Use blkdebug to inject an error on first flush, then attempt to flush
on the first guest. When the error halts the VM, migrate to the
second VM, and attempt to resume the command.

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

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 90f631c..f770c9d 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1069,7 +1069,7 @@ static void test_flush_retry(void)
                                 debug_path,
                                 tmp_path);
 
-    /* Issue Flush Command */
+    /* Issue Flush Command and wait for error */
     port = ahci_port_select(ahci);
     ahci_port_clear(ahci, port);
     cmd = ahci_command_create(CMD_FLUSH_CACHE);
@@ -1152,6 +1152,55 @@ static void test_migrate_dma(void)
     g_free(tx);
 }
 
+/**
+ * Migration test: Try to flush, migrate, then resume.
+ */
+static void test_flush_migrate(void)
+{
+    AHCIQState *src, *dst;
+    AHCICommand *cmd;
+    uint8_t px;
+    const char *s;
+    const char *uri = "tcp:127.0.0.1:1234";
+
+    prepare_blkdebug_script(debug_path, "flush_to_disk");
+
+    src = ahci_boot_and_enable("-drive file=blkdebug:%s:%s,if=none,id=drive0,"
+                               "cache=writeback,rerror=stop,werror=stop "
+                               "-M q35 "
+                               "-device ide-hd,drive=drive0 ",
+                               debug_path, tmp_path);
+    dst = ahci_boot("-drive file=%s,if=none,id=drive0,"
+                    "cache=writeback,rerror=stop,werror=stop "
+                    "-M q35 "
+                    "-device ide-hd,drive=drive0 "
+                    "-incoming %s", tmp_path, uri);
+
+    set_context(src->parent);
+
+    /* Issue Flush Command */
+    px = ahci_port_select(src);
+    ahci_port_clear(src, px);
+    cmd = ahci_command_create(CMD_FLUSH_CACHE);
+    ahci_command_commit(src, cmd, px);
+    ahci_command_issue_async(src, cmd);
+    qmp_eventwait("STOP");
+
+    /* Migrate over */
+    ahci_migrate(src, dst, uri);
+
+    /* Complete the command */
+    s = "{'execute':'cont' }";
+    qmp_async(s);
+    qmp_eventwait("RESUME");
+    ahci_command_wait(dst, cmd);
+    ahci_command_verify(dst, cmd);
+
+    ahci_command_free(cmd);
+    ahci_shutdown(src);
+    ahci_shutdown(dst);
+}
+
 /******************************************************************************/
 /* AHCI I/O Test Matrix Definitions                                           */
 
@@ -1400,6 +1449,7 @@ int main(int argc, char **argv)
 
     qtest_add_func("/ahci/flush/simple", test_flush);
     qtest_add_func("/ahci/flush/retry", test_flush_retry);
+    qtest_add_func("/ahci/flush/migrate", test_flush_migrate);
 
     qtest_add_func("/ahci/migrate/sanity", test_migrate_sanity);
     qtest_add_func("/ahci/migrate/dma", test_migrate_dma);
-- 
2.1.0

[Qemu-devel] [PATCH v3 8/9] qtest/ahci: add halted dma test

[John Snow]

If we're going to test the migration of halted DMA jobs,
we should probably check to make sure we can resume them
locally as a first step.

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

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index f770c9d..aa0db92 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1153,6 +1153,65 @@ static void test_migrate_dma(void)
 }
 
 /**
+ * DMA Error Test
+ *
+ * Simulate an error on first write, Try to write a pattern,
+ * Confirm the VM has stopped, resume the VM, verify command
+ * has completed, then read back the data and verify.
+ */
+static void test_halted_dma(void)
+{
+    AHCIQState *ahci;
+    uint8_t port;
+    size_t bufsize = 4096;
+    unsigned char *tx = g_malloc(bufsize);
+    unsigned char *rx = g_malloc0(bufsize);
+    unsigned i;
+    uint64_t ptr;
+    AHCICommand *cmd;
+
+    prepare_blkdebug_script(debug_path, "write_aio");
+
+    ahci = ahci_boot_and_enable("-drive file=blkdebug:%s:%s,if=none,id=drive0,"
+                                "format=qcow2,cache=writeback,"
+                                "rerror=stop,werror=stop "
+                                "-M q35 "
+                                "-device ide-hd,drive=drive0 ",
+                                debug_path,
+                                tmp_path);
+
+    /* Initialize and prepare */
+    port = ahci_port_select(ahci);
+    ahci_port_clear(ahci, port);
+
+    for (i = 0; i < bufsize; i++) {
+        tx[i] = (bufsize - i);
+    }
+
+    /* create DMA source buffer and write pattern */
+    ptr = ahci_alloc(ahci, bufsize);
+    g_assert(ptr);
+    memwrite(ptr, tx, bufsize);
+
+    /* Attempt to write (and fail) */
+    cmd = ahci_guest_io_halt(ahci, port, CMD_WRITE_DMA,
+                             ptr, bufsize, 0);
+
+    /* Attempt to resume the command */
+    ahci_guest_io_resume(ahci, cmd);
+    ahci_free(ahci, ptr);
+
+    /* Read back and verify */
+    ahci_io(ahci, port, CMD_READ_DMA, rx, bufsize, 0);
+    g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0);
+
+    /* Cleanup and go home */
+    ahci_shutdown(ahci);
+    g_free(rx);
+    g_free(tx);
+}
+
+/**
  * Migration test: Try to flush, migrate, then resume.
  */
 static void test_flush_migrate(void)
@@ -1453,6 +1512,7 @@ int main(int argc, char **argv)
 
     qtest_add_func("/ahci/migrate/sanity", test_migrate_sanity);
     qtest_add_func("/ahci/migrate/dma", test_migrate_dma);
+    qtest_add_func("/ahci/io/dma/lba28/retry", test_halted_dma);
 
     ret = g_test_run();
 
-- 
2.1.0

[Qemu-devel] [PATCH v3 9/9] qtest/ahci: add migrate halted dma test

[John Snow]

Test migrating a halted DMA transaction.
Resume, then test data integrity.

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

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index aa0db92..020acdd 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1212,6 +1212,78 @@ static void test_halted_dma(void)
 }
 
 /**
+ * DMA Error Migration Test
+ *
+ * Simulate an error on first write, Try to write a pattern,
+ * Confirm the VM has stopped, migrate, resume the VM,
+ * verify command has completed, then read back the data and verify.
+ */
+static void test_migrate_halted_dma(void)
+{
+    AHCIQState *src, *dst;
+    uint8_t port;
+    size_t bufsize = 4096;
+    unsigned char *tx = g_malloc(bufsize);
+    unsigned char *rx = g_malloc0(bufsize);
+    unsigned i;
+    uint64_t ptr;
+    AHCICommand *cmd;
+    const char *uri = "tcp:127.0.0.1:1234";
+
+    prepare_blkdebug_script(debug_path, "write_aio");
+
+    src = ahci_boot_and_enable("-drive file=blkdebug:%s:%s,if=none,id=drive0,"
+                               "format=qcow2,cache=writeback,"
+                               "rerror=stop,werror=stop "
+                               "-M q35 "
+                               "-device ide-hd,drive=drive0 ",
+                               debug_path,
+                               tmp_path);
+
+    dst = ahci_boot("-drive file=%s,if=none,id=drive0,"
+                    "format=qcow2,cache=writeback,"
+                    "rerror=stop,werror=stop "
+                    "-M q35 "
+                    "-device ide-hd,drive=drive0 "
+                    "-incoming %s",
+                    tmp_path, uri);
+
+    set_context(src->parent);
+
+    /* Initialize and prepare */
+    port = ahci_port_select(src);
+    ahci_port_clear(src, port);
+
+    for (i = 0; i < bufsize; i++) {
+        tx[i] = (bufsize - i);
+    }
+
+    /* create DMA source buffer and write pattern */
+    ptr = ahci_alloc(src, bufsize);
+    g_assert(ptr);
+    memwrite(ptr, tx, bufsize);
+
+    /* Write, trigger the VM to stop, migrate, then resume. */
+    cmd = ahci_guest_io_halt(src, port, CMD_WRITE_DMA,
+                             ptr, bufsize, 0);
+    ahci_migrate(src, dst, uri);
+    ahci_guest_io_resume(dst, cmd);
+    ahci_free(dst, ptr);
+
+    /* Read back */
+    ahci_io(dst, port, CMD_READ_DMA, rx, bufsize, 0);
+
+    /* Verify TX and RX are identical */
+    g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0);
+
+    /* Cleanup and go home. */
+    ahci_shutdown(src);
+    ahci_shutdown(dst);
+    g_free(rx);
+    g_free(tx);
+}
+
+/**
  * Migration test: Try to flush, migrate, then resume.
  */
 static void test_flush_migrate(void)
@@ -1511,8 +1583,9 @@ int main(int argc, char **argv)
     qtest_add_func("/ahci/flush/migrate", test_flush_migrate);
 
     qtest_add_func("/ahci/migrate/sanity", test_migrate_sanity);
-    qtest_add_func("/ahci/migrate/dma", test_migrate_dma);
+    qtest_add_func("/ahci/migrate/dma/simple", test_migrate_dma);
     qtest_add_func("/ahci/io/dma/lba28/retry", test_halted_dma);
+    qtest_add_func("/ahci/migrate/dma/halted", test_migrate_halted_dma);
 
     ret = g_test_run();
 
-- 
2.1.0

Re: [Qemu-devel] [Qemu-block] [PATCH v3 3/9] libqos: Add migration helpers

[Kevin Wolf]

Am 30.04.2015 um 20:07 hat John Snow geschrieben:
> libqos.c:
>     -set_context for addressing which commands go where
>     -migrate performs the actual migration
> 
> malloc.c:
>     - Structure of the allocator is adjusted slightly with
>       a second-tier malloc to make swapping around the allocators
>       easy when we "migrate" the lists from the source to the destination.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/libqos/libqos.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/libqos/libqos.h |  2 ++
>  tests/libqos/malloc.c | 74 ++++++++++++++++++++++++++++++++++-----------
>  tests/libqos/malloc.h |  1 +
>  4 files changed, 144 insertions(+), 17 deletions(-)
> 
> diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
> index 7e72078..ac1bae1 100644
> --- a/tests/libqos/libqos.c
> +++ b/tests/libqos/libqos.c
> @@ -1,5 +1,6 @@
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <string.h>
>  #include <glib.h>
>  #include <unistd.h>
>  #include <fcntl.h>
> @@ -62,6 +63,89 @@ void qtest_shutdown(QOSState *qs)
>      g_free(qs);
>  }
>  
> +void set_context(QOSState *s)
> +{
> +    global_qtest = s->qts;
> +}
> +
> +static QDict *qmp_execute(const char *command)
> +{
> +    char *fmt;
> +    QDict *rsp;
> +
> +    fmt = g_strdup_printf("{ 'execute': '%s' }", command);
> +    rsp = qmp(fmt);
> +    g_free(fmt);
> +
> +    return rsp;
> +}
> +
> +void migrate(QOSState *from, QOSState *to, const char *uri)
> +{
> +    const char *st;
> +    char *s;
> +    QDict *rsp, *sub;
> +    bool running;
> +
> +    set_context(from);
> +
> +    /* Is the machine currently running? */
> +    rsp = qmp_execute("query-status");
> +    g_assert(qdict_haskey(rsp, "return"));
> +    sub = qdict_get_qdict(rsp, "return");
> +    g_assert(qdict_haskey(sub, "running"));
> +    running = qdict_get_bool(sub, "running");
> +    QDECREF(rsp);
> +
> +    /* Issue the migrate command. */
> +    s = g_strdup_printf("{ 'execute': 'migrate',"
> +                        "'arguments': { 'uri': '%s' } }",
> +                        uri);
> +    rsp = qmp(s);
> +    g_free(s);
> +    g_assert(qdict_haskey(rsp, "return"));
> +    QDECREF(rsp);
> +
> +    /* Wait for STOP event, but only if we were running: */
> +    if (running) {
> +        qmp_eventwait("STOP");
> +    }
> +
> +    /* If we were running, we can wait for an event. */
> +    if (running) {
> +        migrate_allocator(from->alloc, to->alloc);
> +        set_context(to);
> +        qmp_eventwait("RESUME");
> +        return;
> +    }
> +
> +    /* Otherwise, we need to wait: poll until migration is completed. */
> +    while (1) {
> +        rsp = qmp_execute("query-migrate");
> +        g_assert(qdict_haskey(rsp, "return"));
> +        sub = qdict_get_qdict(rsp, "return");
> +        g_assert(qdict_haskey(sub, "status"));
> +        st = qdict_get_str(sub, "status");
> +
> +        /* "setup", "active", "completed", "failed", "cancelled" */
> +        if (strcmp(st, "completed") == 0) {
> +            QDECREF(rsp);
> +            break;
> +        }
> +
> +        if ((strcmp(st, "setup") == 0) || (strcmp(st, "active") == 0)) {
> +            QDECREF(rsp);
> +            continue;

Wouldn't it be nicer to sleep a bit before retrying?

> +        }
> +
> +        fprintf(stderr, "Migration did not complete, status: %s\n", st);
> +        g_assert_not_reached();
> +    }
> +
> +    migrate_allocator(from->alloc, to->alloc);
> +    set_context(to);
> +}

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/9] ahci: enable migration

[Kevin Wolf]

Am 30.04.2015 um 20:07 hat John Snow geschrieben:
> The day we all feared is here, and I am proposing we allow the migration
> of the AHCI device tentatively for the 2.4 development window.
> 
> There are some more NCQ migration tests are needed, but I felt that it was
> important to get migration enabled as close to the start of the 2.4
> development window as possible.
> 
> If the NCQ patches don't pan out by the time the 2.4 freeze occurs, we can
> revert the migration boolean and add a conditional around the ahci tests
> that rely on the migration feature being enabled.
> 
> I am justifying this checkin based on a series of ping-pong
> migration tests I ran under heavy load (using google's stressapptest)
> and saw over 300 successful migrations without a single failure.
> 
> This series does a few things:
> (1) Add migration facilities to libqos
> (2) Enable AHCI and ICH9 migration
> (3) Add a series of migration tests to ahci-test

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

I think besides the NCQ tests, we'll definitely also want to test
migration with READ DMA in flight (this series tests only WRITE DMA and
FLUSH CACHE). Probably also discard.

Nice to have, but less important, would be the other variants that
exist, like the EXT version of each command, and READ/WRITE SECTOR.

But all of that can be added in a follow-up series, it's not a reason to
hold up what's already there.

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/9] ahci: enable migration

[John Snow]

On 05/04/2015 08:29 AM, Kevin Wolf wrote:
> Am 30.04.2015 um 20:07 hat John Snow geschrieben:
>> The day we all feared is here, and I am proposing we allow the migration
>> of the AHCI device tentatively for the 2.4 development window.
>>
>> There are some more NCQ migration tests are needed, but I felt that it was
>> important to get migration enabled as close to the start of the 2.4
>> development window as possible.
>>
>> If the NCQ patches don't pan out by the time the 2.4 freeze occurs, we can
>> revert the migration boolean and add a conditional around the ahci tests
>> that rely on the migration feature being enabled.
>>
>> I am justifying this checkin based on a series of ping-pong
>> migration tests I ran under heavy load (using google's stressapptest)
>> and saw over 300 successful migrations without a single failure.
>>
>> This series does a few things:
>> (1) Add migration facilities to libqos
>> (2) Enable AHCI and ICH9 migration
>> (3) Add a series of migration tests to ahci-test
>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>
> I think besides the NCQ tests, we'll definitely also want to test
> migration with READ DMA in flight (this series tests only WRITE DMA and
> FLUSH CACHE). Probably also discard.
>

Added to the todo list, thanks.

> Nice to have, but less important, would be the other variants that
> exist, like the EXT version of each command, and READ/WRITE SECTOR.
>
> But all of that can be added in a follow-up series, it's not a reason to
> hold up what's already there.
>
> Kevin
>

Thanks!

--js

Re: [Qemu-devel] [Qemu-block] [PATCH v3 3/9] libqos: Add migration helpers

[John Snow]

On 05/04/2015 08:07 AM, Kevin Wolf wrote:
> Am 30.04.2015 um 20:07 hat John Snow geschrieben:
>> libqos.c:
>>      -set_context for addressing which commands go where
>>      -migrate performs the actual migration
>>
>> malloc.c:
>>      - Structure of the allocator is adjusted slightly with
>>        a second-tier malloc to make swapping around the allocators
>>        easy when we "migrate" the lists from the source to the destination.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   tests/libqos/libqos.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/libqos/libqos.h |  2 ++
>>   tests/libqos/malloc.c | 74 ++++++++++++++++++++++++++++++++++-----------
>>   tests/libqos/malloc.h |  1 +
>>   4 files changed, 144 insertions(+), 17 deletions(-)
>>
>> diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
>> index 7e72078..ac1bae1 100644
>> --- a/tests/libqos/libqos.c
>> +++ b/tests/libqos/libqos.c
>> @@ -1,5 +1,6 @@
>>   #include <stdio.h>
>>   #include <stdlib.h>
>> +#include <string.h>
>>   #include <glib.h>
>>   #include <unistd.h>
>>   #include <fcntl.h>
>> @@ -62,6 +63,89 @@ void qtest_shutdown(QOSState *qs)
>>       g_free(qs);
>>   }
>>
>> +void set_context(QOSState *s)
>> +{
>> +    global_qtest = s->qts;
>> +}
>> +
>> +static QDict *qmp_execute(const char *command)
>> +{
>> +    char *fmt;
>> +    QDict *rsp;
>> +
>> +    fmt = g_strdup_printf("{ 'execute': '%s' }", command);
>> +    rsp = qmp(fmt);
>> +    g_free(fmt);
>> +
>> +    return rsp;
>> +}
>> +
>> +void migrate(QOSState *from, QOSState *to, const char *uri)
>> +{
>> +    const char *st;
>> +    char *s;
>> +    QDict *rsp, *sub;
>> +    bool running;
>> +
>> +    set_context(from);
>> +
>> +    /* Is the machine currently running? */
>> +    rsp = qmp_execute("query-status");
>> +    g_assert(qdict_haskey(rsp, "return"));
>> +    sub = qdict_get_qdict(rsp, "return");
>> +    g_assert(qdict_haskey(sub, "running"));
>> +    running = qdict_get_bool(sub, "running");
>> +    QDECREF(rsp);
>> +
>> +    /* Issue the migrate command. */
>> +    s = g_strdup_printf("{ 'execute': 'migrate',"
>> +                        "'arguments': { 'uri': '%s' } }",
>> +                        uri);
>> +    rsp = qmp(s);
>> +    g_free(s);
>> +    g_assert(qdict_haskey(rsp, "return"));
>> +    QDECREF(rsp);
>> +
>> +    /* Wait for STOP event, but only if we were running: */
>> +    if (running) {
>> +        qmp_eventwait("STOP");
>> +    }
>> +
>> +    /* If we were running, we can wait for an event. */
>> +    if (running) {
>> +        migrate_allocator(from->alloc, to->alloc);
>> +        set_context(to);
>> +        qmp_eventwait("RESUME");
>> +        return;
>> +    }
>> +
>> +    /* Otherwise, we need to wait: poll until migration is completed. */
>> +    while (1) {
>> +        rsp = qmp_execute("query-migrate");
>> +        g_assert(qdict_haskey(rsp, "return"));
>> +        sub = qdict_get_qdict(rsp, "return");
>> +        g_assert(qdict_haskey(sub, "status"));
>> +        st = qdict_get_str(sub, "status");
>> +
>> +        /* "setup", "active", "completed", "failed", "cancelled" */
>> +        if (strcmp(st, "completed") == 0) {
>> +            QDECREF(rsp);
>> +            break;
>> +        }
>> +
>> +        if ((strcmp(st, "setup") == 0) || (strcmp(st, "active") == 0)) {
>> +            QDECREF(rsp);
>> +            continue;
>
> Wouldn't it be nicer to sleep a bit before retrying?
>

I actually figured that all the string and stream manipulation for 
sending and receiving QMP queries was "enough sleep" because of how 
quick a migration without any guest should complete -- in practice this 
loop doesn't ever seem to trigger more than once.

If you still think sleep is necessary, I can add some very small sleep 
in a separate patch, or when I merge the tree. Something like:

g_usleep(5000) /* 5 msec */

>> +        }
>> +
>> +        fprintf(stderr, "Migration did not complete, status: %s\n", st);
>> +        g_assert_not_reached();
>> +    }
>> +
>> +    migrate_allocator(from->alloc, to->alloc);
>> +    set_context(to);
>> +}
>
> Kevin
>

Re: [Qemu-devel] [Qemu-block] [PATCH v3 3/9] libqos: Add migration helpers

[Kevin Wolf]

Am 04.05.2015 um 19:52 hat John Snow geschrieben:
> 
> 
> On 05/04/2015 08:07 AM, Kevin Wolf wrote:
> >Am 30.04.2015 um 20:07 hat John Snow geschrieben:
> >>+    /* Otherwise, we need to wait: poll until migration is completed. */
> >>+    while (1) {
> >>+        rsp = qmp_execute("query-migrate");
> >>+        g_assert(qdict_haskey(rsp, "return"));
> >>+        sub = qdict_get_qdict(rsp, "return");
> >>+        g_assert(qdict_haskey(sub, "status"));
> >>+        st = qdict_get_str(sub, "status");
> >>+
> >>+        /* "setup", "active", "completed", "failed", "cancelled" */
> >>+        if (strcmp(st, "completed") == 0) {
> >>+            QDECREF(rsp);
> >>+            break;
> >>+        }
> >>+
> >>+        if ((strcmp(st, "setup") == 0) || (strcmp(st, "active") == 0)) {
> >>+            QDECREF(rsp);
> >>+            continue;
> >
> >Wouldn't it be nicer to sleep a bit before retrying?
> >
> 
> I actually figured that all the string and stream manipulation for
> sending and receiving QMP queries was "enough sleep" because of how
> quick a migration without any guest should complete -- in practice
> this loop doesn't ever seem to trigger more than once.

This surprised me a bit at first because there's no way that string
operations are _that_ slow. You would definitely spin a while in this
loop (and potentially slow down the migration by that).

I think what saves you is that you wait for the STOP event first, and
when qemu's migration thread sends that event, it happens to have
already taken the global mutex. This means that you get your "enough
sleep" from the qemu monitor, which won't respond before migration has
completed.

> If you still think sleep is necessary, I can add some very small
> sleep in a separate patch, or when I merge the tree. Something like:
> 
> g_usleep(5000) /* 5 msec */

If I were you, I'd add it just to be nice (just applying it to your tree
instead of sending out a new version would be okay). If you don't want
to, I won't insist, though. I mean, I already gave my R-b...

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH v3 3/9] libqos: Add migration helpers

[John Snow]

On 05/05/2015 07:35 AM, Kevin Wolf wrote:
> Am 04.05.2015 um 19:52 hat John Snow geschrieben:
>>
>>
>> On 05/04/2015 08:07 AM, Kevin Wolf wrote:
>>> Am 30.04.2015 um 20:07 hat John Snow geschrieben:
>>>> +    /* Otherwise, we need to wait: poll until migration is completed. */
>>>> +    while (1) {
>>>> +        rsp = qmp_execute("query-migrate");
>>>> +        g_assert(qdict_haskey(rsp, "return"));
>>>> +        sub = qdict_get_qdict(rsp, "return");
>>>> +        g_assert(qdict_haskey(sub, "status"));
>>>> +        st = qdict_get_str(sub, "status");
>>>> +
>>>> +        /* "setup", "active", "completed", "failed", "cancelled" */
>>>> +        if (strcmp(st, "completed") == 0) {
>>>> +            QDECREF(rsp);
>>>> +            break;
>>>> +        }
>>>> +
>>>> +        if ((strcmp(st, "setup") == 0) || (strcmp(st, "active") == 0)) {
>>>> +            QDECREF(rsp);
>>>> +            continue;
>>>
>>> Wouldn't it be nicer to sleep a bit before retrying?
>>>
>>
>> I actually figured that all the string and stream manipulation for
>> sending and receiving QMP queries was "enough sleep" because of how
>> quick a migration without any guest should complete -- in practice
>> this loop doesn't ever seem to trigger more than once.
>
> This surprised me a bit at first because there's no way that string
> operations are _that_ slow. You would definitely spin a while in this
> loop (and potentially slow down the migration by that).
>
> I think what saves you is that you wait for the STOP event first, and
> when qemu's migration thread sends that event, it happens to have
> already taken the global mutex. This means that you get your "enough
> sleep" from the qemu monitor, which won't respond before migration has
> completed.
>
>> If you still think sleep is necessary, I can add some very small
>> sleep in a separate patch, or when I merge the tree. Something like:
>>
>> g_usleep(5000) /* 5 msec */
>
> If I were you, I'd add it just to be nice (just applying it to your tree
> instead of sending out a new version would be okay). If you don't want
> to, I won't insist, though. I mean, I already gave my R-b...
>
> Kevin
>

It's worth finding out if my reasoning is sane, and you cared enough to 
comment.

I'll add the sleep when I merge, no problem :)

Thanks!
--js

Re: [Qemu-devel] [PATCH v3 0/9] ahci: enable migration

[John Snow]

On 04/30/2015 02:07 PM, John Snow wrote:
> The day we all feared is here, and I am proposing we allow the migration
> of the AHCI device tentatively for the 2.4 development window.
> 
> There are some more NCQ migration tests are needed, but I felt that it was
> important to get migration enabled as close to the start of the 2.4
> development window as possible.
> 
> If the NCQ patches don't pan out by the time the 2.4 freeze occurs, we can
> revert the migration boolean and add a conditional around the ahci tests
> that rely on the migration feature being enabled.
> 
> I am justifying this checkin based on a series of ping-pong
> migration tests I ran under heavy load (using google's stressapptest)
> and saw over 300 successful migrations without a single failure.
> 
> This series does a few things:
> (1) Add migration facilities to libqos
> (2) Enable AHCI and ICH9 migration
> (3) Add a series of migration tests to ahci-test
> 
> v3:
>  - Rebase and resend for 2.4.
>  - Minor style guide fix.
> 
> v2:
>  - Added a URI parameter to the migrate() helper
>  - Adjust ahci_shutdown to set qtest context for itself
>  - Make sure verify() is part of ahci_migrate() and redundant
>    calls are eliminated
>  - Add new helpers to make tests with blkdebug injections more
>    succint
>  - Change the flush migrate test to not load the blkdebug rule
>    on the destination host
>  - Modify the migrate() function so that it does not poll the
>    VM for migration status if it can rely on RESUME events.
>  - New patch: Repair the ahci_command_set_offset helper.
>  - New test: Test DMA halt and resume.
>  - New test: Test DMA halt, migrate, and resume.
> 
> ==
> For convenience, this branch is available at:
> https://github.com/jnsnow/qemu.git branch ahci-migration-test
> https://github.com/jnsnow/qemu/tree/ahci-migration-test
> 
> This version is tagged ahci-migration-test-v3:
> https://github.com/jnsnow/qemu/releases/tag/ahci-migration-test-v3
> ==
> 
> John Snow (9):
>   libqos/ahci: Add halted command helpers
>   libqos/ahci: Fix sector set method
>   libqos: Add migration helpers
>   ich9/ahci: Enable Migration
>   qtest/ahci: Add migration test
>   qtest/ahci: add migrate dma test
>   qtest/ahci: add flush migrate test
>   qtest/ahci: add halted dma test
>   qtest/ahci: add migrate halted dma test
> 
>  hw/ide/ahci.c         |   1 -
>  hw/ide/ich.c          |   1 -
>  tests/ahci-test.c     | 318 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  tests/libqos/ahci.c   |  34 +++++-
>  tests/libqos/ahci.h   |   3 +
>  tests/libqos/libqos.c |  84 +++++++++++++
>  tests/libqos/libqos.h |   2 +
>  tests/libqos/malloc.c |  74 +++++++++---
>  tests/libqos/malloc.h |   1 +
>  9 files changed, 496 insertions(+), 22 deletions(-)
> 

Staged: https://github.com/jnsnow/qemu/commits/ide
(with one edit to patch 3 as suggested by Kevin.)

--js