It is called from qcow2_invalidate_cache in coroutine context, so always
load metadata from a coroutine.
---
 block/qcow2.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index cb081ea47f..b5de67d113 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -814,8 +814,8 @@ static int qcow2_update_options(BlockDriverState *bs, QDict *options,
     return ret;
 }
 
-static int qcow2_do_open(BlockDriverState *bs, QDict *options, int flags,
-                         Error **errp)
+static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
+                                      int flags, Error **errp)
 {
     BDRVQcow2State *s = bs->opaque;
     unsigned int len, i;
@@ -1161,8 +1161,6 @@ static int qcow2_do_open(BlockDriverState *bs, QDict *options, int flags,
         }
     }
 
-    /* Initialise locks */
-    qemu_co_mutex_init(&s->lock);
     bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP;
 
     /* Repair image if dirty */
@@ -1205,16 +1203,53 @@ static int qcow2_do_open(BlockDriverState *bs, QDict *options, int flags,
     return ret;
 }
 
+typedef struct QCow2OpenCo {
+    BlockDriverState *bs;
+    QDict *options;
+    int flags;
+    Error **errp;
+    int ret;
+} QCow2OpenCo;
+
+static void coroutine_fn qcow2_open_entry(void *opaque)
+{
+    QCow2OpenCo *qoc = opaque;
+    BDRVQcow2State *s = qoc->bs->opaque;
+
+    qemu_co_mutex_lock(&s->lock);
+    qoc->ret = qcow2_do_open(qoc->bs, qoc->options, qoc->flags, qoc->errp);
+    qemu_co_mutex_unlock(&s->lock);
+}
+
 static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
                       Error **errp)
 {
+    BDRVQcow2State *s = bs->opaque;
+    QCow2OpenCo qoc = {
+        .bs = bs,
+        .options = options,
+        .flags = flags,
+        .errp = errp,
+        .ret = -EINPROGRESS
+    };
+
     bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
                                false, errp);
     if (!bs->file) {
         return -EINVAL;
     }
 
-    return qcow2_do_open(bs, options, flags, errp);
+    /* Initialise locks */
+    qemu_co_mutex_init(&s->lock);
+
+    if (qemu_in_coroutine()) {
+        /* From bdrv_co_create.  */
+        qcow2_open_entry(&qoc);
+    } else {
+        qemu_coroutine_enter(qemu_coroutine_create(qcow2_open_entry, &qoc));
+        BDRV_POLL_WHILE(bs, qoc.ret == -EINPROGRESS);
+    }
+    return qoc.ret;
 }
 
 static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp)
-- 
2.13.0

Am 10.07.2017 um 18:58 hat Paolo Bonzini geschrieben:
> It is called from qcow2_invalidate_cache in coroutine context, so always
> load metadata from a coroutine.
> ---
>  block/qcow2.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 40 insertions(+), 5 deletions(-)

Missing S-o-b on patches 1 and 2?

These patches suggest that .bdrv_co_open() might be the next thing to
get rid of the coroutine wrappers again in driver code. But we'll always
have some wrappers in the drivers in an intermediate state, so doing
.bdrv_invalidate_cache first is fine.

Kevin

On 11/07/2017 10:28, Kevin Wolf wrote:
> Am 10.07.2017 um 18:58 hat Paolo Bonzini geschrieben:
>> It is called from qcow2_invalidate_cache in coroutine context, so always
>> load metadata from a coroutine.
>> ---
>>  block/qcow2.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 40 insertions(+), 5 deletions(-)
> 
> Missing S-o-b on patches 1 and 2?

Oops, yes.  Though I don't think these patches are for 2.10.  They are
on top of Stefan's bdrv_co_create series, by the way.

Paolo

> These patches suggest that .bdrv_co_open() might be the next thing to
> get rid of the coroutine wrappers again in driver code. But we'll always
> have some wrappers in the drivers in an intermediate state, so doing
> .bdrv_invalidate_cache first is fine.
> 
> Kevin
> 

Am 11.07.2017 um 10:34 hat Paolo Bonzini geschrieben:
> On 11/07/2017 10:28, Kevin Wolf wrote:
> > Am 10.07.2017 um 18:58 hat Paolo Bonzini geschrieben:
> >> It is called from qcow2_invalidate_cache in coroutine context, so always
> >> load metadata from a coroutine.
> >> ---
> >>  block/qcow2.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
> >>  1 file changed, 40 insertions(+), 5 deletions(-)
> > 
> > Missing S-o-b on patches 1 and 2?
> 
> Oops, yes.  Though I don't think these patches are for 2.10.  They are
> on top of Stefan's bdrv_co_create series, by the way.

Why not for 2.10? They look simple and self-contained. I haven't looked
at bdrv_co_create yet, but it shouldn't be much more complicated either.

Kevin

On 11/07/2017 11:07, Kevin Wolf wrote:
> Am 11.07.2017 um 10:34 hat Paolo Bonzini geschrieben:
>> On 11/07/2017 10:28, Kevin Wolf wrote:
>>> Am 10.07.2017 um 18:58 hat Paolo Bonzini geschrieben:
>>>> It is called from qcow2_invalidate_cache in coroutine context, so always
>>>> load metadata from a coroutine.
>>>> ---
>>>>  block/qcow2.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
>>>>  1 file changed, 40 insertions(+), 5 deletions(-)
>>>
>>> Missing S-o-b on patches 1 and 2?
>>
>> Oops, yes.  Though I don't think these patches are for 2.10.  They are
>> on top of Stefan's bdrv_co_create series, by the way.
> 
> Why not for 2.10?

Because soft freeze is next Monday? :)  But I can certainly send a v2
today or tomorrow.

> They look simple and self-contained. I haven't looked
> at bdrv_co_create yet, but it shouldn't be much more complicated either.

It's simpler, as it's only renaming the callback.

Paolo