Block patches for stable-0.13
The following changes since commit 8f6e28789faeac4f01f8dbfdac147a3d3b635f24:
savevm: Fix memory leak of compat struct (2010-07-30 23:02:03 +0200)
are available in the git repository at:
git://repo.or.cz/qemu/kevin.git for-stable-0.13
Andrea Arcangeli (1):
ide: Avoid canceling IDE DMA
Kevin Wolf (1):
block: Fix bdrv_has_zero_init
Markus Armbruster (1):
block: Change bdrv_eject() not to drop the image
block.c | 13 ++++++------- block/raw-posix.c | 13 +++++++++---- block/raw-win32.c | 6 ++++++ block/raw.c | 6 ++++++ block_int.h | 8 ++++++-- hw/ide/pci.c | 23 +++++++++++++++++++++-- 6 files changed, 54 insertions(+), 15 deletions(-)
PATCH 1/3 block: Fix bdrv_has_zero_init
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.
--- 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(-)
block.c +2 -4
@@ -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 ;
block/raw-posix.c +9 -4
@@ -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,
block/raw-win32.c +6 -0
@@ -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,
block/raw.c +6 -0
@@ -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 )
block_int.h +5 -2
@@ -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; };
Signature · 1 line
1.7.2
PATCH 2/3 block: Change bdrv_eject() not to drop the image
In reply to Kevin WolfFrom: Markus Armbruster <[email protected]>
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.
--- block.c | 7 ++++--- block_int.h | 1 + 2 files changed, 5 insertions(+), 3 deletions(-)
block.c +4 -3
@@ -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; }
block_int.h +1 -0
@@ -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* */
Signature · 1 line
1.7.2
PATCH 3/3 ide: Avoid canceling IDE DMA
In reply to Kevin WolfFrom: Andrea Arcangeli <[email protected]>
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.
--- hw/ide/pci.c | 23 +++++++++++++++++++++-- 1 files changed, 21 insertions(+), 2 deletions(-)
hw/ide/pci.c +21 -2
@@ -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 )) {
Signature · 1 line
1.7.2
Quoted 4 lines · Kevin Wolf
The following changes since commit
8f6e28789faeac4f01f8dbfdac147a3d3b635f24:savevm: Fix memory leak of compat struct (2010-07-30 23:02:03 +0200)
Pulled, thanks.
Regards,
Anthony Liguori
Quoted 22 lines
are available in the git repository at:
git://repo.or.cz/qemu/kevin.git for-stable-0.13Andrea Arcangeli (1):
ide: Avoid canceling IDE DMAKevin Wolf (1):
block: Fixbdrv_has_zero_initMarkus Armbruster (1):
block: Changebdrv_eject()not to drop the imageblock.c | 13 ++++++------- block/raw-posix.c | 13 +++++++++---- block/raw-win32.c | 6 ++++++ block/raw.c | 6 ++++++ block_int.h | 8 ++++++-- hw/ide/pci.c | 23 +++++++++++++++++++++-- 6 files changed, 54 insertions(+), 15 deletions(-)