* [Qemu-devel] [STABLE 0.13][PATCH 1/3] block: Fix bdrv_has_zero_init
2010-08-03 14:46 [Qemu-devel] [STABLE 0.13][PULL 0/3] Block patches for stable-0.13 Kevin Wolf
@ 2010-08-03 14:46 ` Kevin Wolf
2010-08-03 14:46 ` [Qemu-devel] [STABLE 0.13][PATCH 2/3] block: Change bdrv_eject() not to drop the image Kevin Wolf
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Kevin Wolf @ 2010-08-03 14:46 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
Assuming that any image on a block device is not properly zero-initialized is
actually wrong: Only raw images have this problem. Any other image format
shouldn't care about it, they initialize everything properly themselves.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 336c1c12551ff0a6e1a2af226d6cbdbadd2e02b5)
---
block.c | 6 ++----
block/raw-posix.c | 13 +++++++++----
block/raw-win32.c | 6 ++++++
block/raw.c | 6 ++++++
block_int.h | 7 +++++--
5 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/block.c b/block.c
index 452ae94..40e78f9 100644
--- a/block.c
+++ b/block.c
@@ -1476,10 +1476,8 @@ int bdrv_has_zero_init(BlockDriverState *bs)
{
assert(bs->drv);
- if (bs->drv->no_zero_init) {
- return 0;
- } else if (bs->file) {
- return bdrv_has_zero_init(bs->file);
+ if (bs->drv->bdrv_has_zero_init) {
+ return bs->drv->bdrv_has_zero_init(bs);
}
return 1;
diff --git a/block/raw-posix.c b/block/raw-posix.c
index a11170e..72fb8ce 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -993,6 +993,11 @@ static int hdev_create(const char *filename, QEMUOptionParameter *options)
return ret;
}
+static int hdev_has_zero_init(BlockDriverState *bs)
+{
+ return 0;
+}
+
static BlockDriver bdrv_host_device = {
.format_name = "host_device",
.protocol_name = "host_device",
@@ -1002,7 +1007,7 @@ static BlockDriver bdrv_host_device = {
.bdrv_close = raw_close,
.bdrv_create = hdev_create,
.create_options = raw_create_options,
- .no_zero_init = 1,
+ .bdrv_has_zero_init = hdev_has_zero_init,
.bdrv_flush = raw_flush,
.bdrv_aio_readv = raw_aio_readv,
@@ -1117,7 +1122,7 @@ static BlockDriver bdrv_host_floppy = {
.bdrv_close = raw_close,
.bdrv_create = hdev_create,
.create_options = raw_create_options,
- .no_zero_init = 1,
+ .bdrv_has_zero_init = hdev_has_zero_init,
.bdrv_flush = raw_flush,
.bdrv_aio_readv = raw_aio_readv,
@@ -1217,7 +1222,7 @@ static BlockDriver bdrv_host_cdrom = {
.bdrv_close = raw_close,
.bdrv_create = hdev_create,
.create_options = raw_create_options,
- .no_zero_init = 1,
+ .bdrv_has_zero_init = hdev_has_zero_init,
.bdrv_flush = raw_flush,
.bdrv_aio_readv = raw_aio_readv,
@@ -1340,7 +1345,7 @@ static BlockDriver bdrv_host_cdrom = {
.bdrv_close = raw_close,
.bdrv_create = hdev_create,
.create_options = raw_create_options,
- .no_zero_init = 1,
+ .bdrv_has_zero_init = hdev_has_zero_init,
.bdrv_flush = raw_flush,
.bdrv_aio_readv = raw_aio_readv,
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 745bbde..503ed39 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -394,6 +394,11 @@ static int raw_set_locked(BlockDriverState *bs, int locked)
}
#endif
+static int hdev_has_zero_init(BlockDriverState *bs)
+{
+ return 0;
+}
+
static BlockDriver bdrv_host_device = {
.format_name = "host_device",
.protocol_name = "host_device",
@@ -402,6 +407,7 @@ static BlockDriver bdrv_host_device = {
.bdrv_file_open = hdev_open,
.bdrv_close = raw_close,
.bdrv_flush = raw_flush,
+ .bdrv_has_zero_init = hdev_has_zero_init,
.bdrv_read = raw_read,
.bdrv_write = raw_write,
diff --git a/block/raw.c b/block/raw.c
index 1414e77..61e6748 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -237,6 +237,11 @@ static QEMUOptionParameter raw_create_options[] = {
{ NULL }
};
+static int raw_has_zero_init(BlockDriverState *bs)
+{
+ return bdrv_has_zero_init(bs->file);
+}
+
static BlockDriver bdrv_raw = {
.format_name = "raw",
@@ -264,6 +269,7 @@ static BlockDriver bdrv_raw = {
.bdrv_create = raw_create,
.create_options = raw_create_options,
+ .bdrv_has_zero_init = raw_has_zero_init,
};
static void bdrv_raw_init(void)
diff --git a/block_int.h b/block_int.h
index f075a8c..7d5e751 100644
--- a/block_int.h
+++ b/block_int.h
@@ -127,8 +127,11 @@ struct BlockDriver {
void (*bdrv_debug_event)(BlockDriverState *bs, BlkDebugEvent event);
- /* Set if newly created images are not guaranteed to contain only zeros */
- int no_zero_init;
+ /*
+ * Returns 1 if newly created images are guaranteed to contain only
+ * zeros, 0 otherwise.
+ */
+ int (*bdrv_has_zero_init)(BlockDriverState *bs);
QLIST_ENTRY(BlockDriver) list;
};
--
1.7.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [STABLE 0.13][PATCH 2/3] block: Change bdrv_eject() not to drop the image
2010-08-03 14:46 [Qemu-devel] [STABLE 0.13][PULL 0/3] Block patches for stable-0.13 Kevin Wolf
2010-08-03 14:46 ` [Qemu-devel] [STABLE 0.13][PATCH 1/3] block: Fix bdrv_has_zero_init Kevin Wolf
@ 2010-08-03 14:46 ` Kevin Wolf
2010-08-03 14:46 ` [Qemu-devel] [STABLE 0.13][PATCH 3/3] ide: Avoid canceling IDE DMA Kevin Wolf
2010-08-09 13:41 ` [Qemu-devel] [STABLE 0.13][PULL 0/3] Block patches for stable-0.13 Anthony Liguori
3 siblings, 0 replies; 5+ messages in thread
From: Kevin Wolf @ 2010-08-03 14:46 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
bdrv_eject() gets called when a device model opens or closes the tray.
If the block driver implements method bdrv_eject(), that method gets
called. Drivers host_cdrom implements it, and it opens and closes the
physical tray, and nothing else. When a device model opens, then
closes the tray, media changes only if the user actively changes the
physical media while the tray is open. This is matches how physical
hardware behaves.
If the block driver doesn't implement method bdrv_eject(), we do
something quite different: opening the tray severs the connection to
the image by calling bdrv_close(), and closing the tray does nothing.
When the device model opens, then closes the tray, media is gone,
unless the user actively inserts another one while the tray is open,
with a suitable change command in the monitor. This isn't how
physical hardware behaves. Rather inconvenient when programs
"helpfully" eject media to give you a chance to change it. The way
bdrv_eject() behaves here turns that chance into a must, which is not
what these programs or their users expect.
Change the default action not to call bdrv_close(). Instead, note the
tray status in new BlockDriverState member tray_open. Use it in
bdrv_is_inserted().
Arguably, the device models should keep track of tray status
themselves. But this is less invasive.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 4be9762adb0947a353e6efef2fed354f69218bfb)
---
block.c | 7 ++++---
block_int.h | 1 +
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 40e78f9..8014a5c 100644
--- a/block.c
+++ b/block.c
@@ -2516,7 +2516,7 @@ int bdrv_is_inserted(BlockDriverState *bs)
if (!drv)
return 0;
if (!drv->bdrv_is_inserted)
- return 1;
+ return !bs->tray_open;
ret = drv->bdrv_is_inserted(bs);
return ret;
}
@@ -2558,10 +2558,11 @@ int bdrv_eject(BlockDriverState *bs, int eject_flag)
ret = drv->bdrv_eject(bs, eject_flag);
}
if (ret == -ENOTSUP) {
- if (eject_flag)
- bdrv_close(bs);
ret = 0;
}
+ if (ret >= 0) {
+ bs->tray_open = eject_flag;
+ }
return ret;
}
diff --git a/block_int.h b/block_int.h
index 7d5e751..b863451 100644
--- a/block_int.h
+++ b/block_int.h
@@ -144,6 +144,7 @@ struct BlockDriverState {
int open_flags; /* flags used to open the file, re-used for re-open */
int removable; /* if true, the media can be removed */
int locked; /* if true, the media cannot temporarily be ejected */
+ int tray_open; /* if true, the virtual tray is open */
int encrypted; /* if true, the media is encrypted */
int valid_key; /* if true, a valid encryption key has been set */
int sg; /* if true, the device is a /dev/sg* */
--
1.7.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [STABLE 0.13][PATCH 3/3] ide: Avoid canceling IDE DMA
2010-08-03 14:46 [Qemu-devel] [STABLE 0.13][PULL 0/3] Block patches for stable-0.13 Kevin Wolf
2010-08-03 14:46 ` [Qemu-devel] [STABLE 0.13][PATCH 1/3] block: Fix bdrv_has_zero_init Kevin Wolf
2010-08-03 14:46 ` [Qemu-devel] [STABLE 0.13][PATCH 2/3] block: Change bdrv_eject() not to drop the image Kevin Wolf
@ 2010-08-03 14:46 ` Kevin Wolf
2010-08-09 13:41 ` [Qemu-devel] [STABLE 0.13][PULL 0/3] Block patches for stable-0.13 Anthony Liguori
3 siblings, 0 replies; 5+ messages in thread
From: Kevin Wolf @ 2010-08-03 14:46 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Andrea Arcangeli <aarcange@redhat.com>
The reason for not actually canceling the I/O is because with
virtualization and lots of VM running, a guest fs may mistake a
overload of the host, as an IDE timeout. So rather than canceling the
I/O, it's safer to wait I/O completion and simulate that the I/O has
completed just before the io cancellation was requested by the
guest. This way if ntfs or an app writes data without checking for
-EIO retval, and it thinks the write has succeeded, it's less likely
to run into troubles. Similar issues for reads.
Furthermore because the DMA operation is splitted into many synchronous
aio_read/write if there's more than one entry in the SG table, without this
patch the DMA would be cancelled in the middle, something we've no idea if it
happens on real hardware too or not. Overall this seems a great risk for zero
gain.
This approach is sure safer than previous code given we can't pretend all guest
fs code out there to check for errors and reply the DMA if it was completed
partially, given a timeout would never materialize on a real harddisk unless
there are defective blocks (and defective blocks are practically only an issue
for reads never for writes in any recent hardware as writing to blocks is the
way to fix them) or the harddisk breaks as a whole.
Signed-off-by: Izik Eidus <ieidus@redhat.com>
Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 953844d102f5b682f0835f021f2ed2ad9fb7734c)
---
hw/ide/pci.c | 23 +++++++++++++++++++++--
1 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 4331d77..ec90f26 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -40,8 +40,27 @@ void bmdma_cmd_writeb(void *opaque, uint32_t addr, uint32_t val)
printf("%s: 0x%08x\n", __func__, val);
#endif
if (!(val & BM_CMD_START)) {
- /* XXX: do it better */
- ide_dma_cancel(bm);
+ /*
+ * We can't cancel Scatter Gather DMA in the middle of the
+ * operation or a partial (not full) DMA transfer would reach
+ * the storage so we wait for completion instead (we beahve
+ * like if the DMA was completed by the time the guest trying
+ * to cancel dma with bmdma_cmd_writeb with BM_CMD_START not
+ * set).
+ *
+ * In the future we'll be able to safely cancel the I/O if the
+ * whole DMA operation will be submitted to disk with a single
+ * aio operation with preadv/pwritev.
+ */
+ if (bm->aiocb) {
+ qemu_aio_flush();
+#ifdef DEBUG_IDE
+ if (bm->aiocb)
+ printf("ide_dma_cancel: aiocb still pending");
+ if (bm->status & BM_STATUS_DMAING)
+ printf("ide_dma_cancel: BM_STATUS_DMAING still pending");
+#endif
+ }
bm->cmd = val & 0x09;
} else {
if (!(bm->status & BM_STATUS_DMAING)) {
--
1.7.2
^ permalink raw reply related [flat|nested] 5+ messages in thread