From d6ad222673fa4db2d76b4fcd1b839c8dbdc5e6b0 Mon Sep 17 00:00:00 2001
Message-Id: <d6ad222673fa4db2d76b4fcd1b839c8dbdc5e6b0.1383826425.git.minovotn@redhat.com>
From: Kevin Wolf <kwolf@redhat.com>
Date: Tue, 29 Oct 2013 17:52:10 +0100
Subject: [PATCH] block: Avoid unecessary drv->bdrv_getlength() calls

RH-Author: Kevin Wolf <kwolf@redhat.com>
Message-id: <1383069130-24660-1-git-send-email-kwolf@redhat.com>
Patchwork-id: 55098
O-Subject: [RHEL-6.5 qemu-kvm PATCH] block: Avoid unecessary drv->bdrv_getlength() calls
Bugzilla: 1022548
RH-Acked-by: Fam Zheng <famz@redhat.com>
RH-Acked-by: Max Reitz <mreitz@redhat.com>
RH-Acked-by: Markus Armbruster <armbru@redhat.com>

Bugzilla: 1022548
Brew: https://brewweb.devel.redhat.com/taskinfo?taskID=6480905

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 on POSIX, host_device on win32; 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 <kwolf@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit b94a2610573cd9314f244207c8b04cb75e42d7f8)

Conflicts:
	block/raw-win32.c
	block/raw_bsd.c

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c           | 7 ++++---
 block/raw-posix.c | 9 ++++++---
 block/raw-win32.c | 5 ++++-
 block/raw.c       | 1 +
 block_int.h       | 3 +++
 5 files changed, 18 insertions(+), 7 deletions(-)

Signed-off-by: Michal Novotny <minovotn@redhat.com>
---
 block.c           | 7 ++++---
 block/raw-posix.c | 9 ++++++---
 block/raw-win32.c | 5 ++++-
 block/raw.c       | 1 +
 block_int.h       | 3 +++
 5 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index d5cbfe7..4c57615 100644
--- a/block.c
+++ b/block.c
@@ -2390,9 +2390,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 76149f8..bd19cad 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1117,7 +1117,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,
 
@@ -1216,7 +1217,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,
 
@@ -1338,7 +1340,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-win32.c b/block/raw-win32.c
index 8002100..1f03077 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -413,7 +413,10 @@ static BlockDriver bdrv_host_device = {
 
     .bdrv_read		= raw_read,
     .bdrv_write	        = raw_write,
-    .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.c b/block/raw.c
index 40279d0..b55074b 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -135,6 +135,7 @@ static BlockDriver bdrv_raw = {
 
     .bdrv_probe         = raw_probe,
     .bdrv_getlength     = raw_getlength,
+    .has_variable_length = true,
     .bdrv_truncate      = raw_truncate,
 
     .bdrv_is_inserted   = raw_is_inserted,
diff --git a/block_int.h b/block_int.h
index bea2a48..a6cb355 100644
--- a/block_int.h
+++ b/block_int.h
@@ -182,8 +182,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);
 
-- 
1.7.11.7

