Il 29/10/2013 12:35, Kevin Wolf ha scritto: > The block layer generally keeps the size of an image cached in > bs->total_sectors so that it doesn't have to perform expensive > operations to get the size whenever it needs it. > > This doesn't work however when using a backend that can change its size > without qemu being aware of it, i.e. passthrough of removable media like > CD-ROMs or floppy disks. For this reason, the caching is disabled when a > removable device is used. > > It is obvious that checking whether the _guest_ device has removable > media isn't the right thing to do when we want to know whether the size > of the host backend can change. To make things worse, non-top-level > BlockDriverStates never have any device attached, which makes qemu > assume they are removable, so drv->bdrv_getlength() is always called on > the protocol layer. In the case of raw-posix, this causes unnecessary > lseek() system calls, which turned out to be rather expensive. > > This patch completely changes the logic and disables bs->total_sectors > caching only for certain block driver types, for which a size change is > expected: host_cdrom and host_floppy; also the raw format in case it > sits on top of one of these protocols, but in the common case the nested > bdrv_getlength() call on the protocol driver will use the cache again > and avoid an expensive drv->bdrv_getlength() call. > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > --- > block.c | 7 ++++--- > block/raw-posix.c | 9 ++++++--- > block/raw_bsd.c | 1 + > include/block/block_int.h | 3 +++ > 4 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/block.c b/block.c > index 366999b..da88be0 100644 > --- a/block.c > +++ b/block.c > @@ -2868,9 +2868,10 @@ int64_t bdrv_getlength(BlockDriverState *bs) > if (!drv) > return -ENOMEDIUM; > > - if (bdrv_dev_has_removable_media(bs)) { > - if (drv->bdrv_getlength) { > - return drv->bdrv_getlength(bs); > + if (drv->has_variable_length) { > + int ret = refresh_total_sectors(bs, bs->total_sectors); > + if (ret < 0) { > + return ret; > } > } > return bs->total_sectors * BDRV_SECTOR_SIZE; > diff --git a/block/raw-posix.c b/block/raw-posix.c > index 6f03fbf..f6d48bb 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -1715,7 +1715,8 @@ static BlockDriver bdrv_host_floppy = { > .bdrv_aio_flush = raw_aio_flush, > > .bdrv_truncate = raw_truncate, > - .bdrv_getlength = raw_getlength, > + .bdrv_getlength = raw_getlength, > + .has_variable_length = true, > .bdrv_get_allocated_file_size > = raw_get_allocated_file_size, > > @@ -1824,7 +1825,8 @@ static BlockDriver bdrv_host_cdrom = { > .bdrv_aio_flush = raw_aio_flush, > > .bdrv_truncate = raw_truncate, > - .bdrv_getlength = raw_getlength, > + .bdrv_getlength = raw_getlength, > + .has_variable_length = true, > .bdrv_get_allocated_file_size > = raw_get_allocated_file_size, > > @@ -1951,7 +1953,8 @@ static BlockDriver bdrv_host_cdrom = { > .bdrv_aio_flush = raw_aio_flush, > > .bdrv_truncate = raw_truncate, > - .bdrv_getlength = raw_getlength, > + .bdrv_getlength = raw_getlength, > + .has_variable_length = true, > .bdrv_get_allocated_file_size > = raw_get_allocated_file_size, > > diff --git a/block/raw_bsd.c b/block/raw_bsd.c > index 0078c1b..2265dcc 100644 > --- a/block/raw_bsd.c > +++ b/block/raw_bsd.c > @@ -178,6 +178,7 @@ static BlockDriver bdrv_raw = { > .bdrv_co_get_block_status = &raw_co_get_block_status, > .bdrv_truncate = &raw_truncate, > .bdrv_getlength = &raw_getlength, > + .has_variable_length = true, > .bdrv_get_info = &raw_get_info, > .bdrv_is_inserted = &raw_is_inserted, > .bdrv_media_changed = &raw_media_changed, > diff --git a/include/block/block_int.h b/include/block/block_int.h > index a48731d..1666066 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -156,8 +156,11 @@ struct BlockDriver { > > const char *protocol_name; > int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset); > + > int64_t (*bdrv_getlength)(BlockDriverState *bs); > + bool has_variable_length; > int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs); > + > int (*bdrv_write_compressed)(BlockDriverState *bs, int64_t sector_num, > const uint8_t *buf, int nb_sectors); > >
raw-win32.c probably needs to have a .has_variable_length=true in bdrv_host_device. Apart from that, Reviewed-by: Paolo Bonzini <pbonz...@redhat.com>