[Qemu-devel] [PATCH v2 0/9] block: replace .bdrv_is_allocated() with .bdrv_co_is_allocated()

[Qemu-devel] [PATCH v2 0/9] block: replace .bdrv_is_allocated() with .bdrv_co_is_allocated()

[Stefan Hajnoczi]

The bdrv_is_allocated() interface is not suitable for use while the VM is
running.  It is a synchronous interface so it may block the running VM for
arbitrary amounts of time.  It also assumes it is the only block driver
operation and there is a risk that internal state could be corrupted if
asynchronous I/O is currently pending.

This patch series refactors the block layer so that the bdrv_is_allocated()
operation is performed internally in coroutine context.  We can then introduce
a new public bdrv_co_is_allocated() function that is suitable for use when the
VM is running.

Making these changes without breaking git bisect means we need to first
introduce BlockDriver .bdrv_co_is_allocated() and add wrapper code into
bdrv_is_allocated() so existing callers can use converted BlockDrivers.

After converting all block drivers there are no .bdrv_is_allocated() functions
left and the interface can be removed from BlockDriver.

Finally, we can add the new public bdrv_co_is_allocated() public interface and
turn the old bdrv_is_allocated() into a wrapper.

This series is a prerequisite for copy-on-read, which needs to check whether
sectors are allocated while the VM is running.

v2:
 * Rebased on kevin/block and resolved conflicts

Stefan Hajnoczi (9):
  block: use public bdrv_is_allocated() interface
  block: add .bdrv_co_is_allocated()
  qed: convert to .bdrv_co_is_allocated()
  block: convert qcow2, qcow2, and vmdk to .bdrv_co_is_allocated()
  vvfat: convert to .bdrv_co_is_allocated()
  vdi: convert to .bdrv_co_is_allocated()
  cow: convert to .bdrv_co_is_allocated()
  block: drop .bdrv_is_allocated() interface
  block: add bdrv_co_is_allocated() interface

 block.c       |   58 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 block.h       |    2 +
 block/cow.c   |    8 +++---
 block/qcow.c  |    8 ++++--
 block/qcow2.c |   13 +++++++----
 block/qed.c   |   15 ++++++++++---
 block/vdi.c   |    6 ++--
 block/vmdk.c  |    8 ++++--
 block/vvfat.c |    4 +-
 block_int.h   |    4 +-
 10 files changed, 94 insertions(+), 32 deletions(-)

-- 
1.7.7.1

[Qemu-devel] [PATCH v2 1/9] block: use public bdrv_is_allocated() interface

[Stefan Hajnoczi]

There is no need for bdrv_commit() to use the BlockDriver
.bdrv_is_allocated() interface directly.  Converting to the public
interface gives us the freedom to drop .bdrv_is_allocated() entirely in
favor of a new .bdrv_co_is_allocated() in the future.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/block.c b/block.c
index d565cde..ab68e53 100644
--- a/block.c
+++ b/block.c
@@ -1013,7 +1013,7 @@ int bdrv_commit(BlockDriverState *bs)
     buf = g_malloc(COMMIT_BUF_SECTORS * BDRV_SECTOR_SIZE);
 
     for (sector = 0; sector < total_sectors; sector += n) {
-        if (drv->bdrv_is_allocated(bs, sector, COMMIT_BUF_SECTORS, &n)) {
+        if (bdrv_is_allocated(bs, sector, COMMIT_BUF_SECTORS, &n)) {
 
             if (bdrv_read(bs, sector, buf, n) != 0) {
                 ret = -EIO;
-- 
1.7.7.1

[Qemu-devel] [PATCH v2 2/9] block: add .bdrv_co_is_allocated()

[Stefan Hajnoczi]

This patch adds the .bdrv_co_is_allocated() interface which is identical
to .bdrv_is_allocated() but runs in coroutine context.  Running in
coroutine context implies that other coroutines might be performing I/O
at the same time.   Therefore it must be safe to run while the following
BlockDriver functions are in-flight:

    .bdrv_co_readv()
    .bdrv_co_writev()
    .bdrv_co_flush()
    .bdrv_co_is_allocated()

The new .bdrv_co_is_allocated() interface is useful because it can be
used when a VM is running, whereas .bdrv_is_allocated() is a synchronous
interface that does not cope with parallel requests.

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

diff --git a/block.c b/block.c
index ab68e53..69989b0 100644
--- a/block.c
+++ b/block.c
@@ -1887,6 +1887,26 @@ int bdrv_has_zero_init(BlockDriverState *bs)
     return 1;
 }
 
+typedef struct BdrvCoIsAllocatedData {
+    BlockDriverState *bs;
+    int64_t sector_num;
+    int nb_sectors;
+    int *pnum;
+    int ret;
+    bool done;
+} BdrvCoIsAllocatedData;
+
+/* Coroutine wrapper for bdrv_is_allocated() */
+static void coroutine_fn bdrv_is_allocated_co_entry(void *opaque)
+{
+    BdrvCoIsAllocatedData *data = opaque;
+    BlockDriverState *bs = data->bs;
+
+    data->ret = bs->drv->bdrv_co_is_allocated(bs, data->sector_num,
+                                              data->nb_sectors, data->pnum);
+    data->done = true;
+}
+
 /*
  * Returns true iff the specified sector is present in the disk image. Drivers
  * not implementing the functionality are assumed to not support backing files,
@@ -1902,6 +1922,23 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
 	int *pnum)
 {
     int64_t n;
+    if (bs->drv->bdrv_co_is_allocated) {
+        Coroutine *co;
+        BdrvCoIsAllocatedData data = {
+            .bs = bs,
+            .sector_num = sector_num,
+            .nb_sectors = nb_sectors,
+            .pnum = pnum,
+            .done = false,
+        };
+
+        co = qemu_coroutine_create(bdrv_is_allocated_co_entry);
+        qemu_coroutine_enter(co, &data);
+        while (!data.done) {
+            qemu_aio_wait();
+        }
+        return data.ret;
+    }
     if (!bs->drv->bdrv_is_allocated) {
         if (sector_num >= bs->total_sectors) {
             *pnum = 0;
diff --git a/block_int.h b/block_int.h
index b537d5a..01f73ba 100644
--- a/block_int.h
+++ b/block_int.h
@@ -103,6 +103,8 @@ struct BlockDriver {
         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
     int coroutine_fn (*bdrv_co_discard)(BlockDriverState *bs,
         int64_t sector_num, int nb_sectors);
+    int coroutine_fn (*bdrv_co_is_allocated)(BlockDriverState *bs,
+        int64_t sector_num, int nb_sectors, int *pnum);
 
     /*
      * Flushes all data that was already written to the OS all the way down to
-- 
1.7.7.1

[Qemu-devel] [PATCH v2 3/9] qed: convert to .bdrv_co_is_allocated()

[Stefan Hajnoczi]

The bdrv_qed_is_allocated() function is a synchronous wrapper around
qed_find_cluster(), which performs the cluster lookup.  In order to
convert the synchronous function to a coroutine function we yield
instead of using qemu_aio_wait().  Note that QED's cache is already safe
for parallel requests so no locking is needed.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block/qed.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index d032a45..8a05445 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -651,6 +651,7 @@ static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options)
 }
 
 typedef struct {
+    Coroutine *co;
     int is_allocated;
     int *pnum;
 } QEDIsAllocatedCB;
@@ -660,10 +661,14 @@ static void qed_is_allocated_cb(void *opaque, int ret, uint64_t offset, size_t l
     QEDIsAllocatedCB *cb = opaque;
     *cb->pnum = len / BDRV_SECTOR_SIZE;
     cb->is_allocated = (ret == QED_CLUSTER_FOUND || ret == QED_CLUSTER_ZERO);
+    if (cb->co) {
+        qemu_coroutine_enter(cb->co, NULL);
+    }
 }
 
-static int bdrv_qed_is_allocated(BlockDriverState *bs, int64_t sector_num,
-                                  int nb_sectors, int *pnum)
+static int coroutine_fn bdrv_qed_co_is_allocated(BlockDriverState *bs,
+                                                 int64_t sector_num,
+                                                 int nb_sectors, int *pnum)
 {
     BDRVQEDState *s = bs->opaque;
     uint64_t pos = (uint64_t)sector_num * BDRV_SECTOR_SIZE;
@@ -676,8 +681,10 @@ static int bdrv_qed_is_allocated(BlockDriverState *bs, int64_t sector_num,
 
     qed_find_cluster(s, &request, pos, len, qed_is_allocated_cb, &cb);
 
+    /* Now sleep if the callback wasn't invoked immediately */
     while (cb.is_allocated == -1) {
-        qemu_aio_wait();
+        cb.co = qemu_coroutine_self();
+        qemu_coroutine_yield();
     }
 
     qed_unref_l2_cache_entry(request.l2_table);
@@ -1475,7 +1482,7 @@ static BlockDriver bdrv_qed = {
     .bdrv_open                = bdrv_qed_open,
     .bdrv_close               = bdrv_qed_close,
     .bdrv_create              = bdrv_qed_create,
-    .bdrv_is_allocated        = bdrv_qed_is_allocated,
+    .bdrv_co_is_allocated     = bdrv_qed_co_is_allocated,
     .bdrv_make_empty          = bdrv_qed_make_empty,
     .bdrv_aio_readv           = bdrv_qed_aio_readv,
     .bdrv_aio_writev          = bdrv_qed_aio_writev,
-- 
1.7.7.1

[Qemu-devel] [PATCH v2 4/9] block: convert qcow2, qcow2, and vmdk to .bdrv_co_is_allocated()

[Stefan Hajnoczi]

The qcow2, qcow, and vmdk block drivers are based on coroutines.  They have a
coroutine mutex which protects internal state.  We can convert the
.bdrv_is_allocated() function to .bdrv_co_is_allocated() by holding the mutex
around the cluster lookup operation.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block/qcow.c  |    8 +++++---
 block/qcow2.c |   13 ++++++++-----
 block/vmdk.c  |    8 +++++---
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 5314697..a7049e6 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -360,14 +360,16 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
     return cluster_offset;
 }
 
-static int qcow_is_allocated(BlockDriverState *bs, int64_t sector_num,
-                             int nb_sectors, int *pnum)
+static int coroutine_fn qcow_co_is_allocated(BlockDriverState *bs,
+        int64_t sector_num, int nb_sectors, int *pnum)
 {
     BDRVQcowState *s = bs->opaque;
     int index_in_cluster, n;
     uint64_t cluster_offset;
 
+    qemu_co_mutex_lock(&s->lock);
     cluster_offset = get_cluster_offset(bs, sector_num << 9, 0, 0, 0, 0);
+    qemu_co_mutex_unlock(&s->lock);
     index_in_cluster = sector_num & (s->cluster_sectors - 1);
     n = s->cluster_sectors - index_in_cluster;
     if (n > nb_sectors)
@@ -832,7 +834,7 @@ static BlockDriver bdrv_qcow = {
     .bdrv_co_readv          = qcow_co_readv,
     .bdrv_co_writev         = qcow_co_writev,
     .bdrv_co_flush_to_disk  = qcow_co_flush,
-    .bdrv_is_allocated      = qcow_is_allocated,
+    .bdrv_co_is_allocated   = qcow_co_is_allocated,
 
     .bdrv_set_key           = qcow_set_key,
     .bdrv_make_empty        = qcow_make_empty,
diff --git a/block/qcow2.c b/block/qcow2.c
index a56b011..eab35d1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -342,16 +342,19 @@ static int qcow2_set_key(BlockDriverState *bs, const char *key)
     return 0;
 }
 
-static int qcow2_is_allocated(BlockDriverState *bs, int64_t sector_num,
-                              int nb_sectors, int *pnum)
+static int coroutine_fn qcow2_co_is_allocated(BlockDriverState *bs,
+        int64_t sector_num, int nb_sectors, int *pnum)
 {
+    BDRVQcowState *s = bs->opaque;
     uint64_t cluster_offset;
     int ret;
 
     *pnum = nb_sectors;
-    /* FIXME We can get errors here, but the bdrv_is_allocated interface can't
-     * pass them on today */
+    /* FIXME We can get errors here, but the bdrv_co_is_allocated interface
+     * can't pass them on today */
+    qemu_co_mutex_lock(&s->lock);
     ret = qcow2_get_cluster_offset(bs, sector_num << 9, pnum, &cluster_offset);
+    qemu_co_mutex_unlock(&s->lock);
     if (ret < 0) {
         *pnum = 0;
     }
@@ -1244,7 +1247,7 @@ static BlockDriver bdrv_qcow2 = {
     .bdrv_open          = qcow2_open,
     .bdrv_close         = qcow2_close,
     .bdrv_create        = qcow2_create,
-    .bdrv_is_allocated  = qcow2_is_allocated,
+    .bdrv_co_is_allocated = qcow2_co_is_allocated,
     .bdrv_set_key       = qcow2_set_key,
     .bdrv_make_empty    = qcow2_make_empty,
 
diff --git a/block/vmdk.c b/block/vmdk.c
index e53a2f0..e3631fc 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -852,8 +852,8 @@ static VmdkExtent *find_extent(BDRVVmdkState *s,
     return NULL;
 }
 
-static int vmdk_is_allocated(BlockDriverState *bs, int64_t sector_num,
-                             int nb_sectors, int *pnum)
+static int coroutine_fn vmdk_co_is_allocated(BlockDriverState *bs,
+        int64_t sector_num, int nb_sectors, int *pnum)
 {
     BDRVVmdkState *s = bs->opaque;
     int64_t index_in_cluster, n, ret;
@@ -864,8 +864,10 @@ static int vmdk_is_allocated(BlockDriverState *bs, int64_t sector_num,
     if (!extent) {
         return 0;
     }
+    qemu_co_mutex_lock(&s->lock);
     ret = get_cluster_offset(bs, extent, NULL,
                             sector_num * 512, 0, &offset);
+    qemu_co_mutex_unlock(&s->lock);
     /* get_cluster_offset returning 0 means success */
     ret = !ret;
 
@@ -1582,7 +1584,7 @@ static BlockDriver bdrv_vmdk = {
     .bdrv_close     = vmdk_close,
     .bdrv_create    = vmdk_create,
     .bdrv_co_flush_to_disk  = vmdk_co_flush,
-    .bdrv_is_allocated      = vmdk_is_allocated,
+    .bdrv_co_is_allocated   = vmdk_co_is_allocated,
     .bdrv_get_allocated_file_size  = vmdk_get_allocated_file_size,
 
     .create_options = vmdk_create_options,
-- 
1.7.7.1

[Qemu-devel] [PATCH v2 5/9] vvfat: convert to .bdrv_co_is_allocated()

[Stefan Hajnoczi]

It is trivial to switch from the synchronous .bdrv_is_allocated()
interface to .bdrv_co_is_allocated() since vvfat_is_allocated() does not
block.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block/vvfat.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index 131680f..11dab50 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2746,7 +2746,7 @@ static coroutine_fn int vvfat_co_write(BlockDriverState *bs, int64_t sector_num,
     return ret;
 }
 
-static int vvfat_is_allocated(BlockDriverState *bs,
+static int coroutine_fn vvfat_co_is_allocated(BlockDriverState *bs,
 	int64_t sector_num, int nb_sectors, int* n)
 {
     BDRVVVFATState* s = bs->opaque;
@@ -2838,7 +2838,7 @@ static BlockDriver bdrv_vvfat = {
     .bdrv_read          = vvfat_co_read,
     .bdrv_write         = vvfat_co_write,
     .bdrv_close		= vvfat_close,
-    .bdrv_is_allocated	= vvfat_is_allocated,
+    .bdrv_co_is_allocated = vvfat_co_is_allocated,
     .protocol_name	= "fat",
 };
 
-- 
1.7.7.1

[Qemu-devel] [PATCH v2 6/9] vdi: convert to .bdrv_co_is_allocated()

[Stefan Hajnoczi]

It is trivial to switch from the synchronous .bdrv_is_allocated()
interface to .bdrv_co_is_allocated() since vdi_is_allocated() does not
block.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block/vdi.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/vdi.c b/block/vdi.c
index 684a4a8..e852456 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -463,8 +463,8 @@ static int vdi_open(BlockDriverState *bs, int flags)
     return -1;
 }
 
-static int vdi_is_allocated(BlockDriverState *bs, int64_t sector_num,
-                             int nb_sectors, int *pnum)
+static int coroutine_fn vdi_co_is_allocated(BlockDriverState *bs,
+        int64_t sector_num, int nb_sectors, int *pnum)
 {
     /* TODO: Check for too large sector_num (in bdrv_is_allocated or here). */
     BDRVVdiState *s = (BDRVVdiState *)bs->opaque;
@@ -981,7 +981,7 @@ static BlockDriver bdrv_vdi = {
     .bdrv_close = vdi_close,
     .bdrv_create = vdi_create,
     .bdrv_co_flush_to_disk = vdi_co_flush,
-    .bdrv_is_allocated = vdi_is_allocated,
+    .bdrv_co_is_allocated = vdi_co_is_allocated,
     .bdrv_make_empty = vdi_make_empty,
 
     .bdrv_aio_readv = vdi_aio_readv,
-- 
1.7.7.1

[Qemu-devel] [PATCH v2 7/9] cow: convert to .bdrv_co_is_allocated()

[Stefan Hajnoczi]

The cow block driver does not keep internal state for cluster lookups.
This means it is safe to perform cluster lookups in coroutine context
without risk of race conditions that corrupt internal state.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block/cow.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/cow.c b/block/cow.c
index 9858d71..7ae887b 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -132,8 +132,8 @@ static inline int is_bit_set(BlockDriverState *bs, int64_t bitnum)
 /* Return true if first block has been changed (ie. current version is
  * in COW file).  Set the number of continuous blocks for which that
  * is true. */
-static int cow_is_allocated(BlockDriverState *bs, int64_t sector_num,
-        int nb_sectors, int *num_same)
+static int coroutine_fn cow_co_is_allocated(BlockDriverState *bs,
+        int64_t sector_num, int nb_sectors, int *num_same)
 {
     int changed;
 
@@ -178,7 +178,7 @@ static int cow_read(BlockDriverState *bs, int64_t sector_num,
     int ret, n;
 
     while (nb_sectors > 0) {
-        if (cow_is_allocated(bs, sector_num, nb_sectors, &n)) {
+        if (bdrv_is_allocated(bs, sector_num, nb_sectors, &n)) {
             ret = bdrv_pread(bs->file,
                         s->cow_sectors_offset + sector_num * 512,
                         buf, n * 512);
@@ -335,7 +335,7 @@ static BlockDriver bdrv_cow = {
     .bdrv_read              = cow_co_read,
     .bdrv_write             = cow_co_write,
     .bdrv_co_flush_to_disk  = cow_co_flush,
-    .bdrv_is_allocated      = cow_is_allocated,
+    .bdrv_co_is_allocated   = cow_co_is_allocated,
 
     .create_options = cow_create_options,
 };
-- 
1.7.7.1

[Qemu-devel] [PATCH v2 8/9] block: drop .bdrv_is_allocated() interface

[Stefan Hajnoczi]

Now that all block drivers have been converted to
.bdrv_co_is_allocated() we can drop .bdrv_is_allocated().

Note that the public bdrv_is_allocated() interface is still available
but is in fact a synchronous wrapper around .bdrv_co_is_allocated().

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

diff --git a/block.c b/block.c
index 69989b0..eda5271 100644
--- a/block.c
+++ b/block.c
@@ -1921,25 +1921,8 @@ static void coroutine_fn bdrv_is_allocated_co_entry(void *opaque)
 int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
 	int *pnum)
 {
-    int64_t n;
-    if (bs->drv->bdrv_co_is_allocated) {
-        Coroutine *co;
-        BdrvCoIsAllocatedData data = {
-            .bs = bs,
-            .sector_num = sector_num,
-            .nb_sectors = nb_sectors,
-            .pnum = pnum,
-            .done = false,
-        };
-
-        co = qemu_coroutine_create(bdrv_is_allocated_co_entry);
-        qemu_coroutine_enter(co, &data);
-        while (!data.done) {
-            qemu_aio_wait();
-        }
-        return data.ret;
-    }
-    if (!bs->drv->bdrv_is_allocated) {
+    if (!bs->drv->bdrv_co_is_allocated) {
+        int64_t n;
         if (sector_num >= bs->total_sectors) {
             *pnum = 0;
             return 0;
@@ -1948,7 +1931,22 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
         *pnum = (n < nb_sectors) ? (n) : (nb_sectors);
         return 1;
     }
-    return bs->drv->bdrv_is_allocated(bs, sector_num, nb_sectors, pnum);
+
+    Coroutine *co;
+    BdrvCoIsAllocatedData data = {
+        .bs = bs,
+        .sector_num = sector_num,
+        .nb_sectors = nb_sectors,
+        .pnum = pnum,
+        .done = false,
+    };
+
+    co = qemu_coroutine_create(bdrv_is_allocated_co_entry);
+    qemu_coroutine_enter(co, &data);
+    while (!data.done) {
+        qemu_aio_wait();
+    }
+    return data.ret;
 }
 
 void bdrv_mon_event(const BlockDriverState *bdrv,
diff --git a/block_int.h b/block_int.h
index 01f73ba..f9e2c9a 100644
--- a/block_int.h
+++ b/block_int.h
@@ -80,8 +80,6 @@ struct BlockDriver {
                       const uint8_t *buf, int nb_sectors);
     void (*bdrv_close)(BlockDriverState *bs);
     int (*bdrv_create)(const char *filename, QEMUOptionParameter *options);
-    int (*bdrv_is_allocated)(BlockDriverState *bs, int64_t sector_num,
-                             int nb_sectors, int *pnum);
     int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
     int (*bdrv_make_empty)(BlockDriverState *bs);
     /* aio */
-- 
1.7.7.1

[Qemu-devel] [PATCH v2 9/9] block: add bdrv_co_is_allocated() interface

[Stefan Hajnoczi]

This patch introduces the public bdrv_co_is_allocated() interface which
can be used to query image allocation status while the VM is running.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c |   37 ++++++++++++++++++++++++-------------
 block.h |    2 ++
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/block.c b/block.c
index eda5271..0df7eb9 100644
--- a/block.c
+++ b/block.c
@@ -1896,17 +1896,6 @@ typedef struct BdrvCoIsAllocatedData {
     bool done;
 } BdrvCoIsAllocatedData;
 
-/* Coroutine wrapper for bdrv_is_allocated() */
-static void coroutine_fn bdrv_is_allocated_co_entry(void *opaque)
-{
-    BdrvCoIsAllocatedData *data = opaque;
-    BlockDriverState *bs = data->bs;
-
-    data->ret = bs->drv->bdrv_co_is_allocated(bs, data->sector_num,
-                                              data->nb_sectors, data->pnum);
-    data->done = true;
-}
-
 /*
  * Returns true iff the specified sector is present in the disk image. Drivers
  * not implementing the functionality are assumed to not support backing files,
@@ -1918,8 +1907,8 @@ static void coroutine_fn bdrv_is_allocated_co_entry(void *opaque)
  *
  * 'nb_sectors' is the max value 'pnum' should be set to.
  */
-int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
-	int *pnum)
+int coroutine_fn bdrv_co_is_allocated(BlockDriverState *bs, int64_t sector_num,
+                                      int nb_sectors, int *pnum)
 {
     if (!bs->drv->bdrv_co_is_allocated) {
         int64_t n;
@@ -1932,6 +1921,28 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
         return 1;
     }
 
+    return bs->drv->bdrv_co_is_allocated(bs, sector_num, nb_sectors, pnum);
+}
+
+/* Coroutine wrapper for bdrv_is_allocated() */
+static void coroutine_fn bdrv_is_allocated_co_entry(void *opaque)
+{
+    BdrvCoIsAllocatedData *data = opaque;
+    BlockDriverState *bs = data->bs;
+
+    data->ret = bdrv_co_is_allocated(bs, data->sector_num, data->nb_sectors,
+                                     data->pnum);
+    data->done = true;
+}
+
+/*
+ * Synchronous wrapper around bdrv_co_is_allocated().
+ *
+ * See bdrv_co_is_allocated() for details.
+ */
+int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
+                      int *pnum)
+{
     Coroutine *co;
     BdrvCoIsAllocatedData data = {
         .bs = bs,
diff --git a/block.h b/block.h
index 9d8909b..ad8dd48 100644
--- a/block.h
+++ b/block.h
@@ -143,6 +143,8 @@ int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num,
     int nb_sectors, QEMUIOVector *qiov);
 int coroutine_fn bdrv_co_writev(BlockDriverState *bs, int64_t sector_num,
     int nb_sectors, QEMUIOVector *qiov);
+int coroutine_fn bdrv_co_is_allocated(BlockDriverState *bs, int64_t sector_num,
+    int nb_sectors, int *pnum);
 int bdrv_truncate(BlockDriverState *bs, int64_t offset);
 int64_t bdrv_getlength(BlockDriverState *bs);
 int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
-- 
1.7.7.1

Re: [Qemu-devel] [PATCH v2 0/9] block: replace .bdrv_is_allocated() with .bdrv_co_is_allocated()

[Kevin Wolf]

Am 14.11.2011 13:44, schrieb Stefan Hajnoczi:
> The bdrv_is_allocated() interface is not suitable for use while the VM is
> running.  It is a synchronous interface so it may block the running VM for
> arbitrary amounts of time.  It also assumes it is the only block driver
> operation and there is a risk that internal state could be corrupted if
> asynchronous I/O is currently pending.
> 
> This patch series refactors the block layer so that the bdrv_is_allocated()
> operation is performed internally in coroutine context.  We can then introduce
> a new public bdrv_co_is_allocated() function that is suitable for use when the
> VM is running.
> 
> Making these changes without breaking git bisect means we need to first
> introduce BlockDriver .bdrv_co_is_allocated() and add wrapper code into
> bdrv_is_allocated() so existing callers can use converted BlockDrivers.
> 
> After converting all block drivers there are no .bdrv_is_allocated() functions
> left and the interface can be removed from BlockDriver.
> 
> Finally, we can add the new public bdrv_co_is_allocated() public interface and
> turn the old bdrv_is_allocated() into a wrapper.
> 
> This series is a prerequisite for copy-on-read, which needs to check whether
> sectors are allocated while the VM is running.
> 
> v2:
>  * Rebased on kevin/block and resolved conflicts
> 
> Stefan Hajnoczi (9):
>   block: use public bdrv_is_allocated() interface
>   block: add .bdrv_co_is_allocated()
>   qed: convert to .bdrv_co_is_allocated()
>   block: convert qcow2, qcow2, and vmdk to .bdrv_co_is_allocated()
>   vvfat: convert to .bdrv_co_is_allocated()
>   vdi: convert to .bdrv_co_is_allocated()
>   cow: convert to .bdrv_co_is_allocated()
>   block: drop .bdrv_is_allocated() interface
>   block: add bdrv_co_is_allocated() interface
> 
>  block.c       |   58 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  block.h       |    2 +
>  block/cow.c   |    8 +++---
>  block/qcow.c  |    8 ++++--
>  block/qcow2.c |   13 +++++++----
>  block/qed.c   |   15 ++++++++++---
>  block/vdi.c   |    6 ++--
>  block/vmdk.c  |    8 ++++--
>  block/vvfat.c |    4 +-
>  block_int.h   |    4 +-
>  10 files changed, 94 insertions(+), 32 deletions(-)

Thanks, applied all to the block branch (for 1.1)

Kevin