All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v6 0/6] block: ignore flush requests when storage is clean
@ 2016-07-14 12:29 Denis V. Lunev
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 1/6] ide: refactor retry_unit set and clear into separate function Denis V. Lunev
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Denis V. Lunev @ 2016-07-14 12:29 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Evgeny Yakovlev, Kevin Wolf, Max Reitz, Stefan Hajnoczi,
	Fam Zheng, John Snow

Changes from v6:
- Removed failed flush traces in block tests 026 071 089
- Changed BLOCK_JOB_READY event order in block tests 141 144

Changes from v4:
- Moved to write generation scheme instead of dirty flag
- Added retry setup to IDE PIO and FLUSH requests

Changes from v3:
- Fixed a typo in commit message
- Rebased on Kevin'n origin/block

Changes from v2:
- Better comments
- Rebased on latest master

Changes from v1:
- Flush requests that should be skipped will now wait for completion
of any previous requests already in flight
- Fixed IDE and AHCI tests to dirty media for new flush behaviour
- Fixed a problem in IDE CMD_FLUSH_CACHE failure handling

Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Fam Zheng <famz@redhat.com>
CC: John Snow <jsnow@redhat.com>

Evgeny Yakovlev (6):
  ide: refactor retry_unit set and clear into separate function
  ide: set retry_unit for PIO and FLUSH requests
  tests: in IDE and AHCI tests perform DMA write before flushing
  block: ignore flush requests when storage is clean
  tests: removed skipped flushes from block test traces
  tests: changed block job ready event generation order

 block.c                            |  3 +++
 block/io.c                         | 21 ++++++++++++++++
 hw/ide/core.c                      | 24 +++++++++++++-----
 include/block/block_int.h          |  5 ++++
 tests/ahci-test.c                  | 34 ++++++++++++++++++++++++--
 tests/ide-test.c                   | 43 ++++++++++++++++++++++++++++++++
 tests/qemu-iotests/026.out.nocache | 50 --------------------------------------
 tests/qemu-iotests/071.out         |  8 ------
 tests/qemu-iotests/089.out         |  2 --
 tests/qemu-iotests/141.out         |  4 +--
 tests/qemu-iotests/144.out         |  2 +-
 11 files changed, 125 insertions(+), 71 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v6 1/6] ide: refactor retry_unit set and clear into separate function
  2016-07-14 12:29 [Qemu-devel] [PATCH v6 0/6] block: ignore flush requests when storage is clean Denis V. Lunev
@ 2016-07-14 12:29 ` Denis V. Lunev
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 2/6] ide: set retry_unit for PIO and FLUSH requests Denis V. Lunev
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Denis V. Lunev @ 2016-07-14 12:29 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Evgeny Yakovlev, Kevin Wolf, Max Reitz, Stefan Hajnoczi,
	Fam Zheng, John Snow

From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>

Code to set and clear state associated with retry in moved into
ide_set_retry and ide_clear_retry to make adding retry setups easier.

Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Fam Zheng <famz@redhat.com>
CC: John Snow <jsnow@redhat.com>
---
 hw/ide/core.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 029f6b9..b72346e 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -466,6 +466,20 @@ void ide_abort_command(IDEState *s)
     s->error = ABRT_ERR;
 }
 
+static void ide_set_retry(IDEState *s)
+{
+    s->bus->retry_unit = s->unit;
+    s->bus->retry_sector_num = ide_get_sector(s);
+    s->bus->retry_nsector = s->nsector;
+}
+
+static void ide_clear_retry(IDEState *s)
+{
+    s->bus->retry_unit = -1;
+    s->bus->retry_sector_num = 0;
+    s->bus->retry_nsector = 0;
+}
+
 /* prepare data transfer and tell what to do after */
 void ide_transfer_start(IDEState *s, uint8_t *buf, int size,
                         EndTransferFunc *end_transfer_func)
@@ -756,9 +770,7 @@ void dma_buf_commit(IDEState *s, uint32_t tx_bytes)
 void ide_set_inactive(IDEState *s, bool more)
 {
     s->bus->dma->aiocb = NULL;
-    s->bus->retry_unit = -1;
-    s->bus->retry_sector_num = 0;
-    s->bus->retry_nsector = 0;
+    ide_clear_retry(s);
     if (s->bus->dma->ops->set_inactive) {
         s->bus->dma->ops->set_inactive(s->bus->dma, more);
     }
@@ -914,9 +926,7 @@ static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
 void ide_start_dma(IDEState *s, BlockCompletionFunc *cb)
 {
     s->io_buffer_index = 0;
-    s->bus->retry_unit = s->unit;
-    s->bus->retry_sector_num = ide_get_sector(s);
-    s->bus->retry_nsector = s->nsector;
+    ide_set_retry(s);
     if (s->bus->dma->ops->start_dma) {
         s->bus->dma->ops->start_dma(s->bus->dma, s, cb);
     }
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v6 2/6] ide: set retry_unit for PIO and FLUSH requests
  2016-07-14 12:29 [Qemu-devel] [PATCH v6 0/6] block: ignore flush requests when storage is clean Denis V. Lunev
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 1/6] ide: refactor retry_unit set and clear into separate function Denis V. Lunev
@ 2016-07-14 12:29 ` Denis V. Lunev
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 3/6] tests: in IDE and AHCI tests perform DMA write before flushing Denis V. Lunev
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Denis V. Lunev @ 2016-07-14 12:29 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Evgeny Yakovlev, Kevin Wolf, Max Reitz, Stefan Hajnoczi,
	Fam Zheng, John Snow

From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>

The following sequence of tests discovered a problem in IDE emulation:
1. Send DMA write to IDE device 0
2. Send CMD_FLUSH_CACHE to same IDE device which will be failed by block
layer using blkdebug script in tests/ide-test:test_retry_flush

When doing DMA request ide/core.c will set s->retry_unit to s->unit in
ide_start_dma. When dma completes ide_set_inactive sets retry_unit to -1.
After that ide_flush_cache runs and fails thanks to blkdebug.
ide_flush_cb calls ide_handle_rw_error which asserts that s->retry_unit
== s->unit. But s->retry_unit is still -1 after previous DMA completion
and flush does not use anything related to retry.

This patch restricts retry unit assertion only to ops that actually use
retry logic.

Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Fam Zheng <famz@redhat.com>
CC: John Snow <jsnow@redhat.com>
---
 hw/ide/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index b72346e..14f03d2 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -487,6 +487,7 @@ void ide_transfer_start(IDEState *s, uint8_t *buf, int size,
     s->end_transfer_func = end_transfer_func;
     s->data_ptr = buf;
     s->data_end = buf + size;
+    ide_set_retry(s);
     if (!(s->status & ERR_STAT)) {
         s->status |= DRQ_STAT;
     }
@@ -1056,6 +1057,7 @@ static void ide_flush_cache(IDEState *s)
     }
 
     s->status |= BUSY_STAT;
+    ide_set_retry(s);
     block_acct_start(blk_get_stats(s->blk), &s->acct, 0, BLOCK_ACCT_FLUSH);
     s->pio_aiocb = blk_aio_flush(s->blk, ide_flush_cb, s);
 }
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v6 3/6] tests: in IDE and AHCI tests perform DMA write before flushing
  2016-07-14 12:29 [Qemu-devel] [PATCH v6 0/6] block: ignore flush requests when storage is clean Denis V. Lunev
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 1/6] ide: refactor retry_unit set and clear into separate function Denis V. Lunev
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 2/6] ide: set retry_unit for PIO and FLUSH requests Denis V. Lunev
@ 2016-07-14 12:29 ` Denis V. Lunev
  2016-07-14 16:05   ` Eric Blake
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 4/6] block: ignore flush requests when storage is clean Denis V. Lunev
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Denis V. Lunev @ 2016-07-14 12:29 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Evgeny Yakovlev, Kevin Wolf, Max Reitz, Stefan Hajnoczi,
	Fam Zheng, John Snow

From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>

Due to changes in flush behaviour clean disks stopped generating
flush_to_disk events and IDE and AHCI tests that test flush commands
started to fail.

This change adds additional DMA writes to affected tests before sending
flush commands so that bdrv_flush actually generates flush_to_disk event.

Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Fam Zheng <famz@redhat.com>
CC: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c | 34 ++++++++++++++++++++++++++++++++--
 tests/ide-test.c  | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 57dc44c..d707714 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1063,11 +1063,34 @@ static void test_dma_fragmented(void)
     g_free(tx);
 }
 
+/*
+ * Write sector 0 with random data to make AHCI storage dirty
+ * Needed for flush tests so that flushes actually go though the block layer
+ */
+static void make_dirty(AHCIQState* ahci, uint8_t port)
+{
+    uint64_t ptr;
+    unsigned bufsize = 512;
+
+    ptr = ahci_alloc(ahci, bufsize);
+    g_assert(ptr);
+
+    ahci_guest_io(ahci, port, CMD_WRITE_DMA, ptr, bufsize, 0);
+    ahci_free(ahci, ptr);
+}
+
 static void test_flush(void)
 {
     AHCIQState *ahci;
+    uint8_t port;
 
     ahci = ahci_boot_and_enable(NULL);
+
+    port = ahci_port_select(ahci);
+    ahci_port_clear(ahci, port);
+
+    make_dirty(ahci, port);
+
     ahci_test_flush(ahci);
     ahci_shutdown(ahci);
 }
@@ -1087,10 +1110,13 @@ static void test_flush_retry(void)
                                 debug_path,
                                 tmp_path, imgfmt);
 
-    /* Issue Flush Command and wait for error */
     port = ahci_port_select(ahci);
     ahci_port_clear(ahci, port);
 
+    /* Issue write so that flush actually goes to disk */
+    make_dirty(ahci, port);
+
+    /* Issue Flush Command and wait for error */
     cmd = ahci_guest_io_halt(ahci, port, CMD_FLUSH_CACHE, 0, 0, 0);
     ahci_guest_io_resume(ahci, cmd);
 
@@ -1343,9 +1369,13 @@ static void test_flush_migrate(void)
 
     set_context(src->parent);
 
-    /* Issue Flush Command */
     px = ahci_port_select(src);
     ahci_port_clear(src, px);
+
+    /* Dirty device so that flush reaches disk */
+    make_dirty(src, px);
+
+    /* Issue Flush Command */
     cmd = ahci_command_create(CMD_FLUSH_CACHE);
     ahci_command_commit(src, cmd, px);
     ahci_command_issue_async(src, cmd);
diff --git a/tests/ide-test.c b/tests/ide-test.c
index fed1b2e..8466d0f 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -499,6 +499,39 @@ static void test_identify(void)
     ide_test_quit();
 }
 
+/*
+ * Write sector 0 with random data to make IDE storage dirty
+ * Needed for flush tests so that flushes actually go though the block layer
+ */
+static void make_dirty(uint8_t device)
+{
+    uint8_t status;
+    size_t len = 512;
+    uintptr_t guest_buf;
+    void* buf;
+
+    guest_buf = guest_alloc(guest_malloc, len);
+    buf = g_malloc(len);
+    g_assert(guest_buf);
+    g_assert(buf);
+
+    memwrite(guest_buf, buf, len);
+
+    PrdtEntry prdt[] = {
+        {
+            .addr = cpu_to_le32(guest_buf),
+            .size = cpu_to_le32(len | PRDT_EOT),
+        },
+    };
+
+    status = send_dma_request(CMD_WRITE_DMA, 0, 1, prdt,
+                              ARRAY_SIZE(prdt), NULL);
+    g_assert_cmphex(status, ==, BM_STS_INTR);
+    assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
+
+    g_free(buf);
+}
+
 static void test_flush(void)
 {
     uint8_t data;
@@ -507,6 +540,11 @@ static void test_flush(void)
         "-drive file=blkdebug::%s,if=ide,cache=writeback,format=raw",
         tmp_path);
 
+    qtest_irq_intercept_in(global_qtest, "ioapic");
+
+    /* Dirty media so that CMD_FLUSH_CACHE will actually go to disk */
+    make_dirty(0);
+
     /* Delay the completion of the flush request until we explicitly do it */
     g_free(hmp("qemu-io ide0-hd0 \"break flush_to_os A\""));
 
@@ -549,6 +587,11 @@ static void test_retry_flush(const char *machine)
         "rerror=stop,werror=stop",
         debug_path, tmp_path);
 
+    qtest_irq_intercept_in(global_qtest, "ioapic");
+
+    /* Dirty media so that CMD_FLUSH_CACHE will actually go to disk */
+    make_dirty(0);
+
     /* FLUSH CACHE command on device 0*/
     outb(IDE_BASE + reg_device, 0);
     outb(IDE_BASE + reg_command, CMD_FLUSH_CACHE);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v6 4/6] block: ignore flush requests when storage is clean
  2016-07-14 12:29 [Qemu-devel] [PATCH v6 0/6] block: ignore flush requests when storage is clean Denis V. Lunev
                   ` (2 preceding siblings ...)
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 3/6] tests: in IDE and AHCI tests perform DMA write before flushing Denis V. Lunev
@ 2016-07-14 12:29 ` Denis V. Lunev
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 5/6] tests: removed skipped flushes from block test traces Denis V. Lunev
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 6/6] tests: changed block job ready event generation order Denis V. Lunev
  5 siblings, 0 replies; 15+ messages in thread
From: Denis V. Lunev @ 2016-07-14 12:29 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Evgeny Yakovlev, Kevin Wolf, Max Reitz, Stefan Hajnoczi,
	Fam Zheng, John Snow

From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>

Some guests (win2008 server for example) do a lot of unnecessary
flushing when underlying media has not changed. This adds additional
overhead on host when calling fsync/fdatasync.

This change introduces a write generation scheme in BlockDriverState.
Current write generation is checked against last flushed generation to
avoid unnessesary flushes.

The problem with excessive flushing was found by a performance test
which does parallel directory tree creation (from 2 processes).
Results improved from 0.424 loops/sec to 0.432 loops/sec.
Each loop creates 10^3 directories with 10 files in each.

Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Fam Zheng <famz@redhat.com>
CC: John Snow <jsnow@redhat.com>
---
 block.c                   |  3 +++
 block/io.c                | 21 +++++++++++++++++++++
 include/block/block_int.h |  5 +++++
 3 files changed, 29 insertions(+)

diff --git a/block.c b/block.c
index c2fb8bd..b88ad31 100644
--- a/block.c
+++ b/block.c
@@ -234,6 +234,8 @@ BlockDriverState *bdrv_new(void)
     bs->refcnt = 1;
     bs->aio_context = qemu_get_aio_context();
 
+    qemu_co_queue_init(&bs->flush_queue);
+
     QTAILQ_INSERT_TAIL(&all_bdrv_states, bs, bs_list);
 
     return bs;
@@ -2472,6 +2474,7 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
         ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
         bdrv_dirty_bitmap_truncate(bs);
         bdrv_parent_cb_resize(bs);
+        ++bs->write_gen;
     }
     return ret;
 }
diff --git a/block/io.c b/block/io.c
index 7086908..f181ff7 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1303,6 +1303,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs,
     }
     bdrv_debug_event(bs, BLKDBG_PWRITEV_DONE);
 
+    ++bs->write_gen;
     bdrv_set_dirty(bs, start_sector, end_sector - start_sector);
 
     if (bs->wr_highest_offset < offset + bytes) {
@@ -2235,6 +2236,15 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
 
     tracked_request_begin(&req, bs, 0, 0, BDRV_TRACKED_FLUSH);
 
+    int current_gen = bs->write_gen;
+
+    /* Wait until any previous flushes are completed */
+    while (bs->flush_started_gen != bs->flushed_gen) {
+        qemu_co_queue_wait(&bs->flush_queue);
+    }
+
+    bs->flush_started_gen = current_gen;
+
     /* Write back all layers by calling one driver function */
     if (bs->drv->bdrv_co_flush) {
         ret = bs->drv->bdrv_co_flush(bs);
@@ -2255,6 +2265,11 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
         goto flush_parent;
     }
 
+    /* Check if we really need to flush anything */
+    if (bs->flushed_gen == current_gen) {
+        goto flush_parent;
+    }
+
     BLKDBG_EVENT(bs->file, BLKDBG_FLUSH_TO_DISK);
     if (bs->drv->bdrv_co_flush_to_disk) {
         ret = bs->drv->bdrv_co_flush_to_disk(bs);
@@ -2285,6 +2300,7 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
          */
         ret = 0;
     }
+
     if (ret < 0) {
         goto out;
     }
@@ -2295,6 +2311,10 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
 flush_parent:
     ret = bs->file ? bdrv_co_flush(bs->file->bs) : 0;
 out:
+    /* Notify any pending flushes that we have completed */
+    bs->flushed_gen = current_gen;
+    qemu_co_queue_restart_all(&bs->flush_queue);
+
     tracked_request_end(&req);
     return ret;
 }
@@ -2420,6 +2440,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
     }
     ret = 0;
 out:
+    ++bs->write_gen;
     bdrv_set_dirty(bs, req.offset >> BDRV_SECTOR_BITS,
                    req.bytes >> BDRV_SECTOR_BITS);
     tracked_request_end(&req);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 042c118..104d9ab 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -439,6 +439,11 @@ struct BlockDriverState {
     int copy_on_read; /* if nonzero, copy read backing sectors into image.
                          note this is a reference count */
 
+    CoQueue flush_queue;            /* Serializing flush queue */
+    unsigned int write_gen;         /* Current data generation */
+    unsigned int flush_started_gen; /* Generation for which flush has started */
+    unsigned int flushed_gen;       /* Flushed write generation */
+
     BlockDriver *drv; /* NULL means no media */
     void *opaque;
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v6 5/6] tests: removed skipped flushes from block test traces
  2016-07-14 12:29 [Qemu-devel] [PATCH v6 0/6] block: ignore flush requests when storage is clean Denis V. Lunev
                   ` (3 preceding siblings ...)
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 4/6] block: ignore flush requests when storage is clean Denis V. Lunev
@ 2016-07-14 12:29 ` Denis V. Lunev
  2016-07-14 16:06   ` Eric Blake
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 6/6] tests: changed block job ready event generation order Denis V. Lunev
  5 siblings, 1 reply; 15+ messages in thread
From: Denis V. Lunev @ 2016-07-14 12:29 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Evgeny Yakovlev, Kevin Wolf, Max Reitz, Stefan Hajnoczi,
	Fam Zheng, John Snow

From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>

bdrv_co_flush is now skipping flushes in case underlying media has no
actual changes. This affected some blkdebug testcases that were
expecting error logs from failure-injected flushes which are now
skipped entirely.

This change removes expected flush error logs from block tests 026 071 089

Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Fam Zheng <famz@redhat.com>
CC: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/026.out.nocache | 50 --------------------------------------
 tests/qemu-iotests/071.out         |  8 ------
 tests/qemu-iotests/089.out         |  2 --
 3 files changed, 60 deletions(-)

diff --git a/tests/qemu-iotests/026.out.nocache b/tests/qemu-iotests/026.out.nocache
index 9c2c8a9..672d77c 100644
--- a/tests/qemu-iotests/026.out.nocache
+++ b/tests/qemu-iotests/026.out.nocache
@@ -14,7 +14,6 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_update; errno: 5; imm: off; once: off; write 
-Failed to flush the L2 table cache: Input/output error
 Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 
@@ -23,7 +22,6 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_update; errno: 5; imm: off; once: off; write -b
-Failed to flush the L2 table cache: Input/output error
 Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 
@@ -42,7 +40,6 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_update; errno: 28; imm: off; once: off; write 
-Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
@@ -51,7 +48,6 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_update; errno: 28; imm: off; once: off; write -b
-Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
@@ -78,11 +74,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 Event: l2_load; errno: 5; imm: off; once: off; write 
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Failed to flush the L2 table cache: Input/output error
-Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-Failed to flush the L2 table cache: Input/output error
-Failed to flush the refcount block cache: Input/output error
 read failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -90,11 +82,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 Event: l2_load; errno: 5; imm: off; once: off; write -b
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Failed to flush the L2 table cache: Input/output error
-Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-Failed to flush the L2 table cache: Input/output error
-Failed to flush the refcount block cache: Input/output error
 read failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -118,11 +106,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 Event: l2_load; errno: 28; imm: off; once: off; write 
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Failed to flush the L2 table cache: No space left on device
-Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-Failed to flush the L2 table cache: No space left on device
-Failed to flush the refcount block cache: No space left on device
 read failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -130,11 +114,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 Event: l2_load; errno: 28; imm: off; once: off; write -b
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Failed to flush the L2 table cache: No space left on device
-Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-Failed to flush the L2 table cache: No space left on device
-Failed to flush the refcount block cache: No space left on device
 read failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -314,14 +294,12 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_load; errno: 5; imm: off; once: off; write 
-Failed to flush the L2 table cache: Input/output error
 Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_load; errno: 5; imm: off; once: off; write -b
-Failed to flush the L2 table cache: Input/output error
 Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
@@ -338,14 +316,12 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_load; errno: 28; imm: off; once: off; write 
-Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_load; errno: 28; imm: off; once: off; write -b
-Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
@@ -362,14 +338,12 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_update_part; errno: 5; imm: off; once: off; write 
-Failed to flush the L2 table cache: Input/output error
 Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_update_part; errno: 5; imm: off; once: off; write -b
-Failed to flush the L2 table cache: Input/output error
 Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
@@ -386,14 +360,12 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_update_part; errno: 28; imm: off; once: off; write 
-Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_update_part; errno: 28; imm: off; once: off; write -b
-Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
@@ -410,14 +382,12 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc; errno: 5; imm: off; once: off; write 
-Failed to flush the L2 table cache: Input/output error
 Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc; errno: 5; imm: off; once: off; write -b
-Failed to flush the L2 table cache: Input/output error
 Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
@@ -434,14 +404,12 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc; errno: 28; imm: off; once: off; write 
-Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc; errno: 28; imm: off; once: off; write -b
-Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
@@ -458,15 +426,11 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: cluster_alloc; errno: 5; imm: off; once: off; write 
-Failed to flush the L2 table cache: Input/output error
-Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: cluster_alloc; errno: 5; imm: off; once: off; write -b
-Failed to flush the L2 table cache: Input/output error
-Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -482,15 +446,11 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: cluster_alloc; errno: 28; imm: off; once: off; write 
-Failed to flush the L2 table cache: No space left on device
-Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: cluster_alloc; errno: 28; imm: off; once: off; write -b
-Failed to flush the L2 table cache: No space left on device
-Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 
@@ -561,7 +521,6 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc_write_blocks; errno: 28; imm: off; once: off; write 
-Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
@@ -570,7 +529,6 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc_write_blocks; errno: 28; imm: off; once: off; write -b
-Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
@@ -589,7 +547,6 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc_write_table; errno: 28; imm: off; once: off; write 
-Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
@@ -598,7 +555,6 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc_write_table; errno: 28; imm: off; once: off; write -b
-Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
@@ -643,8 +599,6 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_grow_alloc_table; errno: 5; imm: off; once: off
-Failed to flush the L2 table cache: Input/output error
-Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -655,8 +609,6 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_grow_alloc_table; errno: 28; imm: off; once: off
-Failed to flush the L2 table cache: No space left on device
-Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -667,7 +619,6 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_grow_write_table; errno: 5; imm: off; once: off
-Failed to flush the L2 table cache: Input/output error
 Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
@@ -679,7 +630,6 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_grow_write_table; errno: 28; imm: off; once: off
-Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
diff --git a/tests/qemu-iotests/071.out b/tests/qemu-iotests/071.out
index 2b40ead..8c6851e 100644
--- a/tests/qemu-iotests/071.out
+++ b/tests/qemu-iotests/071.out
@@ -30,14 +30,10 @@ blkverify: read sector_num=0 nb_sectors=1 contents mismatch in sector 0
 
 === Testing blkdebug through filename ===
 
-Failed to flush the L2 table cache: Input/output error
-Failed to flush the refcount block cache: Input/output error
 read failed: Input/output error
 
 === Testing blkdebug through file blockref ===
 
-Failed to flush the L2 table cache: Input/output error
-Failed to flush the refcount block cache: Input/output error
 read failed: Input/output error
 
 === Testing blkdebug on existing block device ===
@@ -51,8 +47,6 @@ read failed: Input/output error
 {"return": ""}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
-QEMU_PROG: Failed to flush the L2 table cache: Input/output error
-QEMU_PROG: Failed to flush the refcount block cache: Input/output error
 
 
 === Testing blkverify on existing block device ===
@@ -92,7 +86,5 @@ read failed: Input/output error
 {"return": ""}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
-QEMU_PROG: Failed to flush the L2 table cache: Input/output error
-QEMU_PROG: Failed to flush the refcount block cache: Input/output error
 
 *** done
diff --git a/tests/qemu-iotests/089.out b/tests/qemu-iotests/089.out
index 5b541a3..18f5fdd 100644
--- a/tests/qemu-iotests/089.out
+++ b/tests/qemu-iotests/089.out
@@ -24,8 +24,6 @@ read 512/512 bytes at offset 0
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 wrote 512/512 bytes at offset 229376
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Failed to flush the L2 table cache: Input/output error
-Failed to flush the refcount block cache: Input/output error
 read failed: Input/output error
 
 === Testing qemu-img info output ===
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v6 6/6] tests: changed block job ready event generation order
  2016-07-14 12:29 [Qemu-devel] [PATCH v6 0/6] block: ignore flush requests when storage is clean Denis V. Lunev
                   ` (4 preceding siblings ...)
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 5/6] tests: removed skipped flushes from block test traces Denis V. Lunev
@ 2016-07-14 12:29 ` Denis V. Lunev
  2016-07-14 16:07   ` Eric Blake
  5 siblings, 1 reply; 15+ messages in thread
From: Denis V. Lunev @ 2016-07-14 12:29 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Evgeny Yakovlev, Kevin Wolf, Max Reitz, Stefan Hajnoczi,
	Fam Zheng, John Snow

From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>

Due to skipped flushes on bdrv_co_flush BLOCK_JOB_READY events
for driver-mirror and active block-commit commands now arrives faster,
before QMP send successfully returns to caller.

This change expects for block job ready event to arrive before
QMP send successfully returns in block tests 141 144

Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Fam Zheng <famz@redhat.com>
CC: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/141.out | 4 ++--
 tests/qemu-iotests/144.out | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/141.out b/tests/qemu-iotests/141.out
index adceac1..eaf1e60 100644
--- a/tests/qemu-iotests/141.out
+++ b/tests/qemu-iotests/141.out
@@ -18,8 +18,8 @@ Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.
 
 {"return": {}}
 Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT
-{"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}}
+{"return": {}}
 {"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: mirror"}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}}
@@ -28,8 +28,8 @@ Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.
 === Testing active block-commit ===
 
 {"return": {}}
-{"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}
+{"return": {}}
 {"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: commit"}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}
diff --git a/tests/qemu-iotests/144.out b/tests/qemu-iotests/144.out
index 410d741..387855c 100644
--- a/tests/qemu-iotests/144.out
+++ b/tests/qemu-iotests/144.out
@@ -12,9 +12,9 @@ Formatting 'TEST_DIR/tmp.qcow2', fmt=qcow2 size=536870912 backing_file=TEST_DIR/
 
 === Performing block-commit on active layer ===
 
-{"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "virtio0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}
 {"return": {}}
+{"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "virtio0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}
 
 === Performing Live Snapshot 2 ===
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v6 3/6] tests: in IDE and AHCI tests perform DMA write before flushing
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 3/6] tests: in IDE and AHCI tests perform DMA write before flushing Denis V. Lunev
@ 2016-07-14 16:05   ` Eric Blake
  2016-07-15  8:08     ` Evgeny Yakovlev
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Blake @ 2016-07-14 16:05 UTC (permalink / raw)
  To: Denis V. Lunev, qemu-block, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Evgeny Yakovlev, Max Reitz,
	Stefan Hajnoczi, John Snow

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

On 07/14/2016 06:29 AM, Denis V. Lunev wrote:
> From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
> 
> Due to changes in flush behaviour clean disks stopped generating
> flush_to_disk events and IDE and AHCI tests that test flush commands
> started to fail.
> 
> This change adds additional DMA writes to affected tests before sending
> flush commands so that bdrv_flush actually generates flush_to_disk event.
> 
> Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Max Reitz <mreitz@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Fam Zheng <famz@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> ---
>  tests/ahci-test.c | 34 ++++++++++++++++++++++++++++++++--
>  tests/ide-test.c  | 43 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 75 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> index 57dc44c..d707714 100644
> --- a/tests/ahci-test.c
> +++ b/tests/ahci-test.c
> @@ -1063,11 +1063,34 @@ static void test_dma_fragmented(void)
>      g_free(tx);
>  }
>  
> +/*
> + * Write sector 0 with random data to make AHCI storage dirty

If we ever have a case where we open a disk without specifying -raw, the
random data _might_ resemble some other format and cause probe to
misbehave; as such, we also have code in the block layer that
specifically prevents writes to sector 0 for some data. Should you pick
a different sector than 0, so as to avoid any (remote) possibility that
the random data could change probe results or be rejected?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v6 5/6] tests: removed skipped flushes from block test traces
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 5/6] tests: removed skipped flushes from block test traces Denis V. Lunev
@ 2016-07-14 16:06   ` Eric Blake
  2016-07-15  8:11     ` Evgeny Yakovlev
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Blake @ 2016-07-14 16:06 UTC (permalink / raw)
  To: Denis V. Lunev, qemu-block, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Evgeny Yakovlev, Max Reitz,
	Stefan Hajnoczi, John Snow

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

On 07/14/2016 06:29 AM, Denis V. Lunev wrote:
> From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
> 
> bdrv_co_flush is now skipping flushes in case underlying media has no
> actual changes. This affected some blkdebug testcases that were
> expecting error logs from failure-injected flushes which are now
> skipped entirely.
> 
> This change removes expected flush error logs from block tests 026 071 089
> 
> Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Max Reitz <mreitz@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Fam Zheng <famz@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> ---
>  tests/qemu-iotests/026.out.nocache | 50 --------------------------------------
>  tests/qemu-iotests/071.out         |  8 ------
>  tests/qemu-iotests/089.out         |  2 --
>  3 files changed, 60 deletions(-)

If the previous patch broke the testsuite, then this should be squashed
in with that patch so that bisection doesn't land on a broken testsuite.

Seeing the testsuite change alongside code change is just fine; it
proves what impact the code change has.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v6 6/6] tests: changed block job ready event generation order
  2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 6/6] tests: changed block job ready event generation order Denis V. Lunev
@ 2016-07-14 16:07   ` Eric Blake
  2016-07-14 19:46     ` John Snow
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Blake @ 2016-07-14 16:07 UTC (permalink / raw)
  To: Denis V. Lunev, qemu-block, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Evgeny Yakovlev, Max Reitz,
	Stefan Hajnoczi, John Snow

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

On 07/14/2016 06:29 AM, Denis V. Lunev wrote:
> From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
> 
> Due to skipped flushes on bdrv_co_flush BLOCK_JOB_READY events
> for driver-mirror and active block-commit commands now arrives faster,
> before QMP send successfully returns to caller.
> 
> This change expects for block job ready event to arrive before
> QMP send successfully returns in block tests 141 144
> 
> Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Max Reitz <mreitz@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Fam Zheng <famz@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> ---
>  tests/qemu-iotests/141.out | 4 ++--
>  tests/qemu-iotests/144.out | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

Same comment as in 5/6: if the testsuite broke as a result of 4/6, then
this should be squashed in to that patch so that the end result of your
series never lands on a bisection point where the testsuite is broken.


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v6 6/6] tests: changed block job ready event generation order
  2016-07-14 16:07   ` Eric Blake
@ 2016-07-14 19:46     ` John Snow
  0 siblings, 0 replies; 15+ messages in thread
From: John Snow @ 2016-07-14 19:46 UTC (permalink / raw)
  To: Eric Blake, Denis V. Lunev, qemu-block, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Evgeny Yakovlev, Max Reitz, Stefan Hajnoczi



On 07/14/2016 12:07 PM, Eric Blake wrote:
> On 07/14/2016 06:29 AM, Denis V. Lunev wrote:
>> From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
>>
>> Due to skipped flushes on bdrv_co_flush BLOCK_JOB_READY events
>> for driver-mirror and active block-commit commands now arrives faster,
>> before QMP send successfully returns to caller.
>>
>> This change expects for block job ready event to arrive before
>> QMP send successfully returns in block tests 141 144
>>
>> Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Kevin Wolf <kwolf@redhat.com>
>> CC: Max Reitz <mreitz@redhat.com>
>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>> CC: Fam Zheng <famz@redhat.com>
>> CC: John Snow <jsnow@redhat.com>
>> ---
>>  tests/qemu-iotests/141.out | 4 ++--
>>  tests/qemu-iotests/144.out | 2 +-
>>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> Same comment as in 5/6: if the testsuite broke as a result of 4/6, then
> this should be squashed in to that patch so that the end result of your
> series never lands on a bisection point where the testsuite is broken.
> 
> 

Yes, please: all patches must be ordered such that intermediate commits
do not break iotests or 'make check'.

(Sorry for the runaround.)

--js

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v6 3/6] tests: in IDE and AHCI tests perform DMA write before flushing
  2016-07-14 16:05   ` Eric Blake
@ 2016-07-15  8:08     ` Evgeny Yakovlev
  2016-07-15 17:23       ` Eric Blake
  0 siblings, 1 reply; 15+ messages in thread
From: Evgeny Yakovlev @ 2016-07-15  8:08 UTC (permalink / raw)
  To: Eric Blake, Denis V. Lunev, qemu-block, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Max Reitz, Stefan Hajnoczi, John Snow



On 14.07.2016 19:05, Eric Blake wrote:
> On 07/14/2016 06:29 AM, Denis V. Lunev wrote:
>> From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
>>
>> Due to changes in flush behaviour clean disks stopped generating
>> flush_to_disk events and IDE and AHCI tests that test flush commands
>> started to fail.
>>
>> This change adds additional DMA writes to affected tests before sending
>> flush commands so that bdrv_flush actually generates flush_to_disk event.
>>
>> Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>> CC: Kevin Wolf <kwolf@redhat.com>
>> CC: Max Reitz <mreitz@redhat.com>
>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>> CC: Fam Zheng <famz@redhat.com>
>> CC: John Snow <jsnow@redhat.com>
>> ---
>>   tests/ahci-test.c | 34 ++++++++++++++++++++++++++++++++--
>>   tests/ide-test.c  | 43 +++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 75 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
>> index 57dc44c..d707714 100644
>> --- a/tests/ahci-test.c
>> +++ b/tests/ahci-test.c
>> @@ -1063,11 +1063,34 @@ static void test_dma_fragmented(void)
>>       g_free(tx);
>>   }
>>   
>> +/*
>> + * Write sector 0 with random data to make AHCI storage dirty
> If we ever have a case where we open a disk without specifying -raw, the
> random data _might_ resemble some other format and cause probe to
> misbehave; as such, we also have code in the block layer that
> specifically prevents writes to sector 0 for some data. Should you pick
> a different sector than 0, so as to avoid any (remote) possibility that
> the random data could change probe results or be rejected?
>

Not sure if i understand the problem you're referring to here. Those are 
blkdebug tests, those disks are created, emulated with blkdebug backend, 
flushed and then thrown away. So is there really any possibility for 
reopening the image and accidentally parsing a partition table in sector 0?

Also, not sure what you mean by "code in the block layer that
specifically prevents writes to sector 0 for some data". Can you explain 
that bit, because it sounds pretty scary. How can we deny guest VM to 
write anything to sector 0 on its emulated disk?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v6 5/6] tests: removed skipped flushes from block test traces
  2016-07-14 16:06   ` Eric Blake
@ 2016-07-15  8:11     ` Evgeny Yakovlev
  0 siblings, 0 replies; 15+ messages in thread
From: Evgeny Yakovlev @ 2016-07-15  8:11 UTC (permalink / raw)
  To: Eric Blake, Denis V. Lunev, qemu-block, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Max Reitz, Stefan Hajnoczi, John Snow



On 14.07.2016 19:06, Eric Blake wrote:
> On 07/14/2016 06:29 AM, Denis V. Lunev wrote:
>> From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
>>
>> bdrv_co_flush is now skipping flushes in case underlying media has no
>> actual changes. This affected some blkdebug testcases that were
>> expecting error logs from failure-injected flushes which are now
>> skipped entirely.
>>
>> This change removes expected flush error logs from block tests 026 071 089
>>
>> Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Kevin Wolf <kwolf@redhat.com>
>> CC: Max Reitz <mreitz@redhat.com>
>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>> CC: Fam Zheng <famz@redhat.com>
>> CC: John Snow <jsnow@redhat.com>
>> ---
>>   tests/qemu-iotests/026.out.nocache | 50 --------------------------------------
>>   tests/qemu-iotests/071.out         |  8 ------
>>   tests/qemu-iotests/089.out         |  2 --
>>   3 files changed, 60 deletions(-)
> If the previous patch broke the testsuite, then this should be squashed
> in with that patch so that bisection doesn't land on a broken testsuite.
>
> Seeing the testsuite change alongside code change is just fine; it
> proves what impact the code change has.
>

Will do.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v6 3/6] tests: in IDE and AHCI tests perform DMA write before flushing
  2016-07-15  8:08     ` Evgeny Yakovlev
@ 2016-07-15 17:23       ` Eric Blake
  2016-07-15 17:44         ` Evgeny Yakovlev
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Blake @ 2016-07-15 17:23 UTC (permalink / raw)
  To: Evgeny Yakovlev, Denis V. Lunev, qemu-block, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Max Reitz, Stefan Hajnoczi, John Snow

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

On 07/15/2016 02:08 AM, Evgeny Yakovlev wrote:

>>> + * Write sector 0 with random data to make AHCI storage dirty
>> If we ever have a case where we open a disk without specifying -raw, the
>> random data _might_ resemble some other format and cause probe to
>> misbehave; as such, we also have code in the block layer that
>> specifically prevents writes to sector 0 for some data. Should you pick
>> a different sector than 0, so as to avoid any (remote) possibility that
>> the random data could change probe results or be rejected?
>>
> 
> Not sure if i understand the problem you're referring to here. Those are
> blkdebug tests, those disks are created, emulated with blkdebug backend,
> flushed and then thrown away. So is there really any possibility for
> reopening the image and accidentally parsing a partition table in sector 0?
> 
> Also, not sure what you mean by "code in the block layer that
> specifically prevents writes to sector 0 for some data". Can you explain
> that bit, because it sounds pretty scary. How can we deny guest VM to
> write anything to sector 0 on its emulated disk?

Read block/raw_bsd.c:raw_co_writev_flags() for the gory details.  If the
guest ever gets a raw format driver because the user forgot to say
'--format $foo', then we prevent the guest from writing anything into
sector 0 that would be probed as non-raw.  It means there are only a
handful of patterns that the guest cannot write into the first sector,
but it IS a non-zero number of patterns.  How the guest behaves if such
a write is attempted depends on the error policy you have on that
device; it might show up as an EIO error to the guest, or it might stop
the guest from executing and raise a qemu event to the management
application, but the point is that we actively prohibit some writes to
sector 0 on a probed raw disk.  Using any sector other than 0 doesn't
have this limitation, or you can ensure that your test ALWAYS passes the
appropriate --format $foo so that the disk is never probed as another
way to avoid limitations on sector 0.


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v6 3/6] tests: in IDE and AHCI tests perform DMA write before flushing
  2016-07-15 17:23       ` Eric Blake
@ 2016-07-15 17:44         ` Evgeny Yakovlev
  0 siblings, 0 replies; 15+ messages in thread
From: Evgeny Yakovlev @ 2016-07-15 17:44 UTC (permalink / raw)
  To: Eric Blake, Denis V. Lunev, qemu-block, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Max Reitz, Stefan Hajnoczi, John Snow



On 15.07.2016 20:23, Eric Blake wrote:
> On 07/15/2016 02:08 AM, Evgeny Yakovlev wrote:
>
>>>> + * Write sector 0 with random data to make AHCI storage dirty
>>> If we ever have a case where we open a disk without specifying -raw, the
>>> random data _might_ resemble some other format and cause probe to
>>> misbehave; as such, we also have code in the block layer that
>>> specifically prevents writes to sector 0 for some data. Should you pick
>>> a different sector than 0, so as to avoid any (remote) possibility that
>>> the random data could change probe results or be rejected?
>>>
>> Not sure if i understand the problem you're referring to here. Those are
>> blkdebug tests, those disks are created, emulated with blkdebug backend,
>> flushed and then thrown away. So is there really any possibility for
>> reopening the image and accidentally parsing a partition table in sector 0?
>>
>> Also, not sure what you mean by "code in the block layer that
>> specifically prevents writes to sector 0 for some data". Can you explain
>> that bit, because it sounds pretty scary. How can we deny guest VM to
>> write anything to sector 0 on its emulated disk?
> Read block/raw_bsd.c:raw_co_writev_flags() for the gory details.  If the
> guest ever gets a raw format driver because the user forgot to say
> '--format $foo', then we prevent the guest from writing anything into
> sector 0 that would be probed as non-raw.  It means there are only a
> handful of patterns that the guest cannot write into the first sector,
> but it IS a non-zero number of patterns.  How the guest behaves if such
> a write is attempted depends on the error policy you have on that
> device; it might show up as an EIO error to the guest, or it might stop
> the guest from executing and raise a qemu event to the management
> application, but the point is that we actively prohibit some writes to
> sector 0 on a probed raw disk.  Using any sector other than 0 doesn't
> have this limitation, or you can ensure that your test ALWAYS passes the
> appropriate --format $foo so that the disk is never probed as another
> way to avoid limitations on sector 0.
>
>

I think i get it now. Thanks!

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-07-15 18:17 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-14 12:29 [Qemu-devel] [PATCH v6 0/6] block: ignore flush requests when storage is clean Denis V. Lunev
2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 1/6] ide: refactor retry_unit set and clear into separate function Denis V. Lunev
2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 2/6] ide: set retry_unit for PIO and FLUSH requests Denis V. Lunev
2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 3/6] tests: in IDE and AHCI tests perform DMA write before flushing Denis V. Lunev
2016-07-14 16:05   ` Eric Blake
2016-07-15  8:08     ` Evgeny Yakovlev
2016-07-15 17:23       ` Eric Blake
2016-07-15 17:44         ` Evgeny Yakovlev
2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 4/6] block: ignore flush requests when storage is clean Denis V. Lunev
2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 5/6] tests: removed skipped flushes from block test traces Denis V. Lunev
2016-07-14 16:06   ` Eric Blake
2016-07-15  8:11     ` Evgeny Yakovlev
2016-07-14 12:29 ` [Qemu-devel] [PATCH v6 6/6] tests: changed block job ready event generation order Denis V. Lunev
2016-07-14 16:07   ` Eric Blake
2016-07-14 19:46     ` John Snow

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.