[Qemu-devel] [PATCH v5 0/8] block: generic copy-on-read

[Qemu-devel] [PATCH v5 0/8] block: generic copy-on-read

[Stefan Hajnoczi]

[Kevin: As requested, I am only sending Patch 4 and not the rest of the patches
since they are unchanged and already applied to your block tree.]

The new -drive copy-on-read=on|off feature populates the image file with data
from the backing file on read.  This is useful when accessing images backed
over a slow medium (e.g. http over internet).  All read data will be stored in
the local image file so it does not need to be fetched again in the future.

This series is a prerequisite for the image streaming feature, which uses
copy-on-read to populate the image file in the background while the VM is
running.  However, the copy-on-read feature is useful on its own.

Copy-on-read is implemented by checking whether or not data is allocated in the
image file before reading it.  If data is not allocated then it needs to be
read and written back to the image file.

The tricky bit is avoiding races with other I/O requests.  These patches add
request tracking to BlockDriverState so that the list of pending requests is
available.  Copy-on-read prevents races by serializing overlapping requests.

Finally, there is a performance impact when enabling this feature since an
additional write is performed.  Serializing overlapping requests also means
that I/O patterns where multiple requests access the same cluster will see a
loss in parallelism.  Perhaps we can be smarter about preventing corruption in
the future and win back some performance.

v5:
 * Fix refcount on startup [Kevin]

v4:
 * No changes, sorry for the spam v3 emails that were sent out

v3:
 * Improve wait_for_overlapping_requests() comment [Kevin]

v2:
 * Based on bdrv_co_is_allocated patch series - now safe in coroutine context
 * Use QEMU_ALIGN_DOWN/UP() macros for copy-on-read cluster calculations [Zhi Yong]
 * Reset bs->copy_on_read on bdrv_close() [Kevin]
 * Refcount bs->copy_on_read so it doesn't get clobbered by multiple users [Marcelo]
 * Use bool instead of int where appropriate [Kevin]
 * Use compound literal assignment to ensure BdrvTrackedRequest fields always get zeroed [Kevin]
 * Comment rationale for copy-on-read bounce buffer [Kevin]

Stefan Hajnoczi (1):
  block: add interface to toggle copy-on-read

 block.c     |   22 ++++++++++++++++++++++
 block.h     |    4 ++++
 block_int.h |    2 ++
 3 files changed, 28 insertions(+), 0 deletions(-)

-- 
1.7.7.3

[Qemu-devel] [PATCH v5 4/8] block: add interface to toggle copy-on-read

[Stefan Hajnoczi]

The bdrv_enable_copy_on_read()/bdrv_disable_copy_on_read() functions can
be used to programmatically enable or disable copy-on-read for a block
device.  Later patches add the actual copy-on-read logic.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c     |   22 ++++++++++++++++++++++
 block.h     |    4 ++++
 block_int.h |    2 ++
 3 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 27c4e84..f8dc082 100644
--- a/block.c
+++ b/block.c
@@ -538,6 +538,22 @@ int bdrv_parse_cache_flags(const char *mode, int *flags)
     return 0;
 }
 
+/**
+ * The copy-on-read flag is actually a reference count so multiple users may
+ * use the feature without worrying about clobbering its previous state.
+ * Copy-on-read stays enabled until all users have called to disable it.
+ */
+void bdrv_enable_copy_on_read(BlockDriverState *bs)
+{
+    bs->copy_on_read++;
+}
+
+void bdrv_disable_copy_on_read(BlockDriverState *bs)
+{
+    assert(bs->copy_on_read > 0);
+    bs->copy_on_read--;
+}
+
 /*
  * Common part for opening disk images and files
  */
@@ -559,6 +575,11 @@ static int bdrv_open_common(BlockDriverState *bs, const char *filename,
     bs->growable = 0;
     bs->buffer_alignment = 512;
 
+    assert(bs->copy_on_read == 0); /* bdrv_new() and bdrv_close() make it so */
+    if ((flags & BDRV_O_RDWR) && (flags & BDRV_O_COPY_ON_READ)) {
+        bdrv_enable_copy_on_read(bs);
+    }
+
     pstrcpy(bs->filename, sizeof(bs->filename), filename);
     bs->backing_file[0] = '\0';
 
@@ -801,6 +822,7 @@ void bdrv_close(BlockDriverState *bs)
 #endif
         bs->opaque = NULL;
         bs->drv = NULL;
+        bs->copy_on_read = 0;
 
         if (bs->file != NULL) {
             bdrv_close(bs->file);
diff --git a/block.h b/block.h
index ad8dd48..4bbad0b 100644
--- a/block.h
+++ b/block.h
@@ -70,6 +70,7 @@ typedef struct BlockDevOps {
 #define BDRV_O_NATIVE_AIO  0x0080 /* use native AIO instead of the thread pool */
 #define BDRV_O_NO_BACKING  0x0100 /* don't open the backing file */
 #define BDRV_O_NO_FLUSH    0x0200 /* disable flushing on this disk */
+#define BDRV_O_COPY_ON_READ 0x0400 /* copy read backing sectors into image */
 
 #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
 
@@ -308,6 +309,9 @@ void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
                       int nr_sectors);
 int64_t bdrv_get_dirty_count(BlockDriverState *bs);
 
+void bdrv_enable_copy_on_read(BlockDriverState *bs);
+void bdrv_disable_copy_on_read(BlockDriverState *bs);
+
 void bdrv_set_in_use(BlockDriverState *bs, int in_use);
 int bdrv_in_use(BlockDriverState *bs);
 
diff --git a/block_int.h b/block_int.h
index 788fde9..3c5bacb 100644
--- a/block_int.h
+++ b/block_int.h
@@ -193,6 +193,8 @@ struct BlockDriverState {
     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* */
+    int copy_on_read; /* if true, copy read backing sectors into image
+                         note this is a reference count */
 
     BlockDriver *drv; /* NULL means no media */
     void *opaque;
-- 
1.7.7.3

Re: [Qemu-devel] [PATCH v5 4/8] block: add interface to toggle copy-on-read

[Kevin Wolf]

Am 28.11.2011 17:08, schrieb Stefan Hajnoczi:
> The bdrv_enable_copy_on_read()/bdrv_disable_copy_on_read() functions can
> be used to programmatically enable or disable copy-on-read for a block
> device.  Later patches add the actual copy-on-read logic.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Thanks, updated the block branch.

Kevin