* [Qemu-devel] [PATCH 5/7] block: convert bdrv_invalidate_cache callback to coroutine_fn
2018-01-17 11:00 [Qemu-devel] [PATCH 0/7] Call check and invalidate_cache from coroutine context Paolo Bonzini
@ 2018-01-17 11:00 ` Paolo Bonzini
0 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2018-01-17 11:00 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block, mreitz
QED's bdrv_invalidate_cache implementation would like to reuse functions
that acquire/release the metadata locks. Call it from coroutine context
to simplify the logic.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 41 +++++++++++++++++++++++++++++++++++++----
block/iscsi.c | 6 +++---
block/nfs.c | 6 +++---
block/qcow2.c | 7 +++++--
block/qed.c | 13 +++++--------
block/rbd.c | 6 +++---
include/block/block_int.h | 3 ++-
7 files changed, 58 insertions(+), 24 deletions(-)
diff --git a/block.c b/block.c
index cf33135..6ba7471 100644
--- a/block.c
+++ b/block.c
@@ -4214,7 +4214,8 @@ void bdrv_init_with_whitelist(void)
bdrv_init();
}
-void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
+static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
BdrvChild *child, *parent;
uint64_t perm, shared_perm;
@@ -4230,7 +4231,7 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
}
QLIST_FOREACH(child, &bs->children, next) {
- bdrv_invalidate_cache(child->bs, &local_err);
+ bdrv_co_invalidate_cache(child->bs, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
@@ -4260,8 +4261,8 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
}
bdrv_set_perm(bs, perm, shared_perm);
- if (bs->drv->bdrv_invalidate_cache) {
- bs->drv->bdrv_invalidate_cache(bs, &local_err);
+ if (bs->drv->bdrv_co_invalidate_cache) {
+ bs->drv->bdrv_co_invalidate_cache(bs, &local_err);
if (local_err) {
bs->open_flags |= BDRV_O_INACTIVE;
error_propagate(errp, local_err);
@@ -4287,6 +4288,38 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
}
}
+typedef struct InvalidateCacheCo {
+ BlockDriverState *bs;
+ Error **errp;
+ bool done;
+} InvalidateCacheCo;
+
+static void coroutine_fn bdrv_invalidate_cache_co_entry(void *opaque)
+{
+ InvalidateCacheCo *ico = opaque;
+ bdrv_co_invalidate_cache(ico->bs, ico->errp);
+ ico->done = true;
+}
+
+void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
+{
+ Coroutine *co;
+ InvalidateCacheCo ico = {
+ .bs = bs,
+ .done = false,
+ .errp = errp
+ };
+
+ if (qemu_in_coroutine()) {
+ /* Fast-path if already in coroutine context */
+ bdrv_invalidate_cache_co_entry(&ico);
+ } else {
+ co = qemu_coroutine_create(bdrv_invalidate_cache_co_entry, &ico);
+ qemu_coroutine_enter(co);
+ BDRV_POLL_WHILE(bs, !ico.done);
+ }
+}
+
void bdrv_invalidate_cache_all(Error **errp)
{
BlockDriverState *bs;
diff --git a/block/iscsi.c b/block/iscsi.c
index 5f358f9..9b70899 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2164,8 +2164,8 @@ static int iscsi_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
return 0;
}
-static void iscsi_invalidate_cache(BlockDriverState *bs,
- Error **errp)
+static void coroutine_fn iscsi_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
IscsiLun *iscsilun = bs->opaque;
iscsi_allocmap_invalidate(iscsilun);
@@ -2196,7 +2196,7 @@ static BlockDriver bdrv_iscsi = {
.create_opts = &iscsi_create_opts,
.bdrv_reopen_prepare = iscsi_reopen_prepare,
.bdrv_reopen_commit = iscsi_reopen_commit,
- .bdrv_invalidate_cache = iscsi_invalidate_cache,
+ .bdrv_co_invalidate_cache = iscsi_co_invalidate_cache,
.bdrv_getlength = iscsi_getlength,
.bdrv_get_info = iscsi_get_info,
diff --git a/block/nfs.c b/block/nfs.c
index 8029dc2..d7f9a6c 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -876,8 +876,8 @@ static void nfs_refresh_filename(BlockDriverState *bs, QDict *options)
}
#ifdef LIBNFS_FEATURE_PAGECACHE
-static void nfs_invalidate_cache(BlockDriverState *bs,
- Error **errp)
+static void coroutine_fn nfs_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
NFSClient *client = bs->opaque;
nfs_pagecache_invalidate(client->context, client->fh);
@@ -910,7 +910,7 @@ static BlockDriver bdrv_nfs = {
.bdrv_refresh_filename = nfs_refresh_filename,
#ifdef LIBNFS_FEATURE_PAGECACHE
- .bdrv_invalidate_cache = nfs_invalidate_cache,
+ .bdrv_co_invalidate_cache = nfs_co_invalidate_cache,
#endif
};
diff --git a/block/qcow2.c b/block/qcow2.c
index 7c11516..e886a44 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2109,7 +2109,8 @@ static void qcow2_close(BlockDriverState *bs)
qcow2_free_snapshots(bs);
}
-static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
+static void coroutine_fn qcow2_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
BDRVQcow2State *s = bs->opaque;
int flags = s->flags;
@@ -2132,7 +2133,9 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
options = qdict_clone_shallow(bs->options);
flags &= ~BDRV_O_INACTIVE;
+ qemu_co_mutex_lock(&s->lock);
ret = qcow2_do_open(bs, options, flags, &local_err);
+ qemu_co_mutex_unlock(&s->lock);
QDECREF(options);
if (local_err) {
error_propagate(errp, local_err);
@@ -4386,7 +4389,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_change_backing_file = qcow2_change_backing_file,
.bdrv_refresh_limits = qcow2_refresh_limits,
- .bdrv_invalidate_cache = qcow2_invalidate_cache,
+ .bdrv_co_invalidate_cache = qcow2_co_invalidate_cache,
.bdrv_inactivate = qcow2_inactivate,
.create_opts = &qcow2_create_opts,
diff --git a/block/qed.c b/block/qed.c
index 3e1f4bc..5cf519c 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1550,7 +1550,8 @@ static int bdrv_qed_change_backing_file(BlockDriverState *bs,
return ret;
}
-static void bdrv_qed_invalidate_cache(BlockDriverState *bs, Error **errp)
+static void coroutine_fn bdrv_qed_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
BDRVQEDState *s = bs->opaque;
Error *local_err = NULL;
@@ -1559,13 +1560,9 @@ static void bdrv_qed_invalidate_cache(BlockDriverState *bs, Error **errp)
bdrv_qed_close(bs);
bdrv_qed_init_state(bs);
- if (qemu_in_coroutine()) {
- qemu_co_mutex_lock(&s->table_lock);
- }
+ qemu_co_mutex_lock(&s->table_lock);
ret = bdrv_qed_do_open(bs, NULL, bs->open_flags, &local_err);
- if (qemu_in_coroutine()) {
- qemu_co_mutex_unlock(&s->table_lock);
- }
+ qemu_co_mutex_unlock(&s->table_lock);
if (local_err) {
error_propagate(errp, local_err);
error_prepend(errp, "Could not reopen qed layer: ");
@@ -1640,7 +1637,7 @@ static BlockDriver bdrv_qed = {
.bdrv_get_info = bdrv_qed_get_info,
.bdrv_refresh_limits = bdrv_qed_refresh_limits,
.bdrv_change_backing_file = bdrv_qed_change_backing_file,
- .bdrv_invalidate_cache = bdrv_qed_invalidate_cache,
+ .bdrv_co_invalidate_cache = bdrv_qed_co_invalidate_cache,
.bdrv_check = bdrv_qed_check,
.bdrv_detach_aio_context = bdrv_qed_detach_aio_context,
.bdrv_attach_aio_context = bdrv_qed_attach_aio_context,
diff --git a/block/rbd.c b/block/rbd.c
index 67ac2ee..9632355 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1090,8 +1090,8 @@ static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs,
#endif
#ifdef LIBRBD_SUPPORTS_INVALIDATE
-static void qemu_rbd_invalidate_cache(BlockDriverState *bs,
- Error **errp)
+static void coroutine_fn qemu_rbd_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
BDRVRBDState *s = bs->opaque;
int r = rbd_invalidate_cache(s->image);
@@ -1157,7 +1157,7 @@ static BlockDriver bdrv_rbd = {
.bdrv_snapshot_list = qemu_rbd_snap_list,
.bdrv_snapshot_goto = qemu_rbd_snap_rollback,
#ifdef LIBRBD_SUPPORTS_INVALIDATE
- .bdrv_invalidate_cache = qemu_rbd_invalidate_cache,
+ .bdrv_co_invalidate_cache = qemu_rbd_co_invalidate_cache,
#endif
};
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 05f498f..3481da1 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -218,7 +218,8 @@ struct BlockDriver {
/*
* Invalidate any cached meta-data.
*/
- void (*bdrv_invalidate_cache)(BlockDriverState *bs, Error **errp);
+ void coroutine_fn (*bdrv_co_invalidate_cache)(BlockDriverState *bs,
+ Error **errp);
int (*bdrv_inactivate)(BlockDriverState *bs);
/*
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 5/7] block: convert bdrv_invalidate_cache callback to coroutine_fn
2018-01-17 11:25 [Qemu-devel] [PATCH v2 " Paolo Bonzini
@ 2018-01-17 11:25 ` Paolo Bonzini
0 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2018-01-17 11:25 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block, mreitz
QED's bdrv_invalidate_cache implementation would like to reuse functions
that acquire/release the metadata locks. Call it from coroutine context
to simplify the logic.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 41 +++++++++++++++++++++++++++++++++++++----
block/iscsi.c | 6 +++---
block/nfs.c | 6 +++---
block/qcow2.c | 7 +++++--
block/qed.c | 13 +++++--------
block/rbd.c | 6 +++---
include/block/block_int.h | 3 ++-
7 files changed, 58 insertions(+), 24 deletions(-)
diff --git a/block.c b/block.c
index cf33135..6ba7471 100644
--- a/block.c
+++ b/block.c
@@ -4214,7 +4214,8 @@ void bdrv_init_with_whitelist(void)
bdrv_init();
}
-void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
+static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
BdrvChild *child, *parent;
uint64_t perm, shared_perm;
@@ -4230,7 +4231,7 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
}
QLIST_FOREACH(child, &bs->children, next) {
- bdrv_invalidate_cache(child->bs, &local_err);
+ bdrv_co_invalidate_cache(child->bs, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
@@ -4260,8 +4261,8 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
}
bdrv_set_perm(bs, perm, shared_perm);
- if (bs->drv->bdrv_invalidate_cache) {
- bs->drv->bdrv_invalidate_cache(bs, &local_err);
+ if (bs->drv->bdrv_co_invalidate_cache) {
+ bs->drv->bdrv_co_invalidate_cache(bs, &local_err);
if (local_err) {
bs->open_flags |= BDRV_O_INACTIVE;
error_propagate(errp, local_err);
@@ -4287,6 +4288,38 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
}
}
+typedef struct InvalidateCacheCo {
+ BlockDriverState *bs;
+ Error **errp;
+ bool done;
+} InvalidateCacheCo;
+
+static void coroutine_fn bdrv_invalidate_cache_co_entry(void *opaque)
+{
+ InvalidateCacheCo *ico = opaque;
+ bdrv_co_invalidate_cache(ico->bs, ico->errp);
+ ico->done = true;
+}
+
+void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
+{
+ Coroutine *co;
+ InvalidateCacheCo ico = {
+ .bs = bs,
+ .done = false,
+ .errp = errp
+ };
+
+ if (qemu_in_coroutine()) {
+ /* Fast-path if already in coroutine context */
+ bdrv_invalidate_cache_co_entry(&ico);
+ } else {
+ co = qemu_coroutine_create(bdrv_invalidate_cache_co_entry, &ico);
+ qemu_coroutine_enter(co);
+ BDRV_POLL_WHILE(bs, !ico.done);
+ }
+}
+
void bdrv_invalidate_cache_all(Error **errp)
{
BlockDriverState *bs;
diff --git a/block/iscsi.c b/block/iscsi.c
index 5f358f9..9b70899 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2164,8 +2164,8 @@ static int iscsi_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
return 0;
}
-static void iscsi_invalidate_cache(BlockDriverState *bs,
- Error **errp)
+static void coroutine_fn iscsi_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
IscsiLun *iscsilun = bs->opaque;
iscsi_allocmap_invalidate(iscsilun);
@@ -2196,7 +2196,7 @@ static BlockDriver bdrv_iscsi = {
.create_opts = &iscsi_create_opts,
.bdrv_reopen_prepare = iscsi_reopen_prepare,
.bdrv_reopen_commit = iscsi_reopen_commit,
- .bdrv_invalidate_cache = iscsi_invalidate_cache,
+ .bdrv_co_invalidate_cache = iscsi_co_invalidate_cache,
.bdrv_getlength = iscsi_getlength,
.bdrv_get_info = iscsi_get_info,
diff --git a/block/nfs.c b/block/nfs.c
index 8029dc2..d7f9a6c 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -876,8 +876,8 @@ static void nfs_refresh_filename(BlockDriverState *bs, QDict *options)
}
#ifdef LIBNFS_FEATURE_PAGECACHE
-static void nfs_invalidate_cache(BlockDriverState *bs,
- Error **errp)
+static void coroutine_fn nfs_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
NFSClient *client = bs->opaque;
nfs_pagecache_invalidate(client->context, client->fh);
@@ -910,7 +910,7 @@ static BlockDriver bdrv_nfs = {
.bdrv_refresh_filename = nfs_refresh_filename,
#ifdef LIBNFS_FEATURE_PAGECACHE
- .bdrv_invalidate_cache = nfs_invalidate_cache,
+ .bdrv_co_invalidate_cache = nfs_co_invalidate_cache,
#endif
};
diff --git a/block/qcow2.c b/block/qcow2.c
index 7c11516..e886a44 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2109,7 +2109,8 @@ static void qcow2_close(BlockDriverState *bs)
qcow2_free_snapshots(bs);
}
-static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
+static void coroutine_fn qcow2_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
BDRVQcow2State *s = bs->opaque;
int flags = s->flags;
@@ -2132,7 +2133,9 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
options = qdict_clone_shallow(bs->options);
flags &= ~BDRV_O_INACTIVE;
+ qemu_co_mutex_lock(&s->lock);
ret = qcow2_do_open(bs, options, flags, &local_err);
+ qemu_co_mutex_unlock(&s->lock);
QDECREF(options);
if (local_err) {
error_propagate(errp, local_err);
@@ -4386,7 +4389,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_change_backing_file = qcow2_change_backing_file,
.bdrv_refresh_limits = qcow2_refresh_limits,
- .bdrv_invalidate_cache = qcow2_invalidate_cache,
+ .bdrv_co_invalidate_cache = qcow2_co_invalidate_cache,
.bdrv_inactivate = qcow2_inactivate,
.create_opts = &qcow2_create_opts,
diff --git a/block/qed.c b/block/qed.c
index 3e1f4bc..5cf519c 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1550,7 +1550,8 @@ static int bdrv_qed_change_backing_file(BlockDriverState *bs,
return ret;
}
-static void bdrv_qed_invalidate_cache(BlockDriverState *bs, Error **errp)
+static void coroutine_fn bdrv_qed_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
BDRVQEDState *s = bs->opaque;
Error *local_err = NULL;
@@ -1559,13 +1560,9 @@ static void bdrv_qed_invalidate_cache(BlockDriverState *bs, Error **errp)
bdrv_qed_close(bs);
bdrv_qed_init_state(bs);
- if (qemu_in_coroutine()) {
- qemu_co_mutex_lock(&s->table_lock);
- }
+ qemu_co_mutex_lock(&s->table_lock);
ret = bdrv_qed_do_open(bs, NULL, bs->open_flags, &local_err);
- if (qemu_in_coroutine()) {
- qemu_co_mutex_unlock(&s->table_lock);
- }
+ qemu_co_mutex_unlock(&s->table_lock);
if (local_err) {
error_propagate(errp, local_err);
error_prepend(errp, "Could not reopen qed layer: ");
@@ -1640,7 +1637,7 @@ static BlockDriver bdrv_qed = {
.bdrv_get_info = bdrv_qed_get_info,
.bdrv_refresh_limits = bdrv_qed_refresh_limits,
.bdrv_change_backing_file = bdrv_qed_change_backing_file,
- .bdrv_invalidate_cache = bdrv_qed_invalidate_cache,
+ .bdrv_co_invalidate_cache = bdrv_qed_co_invalidate_cache,
.bdrv_check = bdrv_qed_check,
.bdrv_detach_aio_context = bdrv_qed_detach_aio_context,
.bdrv_attach_aio_context = bdrv_qed_attach_aio_context,
diff --git a/block/rbd.c b/block/rbd.c
index 67ac2ee..9632355 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1090,8 +1090,8 @@ static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs,
#endif
#ifdef LIBRBD_SUPPORTS_INVALIDATE
-static void qemu_rbd_invalidate_cache(BlockDriverState *bs,
- Error **errp)
+static void coroutine_fn qemu_rbd_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
BDRVRBDState *s = bs->opaque;
int r = rbd_invalidate_cache(s->image);
@@ -1157,7 +1157,7 @@ static BlockDriver bdrv_rbd = {
.bdrv_snapshot_list = qemu_rbd_snap_list,
.bdrv_snapshot_goto = qemu_rbd_snap_rollback,
#ifdef LIBRBD_SUPPORTS_INVALIDATE
- .bdrv_invalidate_cache = qemu_rbd_invalidate_cache,
+ .bdrv_co_invalidate_cache = qemu_rbd_co_invalidate_cache,
#endif
};
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 05f498f..3481da1 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -218,7 +218,8 @@ struct BlockDriver {
/*
* Invalidate any cached meta-data.
*/
- void (*bdrv_invalidate_cache)(BlockDriverState *bs, Error **errp);
+ void coroutine_fn (*bdrv_co_invalidate_cache)(BlockDriverState *bs,
+ Error **errp);
int (*bdrv_inactivate)(BlockDriverState *bs);
/*
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context
@ 2018-01-18 12:43 Paolo Bonzini
2018-01-18 12:43 ` [Qemu-devel] [PATCH 1/7] block: rename .bdrv_create() to .bdrv_co_create_opts() Paolo Bonzini
` (9 more replies)
0 siblings, 10 replies; 16+ messages in thread
From: Paolo Bonzini @ 2018-01-18 12:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block, mreitz
Check and invalidate_cache share some parts of the implementation
with the regular I/O path. This is sometimes complicated because the
I/O path wants to use a CoMutex but that is not possible outside coroutine
context. By moving things to coroutine context, we can remove special
cases. In fact, invalidate_cache is already called from coroutine context
because incoming migration is placed in a coroutine.
While at it, I'm including two patches from Stefan to rename the
bdrv_create callback to bdrv_co_create, because it is already called
from coroutine context. The name is now bdrv_co_create_opts, with
bdrv_co_create reserved for the QAPI-based version that Kevin is
working on.
qcow2 still has cache flushing in non-coroutine context, coming from
qcow2_reopen_prepare->qcow2_update_options_prepare and
qcow2_close->qcow2_inactivate.
Paolo
v1->v2: fix file-win32
Paolo Bonzini (5):
qcow2: make qcow2_do_open a coroutine_fn
qed: make bdrv_qed_do_open a coroutine_fn
block: convert bdrv_invalidate_cache callback to coroutine_fn
qcow2: introduce qcow2_write_caches and qcow2_flush_caches
block: convert bdrv_check callback to coroutine_fn
Stefan Hajnoczi (2):
block: rename .bdrv_create() to .bdrv_co_create_opts()
qcow2: make qcow2_co_create2() a coroutine_fn
block.c | 88 ++++++++++++++++++++++++++++++----
block/crypto.c | 8 ++--
block/file-posix.c | 15 +++---
block/file-win32.c | 5 +-
block/gluster.c | 12 ++---
block/iscsi.c | 13 ++---
block/nfs.c | 11 +++--
block/parallels.c | 23 +++++----
block/qcow.c | 5 +-
block/qcow2-refcount.c | 28 +++++++++++
block/qcow2.c | 118 +++++++++++++++++++++++++++++++---------------
block/qcow2.h | 2 +
block/qed-check.c | 1 +
block/qed-table.c | 26 ++++------
block/qed.c | 72 +++++++++++++++++++++-------
block/raw-format.c | 5 +-
block/rbd.c | 12 +++--
block/sheepdog.c | 10 ++--
block/ssh.c | 5 +-
block/vdi.c | 11 +++--
block/vhdx.c | 12 +++--
block/vmdk.c | 12 +++--
block/vpc.c | 5 +-
include/block/block_int.h | 11 +++--
24 files changed, 355 insertions(+), 155 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 1/7] block: rename .bdrv_create() to .bdrv_co_create_opts()
2018-01-18 12:43 [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context Paolo Bonzini
@ 2018-01-18 12:43 ` Paolo Bonzini
2018-01-18 12:43 ` [Qemu-devel] [PATCH 2/7] qcow2: make qcow2_co_create2() a coroutine_fn Paolo Bonzini
` (8 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2018-01-18 12:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block, mreitz, Stefan Hajnoczi
From: Stefan Hajnoczi <stefanha@redhat.com>
BlockDriver->bdrv_create() has been called from coroutine context since
commit 5b7e1542cfa41a281af9629d31cef03704d976e6 ("block: make
bdrv_create adopt coroutine").
Make this explicit by renaming to .bdrv_co_create_opts() and add the
coroutine_fn annotation. This makes it obvious to block driver authors
that they may yield, use CoMutex, or other coroutine_fn APIs.
bdrv_co_create is reserved for the QAPI-based version that Kevin is
working on.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20170705102231.20711-2-stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 4 ++--
block/crypto.c | 8 ++++----
block/file-posix.c | 15 ++++++++-------
block/file-win32.c | 5 +++--
block/gluster.c | 12 ++++++------
block/iscsi.c | 7 ++++---
block/nfs.c | 5 +++--
block/parallels.c | 6 ++++--
block/qcow.c | 5 +++--
block/qcow2.c | 5 +++--
block/qed.c | 6 ++++--
block/raw-format.c | 5 +++--
block/rbd.c | 6 ++++--
block/sheepdog.c | 10 +++++-----
block/ssh.c | 5 +++--
block/vdi.c | 5 +++--
block/vhdx.c | 5 +++--
block/vmdk.c | 5 +++--
block/vpc.c | 5 +++--
include/block/block_int.h | 3 ++-
20 files changed, 73 insertions(+), 54 deletions(-)
diff --git a/block.c b/block.c
index a8da4f2..cf33135 100644
--- a/block.c
+++ b/block.c
@@ -417,7 +417,7 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
CreateCo *cco = opaque;
assert(cco->drv);
- ret = cco->drv->bdrv_create(cco->filename, cco->opts, &local_err);
+ ret = cco->drv->bdrv_co_create_opts(cco->filename, cco->opts, &local_err);
error_propagate(&cco->err, local_err);
cco->ret = ret;
}
@@ -436,7 +436,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
.err = NULL,
};
- if (!drv->bdrv_create) {
+ if (!drv->bdrv_co_create_opts) {
error_setg(errp, "Driver '%s' does not support image creation", drv->format_name);
ret = -ENOTSUP;
goto out;
diff --git a/block/crypto.c b/block/crypto.c
index 60ddf86..4acff45 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -554,9 +554,9 @@ static int block_crypto_open_luks(BlockDriverState *bs,
bs, options, flags, errp);
}
-static int block_crypto_create_luks(const char *filename,
- QemuOpts *opts,
- Error **errp)
+static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
+ QemuOpts *opts,
+ Error **errp)
{
return block_crypto_create_generic(Q_CRYPTO_BLOCK_FORMAT_LUKS,
filename, opts, errp);
@@ -616,7 +616,7 @@ BlockDriver bdrv_crypto_luks = {
.bdrv_open = block_crypto_open_luks,
.bdrv_close = block_crypto_close,
.bdrv_child_perm = bdrv_format_default_perms,
- .bdrv_create = block_crypto_create_luks,
+ .bdrv_co_create_opts = block_crypto_co_create_opts_luks,
.bdrv_truncate = block_crypto_truncate,
.create_opts = &block_crypto_create_opts_luks,
diff --git a/block/file-posix.c b/block/file-posix.c
index 36ee89e..39d2795 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1979,7 +1979,8 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
return (int64_t)st.st_blocks * 512;
}
-static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn raw_co_create_opts(const char *filename, QemuOpts *opts,
+ Error **errp)
{
int fd;
int result = 0;
@@ -2278,7 +2279,7 @@ BlockDriver bdrv_file = {
.bdrv_reopen_commit = raw_reopen_commit,
.bdrv_reopen_abort = raw_reopen_abort,
.bdrv_close = raw_close,
- .bdrv_create = raw_create,
+ .bdrv_co_create_opts = raw_co_create_opts,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_co_get_block_status = raw_co_get_block_status,
.bdrv_co_pwrite_zeroes = raw_co_pwrite_zeroes,
@@ -2682,8 +2683,8 @@ static coroutine_fn int hdev_co_pwrite_zeroes(BlockDriverState *bs,
return -ENOTSUP;
}
-static int hdev_create(const char *filename, QemuOpts *opts,
- Error **errp)
+static int coroutine_fn hdev_co_create_opts(const char *filename, QemuOpts *opts,
+ Error **errp)
{
int fd;
int ret = 0;
@@ -2756,7 +2757,7 @@ static BlockDriver bdrv_host_device = {
.bdrv_reopen_prepare = raw_reopen_prepare,
.bdrv_reopen_commit = raw_reopen_commit,
.bdrv_reopen_abort = raw_reopen_abort,
- .bdrv_create = hdev_create,
+ .bdrv_co_create_opts = hdev_co_create_opts,
.create_opts = &raw_create_opts,
.bdrv_co_pwrite_zeroes = hdev_co_pwrite_zeroes,
@@ -2878,7 +2879,7 @@ static BlockDriver bdrv_host_cdrom = {
.bdrv_reopen_prepare = raw_reopen_prepare,
.bdrv_reopen_commit = raw_reopen_commit,
.bdrv_reopen_abort = raw_reopen_abort,
- .bdrv_create = hdev_create,
+ .bdrv_co_create_opts = hdev_co_create_opts,
.create_opts = &raw_create_opts,
@@ -3009,7 +3010,7 @@ static BlockDriver bdrv_host_cdrom = {
.bdrv_reopen_prepare = raw_reopen_prepare,
.bdrv_reopen_commit = raw_reopen_commit,
.bdrv_reopen_abort = raw_reopen_abort,
- .bdrv_create = hdev_create,
+ .bdrv_co_create_opts = hdev_co_create_opts,
.create_opts = &raw_create_opts,
.bdrv_co_preadv = raw_co_preadv,
diff --git a/block/file-win32.c b/block/file-win32.c
index 9e02214..4d88b14 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -550,7 +550,8 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
return st.st_size;
}
-static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn raw_co_create_opts(const char *filename, QemuOpts *opts,
+ Error **errp)
{
int fd;
int64_t total_size = 0;
@@ -596,7 +597,7 @@ BlockDriver bdrv_file = {
.bdrv_file_open = raw_open,
.bdrv_refresh_limits = raw_probe_alignment,
.bdrv_close = raw_close,
- .bdrv_create = raw_create,
+ .bdrv_co_create_opts = raw_co_create_opts,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_aio_readv = raw_aio_readv,
diff --git a/block/gluster.c b/block/gluster.c
index 0f4265a..9419e0c 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -962,8 +962,8 @@ static coroutine_fn int qemu_gluster_co_pwrite_zeroes(BlockDriverState *bs,
}
#endif
-static int qemu_gluster_create(const char *filename,
- QemuOpts *opts, Error **errp)
+static int coroutine_fn qemu_gluster_co_create_opts(const char *filename,
+ QemuOpts *opts, Error **errp)
{
BlockdevOptionsGluster *gconf;
struct glfs *glfs;
@@ -1424,7 +1424,7 @@ static BlockDriver bdrv_gluster = {
.bdrv_reopen_commit = qemu_gluster_reopen_commit,
.bdrv_reopen_abort = qemu_gluster_reopen_abort,
.bdrv_close = qemu_gluster_close,
- .bdrv_create = qemu_gluster_create,
+ .bdrv_co_create_opts = qemu_gluster_co_create_opts,
.bdrv_getlength = qemu_gluster_getlength,
.bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
.bdrv_truncate = qemu_gluster_truncate,
@@ -1452,7 +1452,7 @@ static BlockDriver bdrv_gluster_tcp = {
.bdrv_reopen_commit = qemu_gluster_reopen_commit,
.bdrv_reopen_abort = qemu_gluster_reopen_abort,
.bdrv_close = qemu_gluster_close,
- .bdrv_create = qemu_gluster_create,
+ .bdrv_co_create_opts = qemu_gluster_co_create_opts,
.bdrv_getlength = qemu_gluster_getlength,
.bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
.bdrv_truncate = qemu_gluster_truncate,
@@ -1480,7 +1480,7 @@ static BlockDriver bdrv_gluster_unix = {
.bdrv_reopen_commit = qemu_gluster_reopen_commit,
.bdrv_reopen_abort = qemu_gluster_reopen_abort,
.bdrv_close = qemu_gluster_close,
- .bdrv_create = qemu_gluster_create,
+ .bdrv_co_create_opts = qemu_gluster_co_create_opts,
.bdrv_getlength = qemu_gluster_getlength,
.bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
.bdrv_truncate = qemu_gluster_truncate,
@@ -1514,7 +1514,7 @@ static BlockDriver bdrv_gluster_rdma = {
.bdrv_reopen_commit = qemu_gluster_reopen_commit,
.bdrv_reopen_abort = qemu_gluster_reopen_abort,
.bdrv_close = qemu_gluster_close,
- .bdrv_create = qemu_gluster_create,
+ .bdrv_co_create_opts = qemu_gluster_co_create_opts,
.bdrv_getlength = qemu_gluster_getlength,
.bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
.bdrv_truncate = qemu_gluster_truncate,
diff --git a/block/iscsi.c b/block/iscsi.c
index 6a1c537..5f358f9 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2103,7 +2103,8 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
return 0;
}
-static int iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn iscsi_co_create_opts(const char *filename, QemuOpts *opts,
+ Error **errp)
{
int ret = 0;
int64_t total_size = 0;
@@ -2191,7 +2192,7 @@ static BlockDriver bdrv_iscsi = {
.bdrv_parse_filename = iscsi_parse_filename,
.bdrv_file_open = iscsi_open,
.bdrv_close = iscsi_close,
- .bdrv_create = iscsi_create,
+ .bdrv_co_create_opts = iscsi_co_create_opts,
.create_opts = &iscsi_create_opts,
.bdrv_reopen_prepare = iscsi_reopen_prepare,
.bdrv_reopen_commit = iscsi_reopen_commit,
@@ -2226,7 +2227,7 @@ static BlockDriver bdrv_iser = {
.bdrv_parse_filename = iscsi_parse_filename,
.bdrv_file_open = iscsi_open,
.bdrv_close = iscsi_close,
- .bdrv_create = iscsi_create,
+ .bdrv_co_create_opts = iscsi_co_create_opts,
.create_opts = &iscsi_create_opts,
.bdrv_reopen_prepare = iscsi_reopen_prepare,
.bdrv_reopen_commit = iscsi_reopen_commit,
diff --git a/block/nfs.c b/block/nfs.c
index effc871..8029dc2 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -684,7 +684,8 @@ static QemuOptsList nfs_create_opts = {
}
};
-static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
+static int coroutine_fn nfs_file_co_create_opts(const char *url, QemuOpts *opts,
+ Error **errp)
{
int64_t ret, total_size;
NFSClient *client = g_new0(NFSClient, 1);
@@ -897,7 +898,7 @@ static BlockDriver bdrv_nfs = {
.bdrv_file_open = nfs_file_open,
.bdrv_close = nfs_file_close,
- .bdrv_create = nfs_file_create,
+ .bdrv_co_create_opts = nfs_file_co_create_opts,
.bdrv_reopen_prepare = nfs_reopen_prepare,
.bdrv_co_preadv = nfs_co_preadv,
diff --git a/block/parallels.c b/block/parallels.c
index 9545761..5f1497d 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -479,7 +479,9 @@ static int parallels_check(BlockDriverState *bs, BdrvCheckResult *res,
}
-static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn parallels_co_create_opts(const char *filename,
+ QemuOpts *opts,
+ Error **errp)
{
int64_t total_size, cl_size;
uint8_t tmp[BDRV_SECTOR_SIZE];
@@ -799,7 +801,7 @@ static BlockDriver bdrv_parallels = {
.bdrv_co_readv = parallels_co_readv,
.bdrv_co_writev = parallels_co_writev,
- .bdrv_create = parallels_create,
+ .bdrv_co_create_opts = parallels_co_create_opts,
.bdrv_check = parallels_check,
.create_opts = ¶llels_create_opts,
};
diff --git a/block/qcow.c b/block/qcow.c
index 9569dee..ebb7cea 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -789,7 +789,8 @@ static void qcow_close(BlockDriverState *bs)
error_free(s->migration_blocker);
}
-static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn qcow_co_create_opts(const char *filename, QemuOpts *opts,
+ Error **errp)
{
int header_size, backing_filename_len, l1_size, shift, i;
QCowHeader header;
@@ -1105,7 +1106,7 @@ static BlockDriver bdrv_qcow = {
.bdrv_close = qcow_close,
.bdrv_child_perm = bdrv_format_default_perms,
.bdrv_reopen_prepare = qcow_reopen_prepare,
- .bdrv_create = qcow_create,
+ .bdrv_co_create_opts = qcow_co_create_opts,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.supports_backing = true,
diff --git a/block/qcow2.c b/block/qcow2.c
index 4348b2c..be2d485 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2881,7 +2881,8 @@ out:
return ret;
}
-static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn qcow2_co_create_opts(const char *filename, QemuOpts *opts,
+ Error **errp)
{
char *backing_file = NULL;
char *backing_fmt = NULL;
@@ -4318,7 +4319,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_reopen_abort = qcow2_reopen_abort,
.bdrv_join_options = qcow2_join_options,
.bdrv_child_perm = bdrv_format_default_perms,
- .bdrv_create = qcow2_create,
+ .bdrv_co_create_opts = qcow2_co_create_opts,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_co_get_block_status = qcow2_co_get_block_status,
diff --git a/block/qed.c b/block/qed.c
index 821dcaa..e0e08ce 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -638,7 +638,9 @@ out:
return ret;
}
-static int bdrv_qed_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn bdrv_qed_co_create_opts(const char *filename,
+ QemuOpts *opts,
+ Error **errp)
{
uint64_t image_size = 0;
uint32_t cluster_size = QED_DEFAULT_CLUSTER_SIZE;
@@ -1593,7 +1595,7 @@ static BlockDriver bdrv_qed = {
.bdrv_close = bdrv_qed_close,
.bdrv_reopen_prepare = bdrv_qed_reopen_prepare,
.bdrv_child_perm = bdrv_format_default_perms,
- .bdrv_create = bdrv_qed_create,
+ .bdrv_co_create_opts = bdrv_qed_co_create_opts,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_co_get_block_status = bdrv_qed_co_get_block_status,
.bdrv_co_readv = bdrv_qed_co_readv,
diff --git a/block/raw-format.c b/block/raw-format.c
index ab552c0..92b4753 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -396,7 +396,8 @@ static int raw_has_zero_init(BlockDriverState *bs)
return bdrv_has_zero_init(bs->file->bs);
}
-static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn raw_co_create_opts(const char *filename, QemuOpts *opts,
+ Error **errp)
{
return bdrv_create_file(filename, opts, errp);
}
@@ -491,7 +492,7 @@ BlockDriver bdrv_raw = {
.bdrv_open = &raw_open,
.bdrv_close = &raw_close,
.bdrv_child_perm = bdrv_filter_default_perms,
- .bdrv_create = &raw_create,
+ .bdrv_co_create_opts = &raw_co_create_opts,
.bdrv_co_preadv = &raw_co_preadv,
.bdrv_co_pwritev = &raw_co_pwritev,
.bdrv_co_pwrite_zeroes = &raw_co_pwrite_zeroes,
diff --git a/block/rbd.c b/block/rbd.c
index a76a5e8..67ac2ee 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -348,7 +348,9 @@ static QemuOptsList runtime_opts = {
},
};
-static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn qemu_rbd_co_create_opts(const char *filename,
+ QemuOpts *opts,
+ Error **errp)
{
Error *local_err = NULL;
int64_t bytes = 0;
@@ -1129,7 +1131,7 @@ static BlockDriver bdrv_rbd = {
.bdrv_file_open = qemu_rbd_open,
.bdrv_close = qemu_rbd_close,
.bdrv_reopen_prepare = qemu_rbd_reopen_prepare,
- .bdrv_create = qemu_rbd_create,
+ .bdrv_co_create_opts = qemu_rbd_co_create_opts,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_get_info = qemu_rbd_getinfo,
.create_opts = &qemu_rbd_create_opts,
diff --git a/block/sheepdog.c b/block/sheepdog.c
index f684477..c90bdc3 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1964,8 +1964,8 @@ static int parse_block_size_shift(BDRVSheepdogState *s, QemuOpts *opt)
return 0;
}
-static int sd_create(const char *filename, QemuOpts *opts,
- Error **errp)
+static int coroutine_fn sd_co_create_opts(const char *filename, QemuOpts *opts,
+ Error **errp)
{
Error *err = NULL;
int ret = 0;
@@ -3086,7 +3086,7 @@ static BlockDriver bdrv_sheepdog = {
.bdrv_reopen_commit = sd_reopen_commit,
.bdrv_reopen_abort = sd_reopen_abort,
.bdrv_close = sd_close,
- .bdrv_create = sd_create,
+ .bdrv_co_create_opts = sd_co_create_opts,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_getlength = sd_getlength,
.bdrv_get_allocated_file_size = sd_get_allocated_file_size,
@@ -3122,7 +3122,7 @@ static BlockDriver bdrv_sheepdog_tcp = {
.bdrv_reopen_commit = sd_reopen_commit,
.bdrv_reopen_abort = sd_reopen_abort,
.bdrv_close = sd_close,
- .bdrv_create = sd_create,
+ .bdrv_co_create_opts = sd_co_create_opts,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_getlength = sd_getlength,
.bdrv_get_allocated_file_size = sd_get_allocated_file_size,
@@ -3158,7 +3158,7 @@ static BlockDriver bdrv_sheepdog_unix = {
.bdrv_reopen_commit = sd_reopen_commit,
.bdrv_reopen_abort = sd_reopen_abort,
.bdrv_close = sd_close,
- .bdrv_create = sd_create,
+ .bdrv_co_create_opts = sd_co_create_opts,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_getlength = sd_getlength,
.bdrv_get_allocated_file_size = sd_get_allocated_file_size,
diff --git a/block/ssh.c b/block/ssh.c
index b049a16..72f44ce 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -813,7 +813,8 @@ static QemuOptsList ssh_create_opts = {
}
};
-static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn ssh_co_create_opts(const char *filename, QemuOpts *opts,
+ Error **errp)
{
int r, ret;
int64_t total_size = 0;
@@ -1201,7 +1202,7 @@ static BlockDriver bdrv_ssh = {
.instance_size = sizeof(BDRVSSHState),
.bdrv_parse_filename = ssh_parse_filename,
.bdrv_file_open = ssh_file_open,
- .bdrv_create = ssh_create,
+ .bdrv_co_create_opts = ssh_co_create_opts,
.bdrv_close = ssh_close,
.bdrv_has_zero_init = ssh_has_zero_init,
.bdrv_co_readv = ssh_co_readv,
diff --git a/block/vdi.c b/block/vdi.c
index 8da5dfc..01501d7 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -716,7 +716,8 @@ nonallocating_write:
return ret;
}
-static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn vdi_co_create_opts(const char *filename, QemuOpts *opts,
+ Error **errp)
{
int ret = 0;
uint64_t bytes = 0;
@@ -894,7 +895,7 @@ static BlockDriver bdrv_vdi = {
.bdrv_close = vdi_close,
.bdrv_reopen_prepare = vdi_reopen_prepare,
.bdrv_child_perm = bdrv_format_default_perms,
- .bdrv_create = vdi_create,
+ .bdrv_co_create_opts = vdi_co_create_opts,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_co_get_block_status = vdi_co_get_block_status,
.bdrv_make_empty = vdi_make_empty,
diff --git a/block/vhdx.c b/block/vhdx.c
index 9956933..7fad8b2 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1792,7 +1792,8 @@ exit:
* .---- ~ ----------- ~ ------------ ~ ---------------- ~ -----------.
* 1MB
*/
-static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn vhdx_co_create_opts(const char *filename, QemuOpts *opts,
+ Error **errp)
{
int ret = 0;
uint64_t image_size = (uint64_t) 2 * GiB;
@@ -2003,7 +2004,7 @@ static BlockDriver bdrv_vhdx = {
.bdrv_child_perm = bdrv_format_default_perms,
.bdrv_co_readv = vhdx_co_readv,
.bdrv_co_writev = vhdx_co_writev,
- .bdrv_create = vhdx_create,
+ .bdrv_co_create_opts = vhdx_co_create_opts,
.bdrv_get_info = vhdx_get_info,
.bdrv_check = vhdx_check,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
diff --git a/block/vmdk.c b/block/vmdk.c
index c665bcc..d4beb25 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1875,7 +1875,8 @@ static int filename_decompose(const char *filename, char *path, char *prefix,
return VMDK_OK;
}
-static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn vmdk_co_create_opts(const char *filename, QemuOpts *opts,
+ Error **errp)
{
int idx = 0;
BlockBackend *new_blk = NULL;
@@ -2391,7 +2392,7 @@ static BlockDriver bdrv_vmdk = {
.bdrv_co_pwritev_compressed = vmdk_co_pwritev_compressed,
.bdrv_co_pwrite_zeroes = vmdk_co_pwrite_zeroes,
.bdrv_close = vmdk_close,
- .bdrv_create = vmdk_create,
+ .bdrv_co_create_opts = vmdk_co_create_opts,
.bdrv_co_flush_to_disk = vmdk_co_flush,
.bdrv_co_get_block_status = vmdk_co_get_block_status,
.bdrv_get_allocated_file_size = vmdk_get_allocated_file_size,
diff --git a/block/vpc.c b/block/vpc.c
index 1576d7b..1817f72 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -895,7 +895,8 @@ static int create_fixed_disk(BlockBackend *blk, uint8_t *buf,
return ret;
}
-static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn vpc_co_create_opts(const char *filename, QemuOpts *opts,
+ Error **errp)
{
uint8_t buf[1024];
VHDFooter *footer = (VHDFooter *) buf;
@@ -1093,7 +1094,7 @@ static BlockDriver bdrv_vpc = {
.bdrv_close = vpc_close,
.bdrv_reopen_prepare = vpc_reopen_prepare,
.bdrv_child_perm = bdrv_format_default_perms,
- .bdrv_create = vpc_create,
+ .bdrv_co_create_opts = vpc_co_create_opts,
.bdrv_co_preadv = vpc_co_preadv,
.bdrv_co_pwritev = vpc_co_pwritev,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 29cafa4..05f498f 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -130,7 +130,8 @@ struct BlockDriver {
int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
Error **errp);
void (*bdrv_close)(BlockDriverState *bs);
- int (*bdrv_create)(const char *filename, QemuOpts *opts, Error **errp);
+ int coroutine_fn (*bdrv_co_create_opts)(const char *filename, QemuOpts *opts,
+ Error **errp);
int (*bdrv_make_empty)(BlockDriverState *bs);
void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 2/7] qcow2: make qcow2_co_create2() a coroutine_fn
2018-01-18 12:43 [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context Paolo Bonzini
2018-01-18 12:43 ` [Qemu-devel] [PATCH 1/7] block: rename .bdrv_create() to .bdrv_co_create_opts() Paolo Bonzini
@ 2018-01-18 12:43 ` Paolo Bonzini
2018-01-18 12:43 ` [Qemu-devel] [PATCH 3/7] qcow2: make qcow2_do_open " Paolo Bonzini
` (7 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2018-01-18 12:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block, mreitz, Stefan Hajnoczi
From: Stefan Hajnoczi <stefanha@redhat.com>
qcow2_create2() calls qemu_co_mutex_lock(). Only a coroutine_fn may
call another coroutine_fn. In fact, qcow2_create2 is always called from
coroutine context.
Rename the function to add the "co" moniker and add coroutine_fn.
Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20170705102231.20711-3-stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/qcow2.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index be2d485..fec065b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2690,11 +2690,12 @@ static uint64_t qcow2_opt_get_refcount_bits_del(QemuOpts *opts, int version,
return refcount_bits;
}
-static int qcow2_create2(const char *filename, int64_t total_size,
- const char *backing_file, const char *backing_format,
- int flags, size_t cluster_size, PreallocMode prealloc,
- QemuOpts *opts, int version, int refcount_order,
- const char *encryptfmt, Error **errp)
+static int coroutine_fn
+qcow2_co_create2(const char *filename, int64_t total_size,
+ const char *backing_file, const char *backing_format,
+ int flags, size_t cluster_size, PreallocMode prealloc,
+ QemuOpts *opts, int version, int refcount_order,
+ const char *encryptfmt, Error **errp)
{
QDict *options;
@@ -2963,9 +2964,9 @@ static int coroutine_fn qcow2_co_create(const char *filename, QemuOpts *opts,
refcount_order = ctz32(refcount_bits);
- ret = qcow2_create2(filename, size, backing_file, backing_fmt, flags,
- cluster_size, prealloc, opts, version, refcount_order,
- encryptfmt, &local_err);
+ ret = qcow2_co_create2(filename, size, backing_file, backing_fmt, flags,
+ cluster_size, prealloc, opts, version, refcount_order,
+ encryptfmt, &local_err);
error_propagate(errp, local_err);
finish:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 3/7] qcow2: make qcow2_do_open a coroutine_fn
2018-01-18 12:43 [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context Paolo Bonzini
2018-01-18 12:43 ` [Qemu-devel] [PATCH 1/7] block: rename .bdrv_create() to .bdrv_co_create_opts() Paolo Bonzini
2018-01-18 12:43 ` [Qemu-devel] [PATCH 2/7] qcow2: make qcow2_co_create2() a coroutine_fn Paolo Bonzini
@ 2018-01-18 12:43 ` Paolo Bonzini
2018-01-18 12:43 ` [Qemu-devel] [PATCH 4/7] qed: make bdrv_qed_do_open " Paolo Bonzini
` (6 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2018-01-18 12:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block, mreitz
It is called from qcow2_invalidate_cache in coroutine context (incoming
migration runs in a coroutine), so it's cleaner if metadata is always
loaded from a coroutine.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/qcow2.c | 46 +++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 41 insertions(+), 5 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index fec065b..7c11516 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1087,8 +1087,9 @@ static int qcow2_update_options(BlockDriverState *bs, QDict *options,
return ret;
}
-static int qcow2_do_open(BlockDriverState *bs, QDict *options, int flags,
- Error **errp)
+/* Called with s->lock held. */
+static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
+ int flags, Error **errp)
{
BDRVQcow2State *s = bs->opaque;
unsigned int len, i;
@@ -1467,8 +1468,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 */
@@ -1514,16 +1513,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)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 4/7] qed: make bdrv_qed_do_open a coroutine_fn
2018-01-18 12:43 [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context Paolo Bonzini
` (2 preceding siblings ...)
2018-01-18 12:43 ` [Qemu-devel] [PATCH 3/7] qcow2: make qcow2_do_open " Paolo Bonzini
@ 2018-01-18 12:43 ` Paolo Bonzini
2018-01-18 20:19 ` Eric Blake
2018-01-18 12:43 ` [Qemu-devel] [PATCH 5/7] block: convert bdrv_invalidate_cache callback to coroutine_fn Paolo Bonzini
` (5 subsequent siblings)
9 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2018-01-18 12:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block, mreitz
It is called from qcow2_invalidate_cache in coroutine context (incoming
migration runs in a coroutine), so it's cleaner if metadata is always
loaded from a coroutine.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/qed.c | 40 +++++++++++++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 3 deletions(-)
diff --git a/block/qed.c b/block/qed.c
index e0e08ce..3e1f4bc 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -381,8 +381,9 @@ static void bdrv_qed_init_state(BlockDriverState *bs)
qemu_co_queue_init(&s->allocating_write_reqs);
}
-static int bdrv_qed_do_open(BlockDriverState *bs, QDict *options, int flags,
- Error **errp)
+/* Called with table_lock held. */
+static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options,
+ int flags, Error **errp)
{
BDRVQEDState *s = bs->opaque;
QEDHeader le_header;
@@ -513,9 +514,35 @@ out:
return ret;
}
+typedef struct QEDOpenCo {
+ BlockDriverState *bs;
+ QDict *options;
+ int flags;
+ Error **errp;
+ int ret;
+} QEDOpenCo;
+
+static void coroutine_fn bdrv_qed_open_entry(void *opaque)
+{
+ QEDOpenCo *qoc = opaque;
+ BDRVQEDState *s = qoc->bs->opaque;
+
+ qemu_co_mutex_lock(&s->table_lock);
+ qoc->ret = bdrv_qed_do_open(qoc->bs, qoc->options, qoc->flags, qoc->errp);
+ qemu_co_mutex_unlock(&s->table_lock);
+}
+
static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
+ QEDOpenCo 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) {
@@ -523,7 +550,14 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
}
bdrv_qed_init_state(bs);
- return bdrv_qed_do_open(bs, options, flags, errp);
+ if (qemu_in_coroutine()) {
+ bdrv_qed_open_entry(&qoc);
+ } else {
+ qemu_coroutine_enter(qemu_coroutine_create(bdrv_qed_open_entry, &qoc));
+ BDRV_POLL_WHILE(bs, qoc.ret == -EINPROGRESS);
+ }
+ BDRV_POLL_WHILE(bs, qoc.ret == -EINPROGRESS);
+ return qoc.ret;
}
static void bdrv_qed_refresh_limits(BlockDriverState *bs, Error **errp)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 5/7] block: convert bdrv_invalidate_cache callback to coroutine_fn
2018-01-18 12:43 [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context Paolo Bonzini
` (3 preceding siblings ...)
2018-01-18 12:43 ` [Qemu-devel] [PATCH 4/7] qed: make bdrv_qed_do_open " Paolo Bonzini
@ 2018-01-18 12:43 ` Paolo Bonzini
2018-03-15 13:21 ` Andreas Schwab
2018-01-18 12:43 ` [Qemu-devel] [PATCH 6/7] qcow2: introduce qcow2_write_caches and qcow2_flush_caches Paolo Bonzini
` (4 subsequent siblings)
9 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2018-01-18 12:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block, mreitz
QED's bdrv_invalidate_cache implementation would like to reuse functions
that acquire/release the metadata locks. Call it from coroutine context
to simplify the logic.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 41 +++++++++++++++++++++++++++++++++++++----
block/iscsi.c | 6 +++---
block/nfs.c | 6 +++---
block/qcow2.c | 7 +++++--
block/qed.c | 13 +++++--------
block/rbd.c | 6 +++---
include/block/block_int.h | 3 ++-
7 files changed, 58 insertions(+), 24 deletions(-)
diff --git a/block.c b/block.c
index cf33135..6ba7471 100644
--- a/block.c
+++ b/block.c
@@ -4214,7 +4214,8 @@ void bdrv_init_with_whitelist(void)
bdrv_init();
}
-void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
+static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
BdrvChild *child, *parent;
uint64_t perm, shared_perm;
@@ -4230,7 +4231,7 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
}
QLIST_FOREACH(child, &bs->children, next) {
- bdrv_invalidate_cache(child->bs, &local_err);
+ bdrv_co_invalidate_cache(child->bs, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
@@ -4260,8 +4261,8 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
}
bdrv_set_perm(bs, perm, shared_perm);
- if (bs->drv->bdrv_invalidate_cache) {
- bs->drv->bdrv_invalidate_cache(bs, &local_err);
+ if (bs->drv->bdrv_co_invalidate_cache) {
+ bs->drv->bdrv_co_invalidate_cache(bs, &local_err);
if (local_err) {
bs->open_flags |= BDRV_O_INACTIVE;
error_propagate(errp, local_err);
@@ -4287,6 +4288,38 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
}
}
+typedef struct InvalidateCacheCo {
+ BlockDriverState *bs;
+ Error **errp;
+ bool done;
+} InvalidateCacheCo;
+
+static void coroutine_fn bdrv_invalidate_cache_co_entry(void *opaque)
+{
+ InvalidateCacheCo *ico = opaque;
+ bdrv_co_invalidate_cache(ico->bs, ico->errp);
+ ico->done = true;
+}
+
+void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
+{
+ Coroutine *co;
+ InvalidateCacheCo ico = {
+ .bs = bs,
+ .done = false,
+ .errp = errp
+ };
+
+ if (qemu_in_coroutine()) {
+ /* Fast-path if already in coroutine context */
+ bdrv_invalidate_cache_co_entry(&ico);
+ } else {
+ co = qemu_coroutine_create(bdrv_invalidate_cache_co_entry, &ico);
+ qemu_coroutine_enter(co);
+ BDRV_POLL_WHILE(bs, !ico.done);
+ }
+}
+
void bdrv_invalidate_cache_all(Error **errp)
{
BlockDriverState *bs;
diff --git a/block/iscsi.c b/block/iscsi.c
index 5f358f9..9b70899 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2164,8 +2164,8 @@ static int iscsi_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
return 0;
}
-static void iscsi_invalidate_cache(BlockDriverState *bs,
- Error **errp)
+static void coroutine_fn iscsi_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
IscsiLun *iscsilun = bs->opaque;
iscsi_allocmap_invalidate(iscsilun);
@@ -2196,7 +2196,7 @@ static BlockDriver bdrv_iscsi = {
.create_opts = &iscsi_create_opts,
.bdrv_reopen_prepare = iscsi_reopen_prepare,
.bdrv_reopen_commit = iscsi_reopen_commit,
- .bdrv_invalidate_cache = iscsi_invalidate_cache,
+ .bdrv_co_invalidate_cache = iscsi_co_invalidate_cache,
.bdrv_getlength = iscsi_getlength,
.bdrv_get_info = iscsi_get_info,
diff --git a/block/nfs.c b/block/nfs.c
index 8029dc2..d7f9a6c 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -876,8 +876,8 @@ static void nfs_refresh_filename(BlockDriverState *bs, QDict *options)
}
#ifdef LIBNFS_FEATURE_PAGECACHE
-static void nfs_invalidate_cache(BlockDriverState *bs,
- Error **errp)
+static void coroutine_fn nfs_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
NFSClient *client = bs->opaque;
nfs_pagecache_invalidate(client->context, client->fh);
@@ -910,7 +910,7 @@ static BlockDriver bdrv_nfs = {
.bdrv_refresh_filename = nfs_refresh_filename,
#ifdef LIBNFS_FEATURE_PAGECACHE
- .bdrv_invalidate_cache = nfs_invalidate_cache,
+ .bdrv_co_invalidate_cache = nfs_co_invalidate_cache,
#endif
};
diff --git a/block/qcow2.c b/block/qcow2.c
index 7c11516..e886a44 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2109,7 +2109,8 @@ static void qcow2_close(BlockDriverState *bs)
qcow2_free_snapshots(bs);
}
-static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
+static void coroutine_fn qcow2_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
BDRVQcow2State *s = bs->opaque;
int flags = s->flags;
@@ -2132,7 +2133,9 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
options = qdict_clone_shallow(bs->options);
flags &= ~BDRV_O_INACTIVE;
+ qemu_co_mutex_lock(&s->lock);
ret = qcow2_do_open(bs, options, flags, &local_err);
+ qemu_co_mutex_unlock(&s->lock);
QDECREF(options);
if (local_err) {
error_propagate(errp, local_err);
@@ -4386,7 +4389,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_change_backing_file = qcow2_change_backing_file,
.bdrv_refresh_limits = qcow2_refresh_limits,
- .bdrv_invalidate_cache = qcow2_invalidate_cache,
+ .bdrv_co_invalidate_cache = qcow2_co_invalidate_cache,
.bdrv_inactivate = qcow2_inactivate,
.create_opts = &qcow2_create_opts,
diff --git a/block/qed.c b/block/qed.c
index 3e1f4bc..5cf519c 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1550,7 +1550,8 @@ static int bdrv_qed_change_backing_file(BlockDriverState *bs,
return ret;
}
-static void bdrv_qed_invalidate_cache(BlockDriverState *bs, Error **errp)
+static void coroutine_fn bdrv_qed_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
BDRVQEDState *s = bs->opaque;
Error *local_err = NULL;
@@ -1559,13 +1560,9 @@ static void bdrv_qed_invalidate_cache(BlockDriverState *bs, Error **errp)
bdrv_qed_close(bs);
bdrv_qed_init_state(bs);
- if (qemu_in_coroutine()) {
- qemu_co_mutex_lock(&s->table_lock);
- }
+ qemu_co_mutex_lock(&s->table_lock);
ret = bdrv_qed_do_open(bs, NULL, bs->open_flags, &local_err);
- if (qemu_in_coroutine()) {
- qemu_co_mutex_unlock(&s->table_lock);
- }
+ qemu_co_mutex_unlock(&s->table_lock);
if (local_err) {
error_propagate(errp, local_err);
error_prepend(errp, "Could not reopen qed layer: ");
@@ -1640,7 +1637,7 @@ static BlockDriver bdrv_qed = {
.bdrv_get_info = bdrv_qed_get_info,
.bdrv_refresh_limits = bdrv_qed_refresh_limits,
.bdrv_change_backing_file = bdrv_qed_change_backing_file,
- .bdrv_invalidate_cache = bdrv_qed_invalidate_cache,
+ .bdrv_co_invalidate_cache = bdrv_qed_co_invalidate_cache,
.bdrv_check = bdrv_qed_check,
.bdrv_detach_aio_context = bdrv_qed_detach_aio_context,
.bdrv_attach_aio_context = bdrv_qed_attach_aio_context,
diff --git a/block/rbd.c b/block/rbd.c
index 67ac2ee..9632355 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1090,8 +1090,8 @@ static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs,
#endif
#ifdef LIBRBD_SUPPORTS_INVALIDATE
-static void qemu_rbd_invalidate_cache(BlockDriverState *bs,
- Error **errp)
+static void coroutine_fn qemu_rbd_co_invalidate_cache(BlockDriverState *bs,
+ Error **errp)
{
BDRVRBDState *s = bs->opaque;
int r = rbd_invalidate_cache(s->image);
@@ -1157,7 +1157,7 @@ static BlockDriver bdrv_rbd = {
.bdrv_snapshot_list = qemu_rbd_snap_list,
.bdrv_snapshot_goto = qemu_rbd_snap_rollback,
#ifdef LIBRBD_SUPPORTS_INVALIDATE
- .bdrv_invalidate_cache = qemu_rbd_invalidate_cache,
+ .bdrv_co_invalidate_cache = qemu_rbd_co_invalidate_cache,
#endif
};
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 05f498f..3481da1 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -218,7 +218,8 @@ struct BlockDriver {
/*
* Invalidate any cached meta-data.
*/
- void (*bdrv_invalidate_cache)(BlockDriverState *bs, Error **errp);
+ void coroutine_fn (*bdrv_co_invalidate_cache)(BlockDriverState *bs,
+ Error **errp);
int (*bdrv_inactivate)(BlockDriverState *bs);
/*
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 6/7] qcow2: introduce qcow2_write_caches and qcow2_flush_caches
2018-01-18 12:43 [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context Paolo Bonzini
` (4 preceding siblings ...)
2018-01-18 12:43 ` [Qemu-devel] [PATCH 5/7] block: convert bdrv_invalidate_cache callback to coroutine_fn Paolo Bonzini
@ 2018-01-18 12:43 ` Paolo Bonzini
2018-01-18 12:43 ` [Qemu-devel] [PATCH 7/7] block: convert bdrv_check callback to coroutine_fn Paolo Bonzini
` (3 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2018-01-18 12:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block, mreitz
Avoid recursively taking s->lock during bdrv_check.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/qcow2-refcount.c | 28 ++++++++++++++++++++++++++++
block/qcow2.c | 20 ++++----------------
block/qcow2.h | 2 ++
3 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 3de1ab5..e0d9dc1 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1172,7 +1172,35 @@ void qcow2_free_any_clusters(BlockDriverState *bs, uint64_t l2_entry,
}
}
+int coroutine_fn qcow2_write_caches(BlockDriverState *bs)
+{
+ BDRVQcow2State *s = bs->opaque;
+ int ret;
+ ret = qcow2_cache_write(bs, s->l2_table_cache);
+ if (ret < 0) {
+ return ret;
+ }
+
+ if (qcow2_need_accurate_refcounts(s)) {
+ ret = qcow2_cache_write(bs, s->refcount_block_cache);
+ if (ret < 0) {
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+int coroutine_fn qcow2_flush_caches(BlockDriverState *bs)
+{
+ int ret = qcow2_write_caches(bs);
+ if (ret < 0) {
+ return ret;
+ }
+
+ return bdrv_flush(bs->file->bs);
+}
/*********************************************************/
/* snapshots and image creation */
diff --git a/block/qcow2.c b/block/qcow2.c
index e886a44..4a10b55 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -492,7 +492,7 @@ static int qcow2_mark_clean(BlockDriverState *bs)
s->incompatible_features &= ~QCOW2_INCOMPAT_DIRTY;
- ret = bdrv_flush(bs);
+ ret = qcow2_flush_caches(bs);
if (ret < 0) {
return ret;
}
@@ -522,7 +522,7 @@ int qcow2_mark_consistent(BlockDriverState *bs)
BDRVQcow2State *s = bs->opaque;
if (s->incompatible_features & QCOW2_INCOMPAT_CORRUPT) {
- int ret = bdrv_flush(bs);
+ int ret = qcow2_flush_caches(bs);
if (ret < 0) {
return ret;
}
@@ -3653,22 +3653,10 @@ static coroutine_fn int qcow2_co_flush_to_os(BlockDriverState *bs)
int ret;
qemu_co_mutex_lock(&s->lock);
- ret = qcow2_cache_write(bs, s->l2_table_cache);
- if (ret < 0) {
- qemu_co_mutex_unlock(&s->lock);
- return ret;
- }
-
- if (qcow2_need_accurate_refcounts(s)) {
- ret = qcow2_cache_write(bs, s->refcount_block_cache);
- if (ret < 0) {
- qemu_co_mutex_unlock(&s->lock);
- return ret;
- }
- }
+ ret = qcow2_write_caches(bs);
qemu_co_mutex_unlock(&s->lock);
- return 0;
+ return ret;
}
static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
diff --git a/block/qcow2.h b/block/qcow2.h
index 46c8cf4..25b50f8 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -570,6 +570,8 @@ void qcow2_free_any_clusters(BlockDriverState *bs, uint64_t l2_entry,
int qcow2_update_snapshot_refcount(BlockDriverState *bs,
int64_t l1_table_offset, int l1_size, int addend);
+int coroutine_fn qcow2_flush_caches(BlockDriverState *bs);
+int coroutine_fn qcow2_write_caches(BlockDriverState *bs);
int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
BdrvCheckMode fix);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 7/7] block: convert bdrv_check callback to coroutine_fn
2018-01-18 12:43 [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context Paolo Bonzini
` (5 preceding siblings ...)
2018-01-18 12:43 ` [Qemu-devel] [PATCH 6/7] qcow2: introduce qcow2_write_caches and qcow2_flush_caches Paolo Bonzini
@ 2018-01-18 12:43 ` Paolo Bonzini
2018-01-18 20:25 ` [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context Eric Blake
` (2 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2018-01-18 12:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block, mreitz
Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 43 ++++++++++++++++++++++++++++++++++++++++---
block/parallels.c | 17 +++++++++++------
block/qcow2.c | 23 +++++++++++++++++++----
block/qed-check.c | 1 +
block/qed-table.c | 26 +++++++++-----------------
block/qed.c | 13 +++++++++----
block/vdi.c | 6 +++---
block/vhdx.c | 7 ++++---
block/vmdk.c | 7 ++++---
include/block/block_int.h | 5 +++--
10 files changed, 103 insertions(+), 45 deletions(-)
diff --git a/block.c b/block.c
index 6ba7471..896031b 100644
--- a/block.c
+++ b/block.c
@@ -3454,17 +3454,54 @@ static void bdrv_delete(BlockDriverState *bs)
* free of errors) or -errno when an internal error occurred. The results of the
* check are stored in res.
*/
-int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix)
+static int coroutine_fn bdrv_co_check(BlockDriverState *bs,
+ BdrvCheckResult *res, BdrvCheckMode fix)
{
if (bs->drv == NULL) {
return -ENOMEDIUM;
}
- if (bs->drv->bdrv_check == NULL) {
+ if (bs->drv->bdrv_co_check == NULL) {
return -ENOTSUP;
}
memset(res, 0, sizeof(*res));
- return bs->drv->bdrv_check(bs, res, fix);
+ return bs->drv->bdrv_co_check(bs, res, fix);
+}
+
+typedef struct CheckCo {
+ BlockDriverState *bs;
+ BdrvCheckResult *res;
+ BdrvCheckMode fix;
+ int ret;
+} CheckCo;
+
+static void bdrv_check_co_entry(void *opaque)
+{
+ CheckCo *cco = opaque;
+ cco->ret = bdrv_co_check(cco->bs, cco->res, cco->fix);
+}
+
+int bdrv_check(BlockDriverState *bs,
+ BdrvCheckResult *res, BdrvCheckMode fix)
+{
+ Coroutine *co;
+ CheckCo cco = {
+ .bs = bs,
+ .res = res,
+ .ret = -EINPROGRESS,
+ .fix = fix,
+ };
+
+ if (qemu_in_coroutine()) {
+ /* Fast-path if already in coroutine context */
+ bdrv_check_co_entry(&cco);
+ } else {
+ co = qemu_coroutine_create(bdrv_check_co_entry, &cco);
+ qemu_coroutine_enter(co);
+ BDRV_POLL_WHILE(bs, cco.ret == -EINPROGRESS);
+ }
+
+ return cco.ret;
}
/*
diff --git a/block/parallels.c b/block/parallels.c
index 5f1497d..803b063 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -382,8 +382,9 @@ static coroutine_fn int parallels_co_readv(BlockDriverState *bs,
}
-static int parallels_check(BlockDriverState *bs, BdrvCheckResult *res,
- BdrvCheckMode fix)
+static int coroutine_fn parallels_co_check(BlockDriverState *bs,
+ BdrvCheckResult *res,
+ BdrvCheckMode fix)
{
BDRVParallelsState *s = bs->opaque;
int64_t size, prev_off, high_off;
@@ -398,6 +399,7 @@ static int parallels_check(BlockDriverState *bs, BdrvCheckResult *res,
return size;
}
+ qemu_co_mutex_lock(&s->lock);
if (s->header_unclean) {
fprintf(stderr, "%s image was not closed correctly\n",
fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR");
@@ -446,11 +448,12 @@ static int parallels_check(BlockDriverState *bs, BdrvCheckResult *res,
prev_off = off;
}
+ ret = 0;
if (flush_bat) {
ret = bdrv_pwrite_sync(bs->file, 0, s->header, s->header_size);
if (ret < 0) {
res->check_errors++;
- return ret;
+ goto out;
}
}
@@ -469,13 +472,15 @@ static int parallels_check(BlockDriverState *bs, BdrvCheckResult *res,
if (ret < 0) {
error_report_err(local_err);
res->check_errors++;
- return ret;
+ goto out;
}
res->leaks_fixed += count;
}
}
- return 0;
+out:
+ qemu_co_mutex_unlock(&s->lock);
+ return ret;
}
@@ -802,7 +807,7 @@ static BlockDriver bdrv_parallels = {
.bdrv_co_writev = parallels_co_writev,
.bdrv_co_create_opts = parallels_co_create_opts,
- .bdrv_check = parallels_check,
+ .bdrv_co_check = parallels_co_check,
.create_opts = ¶llels_create_opts,
};
diff --git a/block/qcow2.c b/block/qcow2.c
index 4a10b55..8f9e37b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -533,8 +533,9 @@ int qcow2_mark_consistent(BlockDriverState *bs)
return 0;
}
-static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
- BdrvCheckMode fix)
+static int coroutine_fn qcow2_co_check_locked(BlockDriverState *bs,
+ BdrvCheckResult *result,
+ BdrvCheckMode fix)
{
int ret = qcow2_check_refcounts(bs, result, fix);
if (ret < 0) {
@@ -551,6 +552,19 @@ static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
return ret;
}
+static int coroutine_fn qcow2_co_check(BlockDriverState *bs,
+ BdrvCheckResult *result,
+ BdrvCheckMode fix)
+{
+ BDRVQcow2State *s = bs->opaque;
+ int ret;
+
+ qemu_co_mutex_lock(&s->lock);
+ ret = qcow2_co_check_locked(bs, result, fix);
+ qemu_co_mutex_unlock(&s->lock);
+ return ret;
+}
+
static int validate_table_offset(BlockDriverState *bs, uint64_t offset,
uint64_t entries, size_t entry_len)
{
@@ -1475,7 +1489,8 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
(s->incompatible_features & QCOW2_INCOMPAT_DIRTY)) {
BdrvCheckResult result = {0};
- ret = qcow2_check(bs, &result, BDRV_FIX_ERRORS | BDRV_FIX_LEAKS);
+ ret = qcow2_co_check_locked(bs, &result,
+ BDRV_FIX_ERRORS | BDRV_FIX_LEAKS);
if (ret < 0 || result.check_errors) {
if (ret >= 0) {
ret = -EIO;
@@ -4381,7 +4396,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_inactivate = qcow2_inactivate,
.create_opts = &qcow2_create_opts,
- .bdrv_check = qcow2_check,
+ .bdrv_co_check = qcow2_co_check,
.bdrv_amend_options = qcow2_amend_options,
.bdrv_detach_aio_context = qcow2_detach_aio_context,
diff --git a/block/qed-check.c b/block/qed-check.c
index dcd4f03..0edac03 100644
--- a/block/qed-check.c
+++ b/block/qed-check.c
@@ -217,6 +217,7 @@ static void qed_check_mark_clean(BDRVQEDState *s, BdrvCheckResult *result)
qed_write_header_sync(s);
}
+/* Called with table_lock held. */
int qed_check(BDRVQEDState *s, BdrvCheckResult *result, bool fix)
{
QEDCheck check = {
diff --git a/block/qed-table.c b/block/qed-table.c
index eead8b0..7df5680 100644
--- a/block/qed-table.c
+++ b/block/qed-table.c
@@ -18,7 +18,7 @@
#include "qed.h"
#include "qemu/bswap.h"
-/* Called either from qed_check or with table_lock held. */
+/* Called with table_lock held. */
static int qed_read_table(BDRVQEDState *s, uint64_t offset, QEDTable *table)
{
QEMUIOVector qiov;
@@ -33,13 +33,9 @@ static int qed_read_table(BDRVQEDState *s, uint64_t offset, QEDTable *table)
trace_qed_read_table(s, offset, table);
- if (qemu_in_coroutine()) {
- qemu_co_mutex_unlock(&s->table_lock);
- }
+ qemu_co_mutex_unlock(&s->table_lock);
ret = bdrv_preadv(s->bs->file, offset, &qiov);
- if (qemu_in_coroutine()) {
- qemu_co_mutex_lock(&s->table_lock);
- }
+ qemu_co_mutex_lock(&s->table_lock);
if (ret < 0) {
goto out;
}
@@ -67,7 +63,7 @@ out:
* @n: Number of elements
* @flush: Whether or not to sync to disk
*
- * Called either from qed_check or with table_lock held.
+ * Called with table_lock held.
*/
static int qed_write_table(BDRVQEDState *s, uint64_t offset, QEDTable *table,
unsigned int index, unsigned int n, bool flush)
@@ -104,13 +100,9 @@ static int qed_write_table(BDRVQEDState *s, uint64_t offset, QEDTable *table,
/* Adjust for offset into table */
offset += start * sizeof(uint64_t);
- if (qemu_in_coroutine()) {
- qemu_co_mutex_unlock(&s->table_lock);
- }
+ qemu_co_mutex_unlock(&s->table_lock);
ret = bdrv_pwritev(s->bs->file, offset, &qiov);
- if (qemu_in_coroutine()) {
- qemu_co_mutex_lock(&s->table_lock);
- }
+ qemu_co_mutex_lock(&s->table_lock);
trace_qed_write_table_cb(s, table, flush, ret);
if (ret < 0) {
goto out;
@@ -134,7 +126,7 @@ int qed_read_l1_table_sync(BDRVQEDState *s)
return qed_read_table(s, s->header.l1_table_offset, s->l1_table);
}
-/* Called either from qed_check or with table_lock held. */
+/* Called with table_lock held. */
int qed_write_l1_table(BDRVQEDState *s, unsigned int index, unsigned int n)
{
BLKDBG_EVENT(s->bs->file, BLKDBG_L1_UPDATE);
@@ -148,7 +140,7 @@ int qed_write_l1_table_sync(BDRVQEDState *s, unsigned int index,
return qed_write_l1_table(s, index, n);
}
-/* Called either from qed_check or with table_lock held. */
+/* Called with table_lock held. */
int qed_read_l2_table(BDRVQEDState *s, QEDRequest *request, uint64_t offset)
{
int ret;
@@ -191,7 +183,7 @@ int qed_read_l2_table_sync(BDRVQEDState *s, QEDRequest *request, uint64_t offset
return qed_read_l2_table(s, request, offset);
}
-/* Called either from qed_check or with table_lock held. */
+/* Called with table_lock held. */
int qed_write_l2_table(BDRVQEDState *s, QEDRequest *request,
unsigned int index, unsigned int n, bool flush)
{
diff --git a/block/qed.c b/block/qed.c
index 5cf519c..77c9b10 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1573,12 +1573,17 @@ static void coroutine_fn bdrv_qed_co_invalidate_cache(BlockDriverState *bs,
}
}
-static int bdrv_qed_check(BlockDriverState *bs, BdrvCheckResult *result,
- BdrvCheckMode fix)
+static int bdrv_qed_co_check(BlockDriverState *bs, BdrvCheckResult *result,
+ BdrvCheckMode fix)
{
BDRVQEDState *s = bs->opaque;
+ int ret;
- return qed_check(s, result, !!fix);
+ qemu_co_mutex_lock(&s->table_lock);
+ ret = qed_check(s, result, !!fix);
+ qemu_co_mutex_unlock(&s->table_lock);
+
+ return ret;
}
static QemuOptsList qed_create_opts = {
@@ -1638,7 +1643,7 @@ static BlockDriver bdrv_qed = {
.bdrv_refresh_limits = bdrv_qed_refresh_limits,
.bdrv_change_backing_file = bdrv_qed_change_backing_file,
.bdrv_co_invalidate_cache = bdrv_qed_co_invalidate_cache,
- .bdrv_check = bdrv_qed_check,
+ .bdrv_co_check = bdrv_qed_co_check,
.bdrv_detach_aio_context = bdrv_qed_detach_aio_context,
.bdrv_attach_aio_context = bdrv_qed_attach_aio_context,
.bdrv_co_drain_begin = bdrv_qed_co_drain_begin,
diff --git a/block/vdi.c b/block/vdi.c
index 01501d7..bb505ee 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -258,8 +258,8 @@ static void vdi_header_print(VdiHeader *header)
}
#endif
-static int vdi_check(BlockDriverState *bs, BdrvCheckResult *res,
- BdrvCheckMode fix)
+static int coroutine_fn vdi_co_check(BlockDriverState *bs, BdrvCheckResult *res,
+ BdrvCheckMode fix)
{
/* TODO: additional checks possible. */
BDRVVdiState *s = (BDRVVdiState *)bs->opaque;
@@ -908,7 +908,7 @@ static BlockDriver bdrv_vdi = {
.bdrv_get_info = vdi_get_info,
.create_opts = &vdi_create_opts,
- .bdrv_check = vdi_check,
+ .bdrv_co_check = vdi_co_check,
};
static void bdrv_vdi_init(void)
diff --git a/block/vhdx.c b/block/vhdx.c
index 7fad8b2..14b33ed 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1944,8 +1944,9 @@ exit:
* r/w and any log has already been replayed, so there is nothing (currently)
* for us to do here
*/
-static int vhdx_check(BlockDriverState *bs, BdrvCheckResult *result,
- BdrvCheckMode fix)
+static int coroutine_fn vhdx_co_check(BlockDriverState *bs,
+ BdrvCheckResult *result,
+ BdrvCheckMode fix)
{
BDRVVHDXState *s = bs->opaque;
@@ -2006,7 +2007,7 @@ static BlockDriver bdrv_vhdx = {
.bdrv_co_writev = vhdx_co_writev,
.bdrv_co_create_opts = vhdx_co_create_opts,
.bdrv_get_info = vhdx_get_info,
- .bdrv_check = vhdx_check,
+ .bdrv_co_check = vhdx_co_check,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.create_opts = &vhdx_create_opts,
diff --git a/block/vmdk.c b/block/vmdk.c
index d4beb25..3614728 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2214,8 +2214,9 @@ static ImageInfo *vmdk_get_extent_info(VmdkExtent *extent)
return info;
}
-static int vmdk_check(BlockDriverState *bs, BdrvCheckResult *result,
- BdrvCheckMode fix)
+static int coroutine_fn vmdk_co_check(BlockDriverState *bs,
+ BdrvCheckResult *result,
+ BdrvCheckMode fix)
{
BDRVVmdkState *s = bs->opaque;
VmdkExtent *extent = NULL;
@@ -2384,7 +2385,7 @@ static BlockDriver bdrv_vmdk = {
.instance_size = sizeof(BDRVVmdkState),
.bdrv_probe = vmdk_probe,
.bdrv_open = vmdk_open,
- .bdrv_check = vmdk_check,
+ .bdrv_co_check = vmdk_co_check,
.bdrv_reopen_prepare = vmdk_reopen_prepare,
.bdrv_child_perm = bdrv_format_default_perms,
.bdrv_co_preadv = vmdk_co_preadv,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3481da1..3e4c93c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -301,8 +301,9 @@ struct BlockDriver {
* Returns 0 for completed check, -errno for internal errors.
* The check results are stored in result.
*/
- int (*bdrv_check)(BlockDriverState *bs, BdrvCheckResult *result,
- BdrvCheckMode fix);
+ int coroutine_fn (*bdrv_co_check)(BlockDriverState *bs,
+ BdrvCheckResult *result,
+ BdrvCheckMode fix);
int (*bdrv_amend_options)(BlockDriverState *bs, QemuOpts *opts,
BlockDriverAmendStatusCB *status_cb,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 4/7] qed: make bdrv_qed_do_open a coroutine_fn
2018-01-18 12:43 ` [Qemu-devel] [PATCH 4/7] qed: make bdrv_qed_do_open " Paolo Bonzini
@ 2018-01-18 20:19 ` Eric Blake
2018-02-01 14:48 ` [Qemu-devel] [Qemu-block] " Paolo Bonzini
0 siblings, 1 reply; 16+ messages in thread
From: Eric Blake @ 2018-01-18 20:19 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: kwolf, qemu-block, mreitz
[-- Attachment #1: Type: text/plain, Size: 651 bytes --]
On 01/18/2018 06:43 AM, Paolo Bonzini wrote:
> It is called from qcow2_invalidate_cache in coroutine context (incoming
Why is a qcow2 function calling qed code? Definitely a copy-paste bug,
but I'm not sure of the right fix.
> migration runs in a coroutine), so it's cleaner if metadata is always
> loaded from a coroutine.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> block/qed.c | 40 +++++++++++++++++++++++++++++++++++++---
> 1 file changed, 37 insertions(+), 3 deletions(-)
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context
2018-01-18 12:43 [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context Paolo Bonzini
` (6 preceding siblings ...)
2018-01-18 12:43 ` [Qemu-devel] [PATCH 7/7] block: convert bdrv_check callback to coroutine_fn Paolo Bonzini
@ 2018-01-18 20:25 ` Eric Blake
2018-02-01 14:48 ` Paolo Bonzini
2018-02-26 16:49 ` Kevin Wolf
9 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2018-01-18 20:25 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: kwolf, qemu-block, mreitz
[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]
On 01/18/2018 06:43 AM, Paolo Bonzini wrote:
> Check and invalidate_cache share some parts of the implementation
> with the regular I/O path. This is sometimes complicated because the
> I/O path wants to use a CoMutex but that is not possible outside coroutine
> context. By moving things to coroutine context, we can remove special
> cases. In fact, invalidate_cache is already called from coroutine context
> because incoming migration is placed in a coroutine.
>
> While at it, I'm including two patches from Stefan to rename the
> bdrv_create callback to bdrv_co_create, because it is already called
> from coroutine context. The name is now bdrv_co_create_opts, with
> bdrv_co_create reserved for the QAPI-based version that Kevin is
> working on.
>
> qcow2 still has cache flushing in non-coroutine context, coming from
> qcow2_reopen_prepare->qcow2_update_options_prepare and
> qcow2_close->qcow2_inactivate.
>
> Paolo
>
Modulo a commit message nit in 4/7, series
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 4/7] qed: make bdrv_qed_do_open a coroutine_fn
2018-01-18 20:19 ` Eric Blake
@ 2018-02-01 14:48 ` Paolo Bonzini
0 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2018-02-01 14:48 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: kwolf, qemu-block, mreitz
On 18/01/2018 15:19, Eric Blake wrote:
> On 01/18/2018 06:43 AM, Paolo Bonzini wrote:
>> It is called from qcow2_invalidate_cache in coroutine context (incoming
> Why is a qcow2 function calling qed code? Definitely a copy-paste bug,
> but I'm not sure of the right fix.
This reference was of course meant to be qed_invalidate_cache.
Thanks,
Paolo
>> migration runs in a coroutine), so it's cleaner if metadata is always
>> loaded from a coroutine.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context
2018-01-18 12:43 [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context Paolo Bonzini
` (7 preceding siblings ...)
2018-01-18 20:25 ` [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context Eric Blake
@ 2018-02-01 14:48 ` Paolo Bonzini
2018-02-26 16:49 ` Kevin Wolf
9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2018-02-01 14:48 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block, mreitz
On 18/01/2018 07:43, Paolo Bonzini wrote:
> Check and invalidate_cache share some parts of the implementation
> with the regular I/O path. This is sometimes complicated because the
> I/O path wants to use a CoMutex but that is not possible outside coroutine
> context. By moving things to coroutine context, we can remove special
> cases. In fact, invalidate_cache is already called from coroutine context
> because incoming migration is placed in a coroutine.
>
> While at it, I'm including two patches from Stefan to rename the
> bdrv_create callback to bdrv_co_create, because it is already called
> from coroutine context. The name is now bdrv_co_create_opts, with
> bdrv_co_create reserved for the QAPI-based version that Kevin is
> working on.
>
> qcow2 still has cache flushing in non-coroutine context, coming from
> qcow2_reopen_prepare->qcow2_update_options_prepare and
> qcow2_close->qcow2_inactivate.
>
> Paolo
Ping?
Thanks,
Paolo
> v1->v2: fix file-win32
>
> Paolo Bonzini (5):
> qcow2: make qcow2_do_open a coroutine_fn
> qed: make bdrv_qed_do_open a coroutine_fn
> block: convert bdrv_invalidate_cache callback to coroutine_fn
> qcow2: introduce qcow2_write_caches and qcow2_flush_caches
> block: convert bdrv_check callback to coroutine_fn
>
> Stefan Hajnoczi (2):
> block: rename .bdrv_create() to .bdrv_co_create_opts()
> qcow2: make qcow2_co_create2() a coroutine_fn
>
> block.c | 88 ++++++++++++++++++++++++++++++----
> block/crypto.c | 8 ++--
> block/file-posix.c | 15 +++---
> block/file-win32.c | 5 +-
> block/gluster.c | 12 ++---
> block/iscsi.c | 13 ++---
> block/nfs.c | 11 +++--
> block/parallels.c | 23 +++++----
> block/qcow.c | 5 +-
> block/qcow2-refcount.c | 28 +++++++++++
> block/qcow2.c | 118 +++++++++++++++++++++++++++++++---------------
> block/qcow2.h | 2 +
> block/qed-check.c | 1 +
> block/qed-table.c | 26 ++++------
> block/qed.c | 72 +++++++++++++++++++++-------
> block/raw-format.c | 5 +-
> block/rbd.c | 12 +++--
> block/sheepdog.c | 10 ++--
> block/ssh.c | 5 +-
> block/vdi.c | 11 +++--
> block/vhdx.c | 12 +++--
> block/vmdk.c | 12 +++--
> block/vpc.c | 5 +-
> include/block/block_int.h | 11 +++--
> 24 files changed, 355 insertions(+), 155 deletions(-)
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context
2018-01-18 12:43 [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context Paolo Bonzini
` (8 preceding siblings ...)
2018-02-01 14:48 ` Paolo Bonzini
@ 2018-02-26 16:49 ` Kevin Wolf
9 siblings, 0 replies; 16+ messages in thread
From: Kevin Wolf @ 2018-02-26 16:49 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-block, mreitz
Am 18.01.2018 um 13:43 hat Paolo Bonzini geschrieben:
> Check and invalidate_cache share some parts of the implementation
> with the regular I/O path. This is sometimes complicated because the
> I/O path wants to use a CoMutex but that is not possible outside coroutine
> context. By moving things to coroutine context, we can remove special
> cases. In fact, invalidate_cache is already called from coroutine context
> because incoming migration is placed in a coroutine.
>
> While at it, I'm including two patches from Stefan to rename the
> bdrv_create callback to bdrv_co_create, because it is already called
> from coroutine context. The name is now bdrv_co_create_opts, with
> bdrv_co_create reserved for the QAPI-based version that Kevin is
> working on.
>
> qcow2 still has cache flushing in non-coroutine context, coming from
> qcow2_reopen_prepare->qcow2_update_options_prepare and
> qcow2_close->qcow2_inactivate.
The patches looked good, but this deadlocks qemu-iotests 165 for me:
#0 0x0000562392ae1cb0 in qemu_coroutine_switch (from_=from_@entry=0x562393de2410, to_=to_@entry=0x7f81bf44ee48, action=action@entry=COROUTINE_YIELD) at util/coroutine-ucontext.c:219
#1 0x0000562392ae0ad1 in qemu_coroutine_yield () at util/qemu-coroutine.c:186
#2 0x0000562392ae0cb4 in qemu_co_mutex_lock_slowpath (mutex=0x562393de1870, ctx=0x562393dc1ad0) at util/qemu-coroutine-lock.c:269
#3 0x0000562392ae0cb4 in qemu_co_mutex_lock (mutex=mutex@entry=0x562393de1870) at util/qemu-coroutine-lock.c:307
#4 0x0000562392a149fc in qcow2_co_flush_to_os (bs=0x562393dd6750) at block/qcow2.c:3705
#5 0x0000562392a461c9 in bdrv_co_flush (bs=0x562393dd6750) at block/io.c:2439
#6 0x0000562392a46639 in bdrv_flush_co_entry (opaque=0x7f8195a7cd50) at block/io.c:2403
#7 0x0000562392a46639 in bdrv_flush (bs=bs@entry=0x562393dd6750) at block/io.c:2528
#8 0x0000562392a26de6 in update_header_sync (bs=bs@entry=0x562393dd6750) at block/qcow2-bitmap.c:113
#9 0x0000562392a26e5a in update_ext_header_and_dir_in_place (bs=bs@entry=0x562393dd6750, bm_list=bm_list@entry=0x562393de14f0) at block/qcow2-bitmap.c:826
#10 0x0000562392a27fa6 in qcow2_load_dirty_bitmaps (bs=bs@entry=0x562393dd6750, errp=errp@entry=0x7f8195a7ce68) at block/qcow2-bitmap.c:982
#11 0x0000562392a1947c in qcow2_do_open (bs=0x562393dd6750, options=<optimized out>, flags=8194, errp=0x7ffc3289a110) at block/qcow2.c:1501
#12 0x0000562392a198e2 in qcow2_open_entry (opaque=0x7ffc3289a0b0) at block/qcow2.c:1578
#13 0x0000562392ae1d1c in coroutine_trampoline (i0=<optimized out>, i1=<optimized out>) at util/coroutine-ucontext.c:116
#14 0x00007f81a338d950 in __start_context () at /lib64/libc.so.6
#15 0x00007ffc32899920 in ()
#16 0x0000000000000000 in ()
Not saving the coroutine pointer anywhere was a bit nasty, too. gdb
only gave the coroutine pointer away with something as indirect as
'p ((BDRVQcow2State*) bs.opaque).lock.holder'.
Kevin
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] block: convert bdrv_invalidate_cache callback to coroutine_fn
2018-01-18 12:43 ` [Qemu-devel] [PATCH 5/7] block: convert bdrv_invalidate_cache callback to coroutine_fn Paolo Bonzini
@ 2018-03-15 13:21 ` Andreas Schwab
0 siblings, 0 replies; 16+ messages in thread
From: Andreas Schwab @ 2018-03-15 13:21 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, kwolf, qemu-block, mreitz
block/iscsi.c:2247:6: error: 'BlockDriver {aka struct BlockDriver}' has no member named 'bdrv_invalidate_cache'; did you mean 'bdrv_co_invalidate_cache'?
.bdrv_invalidate_cache = iscsi_invalidate_cache,
^~~~~~~~~~~~~~~~~~~~~
bdrv_co_invalidate_cache
block/iscsi.c:2247:31: error: 'iscsi_invalidate_cache' undeclared here (not in a function); did you mean 'iscsi_co_invalidate_cache'?
.bdrv_invalidate_cache = iscsi_invalidate_cache,
^~~~~~~~~~~~~~~~~~~~~~
iscsi_co_invalidate_cache
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-03-15 13:22 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-18 12:43 [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context Paolo Bonzini
2018-01-18 12:43 ` [Qemu-devel] [PATCH 1/7] block: rename .bdrv_create() to .bdrv_co_create_opts() Paolo Bonzini
2018-01-18 12:43 ` [Qemu-devel] [PATCH 2/7] qcow2: make qcow2_co_create2() a coroutine_fn Paolo Bonzini
2018-01-18 12:43 ` [Qemu-devel] [PATCH 3/7] qcow2: make qcow2_do_open " Paolo Bonzini
2018-01-18 12:43 ` [Qemu-devel] [PATCH 4/7] qed: make bdrv_qed_do_open " Paolo Bonzini
2018-01-18 20:19 ` Eric Blake
2018-02-01 14:48 ` [Qemu-devel] [Qemu-block] " Paolo Bonzini
2018-01-18 12:43 ` [Qemu-devel] [PATCH 5/7] block: convert bdrv_invalidate_cache callback to coroutine_fn Paolo Bonzini
2018-03-15 13:21 ` Andreas Schwab
2018-01-18 12:43 ` [Qemu-devel] [PATCH 6/7] qcow2: introduce qcow2_write_caches and qcow2_flush_caches Paolo Bonzini
2018-01-18 12:43 ` [Qemu-devel] [PATCH 7/7] block: convert bdrv_check callback to coroutine_fn Paolo Bonzini
2018-01-18 20:25 ` [Qemu-devel] [PATCH v3 0/7] Call check and invalidate_cache from coroutine context Eric Blake
2018-02-01 14:48 ` Paolo Bonzini
2018-02-26 16:49 ` Kevin Wolf
-- strict thread matches above, loose matches on Subject: below --
2018-01-17 11:25 [Qemu-devel] [PATCH v2 " Paolo Bonzini
2018-01-17 11:25 ` [Qemu-devel] [PATCH 5/7] block: convert bdrv_invalidate_cache callback to coroutine_fn Paolo Bonzini
2018-01-17 11:00 [Qemu-devel] [PATCH 0/7] Call check and invalidate_cache from coroutine context Paolo Bonzini
2018-01-17 11:00 ` [Qemu-devel] [PATCH 5/7] block: convert bdrv_invalidate_cache callback to coroutine_fn Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).