[Qemu-devel] [PULL 00/55] Block patches

[Qemu-devel] [PULL 00/55] Block patches

[Stefan Hajnoczi]

The following changes since commit 5c6b3c50cca2106e5fbcbc6efa94c2f8b9d29fd8:

  Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging (2014-08-15 16:37:17 +0100)

are available in the git repository at:


  git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 39ba3bf69c4ef4d8a8b683ee7282efd25b3f01ff:

  qcow2: fix new_blocks double-free in alloc_refcount_block() (2014-08-15 18:03:26 +0100)

----------------------------------------------------------------
Block pull request

----------------------------------------------------------------
Cornelia Huck (3):
      dataplane: print why starting failed
      dataplane: fail notifier setting gracefully
      dataplane: stop trying on notifier error

Denis V. Lunev (4):
      parallels: extend parallels format header with actual data values
      parallels: replace tabs with spaces in block/parallels.c
      parallels: split check for parallels format in parallels_open
      parallels: 2TB+ parallels images support

Gonglei (2):
      qemu-char: using qemu_set_nonblock() instead of fcntl(O_NONBLOCK)
      channel-posix: using qemu_set_nonblock() instead of fcntl(O_NONBLOCK)

John Snow (7):
      q35: Enable the ioapic device to be seen by qtest.
      qtest: Adding qtest_memset and qmemset.
      libqos: Correct memory leak
      libqtest: Correct small memory leak.
      libqos: Fixes a small memory leak.
      libqos: allow qpci_iomap to return BAR mapping size
      qtest/ide: Fix small memory leak

Kevin Wolf (1):
      ide: Fix segfault when flushing a device that doesn't exist

Marc Marí (4):
      libqtest: add QTEST_LOG for debugging qtest testcases
      libqos: Correct mask to align size to PAGE_SIZE in malloc-pc
      libqos: Change free function called in malloc
      virtio-blk: Correct bug in support for flexible descriptor layout

Maria Kustova (9):
      docs: Specification for the image fuzzer
      image-fuzzer: Tool for fuzz tests execution
      image-fuzzer: Fuzzing functions for qcow2 images
      image-fuzzer: Generator of fuzzed qcow2 images
      image-fuzzer: Public API for image-fuzzer/runner/runner.py
      docs: Expand the list of supported image elements with L1/L2 tables
      image-fuzzer: Add fuzzing functions for L1/L2 table entries
      image-fuzzer: Add generators of L1/L2 tables
      image-fuzzer: Reduce number of generator functions in __init__

Mark Cave-Ayland (5):
      cmd646: add constants for CNTRL register access
      cmd646: synchronise DMA interrupt status with UDMA interrupt status
      cmd646: switch cmd646_update_irq() to accept PCIDevice instead of PCIIDEState
      cmd646: allow MRDMODE interrupt status bits clearing from PCI config space
      cmd646: synchronise UDMA interrupt status with DMA interrupt status

Michael Tokarev (1):
      ide: only constrain read/write requests to drive size, not other types

Paolo Bonzini (17):
      blkdebug: report errors on flush too
      libqtest: add QTEST_LOG for debugging qtest testcases
      ide-test: add test for werror=stop
      ide: stash aiocb for flushes
      ide: simplify reset callbacks
      ide: simplify set_inactive callbacks
      ide: simplify async_cmd_done callbacks
      ide: simplify start_transfer callbacks
      ide: wrap start_dma callback
      ide: remove wrong setting of BM_STATUS_INT
      ide: fold add_status callback into set_inactive
      ide: move BM_STATUS bits to pci.[ch]
      ide: move retry constants out of BM_STATUS_* namespace
      ahci: remove duplicate PORT_IRQ_* constants
      ide: stop PIO transfer on errors
      ide: make all commands go through cmd_done
      ahci: construct PIO Setup FIS for PIO commands

Peter Lieven (1):
      qemu-options: add missing -drive discard option to cmdline help

Stefan Hajnoczi (1):
      qcow2: fix new_blocks double-free in alloc_refcount_block()

 block/blkdebug.c                     |  20 ++
 block/parallels.c                    |  52 ++--
 block/qcow2-refcount.c               |   1 +
 docs/image-fuzzer.txt                | 238 ++++++++++++++++++
 hw/block/dataplane/virtio-blk.c      |  39 ++-
 hw/block/virtio-blk.c                |  14 +-
 hw/i386/pc_q35.c                     |   2 +-
 hw/ide/ahci.c                        | 115 +++++----
 hw/ide/ahci.h                        |  21 --
 hw/ide/atapi.c                       |  11 +-
 hw/ide/cmd646.c                      |  94 ++++++-
 hw/ide/core.c                        | 101 ++++----
 hw/ide/internal.h                    |  38 ++-
 hw/ide/macio.c                       |   9 -
 hw/ide/pci.c                         |  45 ++--
 hw/ide/pci.h                         |   7 +
 qemu-char.c                          |   4 +-
 qemu-options.hx                      |   2 +-
 qga/channel-posix.c                  |   2 +-
 tests/ide-test.c                     |  99 +++++++-
 tests/image-fuzzer/qcow2/__init__.py |   1 +
 tests/image-fuzzer/qcow2/fuzz.py     | 355 ++++++++++++++++++++++++++
 tests/image-fuzzer/qcow2/layout.py   | 476 +++++++++++++++++++++++++++++++++++
 tests/image-fuzzer/runner.py         | 405 +++++++++++++++++++++++++++++
 tests/libqos/malloc-pc.c             |   5 +-
 tests/libqos/malloc.h                |   2 +-
 tests/libqos/pci-pc.c                |  12 +-
 tests/libqos/pci-pc.h                |   1 +
 tests/libqos/pci.c                   |   4 +-
 tests/libqos/pci.h                   |   4 +-
 tests/libqtest.c                     |  24 +-
 tests/libqtest.h                     |  24 ++
 tests/usb-hcd-ehci-test.c            |   2 +-
 33 files changed, 1991 insertions(+), 238 deletions(-)
 create mode 100644 docs/image-fuzzer.txt
 create mode 100644 tests/image-fuzzer/qcow2/__init__.py
 create mode 100644 tests/image-fuzzer/qcow2/fuzz.py
 create mode 100644 tests/image-fuzzer/qcow2/layout.py
 create mode 100755 tests/image-fuzzer/runner.py

-- 
1.9.3

[Qemu-devel] [PULL 01/55] blkdebug: report errors on flush too

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/blkdebug.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index f51407d..1586ed9 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -522,6 +522,25 @@ static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs,
     return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
 }
 
+static BlockDriverAIOCB *blkdebug_aio_flush(BlockDriverState *bs,
+    BlockDriverCompletionFunc *cb, void *opaque)
+{
+    BDRVBlkdebugState *s = bs->opaque;
+    BlkdebugRule *rule = NULL;
+
+    QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) {
+        if (rule->options.inject.sector == -1) {
+            break;
+        }
+    }
+
+    if (rule && rule->options.inject.error) {
+        return inject_error(bs, cb, opaque, rule);
+    }
+
+    return bdrv_aio_flush(bs->file, cb, opaque);
+}
+
 
 static void blkdebug_close(BlockDriverState *bs)
 {
@@ -699,6 +718,7 @@ static BlockDriver bdrv_blkdebug = {
 
     .bdrv_aio_readv         = blkdebug_aio_readv,
     .bdrv_aio_writev        = blkdebug_aio_writev,
+    .bdrv_aio_flush         = blkdebug_aio_flush,
 
     .bdrv_debug_event           = blkdebug_debug_event,
     .bdrv_debug_breakpoint      = blkdebug_debug_breakpoint,
-- 
1.9.3

[Qemu-devel] [PULL 02/55] libqtest: add QTEST_LOG for debugging qtest testcases

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqtest.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 98e8f4b..4a75cd3 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -167,11 +167,12 @@ QTestState *qtest_init(const char *extra_args)
     if (s->qemu_pid == 0) {
         command = g_strdup_printf("exec %s "
                                   "-qtest unix:%s,nowait "
-                                  "-qtest-log /dev/null "
+                                  "-qtest-log %s "
                                   "-qmp unix:%s,nowait "
                                   "-machine accel=qtest "
                                   "-display none "
                                   "%s", qemu_binary, socket_path,
+                                  getenv("QTEST_LOG") ? "/dev/fd/2" : "/dev/null",
                                   qmp_socket_path,
                                   extra_args ?: "");
         execlp("/bin/sh", "sh", "-c", command, NULL);
@@ -358,6 +359,7 @@ static void qmp_response(JSONMessageParser *parser, QList *tokens)
 QDict *qtest_qmp_receive(QTestState *s)
 {
     QMPResponseParser qmp;
+    bool log = getenv("QTEST_LOG") != NULL;
 
     qmp.response = NULL;
     json_message_parser_init(&qmp.parser, qmp_response);
@@ -375,6 +377,9 @@ QDict *qtest_qmp_receive(QTestState *s)
             exit(1);
         }
 
+        if (log) {
+            len = write(2, &c, 1);
+        }
         json_message_parser_feed(&qmp.parser, &c, 1);
     }
     json_message_parser_destroy(&qmp.parser);
-- 
1.9.3

[Qemu-devel] [PULL 03/55] ide-test: add test for werror=stop

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/ide-test.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index 4a0d97f..151ef30 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -106,6 +106,7 @@ static QPCIBus *pcibus = NULL;
 static QGuestAllocator *guest_malloc;
 
 static char tmp_path[] = "/tmp/qtest.XXXXXX";
+static char debug_path[] = "/tmp/qtest-blkdebug.XXXXXX";
 
 static void ide_test_start(const char *cmdline_fmt, ...)
 {
@@ -489,6 +490,78 @@ static void test_flush(void)
     ide_test_quit();
 }
 
+static void prepare_blkdebug_script(const char *debug_fn, const char *event)
+{
+    FILE *debug_file = fopen(debug_fn, "w");
+    int ret;
+
+    fprintf(debug_file, "[inject-error]\n");
+    fprintf(debug_file, "event = \"%s\"\n", event);
+    fprintf(debug_file, "errno = \"5\"\n");
+    fprintf(debug_file, "state = \"1\"\n");
+    fprintf(debug_file, "immediately = \"off\"\n");
+    fprintf(debug_file, "once = \"on\"\n");
+
+    fprintf(debug_file, "[set-state]\n");
+    fprintf(debug_file, "event = \"%s\"\n", event);
+    fprintf(debug_file, "new_state = \"2\"\n");
+    fflush(debug_file);
+    g_assert(!ferror(debug_file));
+
+    ret = fclose(debug_file);
+    g_assert(ret == 0);
+}
+
+static void test_retry_flush(void)
+{
+    uint8_t data;
+    const char *s;
+    QDict *response;
+
+    prepare_blkdebug_script(debug_path, "flush_to_disk");
+
+    ide_test_start(
+        "-vnc none "
+        "-drive file=blkdebug:%s:%s,if=ide,cache=writeback,rerror=stop,werror=stop",
+        debug_path, tmp_path);
+
+    /* FLUSH CACHE command on device 0*/
+    outb(IDE_BASE + reg_device, 0);
+    outb(IDE_BASE + reg_command, CMD_FLUSH_CACHE);
+
+    /* Check status while request is in flight*/
+    data = inb(IDE_BASE + reg_status);
+    assert_bit_set(data, BSY | DRDY);
+    assert_bit_clear(data, DF | ERR | DRQ);
+
+    for (;; response = NULL) {
+        response = qmp_receive();
+        if ((qdict_haskey(response, "event")) &&
+            (strcmp(qdict_get_str(response, "event"), "STOP") == 0)) {
+            QDECREF(response);
+            break;
+        }
+        QDECREF(response);
+    }
+
+    /* Complete the command */
+    s = "{'execute':'cont' }";
+    qmp_discard_response(s);
+
+    /* Check registers */
+    data = inb(IDE_BASE + reg_device);
+    g_assert_cmpint(data & DEV, ==, 0);
+
+    do {
+        data = inb(IDE_BASE + reg_status);
+    } while (data & BSY);
+
+    assert_bit_set(data, DRDY);
+    assert_bit_clear(data, BSY | DF | ERR | DRQ);
+
+    ide_test_quit();
+}
+
 int main(int argc, char **argv)
 {
     const char *arch = qtest_get_arch();
@@ -501,6 +574,11 @@ int main(int argc, char **argv)
         return 0;
     }
 
+    /* Create temporary blkdebug instructions */
+    fd = mkstemp(debug_path);
+    g_assert(fd >= 0);
+    close(fd);
+
     /* Create a temporary raw image */
     fd = mkstemp(tmp_path);
     g_assert(fd >= 0);
@@ -522,10 +600,13 @@ int main(int argc, char **argv)
 
     qtest_add_func("/ide/flush", test_flush);
 
+    qtest_add_func("/ide/retry/flush", test_retry_flush);
+
     ret = g_test_run();
 
     /* Cleanup */
     unlink(tmp_path);
+    unlink(debug_path);
 
     return ret;
 }
-- 
1.9.3

[Qemu-devel] [PULL 04/55] ide: stash aiocb for flushes

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

This ensures that operations are completed after a reset

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index db191a6..79985f9 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -831,6 +831,8 @@ static void ide_flush_cb(void *opaque, int ret)
 {
     IDEState *s = opaque;
 
+    s->pio_aiocb = NULL;
+
     if (ret < 0) {
         /* XXX: What sector number to set here? */
         if (ide_handle_rw_error(s, -ret, BM_STATUS_RETRY_FLUSH)) {
@@ -853,7 +855,7 @@ void ide_flush_cache(IDEState *s)
 
     s->status |= BUSY_STAT;
     bdrv_acct_start(s->bs, &s->acct, 0, BDRV_ACCT_FLUSH);
-    bdrv_aio_flush(s->bs, ide_flush_cb, s);
+    s->pio_aiocb = bdrv_aio_flush(s->bs, ide_flush_cb, s);
 }
 
 static void ide_cfata_metadata_inquiry(IDEState *s)
-- 
1.9.3

[Qemu-devel] [PULL 05/55] ide: simplify reset callbacks

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

Drop the unused return value and make the callback optional.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/ahci.c     | 6 ------
 hw/ide/core.c     | 5 +++--
 hw/ide/internal.h | 3 ++-
 hw/ide/macio.c    | 1 -
 hw/ide/pci.c      | 4 +---
 5 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 604152a..273712f 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1147,11 +1147,6 @@ static void ahci_dma_restart_cb(void *opaque, int running, RunState state)
 {
 }
 
-static int ahci_dma_reset(IDEDMA *dma)
-{
-    return 0;
-}
-
 static const IDEDMAOps ahci_dma_ops = {
     .start_dma = ahci_start_dma,
     .start_transfer = ahci_start_transfer,
@@ -1162,7 +1157,6 @@ static const IDEDMAOps ahci_dma_ops = {
     .set_inactive = ahci_dma_set_inactive,
     .async_cmd_done = ahci_async_cmd_done,
     .restart_cb = ahci_dma_restart_cb,
-    .reset = ahci_dma_reset,
 };
 
 void ahci_init(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 79985f9..4183a3a 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2088,7 +2088,9 @@ void ide_bus_reset(IDEBus *bus)
     }
 
     /* reset dma provider too */
-    bus->dma->ops->reset(bus->dma);
+    if (bus->dma->ops->reset) {
+        bus->dma->ops->reset(bus->dma);
+    }
 }
 
 static bool ide_cd_is_tray_open(void *opaque)
@@ -2226,7 +2228,6 @@ static const IDEDMAOps ide_dma_nop_ops = {
     .add_status     = ide_nop_int,
     .set_inactive   = ide_nop,
     .restart_cb     = ide_nop_restart,
-    .reset          = ide_nop,
 };
 
 static IDEDMA ide_dma_nop = {
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 0567a52..6cca46f 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -320,6 +320,7 @@ typedef enum { IDE_HD, IDE_CD, IDE_CFATA } IDEDriveKind;
 typedef void EndTransferFunc(IDEState *);
 
 typedef void DMAStartFunc(IDEDMA *, IDEState *, BlockDriverCompletionFunc *);
+typedef void DMAVoidFunc(IDEDMA *);
 typedef int DMAFunc(IDEDMA *);
 typedef int DMAIntFunc(IDEDMA *, int);
 typedef void DMARestartFunc(void *, int, RunState);
@@ -435,7 +436,7 @@ struct IDEDMAOps {
     DMAFunc *set_inactive;
     DMAFunc *async_cmd_done;
     DMARestartFunc *restart_cb;
-    DMAFunc *reset;
+    DMAVoidFunc *reset;
 };
 
 struct IDEDMA {
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index c14a1dd..ca39e3f 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -578,7 +578,6 @@ static const IDEDMAOps dbdma_ops = {
     .add_status     = ide_nop_int,
     .set_inactive   = ide_nop,
     .restart_cb     = ide_nop_restart,
-    .reset          = ide_nop,
 };
 
 static void macio_ide_realizefn(DeviceState *dev, Error **errp)
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 6257a21..c24496a 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -247,7 +247,7 @@ static void bmdma_cancel(BMDMAState *bm)
     }
 }
 
-static int bmdma_reset(IDEDMA *dma)
+static void bmdma_reset(IDEDMA *dma)
 {
     BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma);
 
@@ -264,8 +264,6 @@ static int bmdma_reset(IDEDMA *dma)
     bm->cur_prd_len = 0;
     bm->sector_num = 0;
     bm->nsector = 0;
-
-    return 0;
 }
 
 static int bmdma_start_transfer(IDEDMA *dma)
-- 
1.9.3

[Qemu-devel] [PULL 06/55] ide: simplify set_inactive callbacks

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

Drop the unused return value and make the callback optional.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/ahci.c     | 6 ------
 hw/ide/core.c     | 5 +++--
 hw/ide/internal.h | 2 +-
 hw/ide/macio.c    | 1 -
 hw/ide/pci.c      | 4 +---
 5 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 273712f..679357f 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1116,11 +1116,6 @@ static int ahci_dma_add_status(IDEDMA *dma, int status)
     return 0;
 }
 
-static int ahci_dma_set_inactive(IDEDMA *dma)
-{
-    return 0;
-}
-
 static int ahci_async_cmd_done(IDEDMA *dma)
 {
     AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
@@ -1154,7 +1149,6 @@ static const IDEDMAOps ahci_dma_ops = {
     .rw_buf = ahci_dma_rw_buf,
     .set_unit = ahci_dma_set_unit,
     .add_status = ahci_dma_add_status,
-    .set_inactive = ahci_dma_set_inactive,
     .async_cmd_done = ahci_async_cmd_done,
     .restart_cb = ahci_dma_restart_cb,
 };
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 4183a3a..d52a6f6 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -595,7 +595,9 @@ static void ide_async_cmd_done(IDEState *s)
 void ide_set_inactive(IDEState *s)
 {
     s->bus->dma->aiocb = NULL;
-    s->bus->dma->ops->set_inactive(s->bus->dma);
+    if (s->bus->dma->ops->set_inactive) {
+        s->bus->dma->ops->set_inactive(s->bus->dma);
+    }
     ide_async_cmd_done(s);
 }
 
@@ -2226,7 +2228,6 @@ static const IDEDMAOps ide_dma_nop_ops = {
     .rw_buf         = ide_nop_int,
     .set_unit       = ide_nop_int,
     .add_status     = ide_nop_int,
-    .set_inactive   = ide_nop,
     .restart_cb     = ide_nop_restart,
 };
 
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 6cca46f..34064cf 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -433,7 +433,7 @@ struct IDEDMAOps {
     DMAIntFunc *rw_buf;
     DMAIntFunc *set_unit;
     DMAIntFunc *add_status;
-    DMAFunc *set_inactive;
+    DMAVoidFunc *set_inactive;
     DMAFunc *async_cmd_done;
     DMARestartFunc *restart_cb;
     DMAVoidFunc *reset;
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index ca39e3f..0e26bac 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -576,7 +576,6 @@ static const IDEDMAOps dbdma_ops = {
     .rw_buf         = ide_nop_int,
     .set_unit       = ide_nop_int,
     .add_status     = ide_nop_int,
-    .set_inactive   = ide_nop,
     .restart_cb     = ide_nop_restart,
 };
 
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index c24496a..9dc3cc5 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -160,15 +160,13 @@ static int bmdma_add_status(IDEDMA *dma, int status)
     return 0;
 }
 
-static int bmdma_set_inactive(IDEDMA *dma)
+static void bmdma_set_inactive(IDEDMA *dma)
 {
     BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma);
 
     bm->status &= ~BM_STATUS_DMAING;
     bm->dma_cb = NULL;
     bm->unit = -1;
-
-    return 0;
 }
 
 static void bmdma_restart_dma(BMDMAState *bm, enum ide_dma_cmd dma_cmd)
-- 
1.9.3

[Qemu-devel] [PULL 07/55] ide: simplify async_cmd_done callbacks

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

Drop the unused return value.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/ahci.c     | 4 +---
 hw/ide/internal.h | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 679357f..e494503 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1116,7 +1116,7 @@ static int ahci_dma_add_status(IDEDMA *dma, int status)
     return 0;
 }
 
-static int ahci_async_cmd_done(IDEDMA *dma)
+static void ahci_async_cmd_done(IDEDMA *dma)
 {
     AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
 
@@ -1130,8 +1130,6 @@ static int ahci_async_cmd_done(IDEDMA *dma)
         ad->check_bh = qemu_bh_new(ahci_check_cmd_bh, ad);
         qemu_bh_schedule(ad->check_bh);
     }
-
-    return 0;
 }
 
 static void ahci_irq_set(void *opaque, int n, int level)
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 34064cf..940dd45 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -434,7 +434,7 @@ struct IDEDMAOps {
     DMAIntFunc *set_unit;
     DMAIntFunc *add_status;
     DMAVoidFunc *set_inactive;
-    DMAFunc *async_cmd_done;
+    DMAVoidFunc *async_cmd_done;
     DMARestartFunc *restart_cb;
     DMAVoidFunc *reset;
 };
-- 
1.9.3

[Qemu-devel] [PULL 08/55] ide: simplify start_transfer callbacks

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

Drop the unused return value and make the callback optional.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/ahci.c     |  4 +---
 hw/ide/core.c     | 10 +++-------
 hw/ide/internal.h |  3 +--
 hw/ide/macio.c    |  6 ------
 hw/ide/pci.c      |  6 ------
 5 files changed, 5 insertions(+), 24 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index e494503..adbac3d 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -991,7 +991,7 @@ out:
 }
 
 /* DMA dev <-> ram */
-static int ahci_start_transfer(IDEDMA *dma)
+static void ahci_start_transfer(IDEDMA *dma)
 {
     AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
     IDEState *s = &ad->port.ifs[0];
@@ -1041,8 +1041,6 @@ out:
         /* done with DMA */
         ahci_trigger_irq(ad->hba, ad, PORT_IRQ_STAT_DSS);
     }
-
-    return 0;
 }
 
 static void ahci_start_dma(IDEDMA *dma, IDEState *s,
diff --git a/hw/ide/core.c b/hw/ide/core.c
index d52a6f6..e5ddecb 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -434,7 +434,9 @@ void ide_transfer_start(IDEState *s, uint8_t *buf, int size,
     if (!(s->status & ERR_STAT)) {
         s->status |= DRQ_STAT;
     }
-    s->bus->dma->ops->start_transfer(s->bus->dma);
+    if (s->bus->dma->ops->start_transfer) {
+        s->bus->dma->ops->start_transfer(s->bus->dma);
+    }
 }
 
 void ide_transfer_stop(IDEState *s)
@@ -2207,11 +2209,6 @@ static void ide_nop_start(IDEDMA *dma, IDEState *s,
 {
 }
 
-static int ide_nop(IDEDMA *dma)
-{
-    return 0;
-}
-
 static int ide_nop_int(IDEDMA *dma, int x)
 {
     return 0;
@@ -2223,7 +2220,6 @@ static void ide_nop_restart(void *opaque, int x, RunState y)
 
 static const IDEDMAOps ide_dma_nop_ops = {
     .start_dma      = ide_nop_start,
-    .start_transfer = ide_nop,
     .prepare_buf    = ide_nop_int,
     .rw_buf         = ide_nop_int,
     .set_unit       = ide_nop_int,
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 940dd45..64fbf2f 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -321,7 +321,6 @@ typedef void EndTransferFunc(IDEState *);
 
 typedef void DMAStartFunc(IDEDMA *, IDEState *, BlockDriverCompletionFunc *);
 typedef void DMAVoidFunc(IDEDMA *);
-typedef int DMAFunc(IDEDMA *);
 typedef int DMAIntFunc(IDEDMA *, int);
 typedef void DMARestartFunc(void *, int, RunState);
 
@@ -428,7 +427,7 @@ struct IDEState {
 
 struct IDEDMAOps {
     DMAStartFunc *start_dma;
-    DMAFunc *start_transfer;
+    DMAVoidFunc *start_transfer;
     DMAIntFunc *prepare_buf;
     DMAIntFunc *rw_buf;
     DMAIntFunc *set_unit;
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 0e26bac..b7cedb6 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -545,11 +545,6 @@ static void macio_ide_reset(DeviceState *dev)
     ide_bus_reset(&d->bus);
 }
 
-static int ide_nop(IDEDMA *dma)
-{
-    return 0;
-}
-
 static int ide_nop_int(IDEDMA *dma, int x)
 {
     return 0;
@@ -571,7 +566,6 @@ static void ide_dbdma_start(IDEDMA *dma, IDEState *s,
 
 static const IDEDMAOps dbdma_ops = {
     .start_dma      = ide_dbdma_start,
-    .start_transfer = ide_nop,
     .prepare_buf    = ide_nop_int,
     .rw_buf         = ide_nop_int,
     .set_unit       = ide_nop_int,
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 9dc3cc5..1ee8c0a 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -264,11 +264,6 @@ static void bmdma_reset(IDEDMA *dma)
     bm->nsector = 0;
 }
 
-static int bmdma_start_transfer(IDEDMA *dma)
-{
-    return 0;
-}
-
 static void bmdma_irq(void *opaque, int n, int level)
 {
     BMDMAState *bm = opaque;
@@ -500,7 +495,6 @@ void pci_ide_create_devs(PCIDevice *dev, DriveInfo **hd_table)
 
 static const struct IDEDMAOps bmdma_ops = {
     .start_dma = bmdma_start_dma,
-    .start_transfer = bmdma_start_transfer,
     .prepare_buf = bmdma_prepare_buf,
     .rw_buf = bmdma_rw_buf,
     .set_unit = bmdma_set_unit,
-- 
1.9.3

[Qemu-devel] [PULL 09/55] ide: wrap start_dma callback

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

Make it optional and prepare for the next patches.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/atapi.c    |  6 ++----
 hw/ide/core.c     | 15 ++++++++-------
 hw/ide/internal.h |  1 +
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index f7d2009..d13395e 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -255,8 +255,7 @@ static void ide_atapi_cmd_reply(IDEState *s, int size, int max_size)
     if (s->atapi_dma) {
         bdrv_acct_start(s->bs, &s->acct, size, BDRV_ACCT_READ);
         s->status = READY_STAT | SEEK_STAT | DRQ_STAT;
-        s->bus->dma->ops->start_dma(s->bus->dma, s,
-                                   ide_atapi_cmd_read_dma_cb);
+        ide_start_dma(s, ide_atapi_cmd_read_dma_cb);
     } else {
         s->status = READY_STAT | SEEK_STAT;
         ide_atapi_cmd_reply_end(s);
@@ -375,8 +374,7 @@ static void ide_atapi_cmd_read_dma(IDEState *s, int lba, int nb_sectors,
 
     /* XXX: check if BUSY_STAT should be set */
     s->status = READY_STAT | SEEK_STAT | DRQ_STAT | BUSY_STAT;
-    s->bus->dma->ops->start_dma(s->bus->dma, s,
-                               ide_atapi_cmd_read_dma_cb);
+    ide_start_dma(s, ide_atapi_cmd_read_dma_cb);
 }
 
 static void ide_atapi_cmd_read(IDEState *s, int lba, int nb_sectors,
diff --git a/hw/ide/core.c b/hw/ide/core.c
index e5ddecb..aa561ae 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -745,7 +745,14 @@ static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
         break;
     }
 
-    s->bus->dma->ops->start_dma(s->bus->dma, s, ide_dma_cb);
+    ide_start_dma(s, ide_dma_cb);
+}
+
+void ide_start_dma(IDEState *s, BlockDriverCompletionFunc *cb)
+{
+    if (s->bus->dma->ops->start_dma) {
+        s->bus->dma->ops->start_dma(s->bus->dma, s, cb);
+    }
 }
 
 static void ide_sector_write_timer_cb(void *opaque)
@@ -2204,11 +2211,6 @@ static void ide_init1(IDEBus *bus, int unit)
                                            ide_sector_write_timer_cb, s);
 }
 
-static void ide_nop_start(IDEDMA *dma, IDEState *s,
-                          BlockDriverCompletionFunc *cb)
-{
-}
-
 static int ide_nop_int(IDEDMA *dma, int x)
 {
     return 0;
@@ -2219,7 +2221,6 @@ static void ide_nop_restart(void *opaque, int x, RunState y)
 }
 
 static const IDEDMAOps ide_dma_nop_ops = {
-    .start_dma      = ide_nop_start,
     .prepare_buf    = ide_nop_int,
     .rw_buf         = ide_nop_int,
     .set_unit       = ide_nop_int,
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 64fbf2f..2fe1f0a 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -532,6 +532,7 @@ void ide_bus_reset(IDEBus *bus);
 int64_t ide_get_sector(IDEState *s);
 void ide_set_sector(IDEState *s, int64_t sector_num);
 
+void ide_start_dma(IDEState *s, BlockDriverCompletionFunc *cb);
 void ide_dma_error(IDEState *s);
 
 void ide_atapi_cmd_ok(IDEState *s);
-- 
1.9.3

[Qemu-devel] [PULL 10/55] ide: remove wrong setting of BM_STATUS_INT

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

Similar to the case removed in commit 69c38b8 (ide/core: Remove explicit
setting of BM_STATUS_INT, 2011-05-19), the only remaining use of
add_status(..., BM_STATUS_INT) is for short PRDs.  The flag should
not be raised in this case.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/ahci.c  | 4 ----
 hw/ide/atapi.c | 1 -
 2 files changed, 5 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index adbac3d..14677ec 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1107,10 +1107,6 @@ static int ahci_dma_add_status(IDEDMA *dma, int status)
     AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
     DPRINTF(ad->port_no, "set status: %x\n", status);
 
-    if (status & BM_STATUS_INT) {
-        ahci_trigger_irq(ad->hba, ad, PORT_IRQ_STAT_DSS);
-    }
-
     return 0;
 }
 
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index d13395e..46ed3f5 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -355,7 +355,6 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
 
 eot:
     bdrv_acct_done(s->bs, &s->acct);
-    s->bus->dma->ops->add_status(s->bus->dma, BM_STATUS_INT);
     ide_set_inactive(s);
 }
 
-- 
1.9.3

[Qemu-devel] [PULL 11/55] ide: fold add_status callback into set_inactive

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

It is now called only after the set_inactive callback.  Put the two together.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/ahci.c     |  9 ---------
 hw/ide/atapi.c    |  2 +-
 hw/ide/core.c     | 12 ++++--------
 hw/ide/internal.h |  6 +++---
 hw/ide/macio.c    |  1 -
 hw/ide/pci.c      | 19 +++++++------------
 6 files changed, 15 insertions(+), 34 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 14677ec..0ec5627 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1102,14 +1102,6 @@ static int ahci_dma_set_unit(IDEDMA *dma, int unit)
     return 0;
 }
 
-static int ahci_dma_add_status(IDEDMA *dma, int status)
-{
-    AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
-    DPRINTF(ad->port_no, "set status: %x\n", status);
-
-    return 0;
-}
-
 static void ahci_async_cmd_done(IDEDMA *dma)
 {
     AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
@@ -1140,7 +1132,6 @@ static const IDEDMAOps ahci_dma_ops = {
     .prepare_buf = ahci_dma_prepare_buf,
     .rw_buf = ahci_dma_rw_buf,
     .set_unit = ahci_dma_set_unit,
-    .add_status = ahci_dma_add_status,
     .async_cmd_done = ahci_async_cmd_done,
     .restart_cb = ahci_dma_restart_cb,
 };
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 46ed3f5..3b419b3 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -355,7 +355,7 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
 
 eot:
     bdrv_acct_done(s->bs, &s->acct);
-    ide_set_inactive(s);
+    ide_set_inactive(s, false);
 }
 
 /* start a CD-CDROM read command with DMA */
diff --git a/hw/ide/core.c b/hw/ide/core.c
index aa561ae..24f24ce 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -594,11 +594,11 @@ static void ide_async_cmd_done(IDEState *s)
     }
 }
 
-void ide_set_inactive(IDEState *s)
+void ide_set_inactive(IDEState *s, bool more)
 {
     s->bus->dma->aiocb = NULL;
     if (s->bus->dma->ops->set_inactive) {
-        s->bus->dma->ops->set_inactive(s->bus->dma);
+        s->bus->dma->ops->set_inactive(s->bus->dma, more);
     }
     ide_async_cmd_done(s);
 }
@@ -608,7 +608,7 @@ void ide_dma_error(IDEState *s)
     ide_transfer_stop(s);
     s->error = ABRT_ERR;
     s->status = READY_STAT | ERR_STAT;
-    ide_set_inactive(s);
+    ide_set_inactive(s, false);
     ide_set_irq(s->bus);
 }
 
@@ -719,10 +719,7 @@ eot:
     if (s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) {
         bdrv_acct_done(s->bs, &s->acct);
     }
-    ide_set_inactive(s);
-    if (stay_active) {
-        s->bus->dma->ops->add_status(s->bus->dma, BM_STATUS_DMAING);
-    }
+    ide_set_inactive(s, stay_active);
 }
 
 static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
@@ -2224,7 +2221,6 @@ static const IDEDMAOps ide_dma_nop_ops = {
     .prepare_buf    = ide_nop_int,
     .rw_buf         = ide_nop_int,
     .set_unit       = ide_nop_int,
-    .add_status     = ide_nop_int,
     .restart_cb     = ide_nop_restart,
 };
 
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 2fe1f0a..b35e52c 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -322,6 +322,7 @@ typedef void EndTransferFunc(IDEState *);
 typedef void DMAStartFunc(IDEDMA *, IDEState *, BlockDriverCompletionFunc *);
 typedef void DMAVoidFunc(IDEDMA *);
 typedef int DMAIntFunc(IDEDMA *, int);
+typedef void DMAStopFunc(IDEDMA *, bool);
 typedef void DMARestartFunc(void *, int, RunState);
 
 struct unreported_events {
@@ -431,8 +432,7 @@ struct IDEDMAOps {
     DMAIntFunc *prepare_buf;
     DMAIntFunc *rw_buf;
     DMAIntFunc *set_unit;
-    DMAIntFunc *add_status;
-    DMAVoidFunc *set_inactive;
+    DMAStopFunc *set_inactive;
     DMAVoidFunc *async_cmd_done;
     DMARestartFunc *restart_cb;
     DMAVoidFunc *reset;
@@ -565,7 +565,7 @@ void ide_flush_cache(IDEState *s);
 void ide_transfer_start(IDEState *s, uint8_t *buf, int size,
                         EndTransferFunc *end_transfer_func);
 void ide_transfer_stop(IDEState *s);
-void ide_set_inactive(IDEState *s);
+void ide_set_inactive(IDEState *s, bool more);
 BlockDriverAIOCB *ide_issue_trim(BlockDriverState *bs,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
         BlockDriverCompletionFunc *cb, void *opaque);
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index b7cedb6..b0c0d40 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -569,7 +569,6 @@ static const IDEDMAOps dbdma_ops = {
     .prepare_buf    = ide_nop_int,
     .rw_buf         = ide_nop_int,
     .set_unit       = ide_nop_int,
-    .add_status     = ide_nop_int,
     .restart_cb     = ide_nop_restart,
 };
 
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 1ee8c0a..73267a4 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -152,21 +152,17 @@ static int bmdma_set_unit(IDEDMA *dma, int unit)
     return 0;
 }
 
-static int bmdma_add_status(IDEDMA *dma, int status)
+static void bmdma_set_inactive(IDEDMA *dma, bool more)
 {
     BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma);
-    bm->status |= status;
 
-    return 0;
-}
-
-static void bmdma_set_inactive(IDEDMA *dma)
-{
-    BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma);
-
-    bm->status &= ~BM_STATUS_DMAING;
     bm->dma_cb = NULL;
     bm->unit = -1;
+    if (more) {
+        bm->status |= BM_STATUS_DMAING;
+    } else {
+        bm->status &= ~BM_STATUS_DMAING;
+    }
 }
 
 static void bmdma_restart_dma(BMDMAState *bm, enum ide_dma_cmd dma_cmd)
@@ -241,7 +237,7 @@ static void bmdma_cancel(BMDMAState *bm)
 {
     if (bm->status & BM_STATUS_DMAING) {
         /* cancel DMA request */
-        bmdma_set_inactive(&bm->dma);
+        bmdma_set_inactive(&bm->dma, false);
     }
 }
 
@@ -498,7 +494,6 @@ static const struct IDEDMAOps bmdma_ops = {
     .prepare_buf = bmdma_prepare_buf,
     .rw_buf = bmdma_rw_buf,
     .set_unit = bmdma_set_unit,
-    .add_status = bmdma_add_status,
     .set_inactive = bmdma_set_inactive,
     .restart_cb = bmdma_restart_cb,
     .reset = bmdma_reset,
-- 
1.9.3

[Qemu-devel] [PULL 12/55] ide: move BM_STATUS bits to pci.[ch]

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

They are not used by AHCI, and should not be even available there.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/internal.h | 11 -----------
 hw/ide/pci.c      |  4 ++++
 hw/ide/pci.h      |  7 +++++++
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index b35e52c..f369b0b 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -484,10 +484,6 @@ struct IDEDevice {
     uint64_t wwn;
 };
 
-#define BM_STATUS_DMAING 0x01
-#define BM_STATUS_ERROR  0x02
-#define BM_STATUS_INT    0x04
-
 /* FIXME These are not status register bits */
 #define BM_STATUS_DMA_RETRY  0x08
 #define BM_STATUS_PIO_RETRY  0x10
@@ -495,13 +491,6 @@ struct IDEDevice {
 #define BM_STATUS_RETRY_FLUSH 0x40
 #define BM_STATUS_RETRY_TRIM 0x80
 
-#define BM_MIGRATION_COMPAT_STATUS_BITS \
-        (BM_STATUS_DMA_RETRY | BM_STATUS_PIO_RETRY | \
-        BM_STATUS_RETRY_READ | BM_STATUS_RETRY_FLUSH)
-
-#define BM_CMD_START     0x01
-#define BM_CMD_READ      0x08
-
 static inline IDEState *idebus_active_if(IDEBus *bus)
 {
     return bus->ifs + bus->unit;
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 73267a4..0e82cab 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -33,6 +33,10 @@
 
 #define BMDMA_PAGE_SIZE 4096
 
+#define BM_MIGRATION_COMPAT_STATUS_BITS \
+        (BM_STATUS_DMA_RETRY | BM_STATUS_PIO_RETRY | \
+        BM_STATUS_RETRY_READ | BM_STATUS_RETRY_FLUSH)
+
 static void bmdma_start_dma(IDEDMA *dma, IDEState *s,
                             BlockDriverCompletionFunc *dma_cb)
 {
diff --git a/hw/ide/pci.h b/hw/ide/pci.h
index 2428275..517711f 100644
--- a/hw/ide/pci.h
+++ b/hw/ide/pci.h
@@ -3,6 +3,13 @@
 
 #include <hw/ide/internal.h>
 
+#define BM_STATUS_DMAING 0x01
+#define BM_STATUS_ERROR  0x02
+#define BM_STATUS_INT    0x04
+
+#define BM_CMD_START     0x01
+#define BM_CMD_READ      0x08
+
 typedef struct BMDMAState {
     IDEDMA dma;
     uint8_t cmd;
-- 
1.9.3

[Qemu-devel] [PULL 13/55] ide: move retry constants out of BM_STATUS_* namespace

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/core.c     | 20 ++++++++++----------
 hw/ide/internal.h | 12 ++++++------
 hw/ide/pci.c      | 14 +++++++-------
 3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 24f24ce..00fe043 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -523,8 +523,8 @@ static void ide_sector_read_cb(void *opaque, int ret)
 
     bdrv_acct_done(s->bs, &s->acct);
     if (ret != 0) {
-        if (ide_handle_rw_error(s, -ret, BM_STATUS_PIO_RETRY |
-                                BM_STATUS_RETRY_READ)) {
+        if (ide_handle_rw_error(s, -ret, IDE_RETRY_PIO |
+                                IDE_RETRY_READ)) {
             return;
         }
     }
@@ -614,14 +614,14 @@ void ide_dma_error(IDEState *s)
 
 static int ide_handle_rw_error(IDEState *s, int error, int op)
 {
-    bool is_read = (op & BM_STATUS_RETRY_READ) != 0;
+    bool is_read = (op & IDE_RETRY_READ) != 0;
     BlockErrorAction action = bdrv_get_error_action(s->bs, is_read, error);
 
     if (action == BLOCK_ERROR_ACTION_STOP) {
         s->bus->dma->ops->set_unit(s->bus->dma, s->unit);
         s->bus->error_status = op;
     } else if (action == BLOCK_ERROR_ACTION_REPORT) {
-        if (op & BM_STATUS_DMA_RETRY) {
+        if (op & IDE_RETRY_DMA) {
             dma_buf_commit(s);
             ide_dma_error(s);
         } else {
@@ -640,12 +640,12 @@ void ide_dma_cb(void *opaque, int ret)
     bool stay_active = false;
 
     if (ret < 0) {
-        int op = BM_STATUS_DMA_RETRY;
+        int op = IDE_RETRY_DMA;
 
         if (s->dma_cmd == IDE_DMA_READ)
-            op |= BM_STATUS_RETRY_READ;
+            op |= IDE_RETRY_READ;
         else if (s->dma_cmd == IDE_DMA_TRIM)
-            op |= BM_STATUS_RETRY_TRIM;
+            op |= IDE_RETRY_TRIM;
 
         if (ide_handle_rw_error(s, -ret, op)) {
             return;
@@ -769,7 +769,7 @@ static void ide_sector_write_cb(void *opaque, int ret)
     s->status &= ~BUSY_STAT;
 
     if (ret != 0) {
-        if (ide_handle_rw_error(s, -ret, BM_STATUS_PIO_RETRY)) {
+        if (ide_handle_rw_error(s, -ret, IDE_RETRY_PIO)) {
             return;
         }
     }
@@ -843,7 +843,7 @@ static void ide_flush_cb(void *opaque, int ret)
 
     if (ret < 0) {
         /* XXX: What sector number to set here? */
-        if (ide_handle_rw_error(s, -ret, BM_STATUS_RETRY_FLUSH)) {
+        if (ide_handle_rw_error(s, -ret, IDE_RETRY_FLUSH)) {
             return;
         }
     }
@@ -2338,7 +2338,7 @@ static bool ide_drive_pio_state_needed(void *opaque)
     IDEState *s = opaque;
 
     return ((s->status & DRQ_STAT) != 0)
-        || (s->bus->error_status & BM_STATUS_PIO_RETRY);
+        || (s->bus->error_status & IDE_RETRY_PIO);
 }
 
 static bool ide_tray_state_needed(void *opaque)
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index f369b0b..b919e96 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -484,12 +484,12 @@ struct IDEDevice {
     uint64_t wwn;
 };
 
-/* FIXME These are not status register bits */
-#define BM_STATUS_DMA_RETRY  0x08
-#define BM_STATUS_PIO_RETRY  0x10
-#define BM_STATUS_RETRY_READ  0x20
-#define BM_STATUS_RETRY_FLUSH 0x40
-#define BM_STATUS_RETRY_TRIM 0x80
+/* These are used for the error_status field of IDEBus */
+#define IDE_RETRY_DMA  0x08
+#define IDE_RETRY_PIO  0x10
+#define IDE_RETRY_READ  0x20
+#define IDE_RETRY_FLUSH 0x40
+#define IDE_RETRY_TRIM 0x80
 
 static inline IDEState *idebus_active_if(IDEBus *bus)
 {
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 0e82cab..2397f35 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -34,8 +34,8 @@
 #define BMDMA_PAGE_SIZE 4096
 
 #define BM_MIGRATION_COMPAT_STATUS_BITS \
-        (BM_STATUS_DMA_RETRY | BM_STATUS_PIO_RETRY | \
-        BM_STATUS_RETRY_READ | BM_STATUS_RETRY_FLUSH)
+        (IDE_RETRY_DMA | IDE_RETRY_PIO | \
+        IDE_RETRY_READ | IDE_RETRY_FLUSH)
 
 static void bmdma_start_dma(IDEDMA *dma, IDEState *s,
                             BlockDriverCompletionFunc *dma_cb)
@@ -198,7 +198,7 @@ static void bmdma_restart_bh(void *opaque)
         return;
     }
 
-    is_read = (bus->error_status & BM_STATUS_RETRY_READ) != 0;
+    is_read = (bus->error_status & IDE_RETRY_READ) != 0;
 
     /* The error status must be cleared before resubmitting the request: The
      * request may fail again, and this case can only be distinguished if the
@@ -206,19 +206,19 @@ static void bmdma_restart_bh(void *opaque)
     error_status = bus->error_status;
     bus->error_status = 0;
 
-    if (error_status & BM_STATUS_DMA_RETRY) {
-        if (error_status & BM_STATUS_RETRY_TRIM) {
+    if (error_status & IDE_RETRY_DMA) {
+        if (error_status & IDE_RETRY_TRIM) {
             bmdma_restart_dma(bm, IDE_DMA_TRIM);
         } else {
             bmdma_restart_dma(bm, is_read ? IDE_DMA_READ : IDE_DMA_WRITE);
         }
-    } else if (error_status & BM_STATUS_PIO_RETRY) {
+    } else if (error_status & IDE_RETRY_PIO) {
         if (is_read) {
             ide_sector_read(bmdma_active_if(bm));
         } else {
             ide_sector_write(bmdma_active_if(bm));
         }
-    } else if (error_status & BM_STATUS_RETRY_FLUSH) {
+    } else if (error_status & IDE_RETRY_FLUSH) {
         ide_flush_cache(bmdma_active_if(bm));
     }
 }
-- 
1.9.3

[Qemu-devel] [PULL 14/55] ahci: remove duplicate PORT_IRQ_* constants

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

These are defined twice, just use one set consistently.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/ahci.c |  6 +++---
 hw/ide/ahci.h | 21 ---------------------
 2 files changed, 3 insertions(+), 24 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 0ec5627..e1f27bd 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -584,7 +584,7 @@ static void ahci_write_fis_sdb(AHCIState *s, int port, uint32_t finished)
     s->dev[port].finished |= finished;
     *(uint32_t*)(sdb_fis + 4) = cpu_to_le32(s->dev[port].finished);
 
-    ahci_trigger_irq(s, &s->dev[port], PORT_IRQ_STAT_SDBS);
+    ahci_trigger_irq(s, &s->dev[port], PORT_IRQ_SDB_FIS);
 }
 
 static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis)
@@ -629,7 +629,7 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis)
     }
 
     if (d2h_fis[2] & ERR_STAT) {
-        ahci_trigger_irq(ad->hba, ad, PORT_IRQ_STAT_TFES);
+        ahci_trigger_irq(ad->hba, ad, PORT_IRQ_TF_ERR);
     }
 
     ahci_trigger_irq(ad->hba, ad, PORT_IRQ_D2H_REG_FIS);
@@ -1039,7 +1039,7 @@ out:
 
     if (!(s->status & DRQ_STAT)) {
         /* done with DMA */
-        ahci_trigger_irq(ad->hba, ad, PORT_IRQ_STAT_DSS);
+        ahci_trigger_irq(ad->hba, ad, PORT_IRQ_D2H_REG_FIS);
     }
 }
 
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index f418b30..1543df7 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -132,27 +132,6 @@
 #define PORT_CMD_ICC_PARTIAL      (0x2 << 28) /* Put i/f in partial state */
 #define PORT_CMD_ICC_SLUMBER      (0x6 << 28) /* Put i/f in slumber state */
 
-#define PORT_IRQ_STAT_DHRS        (1 << 0) /* Device to Host Register FIS */
-#define PORT_IRQ_STAT_PSS         (1 << 1) /* PIO Setup FIS */
-#define PORT_IRQ_STAT_DSS         (1 << 2) /* DMA Setup FIS */
-#define PORT_IRQ_STAT_SDBS        (1 << 3) /* Set Device Bits */
-#define PORT_IRQ_STAT_UFS         (1 << 4) /* Unknown FIS */
-#define PORT_IRQ_STAT_DPS         (1 << 5) /* Descriptor Processed */
-#define PORT_IRQ_STAT_PCS         (1 << 6) /* Port Connect Change Status */
-#define PORT_IRQ_STAT_DMPS        (1 << 7) /* Device Mechanical Presence
-                                              Status */
-#define PORT_IRQ_STAT_PRCS        (1 << 22) /* File Ready Status */
-#define PORT_IRQ_STAT_IPMS        (1 << 23) /* Incorrect Port Multiplier
-                                               Status */
-#define PORT_IRQ_STAT_OFS         (1 << 24) /* Overflow Status */
-#define PORT_IRQ_STAT_INFS        (1 << 26) /* Interface Non-Fatal Error
-                                               Status */
-#define PORT_IRQ_STAT_IFS         (1 << 27) /* Interface Fatal Error */
-#define PORT_IRQ_STAT_HBDS        (1 << 28) /* Host Bus Data Error Status */
-#define PORT_IRQ_STAT_HBFS        (1 << 29) /* Host Bus Fatal Error Status */
-#define PORT_IRQ_STAT_TFES        (1 << 30) /* Task File Error Status */
-#define PORT_IRQ_STAT_CPDS        (1U << 31) /* Code Port Detect Status */
-
 /* ap->flags bits */
 #define AHCI_FLAG_NO_NCQ                  (1 << 24)
 #define AHCI_FLAG_IGN_IRQ_IF_ERR          (1 << 25) /* ignore IRQ_IF_ERR */
-- 
1.9.3

[Qemu-devel] [PULL 15/55] ide: stop PIO transfer on errors

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

This will provide a hook for sending the result of the command via the
FIS receive area.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 00fe043..91a17e6 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -420,6 +420,7 @@ BlockDriverAIOCB *ide_issue_trim(BlockDriverState *bs,
 
 static inline void ide_abort_command(IDEState *s)
 {
+    ide_transfer_stop(s);
     s->status = READY_STAT | ERR_STAT;
     s->error = ABRT_ERR;
 }
@@ -605,9 +606,7 @@ void ide_set_inactive(IDEState *s, bool more)
 
 void ide_dma_error(IDEState *s)
 {
-    ide_transfer_stop(s);
-    s->error = ABRT_ERR;
-    s->status = READY_STAT | ERR_STAT;
+    ide_abort_command(s);
     ide_set_inactive(s, false);
     ide_set_irq(s->bus);
 }
-- 
1.9.3

[Qemu-devel] [PULL 16/55] ide: make all commands go through cmd_done

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

AHCI has code to fill in the D2H FIS trigger the IRQ all over the place.
Centralize this in a single cmd_done callback by generalizing the existing
async_cmd_done callback.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/ahci.c     | 16 +++-------------
 hw/ide/atapi.c    |  2 +-
 hw/ide/core.c     | 20 +++++++++++---------
 hw/ide/internal.h |  2 +-
 4 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index e1f27bd..b40ec06 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -969,11 +969,6 @@ static int handle_cmd(AHCIState *s, int port, int slot)
 
         /* We're ready to process the command in FIS byte 2. */
         ide_exec_cmd(&s->dev[port].port, cmd_fis[2]);
-
-        if ((s->dev[port].port.ifs[0].status & (READY_STAT|DRQ_STAT|BUSY_STAT)) ==
-            READY_STAT) {
-            ahci_write_fis_d2h(&s->dev[port], cmd_fis);
-        }
     }
 
 out:
@@ -1036,11 +1031,6 @@ out:
     }
 
     s->end_transfer_func(s);
-
-    if (!(s->status & DRQ_STAT)) {
-        /* done with DMA */
-        ahci_trigger_irq(ad->hba, ad, PORT_IRQ_D2H_REG_FIS);
-    }
 }
 
 static void ahci_start_dma(IDEDMA *dma, IDEState *s,
@@ -1102,11 +1092,11 @@ static int ahci_dma_set_unit(IDEDMA *dma, int unit)
     return 0;
 }
 
-static void ahci_async_cmd_done(IDEDMA *dma)
+static void ahci_cmd_done(IDEDMA *dma)
 {
     AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
 
-    DPRINTF(ad->port_no, "async cmd done\n");
+    DPRINTF(ad->port_no, "cmd done\n");
 
     /* update d2h status */
     ahci_write_fis_d2h(ad, NULL);
@@ -1132,7 +1122,7 @@ static const IDEDMAOps ahci_dma_ops = {
     .prepare_buf = ahci_dma_prepare_buf,
     .rw_buf = ahci_dma_rw_buf,
     .set_unit = ahci_dma_set_unit,
-    .async_cmd_done = ahci_async_cmd_done,
+    .cmd_done = ahci_cmd_done,
     .restart_cb = ahci_dma_restart_cb,
 };
 
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 3b419b3..3d92b52 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -174,9 +174,9 @@ void ide_atapi_cmd_reply_end(IDEState *s)
 #endif
     if (s->packet_transfer_size <= 0) {
         /* end of transfer */
-        ide_transfer_stop(s);
         s->status = READY_STAT | SEEK_STAT;
         s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO | ATAPI_INT_REASON_CD;
+        ide_transfer_stop(s);
         ide_set_irq(s->bus);
 #ifdef DEBUG_IDE_ATAPI
         printf("status=0x%x\n", s->status);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 91a17e6..bdb0a80 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -440,12 +440,20 @@ void ide_transfer_start(IDEState *s, uint8_t *buf, int size,
     }
 }
 
+static void ide_cmd_done(IDEState *s)
+{
+    if (s->bus->dma->ops->cmd_done) {
+        s->bus->dma->ops->cmd_done(s->bus->dma);
+    }
+}
+
 void ide_transfer_stop(IDEState *s)
 {
     s->end_transfer_func = ide_transfer_stop;
     s->data_ptr = s->io_buffer;
     s->data_end = s->io_buffer;
     s->status &= ~DRQ_STAT;
+    ide_cmd_done(s);
 }
 
 int64_t ide_get_sector(IDEState *s)
@@ -588,20 +596,13 @@ static void dma_buf_commit(IDEState *s)
     qemu_sglist_destroy(&s->sg);
 }
 
-static void ide_async_cmd_done(IDEState *s)
-{
-    if (s->bus->dma->ops->async_cmd_done) {
-        s->bus->dma->ops->async_cmd_done(s->bus->dma);
-    }
-}
-
 void ide_set_inactive(IDEState *s, bool more)
 {
     s->bus->dma->aiocb = NULL;
     if (s->bus->dma->ops->set_inactive) {
         s->bus->dma->ops->set_inactive(s->bus->dma, more);
     }
-    ide_async_cmd_done(s);
+    ide_cmd_done(s);
 }
 
 void ide_dma_error(IDEState *s)
@@ -849,7 +850,7 @@ static void ide_flush_cb(void *opaque, int ret)
 
     bdrv_acct_done(s->bs, &s->acct);
     s->status = READY_STAT | SEEK_STAT;
-    ide_async_cmd_done(s);
+    ide_cmd_done(s);
     ide_set_irq(s->bus);
 }
 
@@ -1773,6 +1774,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
             s->status |= SEEK_STAT;
         }
 
+        ide_cmd_done(s);
         ide_set_irq(s->bus);
     }
 }
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index b919e96..5c19f79 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -433,7 +433,7 @@ struct IDEDMAOps {
     DMAIntFunc *rw_buf;
     DMAIntFunc *set_unit;
     DMAStopFunc *set_inactive;
-    DMAVoidFunc *async_cmd_done;
+    DMAVoidFunc *cmd_done;
     DMARestartFunc *restart_cb;
     DMAVoidFunc *reset;
 };
-- 
1.9.3

[Qemu-devel] [PULL 17/55] ahci: construct PIO Setup FIS for PIO commands

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

PIO commands should put a PIO Setup FIS in the receive area when data
transfer ends.  Currently QEMU does not do this and only places the
D2H FIS at the end of the operation.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/ahci.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index b40ec06..4cda0d0 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -587,6 +587,71 @@ static void ahci_write_fis_sdb(AHCIState *s, int port, uint32_t finished)
     ahci_trigger_irq(s, &s->dev[port], PORT_IRQ_SDB_FIS);
 }
 
+static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len)
+{
+    AHCIPortRegs *pr = &ad->port_regs;
+    uint8_t *pio_fis, *cmd_fis;
+    uint64_t tbl_addr;
+    dma_addr_t cmd_len = 0x80;
+
+    if (!ad->res_fis || !(pr->cmd & PORT_CMD_FIS_RX)) {
+        return;
+    }
+
+    /* map cmd_fis */
+    tbl_addr = le64_to_cpu(ad->cur_cmd->tbl_addr);
+    cmd_fis = dma_memory_map(ad->hba->as, tbl_addr, &cmd_len,
+                             DMA_DIRECTION_TO_DEVICE);
+
+    if (cmd_fis == NULL) {
+        DPRINTF(ad->port_no, "dma_memory_map failed in ahci_write_fis_pio");
+        ahci_trigger_irq(ad->hba, ad, PORT_IRQ_HBUS_ERR);
+        return;
+    }
+
+    if (cmd_len != 0x80) {
+        DPRINTF(ad->port_no,
+                "dma_memory_map mapped too few bytes in ahci_write_fis_pio");
+        dma_memory_unmap(ad->hba->as, cmd_fis, cmd_len,
+                         DMA_DIRECTION_TO_DEVICE, cmd_len);
+        ahci_trigger_irq(ad->hba, ad, PORT_IRQ_HBUS_ERR);
+        return;
+    }
+
+    pio_fis = &ad->res_fis[RES_FIS_PSFIS];
+
+    pio_fis[0] = 0x5f;
+    pio_fis[1] = (ad->hba->control_regs.irqstatus ? (1 << 6) : 0);
+    pio_fis[2] = ad->port.ifs[0].status;
+    pio_fis[3] = ad->port.ifs[0].error;
+
+    pio_fis[4] = cmd_fis[4];
+    pio_fis[5] = cmd_fis[5];
+    pio_fis[6] = cmd_fis[6];
+    pio_fis[7] = cmd_fis[7];
+    pio_fis[8] = cmd_fis[8];
+    pio_fis[9] = cmd_fis[9];
+    pio_fis[10] = cmd_fis[10];
+    pio_fis[11] = cmd_fis[11];
+    pio_fis[12] = cmd_fis[12];
+    pio_fis[13] = cmd_fis[13];
+    pio_fis[14] = 0;
+    pio_fis[15] = ad->port.ifs[0].status;
+    pio_fis[16] = len & 255;
+    pio_fis[17] = len >> 8;
+    pio_fis[18] = 0;
+    pio_fis[19] = 0;
+
+    if (pio_fis[2] & ERR_STAT) {
+        ahci_trigger_irq(ad->hba, ad, PORT_IRQ_TF_ERR);
+    }
+
+    ahci_trigger_irq(ad->hba, ad, PORT_IRQ_PIOS_FIS);
+
+    dma_memory_unmap(ad->hba->as, cmd_fis, cmd_len,
+                     DMA_DIRECTION_TO_DEVICE, cmd_len);
+}
+
 static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis)
 {
     AHCIPortRegs *pr = &ad->port_regs;
@@ -1031,6 +1096,11 @@ out:
     }
 
     s->end_transfer_func(s);
+
+    if (!(s->status & DRQ_STAT)) {
+        /* done with PIO send/receive */
+        ahci_write_fis_pio(ad, le32_to_cpu(ad->cur_cmd->status));
+    }
 }
 
 static void ahci_start_dma(IDEDMA *dma, IDEState *s,
-- 
1.9.3

[Qemu-devel] [PULL 18/55] q35: Enable the ioapic device to be seen by qtest.

[Stefan Hajnoczi]

From: John Snow <jsnow@redhat.com>

Currently, the ioapic device can not be found in a qtest environment
when requesting "irq_interrupt_in ioapic" via the qtest socket.

By mirroring how the ioapic is added in i44ofx (hw/i440/pc_piix.c),
as a child of "q35," the device is able to be seen by qtest.

Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/i386/pc_q35.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c39ee98..0dd7e02 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -236,7 +236,7 @@ static void pc_q35_init(MachineState *machine)
         gsi_state->i8259_irq[i] = i8259[i];
     }
     if (pci_enabled) {
-        ioapic_init_gsi(gsi_state, NULL);
+        ioapic_init_gsi(gsi_state, "q35");
     }
     qdev_init_nofail(icc_bridge);
 
-- 
1.9.3

[Qemu-devel] [PULL 19/55] qtest: Adding qtest_memset and qmemset.

[Stefan Hajnoczi]

From: John Snow <jsnow@redhat.com>

Currently, libqtest allows for memread and memwrite, but
does not offer a simple way to zero out regions of memory.
This patch adds a simple function to do so.

Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqtest.c | 12 ++++++++++++
 tests/libqtest.h | 24 ++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 4a75cd3..e525e6f 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -659,6 +659,18 @@ void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
     qtest_rsp(s, 0);
 }
 
+void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size)
+{
+    size_t i;
+
+    qtest_sendf(s, "write 0x%" PRIx64 " 0x%zx 0x", addr, size);
+    for (i = 0; i < size; i++) {
+        qtest_sendf(s, "%02x", pattern);
+    }
+    qtest_sendf(s, "\n");
+    qtest_rsp(s, 0);
+}
+
 QDict *qmp(const char *fmt, ...)
 {
     va_list ap;
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 8f323c7..1be0934 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -283,6 +283,17 @@ void qtest_memread(QTestState *s, uint64_t addr, void *data, size_t size);
 void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t size);
 
 /**
+ * qtest_memset:
+ * @s: #QTestState instance to operate on.
+ * @addr: Guest address to write to.
+ * @patt: Byte pattern to fill the guest memory region with.
+ * @size: Number of bytes to write.
+ *
+ * Write a pattern to guest memory.
+ */
+void qtest_memset(QTestState *s, uint64_t addr, uint8_t patt, size_t size);
+
+/**
  * qtest_clock_step_next:
  * @s: #QTestState instance to operate on.
  *
@@ -621,6 +632,19 @@ static inline void memwrite(uint64_t addr, const void *data, size_t size)
 }
 
 /**
+ * qmemset:
+ * @addr: Guest address to write to.
+ * @patt: Byte pattern to fill the guest memory region with.
+ * @size: Number of bytes to write.
+ *
+ * Write a pattern to guest memory.
+ */
+static inline void qmemset(uint64_t addr, uint8_t patt, size_t size)
+{
+    qtest_memset(global_qtest, addr, patt, size);
+}
+
+/**
  * clock_step_next:
  *
  * Advance the QEMU_CLOCK_VIRTUAL to the next deadline.
-- 
1.9.3

[Qemu-devel] [PULL 20/55] libqos: Correct memory leak

[Stefan Hajnoczi]

From: John Snow <jsnow@redhat.com>

Fix a small memory leak inside of libqos, in the pc_alloc_init routine.

Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqos/malloc-pc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index db1496c..63af60a 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -67,5 +67,8 @@ QGuestAllocator *pc_alloc_init(void)
     /* Respect PCI hole */
     s->end = MIN(ram_size, 0xE0000000);
 
+    /* clean-up */
+    g_free(fw_cfg);
+
     return &s->alloc;
 }
-- 
1.9.3

[Qemu-devel] [PULL 21/55] libqtest: Correct small memory leak.

[Stefan Hajnoczi]

From: John Snow <jsnow@redhat.com>

Fixes a small memory leak inside of libqtest.
After we produce a test path and glib copies the string
for itself, we should clean up our temporary copy.

Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqtest.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index e525e6f..0bf17aa 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -644,6 +644,7 @@ void qtest_add_func(const char *str, void (*fn))
 {
     gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
     g_test_add_func(path, fn);
+    g_free(path);
 }
 
 void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
-- 
1.9.3

[Qemu-devel] [PULL 22/55] libqos: Fixes a small memory leak.

[Stefan Hajnoczi]

From: John Snow <jsnow@redhat.com>

Allow users the chance to clean up the QPCIBusPC structure
by adding a small cleanup routine. Helps clear up small
memory leaks during setup/teardown, to allow for cleaner
debug output messages.

Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqos/pci-pc.c | 7 +++++++
 tests/libqos/pci-pc.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c
index 4adf400..f5d6469 100644
--- a/tests/libqos/pci-pc.c
+++ b/tests/libqos/pci-pc.c
@@ -237,3 +237,10 @@ QPCIBus *qpci_init_pc(void)
 
     return &ret->bus;
 }
+
+void qpci_free_pc(QPCIBus *bus)
+{
+    QPCIBusPC *s = container_of(bus, QPCIBusPC, bus);
+
+    g_free(s);
+}
diff --git a/tests/libqos/pci-pc.h b/tests/libqos/pci-pc.h
index 4f7475f..2621179 100644
--- a/tests/libqos/pci-pc.h
+++ b/tests/libqos/pci-pc.h
@@ -16,5 +16,6 @@
 #include "libqos/pci.h"
 
 QPCIBus *qpci_init_pc(void);
+void     qpci_free_pc(QPCIBus *bus);
 
 #endif
-- 
1.9.3

[Qemu-devel] [PULL 23/55] libqos: allow qpci_iomap to return BAR mapping size

[Stefan Hajnoczi]

From: John Snow <jsnow@redhat.com>

This patch allows qpci_iomap to return the size of the
BAR mapping that it created, to allow driver applications
(e.g, ahci-test) to make determinations about the suitability
or the mapping size, or in the specific case of AHCI, how
many ports are supported by the HBA.

Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/ide-test.c          | 2 +-
 tests/libqos/pci-pc.c     | 5 ++++-
 tests/libqos/pci.c        | 4 ++--
 tests/libqos/pci.h        | 4 ++--
 tests/usb-hcd-ehci-test.c | 2 +-
 5 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index 151ef30..e2b4efc 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -146,7 +146,7 @@ static QPCIDevice *get_pci_device(uint16_t *bmdma_base)
     g_assert(device_id == PCI_DEVICE_ID_INTEL_82371SB_1);
 
     /* Map bmdma BAR */
-    *bmdma_base = (uint16_t)(uintptr_t) qpci_iomap(dev, 4);
+    *bmdma_base = (uint16_t)(uintptr_t) qpci_iomap(dev, 4, NULL);
 
     qpci_device_enable(dev);
 
diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c
index f5d6469..0609294 100644
--- a/tests/libqos/pci-pc.c
+++ b/tests/libqos/pci-pc.c
@@ -144,7 +144,7 @@ static void qpci_pc_config_writel(QPCIBus *bus, int devfn, uint8_t offset, uint3
     outl(0xcfc, value);
 }
 
-static void *qpci_pc_iomap(QPCIBus *bus, QPCIDevice *dev, int barno)
+static void *qpci_pc_iomap(QPCIBus *bus, QPCIDevice *dev, int barno, uint64_t *sizeptr)
 {
     QPCIBusPC *s = container_of(bus, QPCIBusPC, bus);
     static const int bar_reg_map[] = {
@@ -173,6 +173,9 @@ static void *qpci_pc_iomap(QPCIBus *bus, QPCIDevice *dev, int barno)
     if (size == 0) {
         return NULL;
     }
+    if (sizeptr) {
+        *sizeptr = size;
+    }
 
     if (io_type == PCI_BASE_ADDRESS_SPACE_IO) {
         uint16_t loc;
diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c
index c9a0b91..ce0b308 100644
--- a/tests/libqos/pci.c
+++ b/tests/libqos/pci.c
@@ -138,9 +138,9 @@ void qpci_io_writel(QPCIDevice *dev, void *data, uint32_t value)
     dev->bus->io_writel(dev->bus, data, value);
 }
 
-void *qpci_iomap(QPCIDevice *dev, int barno)
+void *qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
 {
-    return dev->bus->iomap(dev->bus, dev, barno);
+    return dev->bus->iomap(dev->bus, dev, barno, sizeptr);
 }
 
 void qpci_iounmap(QPCIDevice *dev, void *data)
diff --git a/tests/libqos/pci.h b/tests/libqos/pci.h
index 3439431..9ee048b 100644
--- a/tests/libqos/pci.h
+++ b/tests/libqos/pci.h
@@ -41,7 +41,7 @@ struct QPCIBus
     void (*config_writel)(QPCIBus *bus, int devfn,
                           uint8_t offset, uint32_t value);
 
-    void *(*iomap)(QPCIBus *bus, QPCIDevice *dev, int barno);
+    void *(*iomap)(QPCIBus *bus, QPCIDevice *dev, int barno, uint64_t *sizeptr);
     void (*iounmap)(QPCIBus *bus, void *data);
 };
 
@@ -74,7 +74,7 @@ void qpci_io_writeb(QPCIDevice *dev, void *data, uint8_t value);
 void qpci_io_writew(QPCIDevice *dev, void *data, uint16_t value);
 void qpci_io_writel(QPCIDevice *dev, void *data, uint32_t value);
 
-void *qpci_iomap(QPCIDevice *dev, int barno);
+void *qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr);
 void qpci_iounmap(QPCIDevice *dev, void *data);
 
 #endif
diff --git a/tests/usb-hcd-ehci-test.c b/tests/usb-hcd-ehci-test.c
index bcdf62f..c990492 100644
--- a/tests/usb-hcd-ehci-test.c
+++ b/tests/usb-hcd-ehci-test.c
@@ -34,7 +34,7 @@ static void pci_init_one(struct qhc *hc, uint32_t devfn, int bar)
     hc->dev = qpci_device_find(pcibus, devfn);
     g_assert(hc->dev != NULL);
     qpci_device_enable(hc->dev);
-    hc->base = qpci_iomap(hc->dev, bar);
+    hc->base = qpci_iomap(hc->dev, bar, NULL);
     g_assert(hc->base != NULL);
 }
 
-- 
1.9.3

[Qemu-devel] [PULL 24/55] qtest/ide: Fix small memory leak

[Stefan Hajnoczi]

From: John Snow <jsnow@redhat.com>

For libqos debugging purposes, it's nice to
be able to assert that tests and associated libraries
have no memory leaks. To that end, free up the
trivial cmdline leak.

The remaining leaks caused by pc_alloc_init are fixed
instead by my first-fit pc_alloc implementation already
on the qemu-devel mailing list.

Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/ide-test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index e2b4efc..a77a037 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -120,6 +120,8 @@ static void ide_test_start(const char *cmdline_fmt, ...)
     qtest_start(cmdline);
     qtest_irq_intercept_in(global_qtest, "ioapic");
     guest_malloc = pc_alloc_init();
+
+    g_free(cmdline);
 }
 
 static void ide_test_quit(void)
-- 
1.9.3

[Qemu-devel] [PULL 25/55] cmd646: add constants for CNTRL register access

[Stefan Hajnoczi]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/cmd646.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index a8e35fe..d8395ef 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -33,6 +33,9 @@
 #include <hw/ide/pci.h>
 
 /* CMD646 specific */
+#define CNTRL		0x51
+#define   CNTRL_EN_CH0	0x04
+#define   CNTRL_EN_CH1	0x08
 #define MRDMODE		0x71
 #define   MRDMODE_INTR_CH0	0x04
 #define   MRDMODE_INTR_CH1	0x08
@@ -269,10 +272,10 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
 
     pci_conf[PCI_CLASS_PROG] = 0x8f;
 
-    pci_conf[0x51] = 0x04; // enable IDE0
+    pci_conf[CNTRL] = CNTRL_EN_CH0; // enable IDE0
     if (d->secondary) {
         /* XXX: if not enabled, really disable the seconday IDE controller */
-        pci_conf[0x51] |= 0x08; /* enable IDE1 */
+        pci_conf[CNTRL] |= CNTRL_EN_CH1; /* enable IDE1 */
     }
 
     setup_cmd646_bar(d, 0);
-- 
1.9.3

[Qemu-devel] [PULL 26/55] cmd646: synchronise DMA interrupt status with UDMA interrupt status

[Stefan Hajnoczi]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Make sure that the standard DMA interrupt status bits reflect any changes made
to the UDMA interrupt status bits. The CMD646U2 datasheet claims that these
bits are equivalent, and they must be synchronised for guests that manipulate
both registers.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/cmd646.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index d8395ef..c3c6c53 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -33,9 +33,13 @@
 #include <hw/ide/pci.h>
 
 /* CMD646 specific */
+#define CFR		0x50
+#define   CFR_INTR_CH0	0x04
 #define CNTRL		0x51
 #define   CNTRL_EN_CH0	0x04
 #define   CNTRL_EN_CH1	0x08
+#define ARTTIM23	0x57
+#define    ARTTIM23_INTR_CH1	0x10
 #define MRDMODE		0x71
 #define   MRDMODE_INTR_CH0	0x04
 #define   MRDMODE_INTR_CH1	0x08
@@ -126,6 +130,22 @@ static void setup_cmd646_bar(PCIIDEState *d, int bus_num)
                           "cmd646-data", 8);
 }
 
+static void cmd646_update_dma_interrupts(PCIDevice *pd)
+{
+    /* Sync DMA interrupt status from UDMA interrupt status */
+    if (pd->config[MRDMODE] & MRDMODE_INTR_CH0) {
+        pd->config[CFR] |= CFR_INTR_CH0;
+    } else {
+        pd->config[CFR] &= ~CFR_INTR_CH0;
+    }
+
+    if (pd->config[MRDMODE] & MRDMODE_INTR_CH1) {
+        pd->config[ARTTIM23] |= ARTTIM23_INTR_CH1;
+    } else {
+        pd->config[ARTTIM23] &= ~ARTTIM23_INTR_CH1;
+    }
+}
+
 static uint64_t bmdma_read(void *opaque, hwaddr addr,
                            unsigned size)
 {
@@ -184,6 +204,7 @@ static void bmdma_write(void *opaque, hwaddr addr,
     case 1:
         pci_dev->config[MRDMODE] =
             (pci_dev->config[MRDMODE] & ~0x30) | (val & 0x30);
+        cmd646_update_dma_interrupts(pci_dev);
         cmd646_update_irq(bm->pci_dev);
         break;
     case 2:
@@ -249,6 +270,7 @@ static void cmd646_set_irq(void *opaque, int channel, int level)
     } else {
         pd->config[MRDMODE] &= ~irq_mask;
     }
+    cmd646_update_dma_interrupts(pd);
     cmd646_update_irq(d);
 }
 
-- 
1.9.3

[Qemu-devel] [PULL 27/55] cmd646: switch cmd646_update_irq() to accept PCIDevice instead of PCIIDEState

[Stefan Hajnoczi]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

This is in preparation for adding configuration space accessors which accept
PCIDevice as a parameter.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/cmd646.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index c3c6c53..11a3e52 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -48,7 +48,7 @@
 #define UDIDETCR0	0x73
 #define UDIDETCR1	0x7B
 
-static void cmd646_update_irq(PCIIDEState *d);
+static void cmd646_update_irq(PCIDevice *pd);
 
 static uint64_t cmd646_cmd_read(void *opaque, hwaddr addr,
                                 unsigned size)
@@ -205,7 +205,7 @@ static void bmdma_write(void *opaque, hwaddr addr,
         pci_dev->config[MRDMODE] =
             (pci_dev->config[MRDMODE] & ~0x30) | (val & 0x30);
         cmd646_update_dma_interrupts(pci_dev);
-        cmd646_update_irq(bm->pci_dev);
+        cmd646_update_irq(pci_dev);
         break;
     case 2:
         bm->status = (val & 0x60) | (bm->status & 1) | (bm->status & ~val & 0x06);
@@ -245,9 +245,8 @@ static void bmdma_setup_bar(PCIIDEState *d)
 
 /* XXX: call it also when the MRDMODE is changed from the PCI config
    registers */
-static void cmd646_update_irq(PCIIDEState *d)
+static void cmd646_update_irq(PCIDevice *pd)
 {
-    PCIDevice *pd = PCI_DEVICE(d);
     int pci_level;
 
     pci_level = ((pd->config[MRDMODE] & MRDMODE_INTR_CH0) &&
@@ -271,7 +270,7 @@ static void cmd646_set_irq(void *opaque, int channel, int level)
         pd->config[MRDMODE] &= ~irq_mask;
     }
     cmd646_update_dma_interrupts(pd);
-    cmd646_update_irq(d);
+    cmd646_update_irq(pd);
 }
 
 static void cmd646_reset(void *opaque)
-- 
1.9.3

[Qemu-devel] [PULL 28/55] cmd646: allow MRDMODE interrupt status bits clearing from PCI config space

[Stefan Hajnoczi]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Make sure that we also update the normal DMA interrupt status bits at the
same time, and alter the IRQ if being cleared accordingly.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/cmd646.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 11a3e52..b8dc4ab 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -243,8 +243,6 @@ static void bmdma_setup_bar(PCIIDEState *d)
     }
 }
 
-/* XXX: call it also when the MRDMODE is changed from the PCI config
-   registers */
 static void cmd646_update_irq(PCIDevice *pd)
 {
     int pci_level;
@@ -283,6 +281,30 @@ static void cmd646_reset(void *opaque)
     }
 }
 
+static uint32_t cmd646_pci_config_read(PCIDevice *d,
+                                       uint32_t address, int len)
+{
+    return pci_default_read_config(d, address, len);
+}
+
+static void cmd646_pci_config_write(PCIDevice *d, uint32_t addr, uint32_t val,
+                                    int l)
+{
+    uint32_t i;
+
+    pci_default_write_config(d, addr, val, l);
+
+    for (i = addr; i < addr + l; i++) {
+        switch (i) {
+        case MRDMODE:
+            cmd646_update_dma_interrupts(d);
+            break;
+        }
+    }
+
+    cmd646_update_irq(d);
+}
+
 /* CMD646 PCI IDE controller */
 static int pci_cmd646_ide_initfn(PCIDevice *dev)
 {
@@ -299,6 +321,10 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
         pci_conf[CNTRL] |= CNTRL_EN_CH1; /* enable IDE1 */
     }
 
+    /* Set write-to-clear interrupt bits */
+    dev->wmask[MRDMODE] = 0x0;
+    dev->w1cmask[MRDMODE] = MRDMODE_INTR_CH0 | MRDMODE_INTR_CH1;
+
     setup_cmd646_bar(d, 0);
     setup_cmd646_bar(d, 1);
     pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd646_bar[0].data);
@@ -371,6 +397,8 @@ static void cmd646_ide_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_CMD_646;
     k->revision = 0x07;
     k->class_id = PCI_CLASS_STORAGE_IDE;
+    k->config_read = cmd646_pci_config_read;
+    k->config_write = cmd646_pci_config_write;
     dc->props = cmd646_ide_properties;
 }
 
-- 
1.9.3

[Qemu-devel] [PULL 29/55] cmd646: synchronise UDMA interrupt status with DMA interrupt status

[Stefan Hajnoczi]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Make sure that both registers are synchronised when being accessed through
PCI configuration space.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/cmd646.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index b8dc4ab..74d0deb 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -146,6 +146,22 @@ static void cmd646_update_dma_interrupts(PCIDevice *pd)
     }
 }
 
+static void cmd646_update_udma_interrupts(PCIDevice *pd)
+{
+    /* Sync UDMA interrupt status from DMA interrupt status */
+    if (pd->config[CFR] & CFR_INTR_CH0) {
+        pd->config[MRDMODE] |= MRDMODE_INTR_CH0;
+    } else {
+        pd->config[MRDMODE] &= ~MRDMODE_INTR_CH0;
+    }
+
+    if (pd->config[ARTTIM23] & ARTTIM23_INTR_CH1) {
+        pd->config[MRDMODE] |= MRDMODE_INTR_CH1;
+    } else {
+        pd->config[MRDMODE] &= ~MRDMODE_INTR_CH1;
+    }
+}
+
 static uint64_t bmdma_read(void *opaque, hwaddr addr,
                            unsigned size)
 {
@@ -296,6 +312,10 @@ static void cmd646_pci_config_write(PCIDevice *d, uint32_t addr, uint32_t val,
 
     for (i = addr; i < addr + l; i++) {
         switch (i) {
+        case CFR:
+        case ARTTIM23:
+            cmd646_update_udma_interrupts(d);
+            break;
         case MRDMODE:
             cmd646_update_dma_interrupts(d);
             break;
@@ -322,6 +342,10 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
     }
 
     /* Set write-to-clear interrupt bits */
+    dev->wmask[CFR] = 0x0;
+    dev->w1cmask[CFR] = CFR_INTR_CH0;
+    dev->wmask[ARTTIM23] = 0x0;
+    dev->w1cmask[ARTTIM23] = ARTTIM23_INTR_CH1;
     dev->wmask[MRDMODE] = 0x0;
     dev->w1cmask[MRDMODE] = MRDMODE_INTR_CH0 | MRDMODE_INTR_CH1;
 
-- 
1.9.3

[Qemu-devel] [PULL 30/55] qemu-char: using qemu_set_nonblock() instead of fcntl(O_NONBLOCK)

[Stefan Hajnoczi]

From: Gonglei <arei.gonglei@huawei.com>

Technically, fcntl(soc, F_SETFL, O_NONBLOCK)
is incorrect since it clobbers all other file flags.
We can use F_GETFL to get the current flags, set or
clear the O_NONBLOCK flag, then use F_SETFL to set the flags.

Using the qemu_set_nonblock() wrapper.

Signed-off-by: Wangxin <wangxinxin.wang@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qemu-char.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 6964a2d..b1e6a0a 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -975,7 +975,7 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
     s = g_malloc0(sizeof(FDCharDriver));
     s->fd_in = io_channel_from_fd(fd_in);
     s->fd_out = io_channel_from_fd(fd_out);
-    fcntl(fd_out, F_SETFL, O_NONBLOCK);
+    qemu_set_nonblock(fd_out);
     s->chr = chr;
     chr->opaque = s;
     chr->chr_add_watch = fd_chr_add_watch;
@@ -1062,7 +1062,7 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
     }
     old_fd0_flags = fcntl(0, F_GETFL);
     tcgetattr (0, &oldtty);
-    fcntl(0, F_SETFL, O_NONBLOCK);
+    qemu_set_nonblock(0);
     atexit(term_exit);
 
     chr = qemu_chr_open_fd(0, 1);
-- 
1.9.3

[Qemu-devel] [PULL 31/55] channel-posix: using qemu_set_nonblock() instead of fcntl(O_NONBLOCK)

[Stefan Hajnoczi]

From: Gonglei <arei.gonglei@huawei.com>

Technically, fcntl(soc, F_SETFL, O_NONBLOCK)
is incorrect since it clobbers all other file flags.
We can use F_GETFL to get the current flags, set or
clear the O_NONBLOCK flag, then use F_SETFL to set the flags.

Using the qemu_set_nonblock() wrapper.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Wangxin <wangxinxin.wang@huawei.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qga/channel-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index e65dda3..8aad4fe 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -42,7 +42,7 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
         g_warning("error converting fd to gsocket: %s", strerror(errno));
         goto out;
     }
-    fcntl(client_fd, F_SETFL, O_NONBLOCK);
+    qemu_set_nonblock(client_fd);
     ret = ga_channel_client_add(c, client_fd);
     if (ret) {
         g_warning("error setting up connection");
-- 
1.9.3

[Qemu-devel] [PULL 32/55] dataplane: print why starting failed

[Stefan Hajnoczi]

From: Cornelia Huck <cornelia.huck@de.ibm.com>

Setting up guest or host notifiers may fail, but the user will have
no idea why: Let's print the error returned by the callback.

Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index d6ba65c..527a53c 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -218,6 +218,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
     VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
     VirtQueue *vq;
+    int r;
 
     if (s->started) {
         return;
@@ -236,16 +237,18 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
     }
 
     /* Set up guest notifier (irq) */
-    if (k->set_guest_notifiers(qbus->parent, 1, true) != 0) {
-        fprintf(stderr, "virtio-blk failed to set guest notifier, "
-                "ensure -enable-kvm is set\n");
+    r = k->set_guest_notifiers(qbus->parent, 1, true);
+    if (r != 0) {
+        fprintf(stderr, "virtio-blk failed to set guest notifier (%d), "
+                "ensure -enable-kvm is set\n", r);
         exit(1);
     }
     s->guest_notifier = virtio_queue_get_guest_notifier(vq);
 
     /* Set up virtqueue notify */
-    if (k->set_host_notifier(qbus->parent, 0, true) != 0) {
-        fprintf(stderr, "virtio-blk failed to set host notifier\n");
+    r = k->set_host_notifier(qbus->parent, 0, true);
+    if (r != 0) {
+        fprintf(stderr, "virtio-blk failed to set host notifier (%d)\n", r);
         exit(1);
     }
     s->host_notifier = *virtio_queue_get_host_notifier(vq);
-- 
1.9.3

[Qemu-devel] [PULL 33/55] dataplane: fail notifier setting gracefully

[Stefan Hajnoczi]

From: Cornelia Huck <cornelia.huck@de.ibm.com>

The dataplane code is currently doing a hard exit if it fails to set
up either guest or host notifiers. In practice, this may mean that a
guest suddenly dies after a dataplane device failed to come up (e.g.,
when a file descriptor limit is hit for tne nth device).

Let's just try to unwind the setup instead and return.

Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 527a53c..94e1a29 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -232,8 +232,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
 
     vq = virtio_get_queue(s->vdev, 0);
     if (!vring_setup(&s->vring, s->vdev, 0)) {
-        s->starting = false;
-        return;
+        goto fail_vring;
     }
 
     /* Set up guest notifier (irq) */
@@ -241,7 +240,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
     if (r != 0) {
         fprintf(stderr, "virtio-blk failed to set guest notifier (%d), "
                 "ensure -enable-kvm is set\n", r);
-        exit(1);
+        goto fail_guest_notifiers;
     }
     s->guest_notifier = virtio_queue_get_guest_notifier(vq);
 
@@ -249,7 +248,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
     r = k->set_host_notifier(qbus->parent, 0, true);
     if (r != 0) {
         fprintf(stderr, "virtio-blk failed to set host notifier (%d)\n", r);
-        exit(1);
+        goto fail_host_notifier;
     }
     s->host_notifier = *virtio_queue_get_host_notifier(vq);
 
@@ -269,6 +268,14 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
     aio_context_acquire(s->ctx);
     aio_set_event_notifier(s->ctx, &s->host_notifier, handle_notify);
     aio_context_release(s->ctx);
+    return;
+
+  fail_host_notifier:
+    k->set_guest_notifiers(qbus->parent, 1, false);
+  fail_guest_notifiers:
+    vring_teardown(&s->vring, s->vdev, 0);
+  fail_vring:
+    s->starting = false;
 }
 
 /* Context: QEMU global mutex held */
-- 
1.9.3

[Qemu-devel] [PULL 34/55] dataplane: stop trying on notifier error

[Stefan Hajnoczi]

From: Cornelia Huck <cornelia.huck@de.ibm.com>

If we fail to set up guest or host notifiers, there's no use trying again
every time the guest kicks, so disable dataplane in that case.

Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 94e1a29..24a6b71 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -28,6 +28,7 @@ struct VirtIOBlockDataPlane {
     bool started;
     bool starting;
     bool stopping;
+    bool disabled;
 
     VirtIOBlkConf *blk;
 
@@ -220,7 +221,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
     VirtQueue *vq;
     int r;
 
-    if (s->started) {
+    if (s->started || s->disabled) {
         return;
     }
 
@@ -274,6 +275,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
     k->set_guest_notifiers(qbus->parent, 1, false);
   fail_guest_notifiers:
     vring_teardown(&s->vring, s->vdev, 0);
+    s->disabled = true;
   fail_vring:
     s->starting = false;
 }
@@ -284,6 +286,13 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
     BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s->vdev)));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
     VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
+
+
+    /* Better luck next time. */
+    if (s->disabled) {
+        s->disabled = false;
+        return;
+    }
     if (!s->started || s->stopping) {
         return;
     }
-- 
1.9.3

[Qemu-devel] [PULL 35/55] parallels: extend parallels format header with actual data values

[Stefan Hajnoczi]

From: "Denis V. Lunev" <den@openvz.org>

Parallels image format has several additional fields inside:
- nb_sectors is actually 64 bit wide. Upper 32bits are not used for
  images with signature "WithoutFreeSpace" and must be explicitly
  zeroed according to Parallels. They will be used for images with
  signature "WithouFreSpacExt"
- inuse is magic which means that the image is currently opened for
  read/write or was not closed correctly, the magic is 0x746f6e59
- data_off is the location of the first data block. It can be zero
  and in this case data starts just beyond the header aligned to
  512 bytes. Though this field does not matter for read-only driver

This patch adds these values to struct parallels_header and adds
proper handling of nb_sectors for currently supported WithoutFreeSpace
images.

WithouFreSpacExt will be covered in next patches.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Jeff Cody <jcody@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 7325678..59cfad6 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -41,8 +41,10 @@ struct parallels_header {
     uint32_t cylinders;
     uint32_t tracks;
     uint32_t catalog_entries;
-    uint32_t nb_sectors;
-    char padding[24];
+    uint64_t nb_sectors;
+    uint32_t inuse;
+    uint32_t data_off;
+    char padding[12];
 } QEMU_PACKED;
 
 typedef struct BDRVParallelsState {
@@ -90,7 +92,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
-    bs->total_sectors = le32_to_cpu(ph.nb_sectors);
+    bs->total_sectors = 0xffffffff & le64_to_cpu(ph.nb_sectors);
 
     s->tracks = le32_to_cpu(ph.tracks);
     if (s->tracks == 0) {
-- 
1.9.3

[Qemu-devel] [PULL 36/55] parallels: replace tabs with spaces in block/parallels.c

[Stefan Hajnoczi]

From: "Denis V. Lunev" <den@openvz.org>

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Jeff Cody <jcody@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 59cfad6..e0cf1d9 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -61,11 +61,11 @@ static int parallels_probe(const uint8_t *buf, int buf_size, const char *filenam
     const struct parallels_header *ph = (const void *)buf;
 
     if (buf_size < HEADER_SIZE)
-	return 0;
+        return 0;
 
     if (!memcmp(ph->magic, HEADER_MAGIC, 16) &&
-	(le32_to_cpu(ph->version) == HEADER_VERSION))
-	return 100;
+        (le32_to_cpu(ph->version) == HEADER_VERSION))
+        return 100;
 
     return 0;
 }
@@ -119,7 +119,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     for (i = 0; i < s->catalog_size; i++)
-	le32_to_cpus(&s->catalog_bitmap[i]);
+        le32_to_cpus(&s->catalog_bitmap[i]);
 
     qemu_co_mutex_init(&s->lock);
     return 0;
@@ -139,7 +139,7 @@ static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)
 
     /* not allocated */
     if ((index > s->catalog_size) || (s->catalog_bitmap[index] == 0))
-	return -1;
+        return -1;
     return (uint64_t)(s->catalog_bitmap[index] + offset) * 512;
 }
 
-- 
1.9.3

[Qemu-devel] [PULL 37/55] parallels: split check for parallels format in parallels_open

[Stefan Hajnoczi]

From: "Denis V. Lunev" <den@openvz.org>

and rework error path a bit. There is no difference at the moment, but
the code will be definitely shorter when additional processing will
be required for WithouFreSpacExt

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index e0cf1d9..f9e18b4 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -85,11 +85,11 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
-    if (memcmp(ph.magic, HEADER_MAGIC, 16) ||
-        (le32_to_cpu(ph.version) != HEADER_VERSION)) {
-        error_setg(errp, "Image not in Parallels format");
-        ret = -EINVAL;
-        goto fail;
+    if (le32_to_cpu(ph.version) != HEADER_VERSION) {
+        goto fail_format;
+    }
+    if (memcmp(ph.magic, HEADER_MAGIC, 16)) {
+        goto fail_format;
     }
 
     bs->total_sectors = 0xffffffff & le64_to_cpu(ph.nb_sectors);
@@ -124,6 +124,9 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
     qemu_co_mutex_init(&s->lock);
     return 0;
 
+fail_format:
+    error_setg(errp, "Image not in Parallels format");
+    ret = -EINVAL;
 fail:
     g_free(s->catalog_bitmap);
     return ret;
-- 
1.9.3

[Qemu-devel] [PULL 38/55] parallels: 2TB+ parallels images support

[Stefan Hajnoczi]

From: "Denis V. Lunev" <den@openvz.org>

Parallels has released in the recent updates of Parallels Server 5/6
new addition to his image format. Images with signature WithouFreSpacExt
have offsets in the catalog coded not as offsets in sectors (multiple
of 512 bytes) but offsets coded in blocks (i.e. header->tracks * 512)

In this case all 64 bits of header->nb_sectors are used for image size.

This patch implements support of this for qemu-img and also adds specific
check for an incorrect image. Images with block size greater than
INT_MAX/513 are not supported. The biggest available Parallels image
cluster size in the field is 1 Mb. Thus this limit will not hurt
anyone.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index f9e18b4..1774ab8 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -30,6 +30,7 @@
 /**************************************************************/
 
 #define HEADER_MAGIC "WithoutFreeSpace"
+#define HEADER_MAGIC2 "WithouFreSpacExt"
 #define HEADER_VERSION 2
 #define HEADER_SIZE 64
 
@@ -54,6 +55,8 @@ typedef struct BDRVParallelsState {
     unsigned int catalog_size;
 
     unsigned int tracks;
+
+    unsigned int off_multiplier;
 } BDRVParallelsState;
 
 static int parallels_probe(const uint8_t *buf, int buf_size, const char *filename)
@@ -63,7 +66,8 @@ static int parallels_probe(const uint8_t *buf, int buf_size, const char *filenam
     if (buf_size < HEADER_SIZE)
         return 0;
 
-    if (!memcmp(ph->magic, HEADER_MAGIC, 16) &&
+    if ((!memcmp(ph->magic, HEADER_MAGIC, 16) ||
+        !memcmp(ph->magic, HEADER_MAGIC2, 16)) &&
         (le32_to_cpu(ph->version) == HEADER_VERSION))
         return 100;
 
@@ -85,21 +89,31 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
+    bs->total_sectors = le64_to_cpu(ph.nb_sectors);
+
     if (le32_to_cpu(ph.version) != HEADER_VERSION) {
         goto fail_format;
     }
-    if (memcmp(ph.magic, HEADER_MAGIC, 16)) {
+    if (!memcmp(ph.magic, HEADER_MAGIC, 16)) {
+        s->off_multiplier = 1;
+        bs->total_sectors = 0xffffffff & bs->total_sectors;
+    } else if (!memcmp(ph.magic, HEADER_MAGIC2, 16)) {
+        s->off_multiplier = le32_to_cpu(ph.tracks);
+    } else {
         goto fail_format;
     }
 
-    bs->total_sectors = 0xffffffff & le64_to_cpu(ph.nb_sectors);
-
     s->tracks = le32_to_cpu(ph.tracks);
     if (s->tracks == 0) {
         error_setg(errp, "Invalid image: Zero sectors per track");
         ret = -EINVAL;
         goto fail;
     }
+    if (s->tracks > INT32_MAX/513) {
+        error_setg(errp, "Invalid image: Too big cluster");
+        ret = -EFBIG;
+        goto fail;
+    }
 
     s->catalog_size = le32_to_cpu(ph.catalog_entries);
     if (s->catalog_size > INT_MAX / 4) {
@@ -143,7 +157,8 @@ static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)
     /* not allocated */
     if ((index > s->catalog_size) || (s->catalog_bitmap[index] == 0))
         return -1;
-    return (uint64_t)(s->catalog_bitmap[index] + offset) * 512;
+    return
+        ((uint64_t)s->catalog_bitmap[index] * s->off_multiplier + offset) * 512;
 }
 
 static int parallels_read(BlockDriverState *bs, int64_t sector_num,
-- 
1.9.3

[Qemu-devel] [PULL 39/55] qemu-options: add missing -drive discard option to cmdline help

[Stefan Hajnoczi]

From: Peter Lieven <pl@kamp.de>

Signed-off-by: Peter Lieven <pl@kamp.de>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qemu-options.hx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 96516c1..44e3be3 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -427,7 +427,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "       [,serial=s][,addr=A][,rerror=ignore|stop|report]\n"
     "       [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
     "       [,readonly=on|off][,copy-on-read=on|off]\n"
-    "       [,detect-zeroes=on|off|unmap]\n"
+    "       [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"
     "       [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n"
     "       [[,iops=i]|[[,iops_rd=r][,iops_wr=w]]]\n"
     "       [[,bps_max=bm]|[[,bps_rd_max=rm][,bps_wr_max=wm]]]\n"
-- 
1.9.3

[Qemu-devel] [PULL 40/55] ide: Fix segfault when flushing a device that doesn't exist

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/core.c    |  4 +++-
 tests/ide-test.c | 14 ++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index bdb0a80..82dd4af 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -848,7 +848,9 @@ static void ide_flush_cb(void *opaque, int ret)
         }
     }
 
-    bdrv_acct_done(s->bs, &s->acct);
+    if (s->bs) {
+        bdrv_acct_done(s->bs, &s->acct);
+    }
     s->status = READY_STAT | SEEK_STAT;
     ide_cmd_done(s);
     ide_set_irq(s->bus);
diff --git a/tests/ide-test.c b/tests/ide-test.c
index a77a037..ffce6ed 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -564,6 +564,19 @@ static void test_retry_flush(void)
     ide_test_quit();
 }
 
+static void test_flush_nodev(void)
+{
+    ide_test_start("");
+
+    /* FLUSH CACHE command on device 0*/
+    outb(IDE_BASE + reg_device, 0);
+    outb(IDE_BASE + reg_command, CMD_FLUSH_CACHE);
+
+    /* Just testing that qemu doesn't crash... */
+
+    ide_test_quit();
+}
+
 int main(int argc, char **argv)
 {
     const char *arch = qtest_get_arch();
@@ -601,6 +614,7 @@ int main(int argc, char **argv)
     qtest_add_func("/ide/bmdma/teardown", test_bmdma_teardown);
 
     qtest_add_func("/ide/flush", test_flush);
+    qtest_add_func("/ide/flush_nodev", test_flush_nodev);
 
     qtest_add_func("/ide/retry/flush", test_retry_flush);
 
-- 
1.9.3

[Qemu-devel] [PULL 41/55] libqtest: add QTEST_LOG for debugging qtest testcases

[Stefan Hajnoczi]

From: Marc Marí <marc.mari.barcelo@gmail.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqtest.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 0bf17aa..ed55686 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -402,10 +402,14 @@ QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
 
     /* No need to send anything for an empty QObject.  */
     if (qobj) {
+        int log = getenv("QTEST_LOG") != NULL;
         QString *qstr = qobject_to_json(qobj);
         const char *str = qstring_get_str(qstr);
         size_t size = qstring_get_length(qstr);
 
+        if (log) {
+            fprintf(stderr, "%s", str);
+        }
         /* Send QMP request */
         socket_send(s->qmp_fd, str, size);
 
-- 
1.9.3

[Qemu-devel] [PULL 42/55] libqos: Correct mask to align size to PAGE_SIZE in malloc-pc

[Stefan Hajnoczi]

From: Marc Marí <marc.mari.barcelo@gmail.com>

Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqos/malloc-pc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index 63af60a..be1d97f 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -36,7 +36,7 @@ static uint64_t pc_alloc(QGuestAllocator *allocator, size_t size)
 
 
     size += (PAGE_SIZE - 1);
-    size &= PAGE_SIZE;
+    size &= -PAGE_SIZE;
 
     g_assert_cmpint((s->start + size), <=, s->end);
 
-- 
1.9.3

[Qemu-devel] [PULL 43/55] libqos: Change free function called in malloc

[Stefan Hajnoczi]

From: Marc Marí <marc.mari.barcelo@gmail.com>

Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqos/malloc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
index 46f6000..5565381 100644
--- a/tests/libqos/malloc.h
+++ b/tests/libqos/malloc.h
@@ -32,7 +32,7 @@ static inline uint64_t guest_alloc(QGuestAllocator *allocator, size_t size)
 
 static inline void guest_free(QGuestAllocator *allocator, uint64_t addr)
 {
-    allocator->alloc(allocator, addr);
+    allocator->free(allocator, addr);
 }
 
 #endif
-- 
1.9.3

[Qemu-devel] [PULL 44/55] virtio-blk: Correct bug in support for flexible descriptor layout

[Stefan Hajnoczi]

From: Marc Marí <marc.mari.barcelo@gmail.com>

Without this correction, only a three descriptor layout is accepted, and
requests with just two descriptors are not completed and no error message is
displayed.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/virtio-blk.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index c241c50..302c39e 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -404,19 +404,19 @@ void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
          * NB: per existing s/n string convention the string is
          * terminated by '\0' only when shorter than buffer.
          */
-        strncpy(req->elem.in_sg[0].iov_base,
-                s->blk.serial ? s->blk.serial : "",
-                MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
+        const char *serial = s->blk.serial ? s->blk.serial : "";
+        size_t size = MIN(strlen(serial) + 1,
+                          MIN(iov_size(in_iov, in_num),
+                              VIRTIO_BLK_ID_BYTES));
+        iov_from_buf(in_iov, in_num, 0, serial, size);
         virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
         virtio_blk_free_request(req);
     } else if (type & VIRTIO_BLK_T_OUT) {
-        qemu_iovec_init_external(&req->qiov, &req->elem.out_sg[1],
-                                 req->elem.out_num - 1);
+        qemu_iovec_init_external(&req->qiov, iov, out_num);
         virtio_blk_handle_write(req, mrb);
     } else if (type == VIRTIO_BLK_T_IN || type == VIRTIO_BLK_T_BARRIER) {
         /* VIRTIO_BLK_T_IN is 0, so we can't just & it. */
-        qemu_iovec_init_external(&req->qiov, &req->elem.in_sg[0],
-                                 req->elem.in_num - 1);
+        qemu_iovec_init_external(&req->qiov, in_iov, in_num);
         virtio_blk_handle_read(req);
     } else {
         virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
-- 
1.9.3

[Qemu-devel] [PULL 45/55] ide: only constrain read/write requests to drive size, not other types

[Stefan Hajnoczi]

From: Michael Tokarev <mjt@tls.msk.ru>

Commit 58ac321135a introduced a check to ide dma processing which
constrains all requests to drive size.  However, apparently, some
valid requests (like TRIM) does not fit in this constraint, and
fails in 2.1.  So check the range only for reads and writes.

Cc: qemu-stable@nongnu.org
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/ide/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 82dd4af..b48127f 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -692,7 +692,8 @@ void ide_dma_cb(void *opaque, int ret)
            sector_num, n, s->dma_cmd);
 #endif
 
-    if (!ide_sect_range_ok(s, sector_num, n)) {
+    if ((s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) &&
+        !ide_sect_range_ok(s, sector_num, n)) {
         dma_buf_commit(s);
         ide_dma_error(s);
         return;
-- 
1.9.3

[Qemu-devel] [PULL 46/55] docs: Specification for the image fuzzer

[Stefan Hajnoczi]

From: Maria Kustova <maxa@catit.be>

'Overall fuzzer requirements' chapter contains the current product vision and
features done and to be done. This chapter is still in progress.

Signed-off-by: Maria Kustova <maria.k@catit.be>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 docs/image-fuzzer.txt | 239 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 239 insertions(+)
 create mode 100644 docs/image-fuzzer.txt

diff --git a/docs/image-fuzzer.txt b/docs/image-fuzzer.txt
new file mode 100644
index 0000000..e73b182
--- /dev/null
+++ b/docs/image-fuzzer.txt
@@ -0,0 +1,239 @@
+# Specification for the fuzz testing tool
+#
+# Copyright (C) 2014 Maria Kustova <maria.k@catit.be>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+Image fuzzer
+============
+
+Description
+-----------
+
+The goal of the image fuzzer is to catch crashes of qemu-io/qemu-img
+by providing to them randomly corrupted images.
+Test images are generated from scratch and have valid inner structure with some
+elements, e.g. L1/L2 tables, having random invalid values.
+
+
+Test runner
+-----------
+
+The test runner generates test images, executes tests utilizing generated
+images, indicates their results and collects all test related artifacts (logs,
+core dumps, test images, backing files).
+The test means execution of all available commands under test with the same
+generated test image.
+By default, the test runner generates new tests and executes them until
+keyboard interruption. But if a test seed is specified via the '--seed' runner
+parameter, then only one test with this seed will be executed, after its finish
+the runner will exit.
+
+The runner uses an external image fuzzer to generate test images. An image
+generator should be specified as a mandatory parameter of the test runner.
+Details about interactions between the runner and fuzzers see "Module
+interfaces".
+
+The runner activates generation of core dumps during test executions, but it
+assumes that core dumps will be generated in the current working directory.
+For comprehensive test results, please, set up your test environment
+properly.
+
+Paths to binaries under test (SUTs) qemu-img and qemu-io are retrieved from
+environment variables. If the environment check fails the runner will
+use SUTs installed in system paths.
+qemu-img is required for creation of backing files, so it's mandatory to set
+the related environment variable if it's not installed in the system path.
+For details about environment variables see qemu-iotests/check.
+
+The runner accepts a JSON array of fields expected to be fuzzed via the
+'--config' argument, e.g.
+
+       '[["feature_name_table"], ["header", "l1_table_offset"]]'
+
+Each sublist can have one or two strings defining image structure elements.
+In the latter case a parent element should be placed on the first position,
+and a field name on the second one.
+
+The runner accepts a list of commands under test as a JSON array via
+the '--command' argument. Each command is a list containing a SUT and all its
+arguments, e.g.
+
+       runner.py -c '[["qemu-io", "$test_img", "-c", "write $off $len"]]'
+     /tmp/test ../qcow2
+
+For variable arguments next aliases can be used:
+    - $test_img for a fuzzed img
+    - $off for an offset in the fuzzed image
+    - $len for a data size
+
+Values for last two aliases will be generated based on a size of a virtual
+disk of the generated image.
+In case when no commands are specified the runner will execute commands from
+the default list:
+    - qemu-img check
+    - qemu-img info
+    - qemu-img convert
+    - qemu-io -c read
+    - qemu-io -c write
+    - qemu-io -c aio_read
+    - qemu-io -c aio_write
+    - qemu-io -c flush
+    - qemu-io -c discard
+    - qemu-io -c truncate
+
+
+Qcow2 image generator
+---------------------
+
+The 'qcow2' generator is a Python package providing 'create_image' method as
+a single public API. See details in 'Test runner/image fuzzer' chapter of
+'Module interfaces'.
+
+Qcow2 contains two submodules: fuzz.py and layout.py.
+
+'fuzz.py' contains all fuzzing functions, one per image field. It's assumed
+that after code analysis every field will have own constraints for its value.
+For now only universal potentially dangerous values are used, e.g. type limits
+for integers or unsafe symbols as '%s' for strings. For bitmasks random amount
+of bits are set to ones. All fuzzed values are checked on non-equality to the
+current valid value of the field. In case of equality the value will be
+regenerated.
+
+'layout.py' creates a random valid image, fuzzes a random subset of the image
+fields by 'fuzz.py' module and writes a fuzzed image to the file specified.
+If a fuzzer configuration is specified, then it has the next interpretation:
+
+    1. If a list contains a parent image element only, then some random portion
+    of fields of this element will be fuzzed every test.
+    The same behavior is applied for the entire image if no configuration is
+    used. This case is useful for the test specialization.
+
+    2. If a list contains a parent element and a field name, then a field
+    will be always fuzzed for every test. This case is useful for regression
+    testing.
+
+For now only header fields and header extensions are generated.
+
+
+Module interfaces
+-----------------
+
+* Test runner/image fuzzer
+
+The runner calls an image generator specifying the path to a test image file,
+path to a backing file and its format and a fuzzer configuration.
+An image generator is expected to provide a
+
+   'create_image(test_img_path, backing_file_path=None,
+                 backing_file_format=None, fuzz_config=None)'
+
+method that creates a test image, writes it to the specified file and returns
+the size of the virtual disk.
+The file should be created if it doesn't exist or overwritten otherwise.
+fuzz_config has a form of a list of lists. Every sublist can have one
+or two elements: first element is a name of a parent image element, second one
+if exists is a name of a field in this element.
+Example,
+        [['header', 'l1_table_offset'],
+         ['header', 'nb_snapshots'],
+         ['feature_name_table']]
+
+Random seed is set by the runner at every test execution for the regression
+purpose, so an image generator is not recommended to modify it internally.
+
+
+Overall fuzzer requirements
+===========================
+
+Input data:
+----------
+
+ - image template (generator)
+ - work directory
+ - action vector (optional)
+ - seed (optional)
+ - SUT and its arguments (optional)
+
+
+Fuzzer requirements:
+-------------------
+
+1.  Should be able to inject random data
+2.  Should be able to select a random value from the manually pregenerated
+    vector (boundary values, e.g. max/min cluster size)
+3.  Image template should describe a general structure invariant for all
+    test images (image format description)
+4.  Image template should be autonomous and other fuzzer parts should not
+    rely on it
+5.  Image template should contain reference rules (not only block+size
+    description)
+6.  Should generate the test image with the correct structure based on an image
+    template
+7.  Should accept a seed as an argument (for regression purpose)
+8.  Should generate a seed if it is not specified as an input parameter.
+9.  The same seed should generate the same image for the same action vector,
+    specified or generated.
+10. Should accept a vector of actions as an argument (for test reproducing and
+    for test case specification, e.g. group of tests for header structure,
+    group of test for snapshots, etc)
+11. Action vector should be randomly generated from the pool of available
+    actions, if it is not specified as an input parameter
+12. Pool of actions should be defined automatically based on an image template
+13. Should accept a SUT and its call parameters as an argument or select them
+    randomly otherwise. As far as it's expected to be rarely changed, the list
+    of all possible test commands can be available in the test runner
+    internally.
+14. Should support an external cancellation of a test run
+15. Seed should be logged (for regression purpose)
+16. All files related to a test result should be collected: a test image,
+    SUT logs, fuzzer logs and crash dumps
+17. Should be compatible with python version 2.4-2.7
+18. Usage of external libraries should be limited as much as possible.
+
+
+Image formats:
+-------------
+
+Main target image format is qcow2, but support of image templates should
+provide an ability to add any other image format.
+
+
+Effectiveness:
+-------------
+
+The fuzzer can be controlled via template, seed and action vector;
+it makes the fuzzer itself invariant to an image format and test logic.
+It should be able to perform rather complex and precise tests, that can be
+specified via an action vector. Otherwise, knowledge about an image structure
+allows the fuzzer to generate the pool of all available areas can be fuzzed
+and randomly select some of them and so compose its own action vector.
+Also complexity of a template defines complexity of the fuzzer, so its
+functionality can be varied from simple model-independent fuzzing to smart
+model-based one.
+
+
+Glossary:
+--------
+
+Action vector is a sequence of structure elements retrieved from an image
+format, each of them will be fuzzed for the test image. It's a subset of
+elements of the action pool. Example: header, refcount table, etc.
+Action pool is all available elements of an image structure that generated
+automatically from an image template.
+Image template is a formal description of an image structure and relations
+between image blocks.
+Test image is an output image of the fuzzer defined by the current seed and
+action vector.
-- 
1.9.3

[Qemu-devel] [PULL 47/55] image-fuzzer: Tool for fuzz tests execution

[Stefan Hajnoczi]

From: Maria Kustova <maxa@catit.be>

The purpose of the test runner is to prepare the test environment (e.g. create
a work directory, a test image, etc), execute a program under test with
parameters, indicate a test failure if the program was killed during the test
execution and collect core dumps, logs and other test artifacts.

The test runner doesn't depend on an image format, so it can be used with any
external image generator.

[Fixed path to qcow2 format module "qcow2" instead of "../qcow2" since
runner.py is no longer in a sub-directory.
--Stefan]

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Maria Kustova <maria.k@catit.be>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/runner.py | 405 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 405 insertions(+)
 create mode 100755 tests/image-fuzzer/runner.py

diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
new file mode 100755
index 0000000..58079d3
--- /dev/null
+++ b/tests/image-fuzzer/runner.py
@@ -0,0 +1,405 @@
+#!/usr/bin/env python
+
+# Tool for running fuzz tests
+#
+# Copyright (C) 2014 Maria Kustova <maria.k@catit.be>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import sys
+import os
+import signal
+import subprocess
+import random
+import shutil
+from itertools import count
+import getopt
+import StringIO
+import resource
+
+try:
+    import json
+except ImportError:
+    try:
+        import simplejson as json
+    except ImportError:
+        print >>sys.stderr, \
+            "Warning: Module for JSON processing is not found.\n" \
+            "'--config' and '--command' options are not supported."
+
+# Backing file sizes in MB
+MAX_BACKING_FILE_SIZE = 10
+MIN_BACKING_FILE_SIZE = 1
+
+
+def multilog(msg, *output):
+    """ Write an object to all of specified file descriptors."""
+    for fd in output:
+        fd.write(msg)
+        fd.flush()
+
+
+def str_signal(sig):
+    """ Convert a numeric value of a system signal to the string one
+    defined by the current operational system.
+    """
+    for k, v in signal.__dict__.items():
+        if v == sig:
+            return k
+
+
+def run_app(fd, q_args):
+    """Start an application with specified arguments and return its exit code
+    or kill signal depending on the result of execution.
+    """
+    devnull = open('/dev/null', 'r+')
+    process = subprocess.Popen(q_args, stdin=devnull,
+                               stdout=subprocess.PIPE,
+                               stderr=subprocess.PIPE)
+    out, err = process.communicate()
+    fd.write(out)
+    fd.write(err)
+    return process.returncode
+
+
+class TestException(Exception):
+    """Exception for errors risen by TestEnv objects."""
+    pass
+
+
+class TestEnv(object):
+
+    """Test object.
+
+    The class sets up test environment, generates backing and test images
+    and executes application under tests with specified arguments and a test
+    image provided.
+
+    All logs are collected.
+
+    The summary log will contain short descriptions and statuses of tests in
+    a run.
+
+    The test log will include application (e.g. 'qemu-img') logs besides info
+    sent to the summary log.
+    """
+
+    def __init__(self, test_id, seed, work_dir, run_log,
+                 cleanup=True, log_all=False):
+        """Set test environment in a specified work directory.
+
+        Path to qemu-img and qemu-io will be retrieved from 'QEMU_IMG' and
+        'QEMU_IO' environment variables.
+        """
+        if seed is not None:
+            self.seed = seed
+        else:
+            self.seed = str(random.randint(0, sys.maxint))
+        random.seed(self.seed)
+
+        self.init_path = os.getcwd()
+        self.work_dir = work_dir
+        self.current_dir = os.path.join(work_dir, 'test-' + test_id)
+        self.qemu_img = os.environ.get('QEMU_IMG', 'qemu-img')\
+                                  .strip().split(' ')
+        self.qemu_io = os.environ.get('QEMU_IO', 'qemu-io').strip().split(' ')
+        self.commands = [['qemu-img', 'check', '-f', 'qcow2', '$test_img'],
+                         ['qemu-img', 'info', '-f', 'qcow2', '$test_img'],
+                         ['qemu-io', '$test_img', '-c', 'read $off $len'],
+                         ['qemu-io', '$test_img', '-c', 'write $off $len'],
+                         ['qemu-io', '$test_img', '-c',
+                          'aio_read $off $len'],
+                         ['qemu-io', '$test_img', '-c',
+                          'aio_write $off $len'],
+                         ['qemu-io', '$test_img', '-c', 'flush'],
+                         ['qemu-io', '$test_img', '-c',
+                          'discard $off $len'],
+                         ['qemu-io', '$test_img', '-c',
+                          'truncate $off']]
+        for fmt in ['raw', 'vmdk', 'vdi', 'cow', 'qcow2', 'file',
+                    'qed', 'vpc']:
+            self.commands.append(
+                ['qemu-img', 'convert', '-f', 'qcow2', '-O', fmt,
+                 '$test_img', 'converted_image.' + fmt])
+
+        try:
+            os.makedirs(self.current_dir)
+        except OSError, e:
+            print >>sys.stderr, \
+                "Error: The working directory '%s' cannot be used. Reason: %s"\
+                % (self.work_dir, e[1])
+            raise TestException
+        self.log = open(os.path.join(self.current_dir, "test.log"), "w")
+        self.parent_log = open(run_log, "a")
+        self.failed = False
+        self.cleanup = cleanup
+        self.log_all = log_all
+
+    def _create_backing_file(self):
+        """Create a backing file in the current directory.
+
+        Return a tuple of a backing file name and format.
+
+        Format of a backing file is randomly chosen from all formats supported
+        by 'qemu-img create'.
+        """
+        # All formats supported by the 'qemu-img create' command.
+        backing_file_fmt = random.choice(['raw', 'vmdk', 'vdi', 'cow', 'qcow2',
+                                          'file', 'qed', 'vpc'])
+        backing_file_name = 'backing_img.' + backing_file_fmt
+        backing_file_size = random.randint(MIN_BACKING_FILE_SIZE,
+                                           MAX_BACKING_FILE_SIZE) * (1 << 20)
+        cmd = self.qemu_img + ['create', '-f', backing_file_fmt,
+                               backing_file_name, str(backing_file_size)]
+        temp_log = StringIO.StringIO()
+        retcode = run_app(temp_log, cmd)
+        if retcode == 0:
+            temp_log.close()
+            return (backing_file_name, backing_file_fmt)
+        else:
+            multilog("Warning: The %s backing file was not created.\n\n"
+                     % backing_file_fmt, sys.stderr, self.log, self.parent_log)
+            self.log.write("Log for the failure:\n" + temp_log.getvalue() +
+                           '\n\n')
+            temp_log.close()
+            return (None, None)
+
+    def execute(self, input_commands=None, fuzz_config=None):
+        """ Execute a test.
+
+        The method creates backing and test images, runs test app and analyzes
+        its exit status. If the application was killed by a signal, the test
+        is marked as failed.
+        """
+        if input_commands is None:
+            commands = self.commands
+        else:
+            commands = input_commands
+
+        os.chdir(self.current_dir)
+        backing_file_name, backing_file_fmt = self._create_backing_file()
+        img_size = image_generator.create_image('test.img',
+                                                backing_file_name,
+                                                backing_file_fmt,
+                                                fuzz_config)
+        for item in commands:
+            shutil.copy('test.img', 'copy.img')
+            # 'off' and 'len' are multiple of the sector size
+            sector_size = 512
+            start = random.randrange(0, img_size + 1, sector_size)
+            end = random.randrange(start, img_size + 1, sector_size)
+
+            if item[0] == 'qemu-img':
+                current_cmd = list(self.qemu_img)
+            elif item[0] == 'qemu-io':
+                current_cmd = list(self.qemu_io)
+            else:
+                multilog("Warning: test command '%s' is not defined.\n" \
+                         % item[0], sys.stderr, self.log, self.parent_log)
+                continue
+            # Replace all placeholders with their real values
+            for v in item[1:]:
+                c = (v
+                     .replace('$test_img', 'copy.img')
+                     .replace('$off', str(start))
+                     .replace('$len', str(end - start)))
+                current_cmd.append(c)
+
+            # Log string with the test header
+            test_summary = "Seed: %s\nCommand: %s\nTest directory: %s\n" \
+                           "Backing file: %s\n" \
+                           % (self.seed, " ".join(current_cmd),
+                              self.current_dir, backing_file_name)
+
+            temp_log = StringIO.StringIO()
+            try:
+                retcode = run_app(temp_log, current_cmd)
+            except OSError, e:
+                multilog(test_summary + "Error: Start of '%s' failed. " \
+                         "Reason: %s\n\n" % (os.path.basename(
+                             current_cmd[0]), e[1]),
+                         sys.stderr, self.log, self.parent_log)
+                raise TestException
+
+            if retcode < 0:
+                self.log.write(temp_log.getvalue())
+                multilog(test_summary + "FAIL: Test terminated by signal " +
+                         "%s\n\n" % str_signal(-retcode), sys.stderr, self.log,
+                         self.parent_log)
+                self.failed = True
+            else:
+                if self.log_all:
+                    self.log.write(temp_log.getvalue())
+                    multilog(test_summary + "PASS: Application exited with" +
+                             " the code '%d'\n\n" % retcode, sys.stdout,
+                             self.log, self.parent_log)
+            temp_log.close()
+            os.remove('copy.img')
+
+    def finish(self):
+        """Restore the test environment after a test execution."""
+        self.log.close()
+        self.parent_log.close()
+        os.chdir(self.init_path)
+        if self.cleanup and not self.failed:
+            shutil.rmtree(self.current_dir)
+
+if __name__ == '__main__':
+
+    def usage():
+        print """
+        Usage: runner.py [OPTION...] TEST_DIR IMG_GENERATOR
+
+        Set up test environment in TEST_DIR and run a test in it. A module for
+        test image generation should be specified via IMG_GENERATOR.
+        Example:
+        runner.py -c '[["qemu-img", "info", "$test_img"]]' /tmp/test qcow2
+
+        Optional arguments:
+          -h, --help                    display this help and exit
+          -c, --command=JSON            run tests for all commands specified in
+                                        the JSON array
+          -s, --seed=STRING             seed for a test image generation,
+                                        by default will be generated randomly
+          --config=JSON                 take fuzzer configuration from the JSON
+                                        array
+          -k, --keep_passed             don't remove folders of passed tests
+          -v, --verbose                 log information about passed tests
+
+        JSON:
+
+        '--command' accepts a JSON array of commands. Each command presents
+        an application under test with all its paramaters as a list of strings,
+        e.g.
+          ["qemu-io", "$test_img", "-c", "write $off $len"]
+
+        Supported application aliases: 'qemu-img' and 'qemu-io'.
+        Supported argument aliases: $test_img for the fuzzed image, $off
+        for an offset, $len for length.
+
+        Values for $off and $len will be generated based on the virtual disk
+        size of the fuzzed image
+        Paths to 'qemu-img' and 'qemu-io' are retrevied from 'QEMU_IMG' and
+        'QEMU_IO' environment variables
+
+        '--config' accepts a JSON array of fields to be fuzzed, e.g.
+          '[["header"], ["header", "version"]]'
+        Each of the list elements can consist of a complex image element only
+        as ["header"] or ["feature_name_table"] or an exact field as
+        ["header", "version"]. In the first case random portion of the element
+        fields will be fuzzed, in the second one the specified field will be
+        fuzzed always.
+
+        If '--config' argument is specified, fields not listed in
+        the configuration array will not be fuzzed.
+        """
+
+    def run_test(test_id, seed, work_dir, run_log, cleanup, log_all,
+                 command, fuzz_config):
+        """Setup environment for one test and execute this test."""
+        try:
+            test = TestEnv(test_id, seed, work_dir, run_log, cleanup,
+                           log_all)
+        except TestException:
+            sys.exit(1)
+
+        # Python 2.4 doesn't support 'finally' and 'except' in the same 'try'
+        # block
+        try:
+            try:
+                test.execute(command, fuzz_config)
+            except TestException:
+                sys.exit(1)
+        finally:
+            test.finish()
+
+    try:
+        opts, args = getopt.gnu_getopt(sys.argv[1:], 'c:hs:kv',
+                                       ['command=', 'help', 'seed=', 'config=',
+                                        'keep_passed', 'verbose'])
+    except getopt.error, e:
+        print >>sys.stderr, \
+            "Error: %s\n\nTry 'runner.py --help' for more information" % e
+        sys.exit(1)
+
+    command = None
+    cleanup = True
+    log_all = False
+    seed = None
+    config = None
+    for opt, arg in opts:
+        if opt in ('-h', '--help'):
+            usage()
+            sys.exit()
+        elif opt in ('-c', '--command'):
+            try:
+                command = json.loads(arg)
+            except (TypeError, ValueError, NameError), e:
+                print >>sys.stderr, \
+                    "Error: JSON array of test commands cannot be loaded.\n" \
+                    "Reason: %s" % e
+                sys.exit(1)
+        elif opt in ('-k', '--keep_passed'):
+            cleanup = False
+        elif opt in ('-v', '--verbose'):
+            log_all = True
+        elif opt in ('-s', '--seed'):
+            seed = arg
+        elif opt == '--config':
+            try:
+                config = json.loads(arg)
+            except (TypeError, ValueError, NameError), e:
+                print >>sys.stderr, \
+                    "Error: JSON array with the fuzzer configuration cannot" \
+                    " be loaded\nReason: %s" % e
+                sys.exit(1)
+
+    if not len(args) == 2:
+        print >>sys.stderr, \
+            "Expected two parameters\nTry 'runner.py --help'" \
+            " for more information."
+        sys.exit(1)
+
+    work_dir = os.path.realpath(args[0])
+    # run_log is created in 'main', because multiple tests are expected to
+    # log in it
+    run_log = os.path.join(work_dir, 'run.log')
+
+    # Add the path to the image generator module to sys.path
+    sys.path.append(os.path.realpath(os.path.dirname(args[1])))
+    # Remove a script extension from image generator module if any
+    generator_name = os.path.splitext(os.path.basename(args[1]))[0]
+
+    try:
+        image_generator = __import__(generator_name)
+    except ImportError, e:
+        print >>sys.stderr, \
+            "Error: The image generator '%s' cannot be imported.\n" \
+            "Reason: %s" % (generator_name, e)
+        sys.exit(1)
+
+    # Enable core dumps
+    resource.setrlimit(resource.RLIMIT_CORE, (-1, -1))
+    # If a seed is specified, only one test will be executed.
+    # Otherwise runner will terminate after a keyboard interruption
+    for test_id in count(1):
+        try:
+            run_test(str(test_id), seed, work_dir, run_log, cleanup,
+                     log_all, command, config)
+        except (KeyboardInterrupt, SystemExit):
+            sys.exit(1)
+
+        if seed is not None:
+            break
-- 
1.9.3

[Qemu-devel] [PULL 48/55] image-fuzzer: Fuzzing functions for qcow2 images

[Stefan Hajnoczi]

From: Maria Kustova <maxa@catit.be>

The fuzz submodule of the qcow2 image generator contains fuzzing functions for
image fields.
Each fuzzing function contains a list of constraints and a call of a helper
function that randomly selects a fuzzed value satisfied to one of constraints.
For now constraints include only known as invalid or potentially dangerous
values. But after investigation of code coverage by fuzz tests they will be
expanded by heuristic values based on inner checks and flows of a program
under test.

Now fuzzing of a header, header extensions and a backing file name is
supported.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Maria Kustova <maria.k@catit.be>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/qcow2/fuzz.py | 327 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 327 insertions(+)
 create mode 100644 tests/image-fuzzer/qcow2/fuzz.py

diff --git a/tests/image-fuzzer/qcow2/fuzz.py b/tests/image-fuzzer/qcow2/fuzz.py
new file mode 100644
index 0000000..a53c84f
--- /dev/null
+++ b/tests/image-fuzzer/qcow2/fuzz.py
@@ -0,0 +1,327 @@
+# Fuzzing functions for qcow2 fields
+#
+# Copyright (C) 2014 Maria Kustova <maria.k@catit.be>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import random
+
+
+UINT8 = 0xff
+UINT32 = 0xffffffff
+UINT64 = 0xffffffffffffffff
+# Most significant bit orders
+UINT32_M = 31
+UINT64_M = 63
+# Fuzz vectors
+UINT8_V = [0, 0x10, UINT8/4, UINT8/2 - 1, UINT8/2, UINT8/2 + 1, UINT8 - 1,
+           UINT8]
+UINT32_V = [0, 0x100, 0x1000, 0x10000, 0x100000, UINT32/4, UINT32/2 - 1,
+            UINT32/2, UINT32/2 + 1, UINT32 - 1, UINT32]
+UINT64_V = UINT32_V + [0x1000000, 0x10000000, 0x100000000, UINT64/4,
+                       UINT64/2 - 1, UINT64/2, UINT64/2 + 1, UINT64 - 1,
+                       UINT64]
+STRING_V = ['%s%p%x%d', '.1024d', '%.2049d', '%p%p%p%p', '%x%x%x%x',
+            '%d%d%d%d', '%s%s%s%s', '%99999999999s', '%08x', '%%20d', '%%20n',
+            '%%20x', '%%20s', '%s%s%s%s%s%s%s%s%s%s', '%p%p%p%p%p%p%p%p%p%p',
+            '%#0123456x%08x%x%s%p%d%n%o%u%c%h%l%q%j%z%Z%t%i%e%g%f%a%C%S%08x%%',
+            '%s x 129', '%x x 257']
+
+
+def random_from_intervals(intervals):
+    """Select a random integer number from the list of specified intervals.
+
+    Each interval is a tuple of lower and upper limits of the interval. The
+    limits are included. Intervals in a list should not overlap.
+    """
+    total = reduce(lambda x, y: x + y[1] - y[0] + 1, intervals, 0)
+    r = random.randint(0, total - 1) + intervals[0][0]
+    for x in zip(intervals, intervals[1:]):
+        r = r + (r > x[0][1]) * (x[1][0] - x[0][1] - 1)
+    return r
+
+
+def random_bits(bit_ranges):
+    """Generate random binary mask with ones in the specified bit ranges.
+
+    Each bit_ranges is a list of tuples of lower and upper limits of bit
+    positions will be fuzzed. The limits are included. Random amount of bits
+    in range limits will be set to ones. The mask is returned in decimal
+    integer format.
+    """
+    bit_numbers = []
+    # Select random amount of random positions in bit_ranges
+    for rng in bit_ranges:
+        bit_numbers += random.sample(range(rng[0], rng[1] + 1),
+                                     random.randint(0, rng[1] - rng[0] + 1))
+    val = 0
+    # Set bits on selected positions to ones
+    for bit in bit_numbers:
+        val |= 1 << bit
+    return val
+
+
+def truncate_string(strings, length):
+    """Return strings truncated to specified length."""
+    if type(strings) == list:
+        return [s[:length] for s in strings]
+    else:
+        return strings[:length]
+
+
+def validator(current, pick, choices):
+    """Return a value not equal to the current selected by the pick
+    function from choices.
+    """
+    while True:
+        val = pick(choices)
+        if not val == current:
+            return val
+
+
+def int_validator(current, intervals):
+    """Return a random value from intervals not equal to the current.
+
+    This function is useful for selection from valid values except current one.
+    """
+    return validator(current, random_from_intervals, intervals)
+
+
+def bit_validator(current, bit_ranges):
+    """Return a random bit mask not equal to the current.
+
+    This function is useful for selection from valid values except current one.
+    """
+    return validator(current, random_bits, bit_ranges)
+
+
+def string_validator(current, strings):
+    """Return a random string value from the list not equal to the current.
+
+    This function is useful for selection from valid values except current one.
+    """
+    return validator(current, random.choice, strings)
+
+
+def selector(current, constraints, validate=int_validator):
+    """Select one value from all defined by constraints.
+
+    Each constraint produces one random value satisfying to it. The function
+    randomly selects one value satisfying at least one constraint (depending on
+    constraints overlaps).
+    """
+    def iter_validate(c):
+        """Apply validate() only to constraints represented as lists.
+
+        This auxiliary function replaces short circuit conditions not supported
+        in Python 2.4
+        """
+        if type(c) == list:
+            return validate(current, c)
+        else:
+            return c
+
+    fuzz_values = [iter_validate(c) for c in constraints]
+    # Remove current for cases it's implicitly specified in constraints
+    # Duplicate validator functionality to prevent decreasing of probability
+    # to get one of allowable values
+    # TODO: remove validators after implementation of intelligent selection
+    # of fields will be fuzzed
+    try:
+        fuzz_values.remove(current)
+    except ValueError:
+        pass
+    return random.choice(fuzz_values)
+
+
+def magic(current):
+    """Fuzz magic header field.
+
+    The function just returns the current magic value and provides uniformity
+    of calls for all fuzzing functions.
+    """
+    return current
+
+
+def version(current):
+    """Fuzz version header field."""
+    constraints = UINT32_V + [
+        [(2, 3)],  # correct values
+        [(0, 1), (4, UINT32)]
+    ]
+    return selector(current, constraints)
+
+
+def backing_file_offset(current):
+    """Fuzz backing file offset header field."""
+    constraints = UINT64_V
+    return selector(current, constraints)
+
+
+def backing_file_size(current):
+    """Fuzz backing file size header field."""
+    constraints = UINT32_V
+    return selector(current, constraints)
+
+
+def cluster_bits(current):
+    """Fuzz cluster bits header field."""
+    constraints = UINT32_V + [
+        [(9, 20)],  # correct values
+        [(0, 9), (20, UINT32)]
+    ]
+    return selector(current, constraints)
+
+
+def size(current):
+    """Fuzz image size header field."""
+    constraints = UINT64_V
+    return selector(current, constraints)
+
+
+def crypt_method(current):
+    """Fuzz crypt method header field."""
+    constraints = UINT32_V + [
+        1,
+        [(2, UINT32)]
+    ]
+    return selector(current, constraints)
+
+
+def l1_size(current):
+    """Fuzz L1 table size header field."""
+    constraints = UINT32_V
+    return selector(current, constraints)
+
+
+def l1_table_offset(current):
+    """Fuzz L1 table offset header field."""
+    constraints = UINT64_V
+    return selector(current, constraints)
+
+
+def refcount_table_offset(current):
+    """Fuzz refcount table offset header field."""
+    constraints = UINT64_V
+    return selector(current, constraints)
+
+
+def refcount_table_clusters(current):
+    """Fuzz refcount table clusters header field."""
+    constraints = UINT32_V
+    return selector(current, constraints)
+
+
+def nb_snapshots(current):
+    """Fuzz number of snapshots header field."""
+    constraints = UINT32_V
+    return selector(current, constraints)
+
+
+def snapshots_offset(current):
+    """Fuzz snapshots offset header field."""
+    constraints = UINT64_V
+    return selector(current, constraints)
+
+
+def incompatible_features(current):
+    """Fuzz incompatible features header field."""
+    constraints = [
+        [(0, 1)],  # allowable values
+        [(0, UINT64_M)]
+    ]
+    return selector(current, constraints, bit_validator)
+
+
+def compatible_features(current):
+    """Fuzz compatible features header field."""
+    constraints = [
+        [(0, UINT64_M)]
+    ]
+    return selector(current, constraints, bit_validator)
+
+
+def autoclear_features(current):
+    """Fuzz autoclear features header field."""
+    constraints = [
+        [(0, UINT64_M)]
+    ]
+    return selector(current, constraints, bit_validator)
+
+
+def refcount_order(current):
+    """Fuzz number of refcount order header field."""
+    constraints = UINT32_V
+    return selector(current, constraints)
+
+
+def header_length(current):
+    """Fuzz number of refcount order header field."""
+    constraints = UINT32_V + [
+        72,
+        104,
+        [(0, UINT32)]
+    ]
+    return selector(current, constraints)
+
+
+def bf_name(current):
+    """Fuzz the backing file name."""
+    constraints = [
+        truncate_string(STRING_V, len(current))
+    ]
+    return selector(current, constraints, string_validator)
+
+
+def ext_magic(current):
+    """Fuzz magic field of a header extension."""
+    constraints = UINT32_V
+    return selector(current, constraints)
+
+
+def ext_length(current):
+    """Fuzz length field of a header extension."""
+    constraints = UINT32_V
+    return selector(current, constraints)
+
+
+def bf_format(current):
+    """Fuzz backing file format in the corresponding header extension."""
+    constraints = [
+        truncate_string(STRING_V, len(current)),
+        truncate_string(STRING_V, (len(current) + 7) & ~7)  # Fuzz padding
+    ]
+    return selector(current, constraints, string_validator)
+
+
+def feature_type(current):
+    """Fuzz feature type field of a feature name table header extension."""
+    constraints = UINT8_V
+    return selector(current, constraints)
+
+
+def feature_bit_number(current):
+    """Fuzz bit number field of a feature name table header extension."""
+    constraints = UINT8_V
+    return selector(current, constraints)
+
+
+def feature_name(current):
+    """Fuzz feature name field of a feature name table header extension."""
+    constraints = [
+        truncate_string(STRING_V, len(current)),
+        truncate_string(STRING_V, 46)  # Fuzz padding (field length = 46)
+    ]
+    return selector(current, constraints, string_validator)
-- 
1.9.3

[Qemu-devel] [PULL 49/55] image-fuzzer: Generator of fuzzed qcow2 images

[Stefan Hajnoczi]

From: Maria Kustova <maxa@catit.be>

The layout submodule of the qcow2 package creates a random valid image,
randomly selects some amount of its fields, fuzzes them and write the fuzzed
image to the file. Fuzzing process can be controlled by an external
configuration.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Maria Kustova <maria.k@catit.be>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/qcow2/layout.py | 369 +++++++++++++++++++++++++++++++++++++
 1 file changed, 369 insertions(+)
 create mode 100644 tests/image-fuzzer/qcow2/layout.py

diff --git a/tests/image-fuzzer/qcow2/layout.py b/tests/image-fuzzer/qcow2/layout.py
new file mode 100644
index 0000000..4c08202
--- /dev/null
+++ b/tests/image-fuzzer/qcow2/layout.py
@@ -0,0 +1,369 @@
+# Generator of fuzzed qcow2 images
+#
+# Copyright (C) 2014 Maria Kustova <maria.k@catit.be>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import random
+import struct
+import fuzz
+
+MAX_IMAGE_SIZE = 10 * (1 << 20)
+# Standard sizes
+UINT32_S = 4
+UINT64_S = 8
+
+
+class Field(object):
+
+    """Atomic image element (field).
+
+    The class represents an image field as quadruple of a data format
+    of value necessary for its packing to binary form, an offset from
+    the beginning of the image, a value and a name.
+
+    The field can be iterated as a list [format, offset, value].
+    """
+
+    __slots__ = ('fmt', 'offset', 'value', 'name')
+
+    def __init__(self, fmt, offset, val, name):
+        self.fmt = fmt
+        self.offset = offset
+        self.value = val
+        self.name = name
+
+    def __iter__(self):
+        return iter([self.fmt, self.offset, self.value])
+
+    def __repr__(self):
+        return "Field(fmt='%s', offset=%d, value=%s, name=%s)" % \
+            (self.fmt, self.offset, str(self.value), self.name)
+
+
+class FieldsList(object):
+
+    """List of fields.
+
+    The class allows access to a field in the list by its name and joins
+    several list in one via in-place addition.
+    """
+
+    def __init__(self, meta_data=None):
+        if meta_data is None:
+            self.data = []
+        else:
+            self.data = [Field(f[0], f[1], f[2], f[3])
+                         for f in meta_data]
+
+    def __getitem__(self, name):
+        return [x for x in self.data if x.name == name]
+
+    def __iter__(self):
+        return iter(self.data)
+
+    def __iadd__(self, other):
+        self.data += other.data
+        return self
+
+    def __len__(self):
+        return len(self.data)
+
+
+class Image(object):
+
+    """ Qcow2 image object.
+
+    This class allows to create qcow2 images with random valid structures and
+    values, fuzz them via external qcow2.fuzz module and write the result to
+    a file.
+    """
+
+    @staticmethod
+    def _size_params():
+        """Generate a random image size aligned to a random correct
+        cluster size.
+        """
+        cluster_bits = random.randrange(9, 21)
+        cluster_size = 1 << cluster_bits
+        img_size = random.randrange(0, MAX_IMAGE_SIZE + 1, cluster_size)
+        return (cluster_bits, img_size)
+
+    @staticmethod
+    def _header(cluster_bits, img_size, backing_file_name=None):
+        """Generate a random valid header."""
+        meta_header = [
+            ['>4s', 0, "QFI\xfb", 'magic'],
+            ['>I', 4, random.randint(2, 3), 'version'],
+            ['>Q', 8, 0, 'backing_file_offset'],
+            ['>I', 16, 0, 'backing_file_size'],
+            ['>I', 20, cluster_bits, 'cluster_bits'],
+            ['>Q', 24, img_size, 'size'],
+            ['>I', 32, 0, 'crypt_method'],
+            ['>I', 36, 0, 'l1_size'],
+            ['>Q', 40, 0, 'l1_table_offset'],
+            ['>Q', 48, 0, 'refcount_table_offset'],
+            ['>I', 56, 0, 'refcount_table_clusters'],
+            ['>I', 60, 0, 'nb_snapshots'],
+            ['>Q', 64, 0, 'snapshots_offset'],
+            ['>Q', 72, 0, 'incompatible_features'],
+            ['>Q', 80, 0, 'compatible_features'],
+            ['>Q', 88, 0, 'autoclear_features'],
+            # Only refcount_order = 4 is supported by current (07.2014)
+            # implementation of QEMU
+            ['>I', 96, 4, 'refcount_order'],
+            ['>I', 100, 0, 'header_length']
+        ]
+        v_header = FieldsList(meta_header)
+
+        if v_header['version'][0].value == 2:
+            v_header['header_length'][0].value = 72
+        else:
+            v_header['incompatible_features'][0].value = random.getrandbits(2)
+            v_header['compatible_features'][0].value = random.getrandbits(1)
+            v_header['header_length'][0].value = 104
+
+        max_header_len = struct.calcsize(v_header['header_length'][0].fmt) + \
+                         v_header['header_length'][0].offset
+        end_of_extension_area_len = 2 * UINT32_S
+        free_space = (1 << cluster_bits) - (max_header_len +
+                                            end_of_extension_area_len)
+        # If the backing file name specified and there is enough space for it
+        # in the first cluster, then it's placed in the very end of the first
+        # cluster.
+        if (backing_file_name is not None) and \
+           (free_space >= len(backing_file_name)):
+            v_header['backing_file_size'][0].value = len(backing_file_name)
+            v_header['backing_file_offset'][0].value = (1 << cluster_bits) - \
+                                                       len(backing_file_name)
+
+        return v_header
+
+    @staticmethod
+    def _backing_file_name(header, backing_file_name=None):
+        """Add the name of the backing file at the offset specified
+        in the header.
+        """
+        if (backing_file_name is not None) and \
+           (not header['backing_file_offset'][0].value == 0):
+            data_len = len(backing_file_name)
+            data_fmt = '>' + str(data_len) + 's'
+            data_field = FieldsList([
+                [data_fmt, header['backing_file_offset'][0].value,
+                 backing_file_name, 'bf_name']
+            ])
+        else:
+            data_field = FieldsList()
+
+        return data_field
+
+    @staticmethod
+    def _backing_file_format(header, backing_file_fmt=None):
+        """Generate the header extension for the backing file
+        format.
+        """
+        ext = FieldsList()
+        offset = struct.calcsize(header['header_length'][0].fmt) + \
+                 header['header_length'][0].offset
+
+        if backing_file_fmt is not None:
+            # Calculation of the free space available in the first cluster
+            end_of_extension_area_len = 2 * UINT32_S
+            high_border = (header['backing_file_offset'][0].value or
+                           ((1 << header['cluster_bits'][0].value) - 1)) - \
+                end_of_extension_area_len
+            free_space = high_border - offset
+            ext_size = 2 * UINT32_S + ((len(backing_file_fmt) + 7) & ~7)
+
+            if free_space >= ext_size:
+                ext_data_len = len(backing_file_fmt)
+                ext_data_fmt = '>' + str(ext_data_len) + 's'
+                ext_padding_len = 7 - (ext_data_len - 1) % 8
+                ext = FieldsList([
+                    ['>I', offset, 0xE2792ACA, 'ext_magic'],
+                    ['>I', offset + UINT32_S, ext_data_len, 'ext_length'],
+                    [ext_data_fmt, offset + UINT32_S * 2, backing_file_fmt,
+                     'bf_format']
+                ])
+                offset = ext['bf_format'][0].offset + \
+                         struct.calcsize(ext['bf_format'][0].fmt) + \
+                         ext_padding_len
+        return (ext, offset)
+
+    @staticmethod
+    def _feature_name_table(header, offset):
+        """Generate a random header extension for names of features used in
+        the image.
+        """
+        def gen_feat_ids():
+            """Return random feature type and feature bit."""
+            return (random.randint(0, 2), random.randint(0, 63))
+
+        end_of_extension_area_len = 2 * UINT32_S
+        high_border = (header['backing_file_offset'][0].value or
+                       (1 << header['cluster_bits'][0].value) - 1) - \
+            end_of_extension_area_len
+        free_space = high_border - offset
+        # Sum of sizes of 'magic' and 'length' header extension fields
+        ext_header_len = 2 * UINT32_S
+        fnt_entry_size = 6 * UINT64_S
+        num_fnt_entries = min(10, (free_space - ext_header_len) /
+                              fnt_entry_size)
+        if not num_fnt_entries == 0:
+            feature_tables = []
+            feature_ids = []
+            inner_offset = offset + ext_header_len
+            feat_name = 'some cool feature'
+            while len(feature_tables) < num_fnt_entries * 3:
+                feat_type, feat_bit = gen_feat_ids()
+                # Remove duplicates
+                while (feat_type, feat_bit) in feature_ids:
+                    feat_type, feat_bit = gen_feat_ids()
+                feature_ids.append((feat_type, feat_bit))
+                feat_fmt = '>' + str(len(feat_name)) + 's'
+                feature_tables += [['B', inner_offset,
+                                    feat_type, 'feature_type'],
+                                   ['B', inner_offset + 1, feat_bit,
+                                    'feature_bit_number'],
+                                   [feat_fmt, inner_offset + 2,
+                                    feat_name, 'feature_name']
+                ]
+                inner_offset += fnt_entry_size
+            # No padding for the extension is necessary, because
+            # the extension length is multiple of 8
+            ext = FieldsList([
+                ['>I', offset, 0x6803f857, 'ext_magic'],
+                # One feature table contains 3 fields and takes 48 bytes
+                ['>I', offset + UINT32_S, len(feature_tables) / 3 * 48,
+                 'ext_length']
+            ] + feature_tables)
+            offset = inner_offset
+        else:
+            ext = FieldsList()
+
+        return (ext, offset)
+
+    @staticmethod
+    def _end_of_extension_area(offset):
+        """Generate a mandatory header extension marking end of header
+        extensions.
+        """
+        ext = FieldsList([
+            ['>I', offset, 0, 'ext_magic'],
+            ['>I', offset + UINT32_S, 0, 'ext_length']
+        ])
+        return ext
+
+    def __init__(self, backing_file_name=None, backing_file_fmt=None):
+        """Create a random valid qcow2 image with the correct inner structure
+        and allowable values.
+        """
+        # Image size is saved as an attribute for the runner needs
+        cluster_bits, self.image_size = self._size_params()
+        # Saved as an attribute, because it's necessary for writing
+        self.cluster_size = 1 << cluster_bits
+        self.header = self._header(cluster_bits, self.image_size,
+                                   backing_file_name)
+        self.backing_file_name = self._backing_file_name(self.header,
+                                                         backing_file_name)
+        self.backing_file_format, \
+            offset = self._backing_file_format(self.header,
+                                               backing_file_fmt)
+        self.feature_name_table, \
+            offset = self._feature_name_table(self.header, offset)
+        self.end_of_extension_area = self._end_of_extension_area(offset)
+        # Container for entire image
+        self.data = FieldsList()
+        # Percentage of fields will be fuzzed
+        self.bias = random.uniform(0.2, 0.5)
+
+    def __iter__(self):
+        return iter([self.header,
+                     self.backing_file_format,
+                     self.feature_name_table,
+                     self.end_of_extension_area,
+                     self.backing_file_name])
+
+    def _join(self):
+        """Join all image structure elements as header, tables, etc in one
+        list of fields.
+        """
+        if len(self.data) == 0:
+            for v in self:
+                self.data += v
+
+    def fuzz(self, fields_to_fuzz=None):
+        """Fuzz an image by corrupting values of a random subset of its fields.
+
+        Without parameters the method fuzzes an entire image.
+        If 'fields_to_fuzz' is specified then only fields in this list will be
+        fuzzed. 'fields_to_fuzz' can contain both individual fields and more
+        general image elements as a header or tables.
+        In the first case the field will be fuzzed always.
+        In the second a random subset of fields will be selected and fuzzed.
+        """
+        def coin():
+            """Return boolean value proportional to a portion of fields to be
+            fuzzed.
+            """
+            return random.random() < self.bias
+
+        if fields_to_fuzz is None:
+            self._join()
+            for field in self.data:
+                if coin():
+                    field.value = getattr(fuzz, field.name)(field.value)
+        else:
+            for item in fields_to_fuzz:
+                if len(item) == 1:
+                    for field in getattr(self, item[0]):
+                        if coin():
+                            field.value = getattr(fuzz,
+                                                  field.name)(field.value)
+                else:
+                    for field in getattr(self, item[0])[item[1]]:
+                        try:
+                            field.value = getattr(fuzz, field.name)(
+                                field.value)
+                        except AttributeError:
+                            # Some fields can be skipped depending on
+                            # references, e.g. FNT header extension is not
+                            # generated for a feature mask header field
+                            # equal to zero
+                            pass
+
+    def write(self, filename):
+        """Write an entire image to the file."""
+        image_file = open(filename, 'w')
+        self._join()
+        for field in self.data:
+            image_file.seek(field.offset)
+            image_file.write(struct.pack(field.fmt, field.value))
+        image_file.seek(0, 2)
+        size = image_file.tell()
+        rounded = (size + self.cluster_size - 1) & ~(self.cluster_size - 1)
+        if rounded > size:
+            image_file.seek(rounded - 1)
+            image_file.write("\0")
+        image_file.close()
+
+
+def create_image(test_img_path, backing_file_name=None, backing_file_fmt=None,
+                 fields_to_fuzz=None):
+    """Create a fuzzed image and write it to the specified file."""
+    image = Image(backing_file_name, backing_file_fmt)
+    image.fuzz(fields_to_fuzz)
+    image.write(test_img_path)
+    return image.image_size
-- 
1.9.3

[Qemu-devel] [PULL 50/55] image-fuzzer: Public API for image-fuzzer/runner/runner.py

[Stefan Hajnoczi]

From: Maria Kustova <maxa@catit.be>

__init__.py provides the public API required by the test runner

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Maria Kustova <maria.k@catit.be>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/qcow2/__init__.py | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 tests/image-fuzzer/qcow2/__init__.py

diff --git a/tests/image-fuzzer/qcow2/__init__.py b/tests/image-fuzzer/qcow2/__init__.py
new file mode 100644
index 0000000..e2ebe19
--- /dev/null
+++ b/tests/image-fuzzer/qcow2/__init__.py
@@ -0,0 +1 @@
+from layout import create_image
-- 
1.9.3

[Qemu-devel] [PULL 51/55] docs: Expand the list of supported image elements with L1/L2 tables

[Stefan Hajnoczi]

From: Maria Kustova <maxa@catit.be>

Signed-off-by: Maria Kustova <maria.k@catit.be>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 docs/image-fuzzer.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/docs/image-fuzzer.txt b/docs/image-fuzzer.txt
index e73b182..0d0005d 100644
--- a/docs/image-fuzzer.txt
+++ b/docs/image-fuzzer.txt
@@ -125,8 +125,7 @@ If a fuzzer configuration is specified, then it has the next interpretation:
     will be always fuzzed for every test. This case is useful for regression
     testing.
 
-For now only header fields and header extensions are generated.
-
+For now only header fields, header extensions and L1/L2 tables are generated.
 
 Module interfaces
 -----------------
-- 
1.9.3

[Qemu-devel] [PULL 52/55] image-fuzzer: Add fuzzing functions for L1/L2 table entries

[Stefan Hajnoczi]

From: Maria Kustova <maxa@catit.be>

Signed-off-by: Maria Kustova <maria.k@catit.be>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/qcow2/fuzz.py | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/tests/image-fuzzer/qcow2/fuzz.py b/tests/image-fuzzer/qcow2/fuzz.py
index a53c84f..57527f9 100644
--- a/tests/image-fuzzer/qcow2/fuzz.py
+++ b/tests/image-fuzzer/qcow2/fuzz.py
@@ -325,3 +325,31 @@ def feature_name(current):
         truncate_string(STRING_V, 46)  # Fuzz padding (field length = 46)
     ]
     return selector(current, constraints, string_validator)
+
+
+def l1_entry(current):
+    """Fuzz an entry of the L1 table."""
+    constraints = UINT64_V
+    # Reserved bits are ignored
+    # Added a possibility when only flags are fuzzed
+    offset = 0x7fffffffffffffff & random.choice([selector(current,
+                                                          constraints),
+                                                 current])
+    is_cow = random.randint(0, 1)
+    return offset + (is_cow << UINT64_M)
+
+
+def l2_entry(current):
+    """Fuzz an entry of an L2 table."""
+    constraints = UINT64_V
+    # Reserved bits are ignored
+    # Add a possibility when only flags are fuzzed
+    offset = 0x3ffffffffffffffe & random.choice([selector(current,
+                                                          constraints),
+                                                 current])
+    is_compressed = random.randint(0, 1)
+    is_cow = random.randint(0, 1)
+    is_zero = random.randint(0, 1)
+    value = offset + (is_cow << UINT64_M) + \
+            (is_compressed << UINT64_M - 1) + is_zero
+    return value
-- 
1.9.3

[Qemu-devel] [PULL 53/55] image-fuzzer: Add generators of L1/L2 tables

[Stefan Hajnoczi]

From: Maria Kustova <maxa@catit.be>

Entries in L1/L2 entries are based on a portion of random guest clusters.
L2 entries contain offsets to host image clusters filled with random data.
Clusters for L1/L2 tables and guest data are selected randomly.

Signed-off-by: Maria Kustova <maria.k@catit.be>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/qcow2/layout.py | 255 +++++++++++++++++++++++++++----------
 1 file changed, 190 insertions(+), 65 deletions(-)

diff --git a/tests/image-fuzzer/qcow2/layout.py b/tests/image-fuzzer/qcow2/layout.py
index 4c08202..deed9ea 100644
--- a/tests/image-fuzzer/qcow2/layout.py
+++ b/tests/image-fuzzer/qcow2/layout.py
@@ -19,6 +19,8 @@
 import random
 import struct
 import fuzz
+from math import ceil
+from os import urandom
 
 MAX_IMAGE_SIZE = 10 * (1 << 20)
 # Standard sizes
@@ -102,7 +104,66 @@ class Image(object):
         return (cluster_bits, img_size)
 
     @staticmethod
-    def _header(cluster_bits, img_size, backing_file_name=None):
+    def _get_available_clusters(used, number):
+        """Return a set of indices of not allocated clusters.
+
+        'used' contains indices of currently allocated clusters.
+        All clusters that cannot be allocated between 'used' clusters will have
+        indices appended to the end of 'used'.
+        """
+        append_id = max(used) + 1
+        free = set(range(1, append_id)) - used
+        if len(free) >= number:
+            return set(random.sample(free, number))
+        else:
+            return free | set(range(append_id, append_id + number - len(free)))
+
+    @staticmethod
+    def _get_adjacent_clusters(used, size):
+        """Return an index of the first cluster in the sequence of free ones.
+
+        'used' contains indices of currently allocated clusters. 'size' is the
+        length of the sequence of free clusters.
+        If the sequence of 'size' is not available between 'used' clusters, its
+        first index will be append to the end of 'used'.
+        """
+        def get_cluster_id(lst, length):
+            """Return the first index of the sequence of the specified length
+            or None if the sequence cannot be inserted in the list.
+            """
+            if len(lst) != 0:
+                pairs = []
+                pair = (lst[0], 1)
+                for i in range(1, len(lst)):
+                    if lst[i] == lst[i-1] + 1:
+                        pair = (lst[i], pair[1] + 1)
+                    else:
+                        pairs.append(pair)
+                        pair = (lst[i], 1)
+                pairs.append(pair)
+                random.shuffle(pairs)
+                for x, s in pairs:
+                    if s >= length:
+                        return x - length + 1
+            return None
+
+        append_id = max(used) + 1
+        free = list(set(range(1, append_id)) - used)
+        idx = get_cluster_id(free, size)
+        if idx is None:
+            return append_id
+        else:
+            return idx
+
+    @staticmethod
+    def _alloc_data(img_size, cluster_size):
+        """Return a set of random indices of clusters allocated for guest data.
+        """
+        num_of_cls = img_size/cluster_size
+        return set(random.sample(range(1, num_of_cls + 1),
+                                 random.randint(0, num_of_cls)))
+
+    def create_header(self, cluster_bits, backing_file_name=None):
         """Generate a random valid header."""
         meta_header = [
             ['>4s', 0, "QFI\xfb", 'magic'],
@@ -110,7 +171,7 @@ class Image(object):
             ['>Q', 8, 0, 'backing_file_offset'],
             ['>I', 16, 0, 'backing_file_size'],
             ['>I', 20, cluster_bits, 'cluster_bits'],
-            ['>Q', 24, img_size, 'size'],
+            ['>Q', 24, self.image_size, 'size'],
             ['>I', 32, 0, 'crypt_method'],
             ['>I', 36, 0, 'l1_size'],
             ['>Q', 40, 0, 'l1_table_offset'],
@@ -126,63 +187,59 @@ class Image(object):
             ['>I', 96, 4, 'refcount_order'],
             ['>I', 100, 0, 'header_length']
         ]
-        v_header = FieldsList(meta_header)
+        self.header = FieldsList(meta_header)
 
-        if v_header['version'][0].value == 2:
-            v_header['header_length'][0].value = 72
+        if self.header['version'][0].value == 2:
+            self.header['header_length'][0].value = 72
         else:
-            v_header['incompatible_features'][0].value = random.getrandbits(2)
-            v_header['compatible_features'][0].value = random.getrandbits(1)
-            v_header['header_length'][0].value = 104
-
-        max_header_len = struct.calcsize(v_header['header_length'][0].fmt) + \
-                         v_header['header_length'][0].offset
+            self.header['incompatible_features'][0].value = \
+                                                        random.getrandbits(2)
+            self.header['compatible_features'][0].value = random.getrandbits(1)
+            self.header['header_length'][0].value = 104
+
+        max_header_len = struct.calcsize(
+            self.header['header_length'][0].fmt) + \
+            self.header['header_length'][0].offset
         end_of_extension_area_len = 2 * UINT32_S
-        free_space = (1 << cluster_bits) - (max_header_len +
-                                            end_of_extension_area_len)
+        free_space = self.cluster_size - max_header_len - \
+                     end_of_extension_area_len
         # If the backing file name specified and there is enough space for it
         # in the first cluster, then it's placed in the very end of the first
         # cluster.
         if (backing_file_name is not None) and \
            (free_space >= len(backing_file_name)):
-            v_header['backing_file_size'][0].value = len(backing_file_name)
-            v_header['backing_file_offset'][0].value = (1 << cluster_bits) - \
-                                                       len(backing_file_name)
-
-        return v_header
+            self.header['backing_file_size'][0].value = len(backing_file_name)
+            self.header['backing_file_offset'][0].value = \
+                                    self.cluster_size - len(backing_file_name)
 
-    @staticmethod
-    def _backing_file_name(header, backing_file_name=None):
+    def set_backing_file_name(self, backing_file_name=None):
         """Add the name of the backing file at the offset specified
         in the header.
         """
         if (backing_file_name is not None) and \
-           (not header['backing_file_offset'][0].value == 0):
+           (not self.header['backing_file_offset'][0].value == 0):
             data_len = len(backing_file_name)
             data_fmt = '>' + str(data_len) + 's'
-            data_field = FieldsList([
-                [data_fmt, header['backing_file_offset'][0].value,
+            self.backing_file_name = FieldsList([
+                [data_fmt, self.header['backing_file_offset'][0].value,
                  backing_file_name, 'bf_name']
             ])
         else:
-            data_field = FieldsList()
+            self.backing_file_name = FieldsList()
 
-        return data_field
-
-    @staticmethod
-    def _backing_file_format(header, backing_file_fmt=None):
+    def set_backing_file_format(self, backing_file_fmt=None):
         """Generate the header extension for the backing file
         format.
         """
-        ext = FieldsList()
-        offset = struct.calcsize(header['header_length'][0].fmt) + \
-                 header['header_length'][0].offset
+        self.backing_file_format = FieldsList()
+        offset = struct.calcsize(self.header['header_length'][0].fmt) + \
+                 self.header['header_length'][0].offset
 
         if backing_file_fmt is not None:
             # Calculation of the free space available in the first cluster
             end_of_extension_area_len = 2 * UINT32_S
-            high_border = (header['backing_file_offset'][0].value or
-                           ((1 << header['cluster_bits'][0].value) - 1)) - \
+            high_border = (self.header['backing_file_offset'][0].value or
+                           (self.cluster_size - 1)) - \
                 end_of_extension_area_len
             free_space = high_border - offset
             ext_size = 2 * UINT32_S + ((len(backing_file_fmt) + 7) & ~7)
@@ -191,19 +248,19 @@ class Image(object):
                 ext_data_len = len(backing_file_fmt)
                 ext_data_fmt = '>' + str(ext_data_len) + 's'
                 ext_padding_len = 7 - (ext_data_len - 1) % 8
-                ext = FieldsList([
+                self.backing_file_format = FieldsList([
                     ['>I', offset, 0xE2792ACA, 'ext_magic'],
                     ['>I', offset + UINT32_S, ext_data_len, 'ext_length'],
                     [ext_data_fmt, offset + UINT32_S * 2, backing_file_fmt,
                      'bf_format']
                 ])
-                offset = ext['bf_format'][0].offset + \
-                         struct.calcsize(ext['bf_format'][0].fmt) + \
-                         ext_padding_len
-        return (ext, offset)
+                offset = self.backing_file_format['bf_format'][0].offset + \
+                         struct.calcsize(self.backing_file_format[
+                             'bf_format'][0].fmt) + ext_padding_len
 
-    @staticmethod
-    def _feature_name_table(header, offset):
+        return offset
+
+    def create_feature_name_table(self, offset):
         """Generate a random header extension for names of features used in
         the image.
         """
@@ -212,8 +269,8 @@ class Image(object):
             return (random.randint(0, 2), random.randint(0, 63))
 
         end_of_extension_area_len = 2 * UINT32_S
-        high_border = (header['backing_file_offset'][0].value or
-                       (1 << header['cluster_bits'][0].value) - 1) - \
+        high_border = (self.header['backing_file_offset'][0].value or
+                       (self.cluster_size - 1)) - \
             end_of_extension_area_len
         free_space = high_border - offset
         # Sum of sizes of 'magic' and 'length' header extension fields
@@ -243,7 +300,7 @@ class Image(object):
                 inner_offset += fnt_entry_size
             # No padding for the extension is necessary, because
             # the extension length is multiple of 8
-            ext = FieldsList([
+            self.feature_name_table = FieldsList([
                 ['>I', offset, 0x6803f857, 'ext_magic'],
                 # One feature table contains 3 fields and takes 48 bytes
                 ['>I', offset + UINT32_S, len(feature_tables) / 3 * 48,
@@ -251,39 +308,101 @@ class Image(object):
             ] + feature_tables)
             offset = inner_offset
         else:
-            ext = FieldsList()
+            self.feature_name_table = FieldsList()
 
-        return (ext, offset)
+        return offset
 
-    @staticmethod
-    def _end_of_extension_area(offset):
+    def set_end_of_extension_area(self, offset):
         """Generate a mandatory header extension marking end of header
         extensions.
         """
-        ext = FieldsList([
+        self.end_of_extension_area = FieldsList([
             ['>I', offset, 0, 'ext_magic'],
             ['>I', offset + UINT32_S, 0, 'ext_length']
         ])
-        return ext
+
+    def create_l_structures(self):
+        """Generate random valid L1 and L2 tables."""
+        def create_l2_entry(host, guest, l2_cluster):
+            """Generate one L2 entry."""
+            offset = l2_cluster * self.cluster_size
+            l2_size = self.cluster_size / UINT64_S
+            entry_offset = offset + UINT64_S * (guest % l2_size)
+            cluster_descriptor = host * self.cluster_size
+            if not self.header['version'][0].value == 2:
+                cluster_descriptor += random.randint(0, 1)
+            # While snapshots are not supported, bit #63 = 1
+            # Compressed clusters are not supported => bit #62 = 0
+            entry_val = (1 << 63) + cluster_descriptor
+            return ['>Q', entry_offset, entry_val, 'l2_entry']
+
+        def create_l1_entry(l2_cluster, l1_offset, guest):
+            """Generate one L1 entry."""
+            l2_size = self.cluster_size / UINT64_S
+            entry_offset = l1_offset + UINT64_S * (guest / l2_size)
+            # While snapshots are not supported bit #63 = 1
+            entry_val = (1 << 63) + l2_cluster * self.cluster_size
+            return ['>Q', entry_offset, entry_val, 'l1_entry']
+
+        if len(self.data_clusters) == 0:
+            # All metadata for an empty guest image needs 4 clusters:
+            # header, rfc table, rfc block, L1 table.
+            # Header takes cluster #0, other clusters ##1-3 can be used
+            l1_offset = random.randint(1, 3) * self.cluster_size
+            l1 = [['>Q', l1_offset, 0, 'l1_entry']]
+            l2 = []
+        else:
+            meta_data = set([0])
+            guest_clusters = random.sample(range(self.image_size /
+                                                 self.cluster_size),
+                                           len(self.data_clusters))
+            # Number of entries in a L1/L2 table
+            l_size = self.cluster_size / UINT64_S
+            # Number of clusters necessary for L1 table
+            l1_size = int(ceil((max(guest_clusters) + 1) / float(l_size**2)))
+            l1_start = self._get_adjacent_clusters(self.data_clusters |
+                                                   meta_data, l1_size)
+            meta_data |= set(range(l1_start, l1_start + l1_size))
+            l1_offset = l1_start * self.cluster_size
+            # Indices of L2 tables
+            l2_ids = []
+            # Host clusters allocated for L2 tables
+            l2_clusters = []
+            # L1 entries
+            l1 = []
+            # L2 entries
+            l2 = []
+            for host, guest in zip(self.data_clusters, guest_clusters):
+                l2_id = guest / l_size
+                if l2_id not in l2_ids:
+                    l2_ids.append(l2_id)
+                    l2_clusters.append(self._get_adjacent_clusters(
+                        self.data_clusters | meta_data | set(l2_clusters),
+                        1))
+                    l1.append(create_l1_entry(l2_clusters[-1], l1_offset,
+                                              guest))
+                l2.append(create_l2_entry(host, guest,
+                                          l2_clusters[l2_ids.index(l2_id)]))
+        self.l2_tables = FieldsList(l2)
+        self.l1_table = FieldsList(l1)
+        self.header['l1_size'][0].value = int(ceil(UINT64_S * self.image_size /
+                                                float(self.cluster_size**2)))
+        self.header['l1_table_offset'][0].value = l1_offset
 
     def __init__(self, backing_file_name=None, backing_file_fmt=None):
         """Create a random valid qcow2 image with the correct inner structure
         and allowable values.
         """
-        # Image size is saved as an attribute for the runner needs
         cluster_bits, self.image_size = self._size_params()
-        # Saved as an attribute, because it's necessary for writing
         self.cluster_size = 1 << cluster_bits
-        self.header = self._header(cluster_bits, self.image_size,
-                                   backing_file_name)
-        self.backing_file_name = self._backing_file_name(self.header,
-                                                         backing_file_name)
-        self.backing_file_format, \
-            offset = self._backing_file_format(self.header,
-                                               backing_file_fmt)
-        self.feature_name_table, \
-            offset = self._feature_name_table(self.header, offset)
-        self.end_of_extension_area = self._end_of_extension_area(offset)
+        self.create_header(cluster_bits, backing_file_name)
+        self.set_backing_file_name(backing_file_name)
+        offset = self.set_backing_file_format(backing_file_fmt)
+        offset = self.create_feature_name_table(offset)
+        self.set_end_of_extension_area(offset)
+        self.data_clusters = self._alloc_data(self.image_size,
+                                              self.cluster_size)
+        self.create_l_structures()
         # Container for entire image
         self.data = FieldsList()
         # Percentage of fields will be fuzzed
@@ -294,7 +413,9 @@ class Image(object):
                      self.backing_file_format,
                      self.feature_name_table,
                      self.end_of_extension_area,
-                     self.backing_file_name])
+                     self.backing_file_name,
+                     self.l1_table,
+                     self.l2_tables])
 
     def _join(self):
         """Join all image structure elements as header, tables, etc in one
@@ -339,9 +460,7 @@ class Image(object):
                                 field.value)
                         except AttributeError:
                             # Some fields can be skipped depending on
-                            # references, e.g. FNT header extension is not
-                            # generated for a feature mask header field
-                            # equal to zero
+                            # their prerequisites
                             pass
 
     def write(self, filename):
@@ -351,6 +470,12 @@ class Image(object):
         for field in self.data:
             image_file.seek(field.offset)
             image_file.write(struct.pack(field.fmt, field.value))
+
+        for cluster in sorted(self.data_clusters):
+            image_file.seek(cluster * self.cluster_size)
+            image_file.write(urandom(self.cluster_size))
+
+        # Align the real image size to the cluster size
         image_file.seek(0, 2)
         size = image_file.tell()
         rounded = (size + self.cluster_size - 1) & ~(self.cluster_size - 1)
-- 
1.9.3

[Qemu-devel] [PULL 54/55] image-fuzzer: Reduce number of generator functions in __init__

[Stefan Hajnoczi]

From: Maria Kustova <maxa@catit.be>

Some issues can be found only when a fuzzed image has a partial structure,
e.g. has L1/L2 tables but no refcount ones. Generation of an entirely
defined image limits these cases. Now the Image constructor creates only
a header and a backing file name (if any), other image elements are generated
in the 'create_image' API.

Signed-off-by: Maria Kustova <maria.k@catit.be>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/qcow2/layout.py | 304 +++++++++++++++++--------------------
 1 file changed, 143 insertions(+), 161 deletions(-)

diff --git a/tests/image-fuzzer/qcow2/layout.py b/tests/image-fuzzer/qcow2/layout.py
index deed9ea..730c771 100644
--- a/tests/image-fuzzer/qcow2/layout.py
+++ b/tests/image-fuzzer/qcow2/layout.py
@@ -21,6 +21,7 @@ import struct
 import fuzz
 from math import ceil
 from os import urandom
+from itertools import chain
 
 MAX_IMAGE_SIZE = 10 * (1 << 20)
 # Standard sizes
@@ -36,7 +37,7 @@ class Field(object):
     of value necessary for its packing to binary form, an offset from
     the beginning of the image, a value and a name.
 
-    The field can be iterated as a list [format, offset, value].
+    The field can be iterated as a list [format, offset, value, name].
     """
 
     __slots__ = ('fmt', 'offset', 'value', 'name')
@@ -48,7 +49,7 @@ class Field(object):
         self.name = name
 
     def __iter__(self):
-        return iter([self.fmt, self.offset, self.value])
+        return iter([self.fmt, self.offset, self.value, self.name])
 
     def __repr__(self):
         return "Field(fmt='%s', offset=%d, value=%s, name=%s)" % \
@@ -59,15 +60,14 @@ class FieldsList(object):
 
     """List of fields.
 
-    The class allows access to a field in the list by its name and joins
-    several list in one via in-place addition.
+    The class allows access to a field in the list by its name.
     """
 
     def __init__(self, meta_data=None):
         if meta_data is None:
             self.data = []
         else:
-            self.data = [Field(f[0], f[1], f[2], f[3])
+            self.data = [Field(*f)
                          for f in meta_data]
 
     def __getitem__(self, name):
@@ -76,10 +76,6 @@ class FieldsList(object):
     def __iter__(self):
         return iter(self.data)
 
-    def __iadd__(self, other):
-        self.data += other.data
-        return self
-
     def __len__(self):
         return len(self.data)
 
@@ -93,75 +89,31 @@ class Image(object):
     a file.
     """
 
-    @staticmethod
-    def _size_params():
-        """Generate a random image size aligned to a random correct
-        cluster size.
-        """
-        cluster_bits = random.randrange(9, 21)
-        cluster_size = 1 << cluster_bits
-        img_size = random.randrange(0, MAX_IMAGE_SIZE + 1, cluster_size)
-        return (cluster_bits, img_size)
-
-    @staticmethod
-    def _get_available_clusters(used, number):
-        """Return a set of indices of not allocated clusters.
-
-        'used' contains indices of currently allocated clusters.
-        All clusters that cannot be allocated between 'used' clusters will have
-        indices appended to the end of 'used'.
+    def __init__(self, backing_file_name=None):
+        """Create a random valid qcow2 image with the correct header and stored
+        backing file name.
         """
-        append_id = max(used) + 1
-        free = set(range(1, append_id)) - used
-        if len(free) >= number:
-            return set(random.sample(free, number))
-        else:
-            return free | set(range(append_id, append_id + number - len(free)))
-
-    @staticmethod
-    def _get_adjacent_clusters(used, size):
-        """Return an index of the first cluster in the sequence of free ones.
-
-        'used' contains indices of currently allocated clusters. 'size' is the
-        length of the sequence of free clusters.
-        If the sequence of 'size' is not available between 'used' clusters, its
-        first index will be append to the end of 'used'.
-        """
-        def get_cluster_id(lst, length):
-            """Return the first index of the sequence of the specified length
-            or None if the sequence cannot be inserted in the list.
-            """
-            if len(lst) != 0:
-                pairs = []
-                pair = (lst[0], 1)
-                for i in range(1, len(lst)):
-                    if lst[i] == lst[i-1] + 1:
-                        pair = (lst[i], pair[1] + 1)
-                    else:
-                        pairs.append(pair)
-                        pair = (lst[i], 1)
-                pairs.append(pair)
-                random.shuffle(pairs)
-                for x, s in pairs:
-                    if s >= length:
-                        return x - length + 1
-            return None
-
-        append_id = max(used) + 1
-        free = list(set(range(1, append_id)) - used)
-        idx = get_cluster_id(free, size)
-        if idx is None:
-            return append_id
-        else:
-            return idx
+        cluster_bits, self.image_size = self._size_params()
+        self.cluster_size = 1 << cluster_bits
+        self.header = FieldsList()
+        self.backing_file_name = FieldsList()
+        self.backing_file_format = FieldsList()
+        self.feature_name_table = FieldsList()
+        self.end_of_extension_area = FieldsList()
+        self.l2_tables = FieldsList()
+        self.l1_table = FieldsList()
+        self.ext_offset = 0
+        self.create_header(cluster_bits, backing_file_name)
+        self.set_backing_file_name(backing_file_name)
+        self.data_clusters = self._alloc_data(self.image_size,
+                                              self.cluster_size)
+        # Percentage of fields will be fuzzed
+        self.bias = random.uniform(0.2, 0.5)
 
-    @staticmethod
-    def _alloc_data(img_size, cluster_size):
-        """Return a set of random indices of clusters allocated for guest data.
-        """
-        num_of_cls = img_size/cluster_size
-        return set(random.sample(range(1, num_of_cls + 1),
-                                 random.randint(0, num_of_cls)))
+    def __iter__(self):
+        return chain(self.header, self.backing_file_format,
+                     self.feature_name_table, self.end_of_extension_area,
+                     self.backing_file_name, self.l1_table, self.l2_tables)
 
     def create_header(self, cluster_bits, backing_file_name=None):
         """Generate a random valid header."""
@@ -196,12 +148,12 @@ class Image(object):
                                                         random.getrandbits(2)
             self.header['compatible_features'][0].value = random.getrandbits(1)
             self.header['header_length'][0].value = 104
-
-        max_header_len = struct.calcsize(
+        # Extensions start at the header last field offset and the field size
+        self.ext_offset = struct.calcsize(
             self.header['header_length'][0].fmt) + \
             self.header['header_length'][0].offset
         end_of_extension_area_len = 2 * UINT32_S
-        free_space = self.cluster_size - max_header_len - \
+        free_space = self.cluster_size - self.ext_offset - \
                      end_of_extension_area_len
         # If the backing file name specified and there is enough space for it
         # in the first cluster, then it's placed in the very end of the first
@@ -224,24 +176,16 @@ class Image(object):
                 [data_fmt, self.header['backing_file_offset'][0].value,
                  backing_file_name, 'bf_name']
             ])
-        else:
-            self.backing_file_name = FieldsList()
 
     def set_backing_file_format(self, backing_file_fmt=None):
-        """Generate the header extension for the backing file
-        format.
-        """
-        self.backing_file_format = FieldsList()
-        offset = struct.calcsize(self.header['header_length'][0].fmt) + \
-                 self.header['header_length'][0].offset
-
+        """Generate the header extension for the backing file format."""
         if backing_file_fmt is not None:
             # Calculation of the free space available in the first cluster
             end_of_extension_area_len = 2 * UINT32_S
             high_border = (self.header['backing_file_offset'][0].value or
                            (self.cluster_size - 1)) - \
                 end_of_extension_area_len
-            free_space = high_border - offset
+            free_space = high_border - self.ext_offset
             ext_size = 2 * UINT32_S + ((len(backing_file_fmt) + 7) & ~7)
 
             if free_space >= ext_size:
@@ -249,18 +193,19 @@ class Image(object):
                 ext_data_fmt = '>' + str(ext_data_len) + 's'
                 ext_padding_len = 7 - (ext_data_len - 1) % 8
                 self.backing_file_format = FieldsList([
-                    ['>I', offset, 0xE2792ACA, 'ext_magic'],
-                    ['>I', offset + UINT32_S, ext_data_len, 'ext_length'],
-                    [ext_data_fmt, offset + UINT32_S * 2, backing_file_fmt,
-                     'bf_format']
+                    ['>I', self.ext_offset, 0xE2792ACA, 'ext_magic'],
+                    ['>I', self.ext_offset + UINT32_S, ext_data_len,
+                     'ext_length'],
+                    [ext_data_fmt, self.ext_offset + UINT32_S * 2,
+                     backing_file_fmt, 'bf_format']
                 ])
-                offset = self.backing_file_format['bf_format'][0].offset + \
-                         struct.calcsize(self.backing_file_format[
-                             'bf_format'][0].fmt) + ext_padding_len
-
-        return offset
+                self.ext_offset = \
+                        struct.calcsize(
+                            self.backing_file_format['bf_format'][0].fmt) + \
+                        ext_padding_len + \
+                        self.backing_file_format['bf_format'][0].offset
 
-    def create_feature_name_table(self, offset):
+    def create_feature_name_table(self):
         """Generate a random header extension for names of features used in
         the image.
         """
@@ -272,7 +217,7 @@ class Image(object):
         high_border = (self.header['backing_file_offset'][0].value or
                        (self.cluster_size - 1)) - \
             end_of_extension_area_len
-        free_space = high_border - offset
+        free_space = high_border - self.ext_offset
         # Sum of sizes of 'magic' and 'length' header extension fields
         ext_header_len = 2 * UINT32_S
         fnt_entry_size = 6 * UINT64_S
@@ -281,7 +226,7 @@ class Image(object):
         if not num_fnt_entries == 0:
             feature_tables = []
             feature_ids = []
-            inner_offset = offset + ext_header_len
+            inner_offset = self.ext_offset + ext_header_len
             feat_name = 'some cool feature'
             while len(feature_tables) < num_fnt_entries * 3:
                 feat_type, feat_bit = gen_feat_ids()
@@ -301,24 +246,20 @@ class Image(object):
             # No padding for the extension is necessary, because
             # the extension length is multiple of 8
             self.feature_name_table = FieldsList([
-                ['>I', offset, 0x6803f857, 'ext_magic'],
+                ['>I', self.ext_offset, 0x6803f857, 'ext_magic'],
                 # One feature table contains 3 fields and takes 48 bytes
-                ['>I', offset + UINT32_S, len(feature_tables) / 3 * 48,
-                 'ext_length']
+                ['>I', self.ext_offset + UINT32_S,
+                 len(feature_tables) / 3 * 48, 'ext_length']
             ] + feature_tables)
-            offset = inner_offset
-        else:
-            self.feature_name_table = FieldsList()
-
-        return offset
+            self.ext_offset = inner_offset
 
-    def set_end_of_extension_area(self, offset):
+    def set_end_of_extension_area(self):
         """Generate a mandatory header extension marking end of header
         extensions.
         """
         self.end_of_extension_area = FieldsList([
-            ['>I', offset, 0, 'ext_magic'],
-            ['>I', offset + UINT32_S, 0, 'ext_length']
+            ['>I', self.ext_offset, 0, 'ext_magic'],
+            ['>I', self.ext_offset + UINT32_S, 0, 'ext_length']
         ])
 
     def create_l_structures(self):
@@ -352,7 +293,7 @@ class Image(object):
             l1 = [['>Q', l1_offset, 0, 'l1_entry']]
             l2 = []
         else:
-            meta_data = set([0])
+            meta_data = self._get_metadata()
             guest_clusters = random.sample(range(self.image_size /
                                                  self.cluster_size),
                                            len(self.data_clusters))
@@ -389,49 +330,15 @@ class Image(object):
                                                 float(self.cluster_size**2)))
         self.header['l1_table_offset'][0].value = l1_offset
 
-    def __init__(self, backing_file_name=None, backing_file_fmt=None):
-        """Create a random valid qcow2 image with the correct inner structure
-        and allowable values.
-        """
-        cluster_bits, self.image_size = self._size_params()
-        self.cluster_size = 1 << cluster_bits
-        self.create_header(cluster_bits, backing_file_name)
-        self.set_backing_file_name(backing_file_name)
-        offset = self.set_backing_file_format(backing_file_fmt)
-        offset = self.create_feature_name_table(offset)
-        self.set_end_of_extension_area(offset)
-        self.data_clusters = self._alloc_data(self.image_size,
-                                              self.cluster_size)
-        self.create_l_structures()
-        # Container for entire image
-        self.data = FieldsList()
-        # Percentage of fields will be fuzzed
-        self.bias = random.uniform(0.2, 0.5)
-
-    def __iter__(self):
-        return iter([self.header,
-                     self.backing_file_format,
-                     self.feature_name_table,
-                     self.end_of_extension_area,
-                     self.backing_file_name,
-                     self.l1_table,
-                     self.l2_tables])
-
-    def _join(self):
-        """Join all image structure elements as header, tables, etc in one
-        list of fields.
-        """
-        if len(self.data) == 0:
-            for v in self:
-                self.data += v
-
     def fuzz(self, fields_to_fuzz=None):
         """Fuzz an image by corrupting values of a random subset of its fields.
 
         Without parameters the method fuzzes an entire image.
+
         If 'fields_to_fuzz' is specified then only fields in this list will be
         fuzzed. 'fields_to_fuzz' can contain both individual fields and more
         general image elements as a header or tables.
+
         In the first case the field will be fuzzed always.
         In the second a random subset of fields will be selected and fuzzed.
         """
@@ -442,8 +349,7 @@ class Image(object):
             return random.random() < self.bias
 
         if fields_to_fuzz is None:
-            self._join()
-            for field in self.data:
+            for field in self:
                 if coin():
                     field.value = getattr(fuzz, field.name)(field.value)
         else:
@@ -454,20 +360,15 @@ class Image(object):
                             field.value = getattr(fuzz,
                                                   field.name)(field.value)
                 else:
+                    # If fields with the requested name were not generated
+                    # getattr(self, item[0])[item[1]] returns an empty list
                     for field in getattr(self, item[0])[item[1]]:
-                        try:
-                            field.value = getattr(fuzz, field.name)(
-                                field.value)
-                        except AttributeError:
-                            # Some fields can be skipped depending on
-                            # their prerequisites
-                            pass
+                        field.value = getattr(fuzz, field.name)(field.value)
 
     def write(self, filename):
         """Write an entire image to the file."""
         image_file = open(filename, 'w')
-        self._join()
-        for field in self.data:
+        for field in self:
             image_file.seek(field.offset)
             image_file.write(struct.pack(field.fmt, field.value))
 
@@ -484,11 +385,92 @@ class Image(object):
             image_file.write("\0")
         image_file.close()
 
+    @staticmethod
+    def _size_params():
+        """Generate a random image size aligned to a random correct
+        cluster size.
+        """
+        cluster_bits = random.randrange(9, 21)
+        cluster_size = 1 << cluster_bits
+        img_size = random.randrange(0, MAX_IMAGE_SIZE + 1, cluster_size)
+        return (cluster_bits, img_size)
+
+    @staticmethod
+    def _get_available_clusters(used, number):
+        """Return a set of indices of not allocated clusters.
+
+        'used' contains indices of currently allocated clusters.
+        All clusters that cannot be allocated between 'used' clusters will have
+        indices appended to the end of 'used'.
+        """
+        append_id = max(used) + 1
+        free = set(range(1, append_id)) - used
+        if len(free) >= number:
+            return set(random.sample(free, number))
+        else:
+            return free | set(range(append_id, append_id + number - len(free)))
+
+    @staticmethod
+    def _get_adjacent_clusters(used, size):
+        """Return an index of the first cluster in the sequence of free ones.
+
+        'used' contains indices of currently allocated clusters. 'size' is the
+        length of the sequence of free clusters.
+        If the sequence of 'size' is not available between 'used' clusters, its
+        first index will be append to the end of 'used'.
+        """
+        def get_cluster_id(lst, length):
+            """Return the first index of the sequence of the specified length
+            or None if the sequence cannot be inserted in the list.
+            """
+            if len(lst) != 0:
+                pairs = []
+                pair = (lst[0], 1)
+                for i in range(1, len(lst)):
+                    if lst[i] == lst[i-1] + 1:
+                        pair = (lst[i], pair[1] + 1)
+                    else:
+                        pairs.append(pair)
+                        pair = (lst[i], 1)
+                pairs.append(pair)
+                random.shuffle(pairs)
+                for x, s in pairs:
+                    if s >= length:
+                        return x - length + 1
+            return None
+
+        append_id = max(used) + 1
+        free = list(set(range(1, append_id)) - used)
+        idx = get_cluster_id(free, size)
+        if idx is None:
+            return append_id
+        else:
+            return idx
+
+    @staticmethod
+    def _alloc_data(img_size, cluster_size):
+        """Return a set of random indices of clusters allocated for guest data.
+        """
+        num_of_cls = img_size/cluster_size
+        return set(random.sample(range(1, num_of_cls + 1),
+                                 random.randint(0, num_of_cls)))
+
+    def _get_metadata(self):
+        """Return indices of clusters allocated for image metadata."""
+        ids = set()
+        for x in self:
+            ids.add(x.offset/self.cluster_size)
+        return ids
+
 
 def create_image(test_img_path, backing_file_name=None, backing_file_fmt=None,
                  fields_to_fuzz=None):
     """Create a fuzzed image and write it to the specified file."""
-    image = Image(backing_file_name, backing_file_fmt)
+    image = Image(backing_file_name)
+    image.set_backing_file_format(backing_file_fmt)
+    image.create_feature_name_table()
+    image.set_end_of_extension_area()
+    image.create_l_structures()
     image.fuzz(fields_to_fuzz)
     image.write(test_img_path)
     return image.image_size
-- 
1.9.3

[Qemu-devel] [PULL 55/55] qcow2: fix new_blocks double-free in alloc_refcount_block()

[Stefan Hajnoczi]

Commit de82815db1c89da058b7fb941dab137d6d9ab738 ("qcow2: Handle failure
for potentially large allocations") introduced a double-free of
new_blocks in the alloc_refcount_block() error path.

The qemu-iotests qcow2 026 test case was failing because qemu-io
segfaulted.

Make sure new_blocks is NULL after we free it the first time.

Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-refcount.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index d60e2fe..3b77470 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -381,6 +381,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
     ret = bdrv_pwrite_sync(bs->file, meta_offset, new_blocks,
         blocks_clusters * s->cluster_size);
     g_free(new_blocks);
+    new_blocks = NULL;
     if (ret < 0) {
         goto fail_table;
     }
-- 
1.9.3

Re: [Qemu-devel] [PULL 00/55] Block patches

[Peter Maydell]

On 15 August 2014 18:06, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The following changes since commit 5c6b3c50cca2106e5fbcbc6efa94c2f8b9d29fd8:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging (2014-08-15 16:37:17 +0100)
>
> are available in the git repository at:
>
>
>   git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 39ba3bf69c4ef4d8a8b683ee7282efd25b3f01ff:
>
>   qcow2: fix new_blocks double-free in alloc_refcount_block() (2014-08-15 18:03:26 +0100)
>
> ----------------------------------------------------------------
> Block pull request
>

Applied, thanks.

-- PMM