[Qemu-devel] [RFC PATCH 1/1] dataplane: alternative approach to locking

["Denis V. Lunev" <den@openvz.org>]

What about this? Is it simple enough for you keeping lock around
qemu_fopen_bdrv/qemu_fclose as suggested in patch 1?

This is not tested at all, just sent as an idea for a discussion.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
---
 block.c               | 17 +++++++++++++++++
 include/block/block.h |  2 ++
 migration/savevm.c    | 23 +++++++++++++++--------
 monitor.c             |  2 +-
 4 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/block.c b/block.c
index 044897e..d376ec2 100644
--- a/block.c
+++ b/block.c
@@ -2741,6 +2741,23 @@ BlockDriverState *bdrv_next(BlockDriverState *bs)
     return QTAILQ_NEXT(bs, device_list);
 }
 
+BlockDriverState *bdrv_next_lock(BlockDriverState *bs)
+{
+    if (bs != NULL) {
+        aio_context_release(bdrv_get_aio_context(bs));
+    }
+    bs = bdrv_next(bs);
+    if (bs != NULL) {
+        aio_context_acquire(bdrv_get_aio_context(bs));
+    }
+    return bs;
+}
+
+void bdrv_unlock(BlockDriverState *bs)
+{
+    aio_context_release(bdrv_get_aio_context(bs));
+}
+
 const char *bdrv_get_node_name(const BlockDriverState *bs)
 {
     return bs->node_name;
diff --git a/include/block/block.h b/include/block/block.h
index 610db92..b29dd5b 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -401,6 +401,8 @@ BlockDriverState *bdrv_lookup_bs(const char *device,
 bool bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base);
 BlockDriverState *bdrv_next_node(BlockDriverState *bs);
 BlockDriverState *bdrv_next(BlockDriverState *bs);
+BlockDriverState *bdrv_next_lock(BlockDriverState *bs);
+void bdrv_unlock(BlockDriverState *bs);
 int bdrv_is_encrypted(BlockDriverState *bs);
 int bdrv_key_required(BlockDriverState *bs);
 int bdrv_set_key(BlockDriverState *bs, const char *key);
diff --git a/migration/savevm.c b/migration/savevm.c
index dbcc39a..cf06a10 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1240,8 +1240,9 @@ out:
 static BlockDriverState *find_vmstate_bs(void)
 {
     BlockDriverState *bs = NULL;
-    while ((bs = bdrv_next(bs))) {
+    while ((bs = bdrv_next_lock(bs))) {
         if (bdrv_can_snapshot(bs)) {
+            bdrv_unlock(bs);
             return bs;
         }
     }
@@ -1258,11 +1259,12 @@ static int del_existing_snapshots(Monitor *mon, const char *name)
     Error *err = NULL;
 
     bs = NULL;
-    while ((bs = bdrv_next(bs))) {
+    while ((bs = bdrv_next_lock(bs))) {
         if (bdrv_can_snapshot(bs) &&
             bdrv_snapshot_find(bs, snapshot, name) >= 0) {
             bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
             if (err) {
+                bdrv_unlock(bs);
                 monitor_printf(mon,
                                "Error while deleting snapshot on device '%s':"
                                " %s\n",
@@ -1292,13 +1294,14 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
 
     /* Verify if there is a device that doesn't support snapshots and is writable */
     bs = NULL;
-    while ((bs = bdrv_next(bs))) {
+    while ((bs = bdrv_next_lock(bs))) {
 
         if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
             continue;
         }
 
         if (!bdrv_can_snapshot(bs)) {
+            bdrv_unlock(bs);
             monitor_printf(mon, "Device '%s' is writable but does not support snapshots.\n",
                                bdrv_get_device_name(bs));
             return;
@@ -1365,7 +1368,7 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
     /* create the snapshots */
 
     bs1 = NULL;
-    while ((bs1 = bdrv_next(bs1))) {
+    while ((bs1 = bdrv_next_lock(bs1))) {
         if (bdrv_can_snapshot(bs1)) {
             /* Write VM state size only to the image that contains the state */
             sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);
@@ -1436,13 +1439,14 @@ int load_vmstate(const char *name)
     /* Verify if there is any device that doesn't support snapshots and is
     writable and check if the requested snapshot is available too. */
     bs = NULL;
-    while ((bs = bdrv_next(bs))) {
+    while ((bs = bdrv_next_lock(bs))) {
 
         if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
             continue;
         }
 
         if (!bdrv_can_snapshot(bs)) {
+            bdrv_unlock(bs);
             error_report("Device '%s' is writable but does not support snapshots.",
                                bdrv_get_device_name(bs));
             return -ENOTSUP;
@@ -1450,6 +1454,7 @@ int load_vmstate(const char *name)
 
         ret = bdrv_snapshot_find(bs, &sn, name);
         if (ret < 0) {
+            bdrv_unlock(bs);
             error_report("Device '%s' does not have the requested snapshot '%s'",
                            bdrv_get_device_name(bs), name);
             return ret;
@@ -1460,10 +1465,11 @@ int load_vmstate(const char *name)
     bdrv_drain_all();
 
     bs = NULL;
-    while ((bs = bdrv_next(bs))) {
+    while ((bs = bdrv_next_lock(bs))) {
         if (bdrv_can_snapshot(bs)) {
             ret = bdrv_snapshot_goto(bs, name);
             if (ret < 0) {
+                bdrv_unlock(bs);
                 error_report("Error %d while activating snapshot '%s' on '%s'",
                              ret, name, bdrv_get_device_name(bs));
                 return ret;
@@ -1504,7 +1510,7 @@ void hmp_delvm(Monitor *mon, const QDict *qdict)
     }
 
     bs = NULL;
-    while ((bs = bdrv_next(bs))) {
+    while ((bs = bdrv_next_lock(bs))) {
         if (bdrv_can_snapshot(bs)) {
             err = NULL;
             bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
@@ -1552,10 +1558,11 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
         available = 1;
         bs1 = NULL;
 
-        while ((bs1 = bdrv_next(bs1))) {
+        while ((bs1 = bdrv_next_lock(bs1))) {
             if (bdrv_can_snapshot(bs1) && bs1 != bs) {
                 ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str);
                 if (ret < 0) {
+                    bdrv_unlock(bs);
                     available = 0;
                     break;
                 }
diff --git a/monitor.c b/monitor.c
index 301a143..ea1a917 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3374,7 +3374,7 @@ static void vm_completion(ReadLineState *rs, const char *str)
 
     len = strlen(str);
     readline_set_completion_index(rs, len);
-    while ((bs = bdrv_next(bs))) {
+    while ((bs = bdrv_next_lock(bs))) {
         SnapshotInfoList *snapshots, *snapshot;
 
         if (!bdrv_can_snapshot(bs)) {
-- 
2.1.4