From 9e52c53b8c7821ce06e8b995b960e81b469e6847 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:02 -0400 Subject: blkdebug: report errors on flush too Signed-off-by: Paolo Bonzini Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- block/blkdebug.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/block/blkdebug.c b/block/blkdebug.c index f51407de3f..1586ed9664 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, -- cgit v1.2.1 From 7c282cb4c5e84119832f777b40255aac5c65b1d7 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:03 -0400 Subject: libqtest: add QTEST_LOG for debugging qtest testcases Signed-off-by: Paolo Bonzini Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- tests/libqtest.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/libqtest.c b/tests/libqtest.c index 98e8f4b648..4a75cd3a94 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); -- cgit v1.2.1 From 14a92e5fe1d81fe36630582dadd387fba10a63c9 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:04 -0400 Subject: ide-test: add test for werror=stop Signed-off-by: Paolo Bonzini Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- tests/ide-test.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/tests/ide-test.c b/tests/ide-test.c index 4a0d97f197..151ef302e8 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; } -- cgit v1.2.1 From 69f72a22213a6909bf4aef06133c976b508e370a Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:05 -0400 Subject: ide: stash aiocb for flushes This ensures that operations are completed after a reset Signed-off-by: Paolo Bonzini Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- 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 db191a6c3e..79985f9f49 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) -- cgit v1.2.1 From 1374bec0634aac9a460fe3b57bbe8b1aa7a99cb7 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:06 -0400 Subject: ide: simplify reset callbacks Drop the unused return value and make the callback optional. Signed-off-by: Paolo Bonzini Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- 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 604152a823..273712faaa 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 79985f9f49..4183a3abe2 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 0567a522f5..6cca46f506 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 c14a1ddddb..ca39e3f9b6 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 6257a21ed2..c24496a992 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) -- cgit v1.2.1 From 829b933b704a329d60d2ea1fe4c8e8e0a5505d8a Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:07 -0400 Subject: ide: simplify set_inactive callbacks Drop the unused return value and make the callback optional. Signed-off-by: Paolo Bonzini Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- 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 273712faaa..679357fc12 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 4183a3abe2..d52a6f6104 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 6cca46f506..34064cf1a8 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 ca39e3f9b6..0e26bac67f 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 c24496a992..9dc3cc5611 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) -- cgit v1.2.1 From c039cb1e5a7bf1f71fddbbe199eff7fbbfddb416 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:08 -0400 Subject: ide: simplify async_cmd_done callbacks Drop the unused return value. Signed-off-by: Paolo Bonzini Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- 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 679357fc12..e494503330 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 34064cf1a8..940dd4510c 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; }; -- cgit v1.2.1 From 446351236b6e9c53b25e30d107c6235347df1dde Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:09 -0400 Subject: ide: simplify start_transfer callbacks Drop the unused return value and make the callback optional. Signed-off-by: Paolo Bonzini Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- 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 e494503330..adbac3d52f 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 d52a6f6104..e5ddecb811 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 940dd4510c..64fbf2fb62 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 0e26bac67f..b7cedb65a5 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 9dc3cc5611..1ee8c0a4d5 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, -- cgit v1.2.1 From 4855b57639d181362678ab09614a7f753df8e466 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:10 -0400 Subject: ide: wrap start_dma callback Make it optional and prepare for the next patches. Signed-off-by: Paolo Bonzini Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- 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 f7d2009c00..d13395ed2c 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 e5ddecb811..aa561ae4f8 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 64fbf2fb62..2fe1f0a316 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); -- cgit v1.2.1 From 0def37baf9add908c5462b0b8e2d3f80b563a9f9 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:11 -0400 Subject: ide: remove wrong setting of BM_STATUS_INT 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 Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- 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 adbac3d52f..14677ece97 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 d13395ed2c..46ed3f54c0 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); } -- cgit v1.2.1 From 0e7ce54cf5fb9b7e8d19a5a4eb1facf123edbcef Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:12 -0400 Subject: ide: fold add_status callback into set_inactive It is now called only after the set_inactive callback. Put the two together. Signed-off-by: Paolo Bonzini Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- 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 14677ece97..0ec5627a6c 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 46ed3f54c0..3b419b3d05 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 aa561ae4f8..24f24ce7e4 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 2fe1f0a316..b35e52ce98 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 b7cedb65a5..b0c0d400d9 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 1ee8c0a4d5..73267a444f 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, -- cgit v1.2.1 From 7e2648df86a36c063a34196b3bede0a13ebc6809 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:13 -0400 Subject: ide: move BM_STATUS bits to pci.[ch] They are not used by AHCI, and should not be even available there. Signed-off-by: Paolo Bonzini Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- 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 b35e52ce98..f369b0bc26 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 73267a444f..0e82cabffe 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 2428275c8d..517711f913 100644 --- a/hw/ide/pci.h +++ b/hw/ide/pci.h @@ -3,6 +3,13 @@ #include +#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; -- cgit v1.2.1 From fd648f10af1ef41ac37d99ccc341261d6db69ff4 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:14 -0400 Subject: ide: move retry constants out of BM_STATUS_* namespace Signed-off-by: Paolo Bonzini Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- 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 24f24ce7e4..00fe043d11 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 f369b0bc26..b919e96ff8 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 0e82cabffe..2397f355cc 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)); } } -- cgit v1.2.1 From 1f88f77348e14bd9f781db7ff66d2f680daa1d62 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:15 -0400 Subject: ahci: remove duplicate PORT_IRQ_* constants These are defined twice, just use one set consistently. Signed-off-by: Paolo Bonzini Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- 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 0ec5627a6c..e1f27bda51 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 f418b30ce7..1543df7b7d 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 */ -- cgit v1.2.1 From 08ee9e3368be0e9394037d339fc4ebf1392a9896 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:16 -0400 Subject: ide: stop PIO transfer on errors This will provide a hook for sending the result of the command via the FIS receive area. Signed-off-by: Paolo Bonzini Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- 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 00fe043d11..91a17e6f1d 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); } -- cgit v1.2.1 From c7e73adb48abb9fc5cbfc4f1ce6090fbdb0bdbea Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:17 -0400 Subject: ide: make all commands go through cmd_done 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 Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- 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 e1f27bda51..b40ec06c3a 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 3b419b3d05..3d92b52dbc 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 91a17e6f1d..bdb0a804cf 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 b919e96ff8..5c19f79437 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; }; -- cgit v1.2.1 From 088415202b9dadb387cb6d9fd25324ae7fd4da4b Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Aug 2014 17:11:18 -0400 Subject: ahci: construct PIO Setup FIS for PIO commands 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 Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- hw/ide/ahci.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index b40ec06c3a..4cda0d0075 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, -- cgit v1.2.1 From 552b48f44df99e095f04245a48d3b839bbe9912c Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 4 Aug 2014 17:11:19 -0400 Subject: q35: Enable the ioapic device to be seen by qtest. 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 Signed-off-by: Stefan Hajnoczi --- 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 c39ee98933..0dd7e02160 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); -- cgit v1.2.1 From 86298845e127365e8a5b7419a5ee9039bbd1837f Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 4 Aug 2014 17:11:20 -0400 Subject: qtest: Adding qtest_memset and qmemset. 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 Signed-off-by: Stefan Hajnoczi --- tests/libqtest.c | 12 ++++++++++++ tests/libqtest.h | 24 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/tests/libqtest.c b/tests/libqtest.c index 4a75cd3a94..e525e6fdc2 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 8f323c7030..1be0934f07 100644 --- a/tests/libqtest.h +++ b/tests/libqtest.h @@ -282,6 +282,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. @@ -620,6 +631,19 @@ static inline void memwrite(uint64_t addr, const void *data, size_t size) qtest_memwrite(global_qtest, addr, data, 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: * -- cgit v1.2.1 From f3cdcbaee16d32b52d5015a8b1e8ddf5a27f7089 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 4 Aug 2014 17:11:21 -0400 Subject: libqos: Correct memory leak Fix a small memory leak inside of libqos, in the pc_alloc_init routine. Signed-off-by: John Snow Signed-off-by: Stefan Hajnoczi --- 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 db1496c667..63af60ac17 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; } -- cgit v1.2.1 From a7afc6b8c13c70e9c40b4f666be80600f8ad0b3d Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 4 Aug 2014 17:11:22 -0400 Subject: libqtest: Correct small memory leak. 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 Signed-off-by: Stefan Hajnoczi --- tests/libqtest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/libqtest.c b/tests/libqtest.c index e525e6fdc2..0bf17aa8cd 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) -- cgit v1.2.1 From 7f2a5ae6c1194b1676f1a7239fbcacd9d59637be Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 4 Aug 2014 17:11:23 -0400 Subject: libqos: Fixes a small memory leak. 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 Signed-off-by: Stefan Hajnoczi --- 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 4adf4006ae..f5d646984c 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 4f7475f6f7..26211790cd 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 -- cgit v1.2.1 From 6ce7100e7ff986c1f214f6bccca89dfd7256d8ec Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 4 Aug 2014 17:11:24 -0400 Subject: libqos: allow qpci_iomap to return BAR mapping size 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 Signed-off-by: Stefan Hajnoczi --- 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 151ef302e8..e2b4efcc3f 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 f5d646984c..0609294af0 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 c9a0b9134a..ce0b308a83 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 3439431540..9ee048b154 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 bcdf62fc3b..c990492835 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); } -- cgit v1.2.1 From e42de189e8eaf3dc93f22e88beca4f5b62ef336c Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 4 Aug 2014 17:11:25 -0400 Subject: qtest/ide: Fix small memory leak 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 Signed-off-by: Stefan Hajnoczi --- tests/ide-test.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ide-test.c b/tests/ide-test.c index e2b4efcc3f..a77a037d1e 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) -- cgit v1.2.1 From 58f16a7b47e0e8418b1222b6adc08d2b7079a4c0 Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Fri, 8 Aug 2014 17:23:32 +0100 Subject: cmd646: add constants for CNTRL register access Signed-off-by: Mark Cave-Ayland Signed-off-by: Stefan Hajnoczi --- 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 a8e35fe38f..d8395ef248 100644 --- a/hw/ide/cmd646.c +++ b/hw/ide/cmd646.c @@ -33,6 +33,9 @@ #include /* 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); -- cgit v1.2.1 From 5bbc0a703d8241a866f51856336aeb2a2d54b79f Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Fri, 8 Aug 2014 17:23:33 +0100 Subject: cmd646: synchronise DMA interrupt status with UDMA interrupt status 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 Signed-off-by: Stefan Hajnoczi --- hw/ide/cmd646.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c index d8395ef248..c3c6c53d91 100644 --- a/hw/ide/cmd646.c +++ b/hw/ide/cmd646.c @@ -33,9 +33,13 @@ #include /* 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); } -- cgit v1.2.1 From dab91a1e138e9d0cb7dc0744c1e41e9f18af0fc4 Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Fri, 8 Aug 2014 17:23:34 +0100 Subject: cmd646: switch cmd646_update_irq() to accept PCIDevice instead of PCIIDEState This is in preparation for adding configuration space accessors which accept PCIDevice as a parameter. Signed-off-by: Mark Cave-Ayland Signed-off-by: Stefan Hajnoczi --- 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 c3c6c53d91..11a3e52d71 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) -- cgit v1.2.1 From 1d113ef874c04486cf4ee2894b2ef84bf8d17543 Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Fri, 8 Aug 2014 17:23:35 +0100 Subject: cmd646: allow MRDMODE interrupt status bits clearing from PCI config space 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 Signed-off-by: Stefan Hajnoczi --- 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 11a3e52d71..b8dc4ab9ea 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; } -- cgit v1.2.1 From 271dddd133125ee00e347b154bb9d44e228929bb Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Fri, 8 Aug 2014 17:23:36 +0100 Subject: cmd646: synchronise UDMA interrupt status with DMA interrupt status Make sure that both registers are synchronised when being accessed through PCI configuration space. Signed-off-by: Mark Cave-Ayland Signed-off-by: Stefan Hajnoczi --- hw/ide/cmd646.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c index b8dc4ab9ea..74d0deb6dd 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; -- cgit v1.2.1 From 4ff12bdb1d7c74d95b7315f0a00d17e5cea32249 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Mon, 11 Aug 2014 17:34:20 +0800 Subject: qemu-char: using qemu_set_nonblock() instead of fcntl(O_NONBLOCK) 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 Signed-off-by: Gonglei Reviewed-by: Eric Blake Signed-off-by: Stefan Hajnoczi --- qemu-char.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index 6964a2d9fd..b1e6a0a9b8 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); -- cgit v1.2.1 From 16b38080e3383252f9606c484eb5b43cff70b673 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Mon, 11 Aug 2014 17:34:21 +0800 Subject: channel-posix: using qemu_set_nonblock() instead of fcntl(O_NONBLOCK) 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 Signed-off-by: Wangxin Reviewed-by: Eric Blake Signed-off-by: Stefan Hajnoczi --- 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 e65dda3822..8aad4fee9f 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"); -- cgit v1.2.1 From 267e1a204c073eac28b1510bcc2404668d306e17 Mon Sep 17 00:00:00 2001 From: Cornelia Huck Date: Fri, 25 Jul 2014 14:10:46 +0200 Subject: dataplane: print why starting failed 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 Signed-off-by: Cornelia Huck Signed-off-by: Stefan Hajnoczi --- 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 d6ba65ca23..527a53ce37 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); -- cgit v1.2.1 From f9907ebc4cc37d0317ee67cfa8d6618eaf8f658b Mon Sep 17 00:00:00 2001 From: Cornelia Huck Date: Fri, 25 Jul 2014 14:10:47 +0200 Subject: dataplane: fail notifier setting gracefully 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 Signed-off-by: Cornelia Huck Signed-off-by: Stefan Hajnoczi --- 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 527a53ce37..94e1a29ef5 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 */ -- cgit v1.2.1 From 2f5f70fa5f41e3893a781c065be76e56db4f2e32 Mon Sep 17 00:00:00 2001 From: Cornelia Huck Date: Fri, 25 Jul 2014 14:10:48 +0200 Subject: dataplane: stop trying on notifier error 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 Signed-off-by: Cornelia Huck Signed-off-by: Stefan Hajnoczi --- 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 94e1a29ef5..24a6b71395 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; } -- cgit v1.2.1 From 8c27d54fa0a289809dc0c62d8c1f44d5bab2b3bb Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Mon, 28 Jul 2014 20:23:52 +0400 Subject: parallels: extend parallels format header with actual data values 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 CC: Kevin Wolf CC: Stefan Hajnoczi CC: Jeff Cody Reviewed-by: Jeff Cody Signed-off-by: Stefan Hajnoczi --- block/parallels.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/block/parallels.c b/block/parallels.c index 7325678a4d..59cfad6a63 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) { -- cgit v1.2.1 From f08e2f8465197c65c61b1ab1e0df3cdef4d8567c Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Mon, 28 Jul 2014 20:23:53 +0400 Subject: parallels: replace tabs with spaces in block/parallels.c Signed-off-by: Denis V. Lunev Reviewed-by: Jeff Cody CC: Kevin Wolf CC: Stefan Hajnoczi Signed-off-by: Stefan Hajnoczi --- block/parallels.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/block/parallels.c b/block/parallels.c index 59cfad6a63..e0cf1d991f 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; } -- cgit v1.2.1 From 418a7adb77abbbc09a17d5626cbaa5e9e54be709 Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Mon, 28 Jul 2014 20:23:54 +0400 Subject: parallels: split check for parallels format in parallels_open 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 CC: Jeff Cody CC: Kevin Wolf CC: Stefan Hajnoczi Reviewed-by: Jeff Cody Signed-off-by: Stefan Hajnoczi --- block/parallels.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/block/parallels.c b/block/parallels.c index e0cf1d991f..f9e18b4537 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; -- cgit v1.2.1 From d25d59802021a747812472780d80a0e792078f40 Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Mon, 28 Jul 2014 20:23:55 +0400 Subject: parallels: 2TB+ parallels images support 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 CC: Jeff Cody CC: Kevin Wolf CC: Stefan Hajnoczi Reviewed-by: Jeff Cody Signed-off-by: Stefan Hajnoczi --- block/parallels.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/block/parallels.c b/block/parallels.c index f9e18b4537..1774ab8e8e 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, -- cgit v1.2.1 From 2f7133b2e540342809a35bb4f52e0abdc5fff9ff Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Mon, 28 Jul 2014 21:53:02 +0200 Subject: qemu-options: add missing -drive discard option to cmdline help Signed-off-by: Peter Lieven Reviewed-by: Eric Blake Signed-off-by: Stefan Hajnoczi --- qemu-options.hx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-options.hx b/qemu-options.hx index 96516c1e23..44e3be3c2d 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" -- cgit v1.2.1 From f7f3ff1da0c451befc8d32f977f9c352d1303f40 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 12 Aug 2014 18:29:41 +0200 Subject: ide: Fix segfault when flushing a device that doesn't exist Signed-off-by: Kevin Wolf Signed-off-by: Stefan Hajnoczi --- 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 bdb0a804cf..82dd4afd84 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 a77a037d1e..ffce6ed669 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); -- cgit v1.2.1 From ae74f18782d83c249e8bb9566bf5a2ad501ab82c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Mar=C3=AD?= Date: Tue, 12 Aug 2014 13:41:48 +0200 Subject: libqtest: add QTEST_LOG for debugging qtest testcases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paolo Bonzini Signed-off-by: Marc Marí Signed-off-by: Stefan Hajnoczi --- tests/libqtest.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/libqtest.c b/tests/libqtest.c index 0bf17aa8cd..ed55686ce0 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); -- cgit v1.2.1 From f75ffc585717a26169d180a905aa1f4299aa57e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Mar=C3=AD?= Date: Tue, 12 Aug 2014 13:41:49 +0200 Subject: libqos: Correct mask to align size to PAGE_SIZE in malloc-pc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed-by: John Snow Reviewed-by: Stefan Hajnoczi Signed-off-by: Paolo Bonzini Signed-off-by: Marc Marí Signed-off-by: Stefan Hajnoczi --- 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 63af60ac17..be1d97f8bb 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); -- cgit v1.2.1 From 220c1a2fadee125909e88d61637be307fea02d88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Mar=C3=AD?= Date: Tue, 12 Aug 2014 13:41:50 +0200 Subject: libqos: Change free function called in malloc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed-by: John Snow Reviewed-by: Stefan Hajnoczi Signed-off-by: Marc Marí Signed-off-by: Stefan Hajnoczi --- 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 46f6000763..556538121e 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 -- cgit v1.2.1 From a83ceea8ffa5ed8f0b09d6ed1f33da6aae8b14e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Mar=C3=AD?= Date: Tue, 12 Aug 2014 13:41:51 +0200 Subject: virtio-blk: Correct bug in support for flexible descriptor layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Marc Marí Signed-off-by: Stefan Hajnoczi --- 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 c241c5002b..302c39e2be 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); -- cgit v1.2.1 From d66168ed687325aa6d338ce3a3cff18ce3098ed6 Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Wed, 13 Aug 2014 11:23:31 +0400 Subject: ide: only constrain read/write requests to drive size, not other types 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 Signed-off-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi --- 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 82dd4afd84..b48127f921 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; -- cgit v1.2.1 From d6dc10aad8f4dd5503bd8b6d42ed9ab07e03fb01 Mon Sep 17 00:00:00 2001 From: Maria Kustova Date: Mon, 11 Aug 2014 14:33:58 +0400 Subject: docs: Specification for the image fuzzer '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 Signed-off-by: Stefan Hajnoczi --- 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 0000000000..e73b182859 --- /dev/null +++ b/docs/image-fuzzer.txt @@ -0,0 +1,239 @@ +# Specification for the fuzz testing tool +# +# Copyright (C) 2014 Maria Kustova +# +# 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 . + + +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. -- cgit v1.2.1 From ad724dd7282520ea13f0626f1c3ef45d65a1b994 Mon Sep 17 00:00:00 2001 From: Maria Kustova Date: Mon, 11 Aug 2014 14:33:59 +0400 Subject: image-fuzzer: Tool for fuzz tests execution 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 Signed-off-by: Maria Kustova Signed-off-by: Stefan Hajnoczi --- 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 0000000000..58079d331e --- /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 +# +# 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 . +# + +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 -- cgit v1.2.1 From 6d5e9372f6d968cbee1d6708198abd087db07260 Mon Sep 17 00:00:00 2001 From: Maria Kustova Date: Mon, 11 Aug 2014 14:34:00 +0400 Subject: image-fuzzer: Fuzzing functions for qcow2 images 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 Signed-off-by: Maria Kustova Signed-off-by: Stefan Hajnoczi --- 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 0000000000..a53c84fc4e --- /dev/null +++ b/tests/image-fuzzer/qcow2/fuzz.py @@ -0,0 +1,327 @@ +# Fuzzing functions for qcow2 fields +# +# Copyright (C) 2014 Maria Kustova +# +# 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 . +# + +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) -- cgit v1.2.1 From e123232331305fce0c69a3fb170d3f5d8003abf4 Mon Sep 17 00:00:00 2001 From: Maria Kustova Date: Mon, 11 Aug 2014 14:34:01 +0400 Subject: image-fuzzer: Generator of fuzzed qcow2 images 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 Signed-off-by: Maria Kustova Signed-off-by: Stefan Hajnoczi --- 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 0000000000..4c08202c3d --- /dev/null +++ b/tests/image-fuzzer/qcow2/layout.py @@ -0,0 +1,369 @@ +# Generator of fuzzed qcow2 images +# +# Copyright (C) 2014 Maria Kustova +# +# 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 . +# + +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 -- cgit v1.2.1 From 071e6491947acff53196c119ea4713593e7a8b11 Mon Sep 17 00:00:00 2001 From: Maria Kustova Date: Mon, 11 Aug 2014 14:34:02 +0400 Subject: image-fuzzer: Public API for image-fuzzer/runner/runner.py __init__.py provides the public API required by the test runner Reviewed-by: Stefan Hajnoczi Signed-off-by: Maria Kustova Signed-off-by: Stefan Hajnoczi --- 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 0000000000..e2ebe19311 --- /dev/null +++ b/tests/image-fuzzer/qcow2/__init__.py @@ -0,0 +1 @@ +from layout import create_image -- cgit v1.2.1 From 489cb4d7f958f443556a6e47fbd23344183408ac Mon Sep 17 00:00:00 2001 From: Maria Kustova Date: Mon, 11 Aug 2014 15:01:08 +0400 Subject: docs: Expand the list of supported image elements with L1/L2 tables Signed-off-by: Maria Kustova Signed-off-by: Stefan Hajnoczi --- 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 e73b182859..0d0005d34a 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 ----------------- -- cgit v1.2.1 From eeadd9248707c3952de22012974ebdc1e17f2cba Mon Sep 17 00:00:00 2001 From: Maria Kustova Date: Mon, 11 Aug 2014 15:01:09 +0400 Subject: image-fuzzer: Add fuzzing functions for L1/L2 table entries Signed-off-by: Maria Kustova Signed-off-by: Stefan Hajnoczi --- 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 a53c84fc4e..57527f9b4a 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 -- cgit v1.2.1 From 38eb193b8b14844a5d5ef65b54a4926e931ac311 Mon Sep 17 00:00:00 2001 From: Maria Kustova Date: Mon, 11 Aug 2014 15:01:10 +0400 Subject: image-fuzzer: Add generators of L1/L2 tables 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 Signed-off-by: Stefan Hajnoczi --- 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 4c08202c3d..deed9ea96e 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) -- cgit v1.2.1 From 94c83a24c1956cd50ab979725a730f7d8649ac15 Mon Sep 17 00:00:00 2001 From: Maria Kustova Date: Mon, 11 Aug 2014 15:27:46 +0400 Subject: image-fuzzer: Reduce number of generator functions in __init__ 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 Signed-off-by: Stefan Hajnoczi --- 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 deed9ea96e..730c771d3c 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 -- cgit v1.2.1 From 39ba3bf69c4ef4d8a8b683ee7282efd25b3f01ff Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Fri, 15 Aug 2014 17:59:54 +0100 Subject: qcow2: fix new_blocks double-free in alloc_refcount_block() 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 Signed-off-by: Stefan Hajnoczi --- block/qcow2-refcount.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index d60e2feb10..3b7747048e 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; } -- cgit v1.2.1