* [Qemu-devel] [RFC v2 02/15] Rename qemu_coroutine_self to qemu_coroutine_self_int and add an annotated wrapper
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
@ 2013-08-09 17:43 ` Charlie Shepherd
2013-08-14 14:21 ` Stefan Hajnoczi
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 03/15] Explicitly mark BlockDriver function .bdrv_create as coroutine and rename it bdrv_co_create Charlie Shepherd
` (14 subsequent siblings)
15 siblings, 1 reply; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-09 17:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, gabriel, Charlie Shepherd, pbonzini
While it only really makes sense to call qemu_coroutine_self() in a coroutine
context, some coroutine internals need to call it from functions not annotated
as coroutine_fn, so add an annotated wrapper and rename the implementation
versions to qemu_coroutine_self_int.
Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
---
coroutine-gthread.c | 2 +-
coroutine-sigaltstack.c | 2 +-
coroutine-ucontext.c | 2 +-
coroutine-win32.c | 2 +-
include/block/coroutine_int.h | 1 +
qemu-coroutine.c | 7 ++++++-
6 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/coroutine-gthread.c b/coroutine-gthread.c
index d3e5b99..a913aeb 100644
--- a/coroutine-gthread.c
+++ b/coroutine-gthread.c
@@ -194,7 +194,7 @@ CoroutineAction qemu_coroutine_switch(Coroutine *from_,
return from->action;
}
-Coroutine *qemu_coroutine_self(void)
+Coroutine *qemu_coroutine_self_int(void)
{
CoroutineGThread *co = get_coroutine_key();
if (!co) {
diff --git a/coroutine-sigaltstack.c b/coroutine-sigaltstack.c
index 3de0bb3..0556539 100644
--- a/coroutine-sigaltstack.c
+++ b/coroutine-sigaltstack.c
@@ -277,7 +277,7 @@ CoroutineAction qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
return ret;
}
-Coroutine *qemu_coroutine_self(void)
+Coroutine *qemu_coroutine_self_int(void)
{
CoroutineThreadState *s = coroutine_get_thread_state();
diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
index 4bf2cde..27d1b79 100644
--- a/coroutine-ucontext.c
+++ b/coroutine-ucontext.c
@@ -210,7 +210,7 @@ CoroutineAction qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
return ret;
}
-Coroutine *qemu_coroutine_self(void)
+Coroutine *qemu_coroutine_self_int(void)
{
CoroutineThreadState *s = coroutine_get_thread_state();
diff --git a/coroutine-win32.c b/coroutine-win32.c
index edc1f72..3f1f79b 100644
--- a/coroutine-win32.c
+++ b/coroutine-win32.c
@@ -77,7 +77,7 @@ void qemu_coroutine_delete(Coroutine *co_)
g_free(co);
}
-Coroutine *qemu_coroutine_self(void)
+Coroutine *qemu_coroutine_self_int(void)
{
if (!current) {
current = &leader.base;
diff --git a/include/block/coroutine_int.h b/include/block/coroutine_int.h
index f133d65..f6191ad 100644
--- a/include/block/coroutine_int.h
+++ b/include/block/coroutine_int.h
@@ -48,6 +48,7 @@ Coroutine *qemu_coroutine_new(void);
void qemu_coroutine_delete(Coroutine *co);
CoroutineAction qemu_coroutine_switch(Coroutine *from, Coroutine *to,
CoroutineAction action);
+Coroutine *qemu_coroutine_self_int(void);
void coroutine_fn qemu_co_queue_run_restart(Coroutine *co);
#endif
diff --git a/qemu-coroutine.c b/qemu-coroutine.c
index 423430d..b147f87 100644
--- a/qemu-coroutine.c
+++ b/qemu-coroutine.c
@@ -104,7 +104,7 @@ static void coroutine_swap(Coroutine *from, Coroutine *to)
void qemu_coroutine_enter(Coroutine *co, void *opaque)
{
- Coroutine *self = qemu_coroutine_self();
+ Coroutine *self = qemu_coroutine_self_int();
trace_qemu_coroutine_enter(self, co, opaque);
@@ -133,3 +133,8 @@ void coroutine_fn qemu_coroutine_yield(void)
self->caller = NULL;
coroutine_swap(self, to);
}
+
+coroutine_fn Coroutine *qemu_coroutine_self(void)
+{
+ return qemu_coroutine_self_int();
+}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [RFC v2 02/15] Rename qemu_coroutine_self to qemu_coroutine_self_int and add an annotated wrapper
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 02/15] Rename qemu_coroutine_self to qemu_coroutine_self_int and add an annotated wrapper Charlie Shepherd
@ 2013-08-14 14:21 ` Stefan Hajnoczi
0 siblings, 0 replies; 28+ messages in thread
From: Stefan Hajnoczi @ 2013-08-14 14:21 UTC (permalink / raw)
To: Charlie Shepherd; +Cc: kwolf, pbonzini, gabriel, qemu-devel
On Fri, Aug 09, 2013 at 07:43:52PM +0200, Charlie Shepherd wrote:
> @@ -133,3 +133,8 @@ void coroutine_fn qemu_coroutine_yield(void)
> self->caller = NULL;
> coroutine_swap(self, to);
> }
> +
> +coroutine_fn Coroutine *qemu_coroutine_self(void)
> +{
> + return qemu_coroutine_self_int();
> +}
The example in include/block/coroutine.h says:
* For example:
*
* static void coroutine_fn foo(void) {
* ....
* }
Not sure how happy compilers are with __attribute__ before the return
type, but I suggest consistently placing coroutine_fn after the return
type.
Also, a comment would be helpful here to explain the need for the
external/internal functions. Otherwise the next person might undo this
again thinking it pointless :).
coroutine_fn Coroutine *qemu_coroutine_self(void)
{
/* Call the internal version of this function, which is
* non-coroutine_fn and can therefore be called from from
* non-coroutine contexts. Internally we know it's always possible
* to pull a Coroutine* out of thin air (or thread-local storage).
* External callers shouldn't assume they can always get a
* Coroutine* since we may not be in coroutine context, hence the
* external version of this function.
*/
return qemu_coroutine_self_int();
}
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [RFC v2 03/15] Explicitly mark BlockDriver function .bdrv_create as coroutine and rename it bdrv_co_create.
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 02/15] Rename qemu_coroutine_self to qemu_coroutine_self_int and add an annotated wrapper Charlie Shepherd
@ 2013-08-09 17:43 ` Charlie Shepherd
2013-08-14 14:26 ` Stefan Hajnoczi
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 04/15] Convert .bdrv_open and .bdrv_file_open to coroutine_fn Charlie Shepherd
` (13 subsequent siblings)
15 siblings, 1 reply; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-09 17:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, gabriel, Charlie Shepherd, pbonzini
Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
---
block.c | 4 ++--
block/cow.c | 4 ++--
block/qcow.c | 4 ++--
block/qcow2.c | 4 ++--
block/qed.c | 4 ++--
block/raw-posix.c | 14 +++++++-------
block/raw.c | 4 ++--
block/sheepdog.c | 8 ++++----
block/ssh.c | 4 ++--
block/vdi.c | 4 ++--
block/vmdk.c | 4 ++--
block/vpc.c | 4 ++--
include/block/block_int.h | 2 +-
13 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/block.c b/block.c
index 01b66d8..40f73ee 100644
--- a/block.c
+++ b/block.c
@@ -374,7 +374,7 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
CreateCo *cco = opaque;
assert(cco->drv);
- cco->ret = cco->drv->bdrv_create(cco->filename, cco->options);
+ cco->ret = cco->drv->bdrv_co_create(cco->filename, cco->options);
}
int bdrv_create(BlockDriver *drv, const char* filename,
@@ -390,7 +390,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
.ret = NOT_DONE,
};
- if (!drv->bdrv_create) {
+ if (!drv->bdrv_co_create) {
ret = -ENOTSUP;
goto out;
}
diff --git a/block/cow.c b/block/cow.c
index 1cc2e89..34c181a 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -255,7 +255,7 @@ static void cow_close(BlockDriverState *bs)
{
}
-static int cow_create(const char *filename, QEMUOptionParameter *options)
+static int coroutine_fn cow_co_create(const char *filename, QEMUOptionParameter *options)
{
struct cow_header_v2 cow_header;
struct stat st;
@@ -339,7 +339,7 @@ static BlockDriver bdrv_cow = {
.bdrv_probe = cow_probe,
.bdrv_open = cow_open,
.bdrv_close = cow_close,
- .bdrv_create = cow_create,
+ .bdrv_co_create = cow_co_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_read = cow_co_read,
diff --git a/block/qcow.c b/block/qcow.c
index 5239bd6..c367c55 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -651,7 +651,7 @@ static void qcow_close(BlockDriverState *bs)
error_free(s->migration_blocker);
}
-static int qcow_create(const char *filename, QEMUOptionParameter *options)
+static int coroutine_fn qcow_create(const char *filename, QEMUOptionParameter *options)
{
int header_size, backing_filename_len, l1_size, shift, i;
QCowHeader header;
@@ -891,7 +891,7 @@ static BlockDriver bdrv_qcow = {
.bdrv_open = qcow_open,
.bdrv_close = qcow_close,
.bdrv_reopen_prepare = qcow_reopen_prepare,
- .bdrv_create = qcow_create,
+ .bdrv_co_create = qcow_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_co_readv = qcow_co_readv,
diff --git a/block/qcow2.c b/block/qcow2.c
index 3376901..656deb9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1394,7 +1394,7 @@ out:
return ret;
}
-static int qcow2_create(const char *filename, QEMUOptionParameter *options)
+static int coroutine_fn qcow2_co_create(const char *filename, QEMUOptionParameter *options)
{
const char *backing_file = NULL;
const char *backing_fmt = NULL;
@@ -1784,7 +1784,7 @@ static BlockDriver bdrv_qcow2 = {
.bdrv_open = qcow2_open,
.bdrv_close = qcow2_close,
.bdrv_reopen_prepare = qcow2_reopen_prepare,
- .bdrv_create = qcow2_create,
+ .bdrv_co_create = qcow2_co_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_co_is_allocated = qcow2_co_is_allocated,
.bdrv_set_key = qcow2_set_key,
diff --git a/block/qed.c b/block/qed.c
index f767b05..083f20f 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -603,7 +603,7 @@ out:
return ret;
}
-static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options)
+static int coroutine_fn bdrv_qed_co_create(const char *filename, QEMUOptionParameter *options)
{
uint64_t image_size = 0;
uint32_t cluster_size = QED_DEFAULT_CLUSTER_SIZE;
@@ -1573,7 +1573,7 @@ static BlockDriver bdrv_qed = {
.bdrv_open = bdrv_qed_open,
.bdrv_close = bdrv_qed_close,
.bdrv_reopen_prepare = bdrv_qed_reopen_prepare,
- .bdrv_create = bdrv_qed_create,
+ .bdrv_co_create = bdrv_qed_co_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_co_is_allocated = bdrv_qed_co_is_allocated,
.bdrv_make_empty = bdrv_qed_make_empty,
diff --git a/block/raw-posix.c b/block/raw-posix.c
index ba721d3..306a0b0 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1040,7 +1040,7 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
return (int64_t)st.st_blocks * 512;
}
-static int raw_create(const char *filename, QEMUOptionParameter *options)
+static int coroutine_fn raw_co_create(const char *filename, QEMUOptionParameter *options)
{
int fd;
int result = 0;
@@ -1198,7 +1198,7 @@ static 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 = raw_co_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_co_is_allocated = raw_co_is_allocated,
@@ -1498,7 +1498,7 @@ static coroutine_fn BlockDriverAIOCB *hdev_aio_discard(BlockDriverState *bs,
cb, opaque, QEMU_AIO_DISCARD|QEMU_AIO_BLKDEV);
}
-static int hdev_create(const char *filename, QEMUOptionParameter *options)
+static int coroutine_fn hdev_co_create(const char *filename, QEMUOptionParameter *options)
{
int fd;
int ret = 0;
@@ -1538,7 +1538,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 = hdev_co_create,
.create_options = raw_create_options,
.bdrv_aio_readv = raw_aio_readv,
@@ -1662,7 +1662,7 @@ static BlockDriver bdrv_host_floppy = {
.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 = hdev_co_create,
.create_options = raw_create_options,
.bdrv_aio_readv = raw_aio_readv,
@@ -1763,7 +1763,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 = hdev_co_create,
.create_options = raw_create_options,
.bdrv_aio_readv = raw_aio_readv,
@@ -1884,7 +1884,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 = hdev_co_create,
.create_options = raw_create_options,
.bdrv_aio_readv = raw_aio_readv,
diff --git a/block/raw.c b/block/raw.c
index f1682d4..6f1f4e7 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -102,7 +102,7 @@ static BlockDriverAIOCB *raw_aio_ioctl(BlockDriverState *bs,
return bdrv_aio_ioctl(bs->file, req, buf, cb, opaque);
}
-static int raw_create(const char *filename, QEMUOptionParameter *options)
+static int coroutine_fn raw_co_create(const char *filename, QEMUOptionParameter *options)
{
return bdrv_create_file(filename, options);
}
@@ -156,7 +156,7 @@ static BlockDriver bdrv_raw = {
.bdrv_ioctl = raw_ioctl,
.bdrv_aio_ioctl = raw_aio_ioctl,
- .bdrv_create = raw_create,
+ .bdrv_co_create = raw_co_create,
.create_options = raw_create_options,
.bdrv_has_zero_init = raw_has_zero_init,
};
diff --git a/block/sheepdog.c b/block/sheepdog.c
index afe0533..7ca70de 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1454,7 +1454,7 @@ out:
return ret;
}
-static int sd_create(const char *filename, QEMUOptionParameter *options)
+static int coroutine_fn sd_co_create(const char *filename, QEMUOptionParameter *options)
{
int ret = 0;
uint32_t vid = 0, base_vid = 0;
@@ -2346,7 +2346,7 @@ static BlockDriver bdrv_sheepdog = {
.instance_size = sizeof(BDRVSheepdogState),
.bdrv_file_open = sd_open,
.bdrv_close = sd_close,
- .bdrv_create = sd_create,
+ .bdrv_co_create = sd_co_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_getlength = sd_getlength,
.bdrv_truncate = sd_truncate,
@@ -2374,7 +2374,7 @@ static BlockDriver bdrv_sheepdog_tcp = {
.instance_size = sizeof(BDRVSheepdogState),
.bdrv_file_open = sd_open,
.bdrv_close = sd_close,
- .bdrv_create = sd_create,
+ .bdrv_co_create = sd_co_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_getlength = sd_getlength,
.bdrv_truncate = sd_truncate,
@@ -2402,7 +2402,7 @@ static BlockDriver bdrv_sheepdog_unix = {
.instance_size = sizeof(BDRVSheepdogState),
.bdrv_file_open = sd_open,
.bdrv_close = sd_close,
- .bdrv_create = sd_create,
+ .bdrv_co_create = sd_co_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_getlength = sd_getlength,
.bdrv_truncate = sd_truncate,
diff --git a/block/ssh.c b/block/ssh.c
index d7e7bf8..3246185 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -650,7 +650,7 @@ static QEMUOptionParameter ssh_create_options[] = {
{ NULL }
};
-static int ssh_create(const char *filename, QEMUOptionParameter *options)
+static int coroutine_fn ssh_co_create(const char *filename, QEMUOptionParameter *options)
{
int r, ret;
Error *local_err = NULL;
@@ -1050,7 +1050,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 = ssh_co_create,
.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 8a91525..38e08fe 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -633,7 +633,7 @@ static int vdi_co_write(BlockDriverState *bs,
return ret;
}
-static int vdi_create(const char *filename, QEMUOptionParameter *options)
+static int coroutine_fn vdi_co_create(const char *filename, QEMUOptionParameter *options)
{
int fd;
int result = 0;
@@ -778,7 +778,7 @@ static BlockDriver bdrv_vdi = {
.bdrv_open = vdi_open,
.bdrv_close = vdi_close,
.bdrv_reopen_prepare = vdi_reopen_prepare,
- .bdrv_create = vdi_create,
+ .bdrv_co_create = vdi_co_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_co_is_allocated = vdi_co_is_allocated,
.bdrv_make_empty = vdi_make_empty,
diff --git a/block/vmdk.c b/block/vmdk.c
index 346bb5c..5e81519 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1532,7 +1532,7 @@ static int filename_decompose(const char *filename, char *path, char *prefix,
return VMDK_OK;
}
-static int vmdk_create(const char *filename, QEMUOptionParameter *options)
+static int coroutine_fn vmdk_co_create(const char *filename, QEMUOptionParameter *options)
{
int fd, idx = 0;
char desc[BUF_SIZE];
@@ -1833,7 +1833,7 @@ static BlockDriver bdrv_vmdk = {
.bdrv_write = vmdk_co_write,
.bdrv_co_write_zeroes = vmdk_co_write_zeroes,
.bdrv_close = vmdk_close,
- .bdrv_create = vmdk_create,
+ .bdrv_co_create = vmdk_co_create,
.bdrv_co_flush_to_disk = vmdk_co_flush,
.bdrv_co_is_allocated = vmdk_co_is_allocated,
.bdrv_get_allocated_file_size = vmdk_get_allocated_file_size,
diff --git a/block/vpc.c b/block/vpc.c
index fe4f311..0dc9812 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -683,7 +683,7 @@ static int create_fixed_disk(int fd, uint8_t *buf, int64_t total_size)
return ret;
}
-static int vpc_create(const char *filename, QEMUOptionParameter *options)
+static int coroutine_fn vpc_co_create(const char *filename, QEMUOptionParameter *options)
{
uint8_t buf[1024];
struct vhd_footer *footer = (struct vhd_footer *) buf;
@@ -834,7 +834,7 @@ static BlockDriver bdrv_vpc = {
.bdrv_open = vpc_open,
.bdrv_close = vpc_close,
.bdrv_reopen_prepare = vpc_reopen_prepare,
- .bdrv_create = vpc_create,
+ .bdrv_co_create = vpc_co_create,
.bdrv_read = vpc_co_read,
.bdrv_write = vpc_co_write,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index e45f2a0..d7f59a8 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -104,7 +104,7 @@ struct BlockDriver {
const uint8_t *buf, int nb_sectors);
void (*bdrv_close)(BlockDriverState *bs);
void (*bdrv_rebind)(BlockDriverState *bs);
- int (*bdrv_create)(const char *filename, QEMUOptionParameter *options);
+ int coroutine_fn (*bdrv_co_create)(const char *filename, QEMUOptionParameter *options);
int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
int (*bdrv_make_empty)(BlockDriverState *bs);
/* aio */
--
1.8.3.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [RFC v2 03/15] Explicitly mark BlockDriver function .bdrv_create as coroutine and rename it bdrv_co_create.
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 03/15] Explicitly mark BlockDriver function .bdrv_create as coroutine and rename it bdrv_co_create Charlie Shepherd
@ 2013-08-14 14:26 ` Stefan Hajnoczi
0 siblings, 0 replies; 28+ messages in thread
From: Stefan Hajnoczi @ 2013-08-14 14:26 UTC (permalink / raw)
To: Charlie Shepherd; +Cc: kwolf, pbonzini, gabriel, qemu-devel
On Fri, Aug 09, 2013 at 07:43:53PM +0200, Charlie Shepherd wrote:
> diff --git a/block/cow.c b/block/cow.c
> index 1cc2e89..34c181a 100644
> --- a/block/cow.c
> +++ b/block/cow.c
> @@ -255,7 +255,7 @@ static void cow_close(BlockDriverState *bs)
> {
> }
>
> -static int cow_create(const char *filename, QEMUOptionParameter *options)
> +static int coroutine_fn cow_co_create(const char *filename, QEMUOptionParameter *options)
Please run scripts/checkpatch.pl on your patches. QEMU coding style
enforces 80-character lines.
See here to automatically run it during git-commit(1):
http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [RFC v2 04/15] Convert .bdrv_open and .bdrv_file_open to coroutine_fn
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 02/15] Rename qemu_coroutine_self to qemu_coroutine_self_int and add an annotated wrapper Charlie Shepherd
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 03/15] Explicitly mark BlockDriver function .bdrv_create as coroutine and rename it bdrv_co_create Charlie Shepherd
@ 2013-08-09 17:43 ` Charlie Shepherd
2013-08-29 12:11 ` Stefan Hajnoczi
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 05/15] Make qcow2_open synchronous Charlie Shepherd
` (12 subsequent siblings)
15 siblings, 1 reply; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-09 17:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, gabriel, Charlie Shepherd, pbonzini
Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
---
block.c | 8 ++++----
block/blkdebug.c | 4 ++--
block/blkverify.c | 4 ++--
block/bochs.c | 4 ++--
block/cloop.c | 4 ++--
block/cow.c | 4 ++--
block/curl.c | 12 ++++++------
block/dmg.c | 4 ++--
block/nbd.c | 6 +++---
block/parallels.c | 4 ++--
block/qcow.c | 4 ++--
block/qcow2.c | 2 +-
block/qed.c | 4 ++--
block/raw-posix.c | 20 ++++++++++----------
block/raw.c | 4 ++--
block/sheepdog.c | 10 +++++-----
block/ssh.c | 4 ++--
block/vdi.c | 4 ++--
block/vhdx.c | 4 ++--
block/vmdk.c | 4 ++--
block/vpc.c | 4 ++--
block/vvfat.c | 6 +++---
include/block/block_int.h | 4 ++--
23 files changed, 64 insertions(+), 64 deletions(-)
diff --git a/block.c b/block.c
index 40f73ee..75a1e6b 100644
--- a/block.c
+++ b/block.c
@@ -699,7 +699,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
/* bdrv_open() with directly using a protocol as drv. This layer is already
* opened, so assign it to bs (while file becomes a closed BlockDriverState)
* and return immediately. */
- if (file != NULL && drv->bdrv_file_open) {
+ if (file != NULL && drv->bdrv_co_file_open) {
bdrv_swap(file, bs);
return 0;
}
@@ -730,10 +730,10 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
bs->enable_write_cache = !!(flags & BDRV_O_CACHE_WB);
/* Open the image, either directly or using a protocol */
- if (drv->bdrv_file_open) {
+ if (drv->bdrv_co_file_open) {
assert(file == NULL);
assert(drv->bdrv_parse_filename || filename != NULL);
- ret = drv->bdrv_file_open(bs, options, open_flags);
+ ret = drv->bdrv_co_file_open(bs, options, open_flags);
} else {
if (file == NULL) {
qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't use '%s' as a "
@@ -744,7 +744,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
}
assert(file != NULL);
bs->file = file;
- ret = drv->bdrv_open(bs, options, open_flags);
+ ret = drv->bdrv_co_open(bs, options, open_flags);
}
if (ret < 0) {
diff --git a/block/blkdebug.c b/block/blkdebug.c
index ccb627a..7e022e4 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -349,7 +349,7 @@ static QemuOptsList runtime_opts = {
},
};
-static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn blkdebug_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVBlkdebugState *s = bs->opaque;
QemuOpts *opts;
@@ -626,7 +626,7 @@ static BlockDriver bdrv_blkdebug = {
.instance_size = sizeof(BDRVBlkdebugState),
.bdrv_parse_filename = blkdebug_parse_filename,
- .bdrv_file_open = blkdebug_open,
+ .bdrv_co_file_open = blkdebug_open,
.bdrv_close = blkdebug_close,
.bdrv_getlength = blkdebug_getlength,
diff --git a/block/blkverify.c b/block/blkverify.c
index 1d58cc3..0b89cfe 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -116,7 +116,7 @@ static QemuOptsList runtime_opts = {
},
};
-static int blkverify_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn blkverify_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVBlkverifyState *s = bs->opaque;
QemuOpts *opts;
@@ -405,7 +405,7 @@ static BlockDriver bdrv_blkverify = {
.instance_size = sizeof(BDRVBlkverifyState),
.bdrv_parse_filename = blkverify_parse_filename,
- .bdrv_file_open = blkverify_open,
+ .bdrv_co_file_open = blkverify_co_open,
.bdrv_close = blkverify_close,
.bdrv_getlength = blkverify_getlength,
diff --git a/block/bochs.c b/block/bochs.c
index d7078c0..aab9028 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -108,7 +108,7 @@ static int bochs_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
}
-static int bochs_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn bochs_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVBochsState *s = bs->opaque;
int i;
@@ -238,8 +238,8 @@ static BlockDriver bdrv_bochs = {
.format_name = "bochs",
.instance_size = sizeof(BDRVBochsState),
.bdrv_probe = bochs_probe,
- .bdrv_open = bochs_open,
.bdrv_read = bochs_co_read,
+ .bdrv_co_open = bochs_open,
.bdrv_close = bochs_close,
};
diff --git a/block/cloop.c b/block/cloop.c
index 6ea7cf4..b7c1551 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -53,7 +53,7 @@ static int cloop_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
}
-static int cloop_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn cloop_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVCloopState *s = bs->opaque;
uint32_t offsets_size, max_compressed_block_size = 1, i;
@@ -191,8 +191,8 @@ static BlockDriver bdrv_cloop = {
.format_name = "cloop",
.instance_size = sizeof(BDRVCloopState),
.bdrv_probe = cloop_probe,
- .bdrv_open = cloop_open,
.bdrv_read = cloop_co_read,
+ .bdrv_co_open = cloop_open,
.bdrv_close = cloop_close,
};
diff --git a/block/cow.c b/block/cow.c
index 34c181a..e2a1550 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -58,7 +58,7 @@ static int cow_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
}
-static int cow_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn cow_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVCowState *s = bs->opaque;
struct cow_header_v2 cow_header;
@@ -337,7 +337,7 @@ static BlockDriver bdrv_cow = {
.instance_size = sizeof(BDRVCowState),
.bdrv_probe = cow_probe,
- .bdrv_open = cow_open,
+ .bdrv_co_open = cow_co_open,
.bdrv_close = cow_close,
.bdrv_co_create = cow_co_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
diff --git a/block/curl.c b/block/curl.c
index 82d39ff..8b01840 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -397,7 +397,7 @@ static QemuOptsList runtime_opts = {
},
};
-static int curl_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn curl_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVCURLState *s = bs->opaque;
CURLState *state = NULL;
@@ -634,7 +634,7 @@ static BlockDriver bdrv_http = {
.instance_size = sizeof(BDRVCURLState),
.bdrv_parse_filename = curl_parse_filename,
- .bdrv_file_open = curl_open,
+ .bdrv_co_file_open = curl_co_open,
.bdrv_close = curl_close,
.bdrv_getlength = curl_getlength,
@@ -647,7 +647,7 @@ static BlockDriver bdrv_https = {
.instance_size = sizeof(BDRVCURLState),
.bdrv_parse_filename = curl_parse_filename,
- .bdrv_file_open = curl_open,
+ .bdrv_co_file_open = curl_co_open,
.bdrv_close = curl_close,
.bdrv_getlength = curl_getlength,
@@ -660,7 +660,7 @@ static BlockDriver bdrv_ftp = {
.instance_size = sizeof(BDRVCURLState),
.bdrv_parse_filename = curl_parse_filename,
- .bdrv_file_open = curl_open,
+ .bdrv_co_file_open = curl_co_open,
.bdrv_close = curl_close,
.bdrv_getlength = curl_getlength,
@@ -673,7 +673,7 @@ static BlockDriver bdrv_ftps = {
.instance_size = sizeof(BDRVCURLState),
.bdrv_parse_filename = curl_parse_filename,
- .bdrv_file_open = curl_open,
+ .bdrv_co_file_open = curl_co_open,
.bdrv_close = curl_close,
.bdrv_getlength = curl_getlength,
@@ -686,7 +686,7 @@ static BlockDriver bdrv_tftp = {
.instance_size = sizeof(BDRVCURLState),
.bdrv_parse_filename = curl_parse_filename,
- .bdrv_file_open = curl_open,
+ .bdrv_co_file_open = curl_co_open,
.bdrv_close = curl_close,
.bdrv_getlength = curl_getlength,
diff --git a/block/dmg.c b/block/dmg.c
index 3141cb5..745703f 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -92,7 +92,7 @@ static int read_uint32(BlockDriverState *bs, int64_t offset, uint32_t *result)
return 0;
}
-static int dmg_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn dmg_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVDMGState *s = bs->opaque;
uint64_t info_begin,info_end,last_in_offset,last_out_offset;
@@ -378,8 +378,8 @@ static BlockDriver bdrv_dmg = {
.format_name = "dmg",
.instance_size = sizeof(BDRVDMGState),
.bdrv_probe = dmg_probe,
- .bdrv_open = dmg_open,
.bdrv_read = dmg_co_read,
+ .bdrv_co_open = dmg_co_open,
.bdrv_close = dmg_close,
};
diff --git a/block/nbd.c b/block/nbd.c
index 9c480b8..a0e3562 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -653,7 +653,7 @@ static BlockDriver bdrv_nbd = {
.protocol_name = "nbd",
.instance_size = sizeof(BDRVNBDState),
.bdrv_parse_filename = nbd_parse_filename,
- .bdrv_file_open = nbd_open,
+ .bdrv_co_file_open = nbd_co_open,
.bdrv_co_readv = nbd_co_readv,
.bdrv_co_writev = nbd_co_writev,
.bdrv_close = nbd_close,
@@ -667,7 +667,7 @@ static BlockDriver bdrv_nbd_tcp = {
.protocol_name = "nbd+tcp",
.instance_size = sizeof(BDRVNBDState),
.bdrv_parse_filename = nbd_parse_filename,
- .bdrv_file_open = nbd_open,
+ .bdrv_co_file_open = nbd_co_open,
.bdrv_co_readv = nbd_co_readv,
.bdrv_co_writev = nbd_co_writev,
.bdrv_close = nbd_close,
@@ -681,7 +681,7 @@ static BlockDriver bdrv_nbd_unix = {
.protocol_name = "nbd+unix",
.instance_size = sizeof(BDRVNBDState),
.bdrv_parse_filename = nbd_parse_filename,
- .bdrv_file_open = nbd_open,
+ .bdrv_co_file_open = nbd_co_open,
.bdrv_co_readv = nbd_co_readv,
.bdrv_co_writev = nbd_co_writev,
.bdrv_close = nbd_close,
diff --git a/block/parallels.c b/block/parallels.c
index 18b3ac0..75175a8 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -68,7 +68,7 @@ static int parallels_probe(const uint8_t *buf, int buf_size, const char *filenam
return 0;
}
-static int parallels_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn parallels_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVParallelsState *s = bs->opaque;
int i;
@@ -164,8 +164,8 @@ static BlockDriver bdrv_parallels = {
.format_name = "parallels",
.instance_size = sizeof(BDRVParallelsState),
.bdrv_probe = parallels_probe,
- .bdrv_open = parallels_open,
.bdrv_read = parallels_co_read,
+ .bdrv_co_open = parallels_co_open,
.bdrv_close = parallels_close,
};
diff --git a/block/qcow.c b/block/qcow.c
index c367c55..04f59f2 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -92,7 +92,7 @@ static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
}
-static int qcow_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn qcow_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVQcowState *s = bs->opaque;
int len, i, shift, ret;
@@ -888,7 +888,7 @@ static BlockDriver bdrv_qcow = {
.format_name = "qcow",
.instance_size = sizeof(BDRVQcowState),
.bdrv_probe = qcow_probe,
- .bdrv_open = qcow_open,
+ .bdrv_co_open = qcow_co_open,
.bdrv_close = qcow_close,
.bdrv_reopen_prepare = qcow_reopen_prepare,
.bdrv_co_create = qcow_create,
diff --git a/block/qcow2.c b/block/qcow2.c
index 656deb9..c6dc209 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1781,7 +1781,7 @@ static BlockDriver bdrv_qcow2 = {
.format_name = "qcow2",
.instance_size = sizeof(BDRVQcowState),
.bdrv_probe = qcow2_probe,
- .bdrv_open = qcow2_open,
+ .bdrv_co_open = qcow2_co_open,
.bdrv_close = qcow2_close,
.bdrv_reopen_prepare = qcow2_reopen_prepare,
.bdrv_co_create = qcow2_co_create,
diff --git a/block/qed.c b/block/qed.c
index 083f20f..5f4ba79 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -373,7 +373,7 @@ static void bdrv_qed_rebind(BlockDriverState *bs)
s->bs = bs;
}
-static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn bdrv_qed_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVQEDState *s = bs->opaque;
QEDHeader le_header;
@@ -1570,7 +1570,7 @@ static BlockDriver bdrv_qed = {
.bdrv_probe = bdrv_qed_probe,
.bdrv_rebind = bdrv_qed_rebind,
- .bdrv_open = bdrv_qed_open,
+ .bdrv_co_open = bdrv_qed_co_open,
.bdrv_close = bdrv_qed_close,
.bdrv_reopen_prepare = bdrv_qed_reopen_prepare,
.bdrv_co_create = bdrv_qed_co_create,
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 306a0b0..e9f0890 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -335,7 +335,7 @@ fail:
return ret;
}
-static int raw_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn raw_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVRawState *s = bs->opaque;
@@ -1193,7 +1193,7 @@ static BlockDriver bdrv_file = {
.protocol_name = "file",
.instance_size = sizeof(BDRVRawState),
.bdrv_probe = NULL, /* no probe for protocols */
- .bdrv_file_open = raw_open,
+ .bdrv_co_file_open = raw_co_open,
.bdrv_reopen_prepare = raw_reopen_prepare,
.bdrv_reopen_commit = raw_reopen_commit,
.bdrv_reopen_abort = raw_reopen_abort,
@@ -1325,7 +1325,7 @@ static int check_hdev_writable(BDRVRawState *s)
return 0;
}
-static int hdev_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn hdev_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVRawState *s = bs->opaque;
int ret;
@@ -1533,7 +1533,7 @@ static BlockDriver bdrv_host_device = {
.protocol_name = "host_device",
.instance_size = sizeof(BDRVRawState),
.bdrv_probe_device = hdev_probe_device,
- .bdrv_file_open = hdev_open,
+ .bdrv_co_file_open = hdev_co_open,
.bdrv_close = raw_close,
.bdrv_reopen_prepare = raw_reopen_prepare,
.bdrv_reopen_commit = raw_reopen_commit,
@@ -1559,7 +1559,7 @@ static BlockDriver bdrv_host_device = {
};
#ifdef __linux__
-static int floppy_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn floppy_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVRawState *s = bs->opaque;
int ret;
@@ -1657,7 +1657,7 @@ static BlockDriver bdrv_host_floppy = {
.protocol_name = "host_floppy",
.instance_size = sizeof(BDRVRawState),
.bdrv_probe_device = floppy_probe_device,
- .bdrv_file_open = floppy_open,
+ .bdrv_co_file_open = floppy_co_open,
.bdrv_close = raw_close,
.bdrv_reopen_prepare = raw_reopen_prepare,
.bdrv_reopen_commit = raw_reopen_commit,
@@ -1680,7 +1680,7 @@ static BlockDriver bdrv_host_floppy = {
.bdrv_eject = floppy_eject,
};
-static int cdrom_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn cdrom_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVRawState *s = bs->opaque;
@@ -1758,7 +1758,7 @@ static BlockDriver bdrv_host_cdrom = {
.protocol_name = "host_cdrom",
.instance_size = sizeof(BDRVRawState),
.bdrv_probe_device = cdrom_probe_device,
- .bdrv_file_open = cdrom_open,
+ .bdrv_co_file_open = cdrom_co_open,
.bdrv_close = raw_close,
.bdrv_reopen_prepare = raw_reopen_prepare,
.bdrv_reopen_commit = raw_reopen_commit,
@@ -1787,7 +1787,7 @@ static BlockDriver bdrv_host_cdrom = {
#endif /* __linux__ */
#if defined (__FreeBSD__) || defined(__FreeBSD_kernel__)
-static int cdrom_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn cdrom_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVRawState *s = bs->opaque;
int ret;
@@ -1879,7 +1879,7 @@ static BlockDriver bdrv_host_cdrom = {
.protocol_name = "host_cdrom",
.instance_size = sizeof(BDRVRawState),
.bdrv_probe_device = cdrom_probe_device,
- .bdrv_file_open = cdrom_open,
+ .bdrv_co_file_open = cdrom_co_open,
.bdrv_close = raw_close,
.bdrv_reopen_prepare = raw_reopen_prepare,
.bdrv_reopen_commit = raw_reopen_commit,
diff --git a/block/raw.c b/block/raw.c
index 6f1f4e7..e4d3db2 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -3,7 +3,7 @@
#include "block/block_int.h"
#include "qemu/module.h"
-static int raw_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn raw_co_open(BlockDriverState *bs, QDict *options, int flags)
{
bs->sg = bs->file->sg;
return 0;
@@ -132,7 +132,7 @@ static BlockDriver bdrv_raw = {
/* It's really 0, but we need to make g_malloc() happy */
.instance_size = 1,
- .bdrv_open = raw_open,
+ .bdrv_co_open = raw_co_open,
.bdrv_close = raw_close,
.bdrv_reopen_prepare = raw_reopen_prepare,
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 7ca70de..7456954 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1259,7 +1259,7 @@ static QemuOptsList runtime_opts = {
},
};
-static int sd_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn sd_co_open(BlockDriverState *bs, QDict *options, int flags)
{
int ret, fd;
uint32_t vid = 0;
@@ -2186,7 +2186,7 @@ out:
return found;
}
-static int do_load_save_vmstate(BDRVSheepdogState *s, uint8_t *data,
+static int coroutine_fn do_load_save_vmstate(BDRVSheepdogState *s, uint8_t *data,
int64_t pos, int size, int load)
{
bool create;
@@ -2344,8 +2344,8 @@ static BlockDriver bdrv_sheepdog = {
.format_name = "sheepdog",
.protocol_name = "sheepdog",
.instance_size = sizeof(BDRVSheepdogState),
- .bdrv_file_open = sd_open,
.bdrv_close = sd_close,
+ .bdrv_co_file_open = sd_co_open,
.bdrv_co_create = sd_co_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_getlength = sd_getlength,
@@ -2372,8 +2372,8 @@ static BlockDriver bdrv_sheepdog_tcp = {
.format_name = "sheepdog",
.protocol_name = "sheepdog+tcp",
.instance_size = sizeof(BDRVSheepdogState),
- .bdrv_file_open = sd_open,
.bdrv_close = sd_close,
+ .bdrv_co_file_open = sd_co_open,
.bdrv_co_create = sd_co_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_getlength = sd_getlength,
@@ -2400,8 +2400,8 @@ static BlockDriver bdrv_sheepdog_unix = {
.format_name = "sheepdog",
.protocol_name = "sheepdog+unix",
.instance_size = sizeof(BDRVSheepdogState),
- .bdrv_file_open = sd_open,
.bdrv_close = sd_close,
+ .bdrv_co_file_open = sd_co_open,
.bdrv_co_create = sd_co_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_getlength = sd_getlength,
diff --git a/block/ssh.c b/block/ssh.c
index 3246185..2afb7cc 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -608,7 +608,7 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
return ret;
}
-static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags)
+static int coroutine_fn ssh_co_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags)
{
BDRVSSHState *s = bs->opaque;
int ret;
@@ -1049,7 +1049,7 @@ static BlockDriver bdrv_ssh = {
.protocol_name = "ssh",
.instance_size = sizeof(BDRVSSHState),
.bdrv_parse_filename = ssh_parse_filename,
- .bdrv_file_open = ssh_file_open,
+ .bdrv_co_file_open = ssh_co_file_open,
.bdrv_co_create = ssh_co_create,
.bdrv_close = ssh_close,
.bdrv_has_zero_init = ssh_has_zero_init,
diff --git a/block/vdi.c b/block/vdi.c
index 38e08fe..40818c4 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -364,7 +364,7 @@ static int vdi_probe(const uint8_t *buf, int buf_size, const char *filename)
return result;
}
-static int vdi_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn vdi_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVVdiState *s = bs->opaque;
VdiHeader header;
@@ -775,7 +775,7 @@ static BlockDriver bdrv_vdi = {
.format_name = "vdi",
.instance_size = sizeof(BDRVVdiState),
.bdrv_probe = vdi_probe,
- .bdrv_open = vdi_open,
+ .bdrv_co_open = vdi_co_open,
.bdrv_close = vdi_close,
.bdrv_reopen_prepare = vdi_reopen_prepare,
.bdrv_co_create = vdi_co_create,
diff --git a/block/vhdx.c b/block/vhdx.c
index e9704b1..af38098 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -715,7 +715,7 @@ exit:
}
-static int vhdx_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn vhdx_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVVHDXState *s = bs->opaque;
int ret = 0;
@@ -957,7 +957,7 @@ static BlockDriver bdrv_vhdx = {
.format_name = "vhdx",
.instance_size = sizeof(BDRVVHDXState),
.bdrv_probe = vhdx_probe,
- .bdrv_open = vhdx_open,
+ .bdrv_co_open = vhdx_co_open,
.bdrv_close = vhdx_close,
.bdrv_reopen_prepare = vhdx_reopen_prepare,
.bdrv_co_readv = vhdx_co_readv,
diff --git a/block/vmdk.c b/block/vmdk.c
index 5e81519..a58b551 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -803,7 +803,7 @@ exit:
return ret;
}
-static int vmdk_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn vmdk_co_open(BlockDriverState *bs, QDict *options, int flags)
{
int ret;
BDRVVmdkState *s = bs->opaque;
@@ -1827,7 +1827,7 @@ static BlockDriver bdrv_vmdk = {
.format_name = "vmdk",
.instance_size = sizeof(BDRVVmdkState),
.bdrv_probe = vmdk_probe,
- .bdrv_open = vmdk_open,
+ .bdrv_co_open = vmdk_co_open,
.bdrv_reopen_prepare = vmdk_reopen_prepare,
.bdrv_read = vmdk_co_read,
.bdrv_write = vmdk_co_write,
diff --git a/block/vpc.c b/block/vpc.c
index 0dc9812..241c1a6 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -155,7 +155,7 @@ static int vpc_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
}
-static int vpc_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn vpc_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVVPCState *s = bs->opaque;
int i;
@@ -831,7 +831,7 @@ static BlockDriver bdrv_vpc = {
.instance_size = sizeof(BDRVVPCState),
.bdrv_probe = vpc_probe,
- .bdrv_open = vpc_open,
+ .bdrv_co_open = vpc_co_open,
.bdrv_close = vpc_close,
.bdrv_reopen_prepare = vpc_reopen_prepare,
.bdrv_co_create = vpc_co_create,
diff --git a/block/vvfat.c b/block/vvfat.c
index cd3b8ed..27129da 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1065,7 +1065,7 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
qdict_put(options, "rw", qbool_from_int(rw));
}
-static int vvfat_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn vvfat_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVVVFATState *s = bs->opaque;
int cyls, heads, secs;
@@ -2886,7 +2886,7 @@ static int coroutine_fn vvfat_co_is_allocated(BlockDriverState *bs,
return 1;
}
-static int write_target_commit(BlockDriverState *bs, int64_t sector_num,
+static int coroutine_fn write_target_commit(BlockDriverState *bs, int64_t sector_num,
const uint8_t* buffer, int nb_sectors) {
BDRVVVFATState* s = *((BDRVVVFATState**) bs->opaque);
return try_commit(s);
@@ -2978,7 +2978,7 @@ static BlockDriver bdrv_vvfat = {
.instance_size = sizeof(BDRVVVFATState),
.bdrv_parse_filename = vvfat_parse_filename,
- .bdrv_file_open = vvfat_open,
+ .bdrv_co_file_open = vvfat_co_open,
.bdrv_close = vvfat_close,
.bdrv_rebind = vvfat_rebind,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index d7f59a8..926cf87 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -96,9 +96,9 @@ struct BlockDriver {
void (*bdrv_reopen_commit)(BDRVReopenState *reopen_state);
void (*bdrv_reopen_abort)(BDRVReopenState *reopen_state);
- int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags);
- int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags);
int (*bdrv_read)(BlockDriverState *bs, int64_t sector_num,
+ int coroutine_fn (*bdrv_co_open)(BlockDriverState *bs, QDict *options, int flags);
+ int coroutine_fn (*bdrv_co_file_open)(BlockDriverState *bs, QDict *options, int flags);
uint8_t *buf, int nb_sectors);
int (*bdrv_write)(BlockDriverState *bs, int64_t sector_num,
const uint8_t *buf, int nb_sectors);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [RFC v2 04/15] Convert .bdrv_open and .bdrv_file_open to coroutine_fn
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 04/15] Convert .bdrv_open and .bdrv_file_open to coroutine_fn Charlie Shepherd
@ 2013-08-29 12:11 ` Stefan Hajnoczi
2013-08-29 12:16 ` Charlie Shepherd
0 siblings, 1 reply; 28+ messages in thread
From: Stefan Hajnoczi @ 2013-08-29 12:11 UTC (permalink / raw)
To: Charlie Shepherd; +Cc: kwolf, pbonzini, gabriel, qemu-devel
On Fri, Aug 09, 2013 at 07:43:54PM +0200, Charlie Shepherd wrote:
> Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
> ---
> block.c | 8 ++++----
> block/blkdebug.c | 4 ++--
> block/blkverify.c | 4 ++--
> block/bochs.c | 4 ++--
> block/cloop.c | 4 ++--
> block/cow.c | 4 ++--
> block/curl.c | 12 ++++++------
> block/dmg.c | 4 ++--
> block/nbd.c | 6 +++---
> block/parallels.c | 4 ++--
> block/qcow.c | 4 ++--
> block/qcow2.c | 2 +-
> block/qed.c | 4 ++--
> block/raw-posix.c | 20 ++++++++++----------
> block/raw.c | 4 ++--
> block/sheepdog.c | 10 +++++-----
> block/ssh.c | 4 ++--
> block/vdi.c | 4 ++--
> block/vhdx.c | 4 ++--
> block/vmdk.c | 4 ++--
> block/vpc.c | 4 ++--
> block/vvfat.c | 6 +++---
> include/block/block_int.h | 4 ++--
> 23 files changed, 64 insertions(+), 64 deletions(-)
>
> diff --git a/block.c b/block.c
> index 40f73ee..75a1e6b 100644
> --- a/block.c
> +++ b/block.c
> @@ -699,7 +699,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
> /* bdrv_open() with directly using a protocol as drv. This layer is already
> * opened, so assign it to bs (while file becomes a closed BlockDriverState)
> * and return immediately. */
> - if (file != NULL && drv->bdrv_file_open) {
> + if (file != NULL && drv->bdrv_co_file_open) {
> bdrv_swap(file, bs);
> return 0;
> }
> @@ -730,10 +730,10 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
> bs->enable_write_cache = !!(flags & BDRV_O_CACHE_WB);
>
> /* Open the image, either directly or using a protocol */
> - if (drv->bdrv_file_open) {
> + if (drv->bdrv_co_file_open) {
> assert(file == NULL);
> assert(drv->bdrv_parse_filename || filename != NULL);
> - ret = drv->bdrv_file_open(bs, options, open_flags);
> + ret = drv->bdrv_co_file_open(bs, options, open_flags);
> } else {
> if (file == NULL) {
> qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't use '%s' as a "
> @@ -744,7 +744,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
> }
> assert(file != NULL);
> bs->file = file;
> - ret = drv->bdrv_open(bs, options, open_flags);
> + ret = drv->bdrv_co_open(bs, options, open_flags);
> }
>
> if (ret < 0) {
bdrv_open_common() needs to be coroutine_fn too. I'm pretty sure
bdrv_open() is called outside coroutine context in some places. How do
you guarantee that it is executed inside a coroutine (a synchronous
wrapper would be necessary)?
Stefan
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [RFC v2 04/15] Convert .bdrv_open and .bdrv_file_open to coroutine_fn
2013-08-29 12:11 ` Stefan Hajnoczi
@ 2013-08-29 12:16 ` Charlie Shepherd
0 siblings, 0 replies; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-29 12:16 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: kwolf, pbonzini, gabriel, qemu-devel
On 29/08/2013 13:11, Stefan Hajnoczi wrote:
> On Fri, Aug 09, 2013 at 07:43:54PM +0200, Charlie Shepherd wrote:
>> Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
>> ---
>> block.c | 8 ++++----
>> block/blkdebug.c | 4 ++--
>> block/blkverify.c | 4 ++--
>> block/bochs.c | 4 ++--
>> block/cloop.c | 4 ++--
>> block/cow.c | 4 ++--
>> block/curl.c | 12 ++++++------
>> block/dmg.c | 4 ++--
>> block/nbd.c | 6 +++---
>> block/parallels.c | 4 ++--
>> block/qcow.c | 4 ++--
>> block/qcow2.c | 2 +-
>> block/qed.c | 4 ++--
>> block/raw-posix.c | 20 ++++++++++----------
>> block/raw.c | 4 ++--
>> block/sheepdog.c | 10 +++++-----
>> block/ssh.c | 4 ++--
>> block/vdi.c | 4 ++--
>> block/vhdx.c | 4 ++--
>> block/vmdk.c | 4 ++--
>> block/vpc.c | 4 ++--
>> block/vvfat.c | 6 +++---
>> include/block/block_int.h | 4 ++--
>> 23 files changed, 64 insertions(+), 64 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 40f73ee..75a1e6b 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -699,7 +699,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
>> /* bdrv_open() with directly using a protocol as drv. This layer is already
>> * opened, so assign it to bs (while file becomes a closed BlockDriverState)
>> * and return immediately. */
>> - if (file != NULL && drv->bdrv_file_open) {
>> + if (file != NULL && drv->bdrv_co_file_open) {
>> bdrv_swap(file, bs);
>> return 0;
>> }
>> @@ -730,10 +730,10 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
>> bs->enable_write_cache = !!(flags & BDRV_O_CACHE_WB);
>>
>> /* Open the image, either directly or using a protocol */
>> - if (drv->bdrv_file_open) {
>> + if (drv->bdrv_co_file_open) {
>> assert(file == NULL);
>> assert(drv->bdrv_parse_filename || filename != NULL);
>> - ret = drv->bdrv_file_open(bs, options, open_flags);
>> + ret = drv->bdrv_co_file_open(bs, options, open_flags);
>> } else {
>> if (file == NULL) {
>> qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't use '%s' as a "
>> @@ -744,7 +744,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
>> }
>> assert(file != NULL);
>> bs->file = file;
>> - ret = drv->bdrv_open(bs, options, open_flags);
>> + ret = drv->bdrv_co_open(bs, options, open_flags);
>> }
>>
>> if (ret < 0) {
> bdrv_open_common() needs to be coroutine_fn too. I'm pretty sure
> bdrv_open() is called outside coroutine context in some places. How do
> you guarantee that it is executed inside a coroutine (a synchronous
> wrapper would be necessary)?
Yes, later patches do that. It's quite difficult to keep the patches
small and separate, so the series tries to keep them compilable, but the
annotations will not necessarily be fully consistent until the end of
the series.
Charlie
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [RFC v2 05/15] Make qcow2_open synchronous
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
` (2 preceding siblings ...)
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 04/15] Convert .bdrv_open and .bdrv_file_open to coroutine_fn Charlie Shepherd
@ 2013-08-09 17:43 ` Charlie Shepherd
2013-08-29 12:29 ` Stefan Hajnoczi
2013-08-29 12:33 ` Stefan Hajnoczi
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 06/15] Explicitly mark BlockDriver functions .bdrv_write and .bdrv_read as coroutine functions Charlie Shepherd
` (11 subsequent siblings)
15 siblings, 2 replies; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-09 17:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, gabriel, Charlie Shepherd, pbonzini
The previous patch convert all .bdrv_open functions to run from a coroutine context. However
qcow2's open method is also called from qcow2_invalidate_cache. bdrv_invalidate_cache is mainly
called by migration.c, which doesn't run in coroutine context, so rather than propagating
coroutine_fn annotations up the call chain, turn qcow2_open into a synchronous wrapper.
Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
---
block/qcow2.c | 38 +++++++++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index c6dc209..59c4200 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -58,6 +58,10 @@ typedef struct {
#define QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
#define QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
+
+#define NOT_DONE 0x7fffffff
+
+
static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
{
const QCowHeader *cow_header = (const void *)buf;
@@ -315,7 +319,7 @@ static QemuOptsList qcow2_runtime_opts = {
},
};
-static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn qcow2_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVQcowState *s = bs->opaque;
int len, i, ret = 0;
@@ -590,6 +594,38 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
return ret;
}
+struct QOpenCo {
+ BlockDriverState *bs;
+ QDict *options;
+ int flags;
+ int ret;
+};
+
+static void coroutine_fn qcow2_co_open_entry(void *opaque)
+{
+ struct QOpenCo *qo = opaque;
+
+ qo->ret = qcow2_co_open(qo->bs, qo->options, qo->flags);
+}
+
+static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
+{
+ Coroutine *co;
+ struct QOpenCo qo = {
+ .bs = bs,
+ .options = options,
+ .flags = flags,
+ .ret = NOT_DONE,
+ };
+
+ co = qemu_coroutine_create(qcow2_co_open_entry);
+ qemu_coroutine_enter(co, &qo);
+ while (qo.ret == NOT_DONE) {
+ qemu_aio_wait();
+ }
+ return qo.ret;
+}
+
static int qcow2_set_key(BlockDriverState *bs, const char *key)
{
BDRVQcowState *s = bs->opaque;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [RFC v2 05/15] Make qcow2_open synchronous
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 05/15] Make qcow2_open synchronous Charlie Shepherd
@ 2013-08-29 12:29 ` Stefan Hajnoczi
2013-08-29 12:33 ` Stefan Hajnoczi
1 sibling, 0 replies; 28+ messages in thread
From: Stefan Hajnoczi @ 2013-08-29 12:29 UTC (permalink / raw)
To: Charlie Shepherd; +Cc: kwolf, pbonzini, gabriel, qemu-devel
On Fri, Aug 09, 2013 at 07:43:55PM +0200, Charlie Shepherd wrote:
> +static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
> +{
> + Coroutine *co;
> + struct QOpenCo qo = {
> + .bs = bs,
> + .options = options,
> + .flags = flags,
> + .ret = NOT_DONE,
> + };
> +
> + co = qemu_coroutine_create(qcow2_co_open_entry);
> + qemu_coroutine_enter(co, &qo);
> + while (qo.ret == NOT_DONE) {
> + qemu_aio_wait();
> + }
I think a cleaner approach is a bool qo.done field instead of using a
magic NOT_DONE value. This way the return value of qcow2_co_open() can
never collide with NOT_DONE.
(In block.c we use NOT_DONE but I'd like to start using a separate field
to indicate completion in new places.)
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [RFC v2 05/15] Make qcow2_open synchronous
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 05/15] Make qcow2_open synchronous Charlie Shepherd
2013-08-29 12:29 ` Stefan Hajnoczi
@ 2013-08-29 12:33 ` Stefan Hajnoczi
2013-08-29 12:43 ` Charlie Shepherd
1 sibling, 1 reply; 28+ messages in thread
From: Stefan Hajnoczi @ 2013-08-29 12:33 UTC (permalink / raw)
To: Charlie Shepherd; +Cc: kwolf, pbonzini, gabriel, qemu-devel
On Fri, Aug 09, 2013 at 07:43:55PM +0200, Charlie Shepherd wrote:
> The previous patch convert all .bdrv_open functions to run from a coroutine context. However
> qcow2's open method is also called from qcow2_invalidate_cache. bdrv_invalidate_cache is mainly
> called by migration.c, which doesn't run in coroutine context, so rather than propagating
> coroutine_fn annotations up the call chain, turn qcow2_open into a synchronous wrapper.
I think it would be cleaner to make .bdrv_open a coroutine function and
push the synchronous wrapper out to the callers. That way we can either
keep synchronous wrappers where necessary, or we can eventually convert
that synchronous code to coroutine code.
If you hide the synchronous wrapper inside qcow2 it blocks coroutine
callers who could otherwise use the event loop.
Stefan
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [RFC v2 05/15] Make qcow2_open synchronous
2013-08-29 12:33 ` Stefan Hajnoczi
@ 2013-08-29 12:43 ` Charlie Shepherd
2013-09-04 11:39 ` Stefan Hajnoczi
0 siblings, 1 reply; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-29 12:43 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: kwolf, pbonzini, gabriel, qemu-devel
On 29/08/2013 13:33, Stefan Hajnoczi wrote:
> On Fri, Aug 09, 2013 at 07:43:55PM +0200, Charlie Shepherd wrote:
>> The previous patch convert all .bdrv_open functions to run from a coroutine context. However
>> qcow2's open method is also called from qcow2_invalidate_cache. bdrv_invalidate_cache is mainly
>> called by migration.c, which doesn't run in coroutine context, so rather than propagating
>> coroutine_fn annotations up the call chain, turn qcow2_open into a synchronous wrapper.
> I think it would be cleaner to make .bdrv_open a coroutine function and
> push the synchronous wrapper out to the callers. That way we can either
> keep synchronous wrappers where necessary, or we can eventually convert
> that synchronous code to coroutine code.
>
> If you hide the synchronous wrapper inside qcow2 it blocks coroutine
> callers who could otherwise use the event loop.
So you want this approach for all synchronous wrappers?
Charlie
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [RFC v2 05/15] Make qcow2_open synchronous
2013-08-29 12:43 ` Charlie Shepherd
@ 2013-09-04 11:39 ` Stefan Hajnoczi
0 siblings, 0 replies; 28+ messages in thread
From: Stefan Hajnoczi @ 2013-09-04 11:39 UTC (permalink / raw)
To: Charlie Shepherd; +Cc: kwolf, pbonzini, gabriel, qemu-devel
On Thu, Aug 29, 2013 at 01:43:02PM +0100, Charlie Shepherd wrote:
> On 29/08/2013 13:33, Stefan Hajnoczi wrote:
> >On Fri, Aug 09, 2013 at 07:43:55PM +0200, Charlie Shepherd wrote:
> >>The previous patch convert all .bdrv_open functions to run from a coroutine context. However
> >>qcow2's open method is also called from qcow2_invalidate_cache. bdrv_invalidate_cache is mainly
> >>called by migration.c, which doesn't run in coroutine context, so rather than propagating
> >>coroutine_fn annotations up the call chain, turn qcow2_open into a synchronous wrapper.
> >I think it would be cleaner to make .bdrv_open a coroutine function and
> >push the synchronous wrapper out to the callers. That way we can either
> >keep synchronous wrappers where necessary, or we can eventually convert
> >that synchronous code to coroutine code.
> >
> >If you hide the synchronous wrapper inside qcow2 it blocks coroutine
> >callers who could otherwise use the event loop.
>
> So you want this approach for all synchronous wrappers?
For .bdrv_*() functions, yes. In other cases the trade-off may be worth
it.
Stefan
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [RFC v2 06/15] Explicitly mark BlockDriver functions .bdrv_write and .bdrv_read as coroutine functions
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
` (3 preceding siblings ...)
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 05/15] Make qcow2_open synchronous Charlie Shepherd
@ 2013-08-09 17:43 ` Charlie Shepherd
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 07/15] Call bdrv->open via a synchronous wrapper in block/snapshot.c Charlie Shepherd
` (10 subsequent siblings)
15 siblings, 0 replies; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-09 17:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, gabriel, Charlie Shepherd, pbonzini
Convert .bdrv_write and .bdrv_read to coroutine functions and rename them to .bdrv_co_write and
.bdrv_co_read.
Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
---
block.c | 4 ++--
block/bochs.c | 2 +-
block/cloop.c | 2 +-
block/cow.c | 4 ++--
block/dmg.c | 2 +-
block/parallels.c | 2 +-
block/ssh.c | 2 +-
block/vdi.c | 4 ++--
block/vmdk.c | 4 ++--
block/vpc.c | 4 ++--
block/vvfat.c | 6 +++---
include/block/block_int.h | 4 ++--
12 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/block.c b/block.c
index 75a1e6b..1f9f653 100644
--- a/block.c
+++ b/block.c
@@ -3790,9 +3790,9 @@ static BlockDriverAIOCB *bdrv_aio_rw_vector(BlockDriverState *bs,
if (is_write) {
qemu_iovec_to_buf(acb->qiov, 0, acb->bounce, qiov->size);
- acb->ret = bs->drv->bdrv_write(bs, sector_num, acb->bounce, nb_sectors);
+ acb->ret = bs->drv->bdrv_co_write(bs, sector_num, acb->bounce, nb_sectors);
} else {
- acb->ret = bs->drv->bdrv_read(bs, sector_num, acb->bounce, nb_sectors);
+ acb->ret = bs->drv->bdrv_co_read(bs, sector_num, acb->bounce, nb_sectors);
}
qemu_bh_schedule(acb->bh);
diff --git a/block/bochs.c b/block/bochs.c
index aab9028..3111ab9 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -238,7 +238,7 @@ static BlockDriver bdrv_bochs = {
.format_name = "bochs",
.instance_size = sizeof(BDRVBochsState),
.bdrv_probe = bochs_probe,
- .bdrv_read = bochs_co_read,
+ .bdrv_co_read = bochs_co_read,
.bdrv_co_open = bochs_open,
.bdrv_close = bochs_close,
};
diff --git a/block/cloop.c b/block/cloop.c
index b7c1551..122f650 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -191,7 +191,7 @@ static BlockDriver bdrv_cloop = {
.format_name = "cloop",
.instance_size = sizeof(BDRVCloopState),
.bdrv_probe = cloop_probe,
- .bdrv_read = cloop_co_read,
+ .bdrv_co_read = cloop_co_read,
.bdrv_co_open = cloop_open,
.bdrv_close = cloop_close,
};
diff --git a/block/cow.c b/block/cow.c
index e2a1550..c68c5ae 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -342,8 +342,8 @@ static BlockDriver bdrv_cow = {
.bdrv_co_create = cow_co_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
- .bdrv_read = cow_co_read,
- .bdrv_write = cow_co_write,
+ .bdrv_co_read = cow_co_read,
+ .bdrv_co_write = cow_co_write,
.bdrv_co_is_allocated = cow_co_is_allocated,
.create_options = cow_create_options,
diff --git a/block/dmg.c b/block/dmg.c
index 745703f..146207a 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -378,7 +378,7 @@ static BlockDriver bdrv_dmg = {
.format_name = "dmg",
.instance_size = sizeof(BDRVDMGState),
.bdrv_probe = dmg_probe,
- .bdrv_read = dmg_co_read,
+ .bdrv_co_read = dmg_co_read,
.bdrv_co_open = dmg_co_open,
.bdrv_close = dmg_close,
};
diff --git a/block/parallels.c b/block/parallels.c
index 75175a8..b765aed 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -164,7 +164,7 @@ static BlockDriver bdrv_parallels = {
.format_name = "parallels",
.instance_size = sizeof(BDRVParallelsState),
.bdrv_probe = parallels_probe,
- .bdrv_read = parallels_co_read,
+ .bdrv_co_read = parallels_co_read,
.bdrv_co_open = parallels_co_open,
.bdrv_close = parallels_close,
};
diff --git a/block/ssh.c b/block/ssh.c
index 2afb7cc..22f112c 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -888,7 +888,7 @@ static coroutine_fn int ssh_co_readv(BlockDriverState *bs,
return ret;
}
-static int ssh_write(BDRVSSHState *s,
+static int coroutine_fn ssh_write(BDRVSSHState *s,
int64_t offset, size_t size,
QEMUIOVector *qiov)
{
diff --git a/block/vdi.c b/block/vdi.c
index 40818c4..577a638 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -783,9 +783,9 @@ static BlockDriver bdrv_vdi = {
.bdrv_co_is_allocated = vdi_co_is_allocated,
.bdrv_make_empty = vdi_make_empty,
- .bdrv_read = vdi_co_read,
+ .bdrv_co_read = vdi_co_read,
#if defined(CONFIG_VDI_WRITE)
- .bdrv_write = vdi_co_write,
+ .bdrv_co_write = vdi_co_write,
#endif
.bdrv_get_info = vdi_get_info,
diff --git a/block/vmdk.c b/block/vmdk.c
index a58b551..16d593b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1829,8 +1829,8 @@ static BlockDriver bdrv_vmdk = {
.bdrv_probe = vmdk_probe,
.bdrv_co_open = vmdk_co_open,
.bdrv_reopen_prepare = vmdk_reopen_prepare,
- .bdrv_read = vmdk_co_read,
- .bdrv_write = vmdk_co_write,
+ .bdrv_co_read = vmdk_co_read,
+ .bdrv_co_write = vmdk_co_write,
.bdrv_co_write_zeroes = vmdk_co_write_zeroes,
.bdrv_close = vmdk_close,
.bdrv_co_create = vmdk_co_create,
diff --git a/block/vpc.c b/block/vpc.c
index 241c1a6..6eb293a 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -836,8 +836,8 @@ static BlockDriver bdrv_vpc = {
.bdrv_reopen_prepare = vpc_reopen_prepare,
.bdrv_co_create = vpc_co_create,
- .bdrv_read = vpc_co_read,
- .bdrv_write = vpc_co_write,
+ .bdrv_co_read = vpc_co_read,
+ .bdrv_co_write = vpc_co_write,
.create_options = vpc_create_options,
.bdrv_has_zero_init = vpc_has_zero_init,
diff --git a/block/vvfat.c b/block/vvfat.c
index 27129da..87ce631 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2900,7 +2900,7 @@ static void write_target_close(BlockDriverState *bs) {
static BlockDriver vvfat_write_target = {
.format_name = "vvfat_write_target",
- .bdrv_write = write_target_commit,
+ .bdrv_co_write = write_target_commit,
.bdrv_close = write_target_close,
};
@@ -2982,8 +2982,8 @@ static BlockDriver bdrv_vvfat = {
.bdrv_close = vvfat_close,
.bdrv_rebind = vvfat_rebind,
- .bdrv_read = vvfat_co_read,
- .bdrv_write = vvfat_co_write,
+ .bdrv_co_read = vvfat_co_read,
+ .bdrv_co_write = vvfat_co_write,
.bdrv_co_is_allocated = vvfat_co_is_allocated,
};
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 926cf87..c9632a0 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -96,11 +96,11 @@ struct BlockDriver {
void (*bdrv_reopen_commit)(BDRVReopenState *reopen_state);
void (*bdrv_reopen_abort)(BDRVReopenState *reopen_state);
- int (*bdrv_read)(BlockDriverState *bs, int64_t sector_num,
int coroutine_fn (*bdrv_co_open)(BlockDriverState *bs, QDict *options, int flags);
int coroutine_fn (*bdrv_co_file_open)(BlockDriverState *bs, QDict *options, int flags);
+ int coroutine_fn (*bdrv_co_read)(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors);
- int (*bdrv_write)(BlockDriverState *bs, int64_t sector_num,
+ int coroutine_fn (*bdrv_co_write)(BlockDriverState *bs, int64_t sector_num,
const uint8_t *buf, int nb_sectors);
void (*bdrv_close)(BlockDriverState *bs);
void (*bdrv_rebind)(BlockDriverState *bs);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [RFC v2 07/15] Call bdrv->open via a synchronous wrapper in block/snapshot.c
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
` (4 preceding siblings ...)
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 06/15] Explicitly mark BlockDriver functions .bdrv_write and .bdrv_read as coroutine functions Charlie Shepherd
@ 2013-08-09 17:43 ` Charlie Shepherd
2013-08-29 15:14 ` Stefan Hajnoczi
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 08/15] Convert bdrv_create and associated functions to be coroutine_fn Charlie Shepherd
` (9 subsequent siblings)
15 siblings, 1 reply; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-09 17:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, gabriel, Charlie Shepherd, pbonzini
block/snapshot.c calls bdrv->open directly from a non-coroutine context. Provide a synchronous
wrapper to ensure correctness.
Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
---
block/snapshot.c | 32 +++++++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/block/snapshot.c b/block/snapshot.c
index 6c6d9de..541d83d 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -25,6 +25,8 @@
#include "block/snapshot.h"
#include "block/block_int.h"
+#define NOT_DONE 0x7fffffff
+
int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
const char *name)
{
@@ -81,6 +83,34 @@ int bdrv_snapshot_create(BlockDriverState *bs,
return -ENOTSUP;
}
+struct SnapOp {
+ BlockDriverState *bs;
+ int ret;
+};
+
+static void coroutine_fn bdrv_snapshot_open_entry(void *opaque)
+{
+ struct SnapOp *so = opaque;
+ so->ret = so->bs->drv->bdrv_co_open(so->bs, NULL, so->bs->open_flags);
+}
+
+static int bdrv_snapshot_open(BlockDriverState *bs)
+{
+ Coroutine *co;
+ struct SnapOp so = {
+ .bs = bs,
+ .ret = NOT_DONE,
+ };
+
+ co = qemu_coroutine_create(bdrv_snapshot_open_entry);
+ qemu_coroutine_enter(co, &so);
+ while (so.ret == NOT_DONE) {
+ qemu_aio_wait();
+ }
+
+ return so.ret;
+}
+
int bdrv_snapshot_goto(BlockDriverState *bs,
const char *snapshot_id)
{
@@ -97,7 +127,7 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
if (bs->file) {
drv->bdrv_close(bs);
ret = bdrv_snapshot_goto(bs->file, snapshot_id);
- open_ret = drv->bdrv_open(bs, NULL, bs->open_flags);
+ open_ret = bdrv_snapshot_open(bs);
if (open_ret < 0) {
bdrv_delete(bs->file);
bs->drv = NULL;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [RFC v2 07/15] Call bdrv->open via a synchronous wrapper in block/snapshot.c
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 07/15] Call bdrv->open via a synchronous wrapper in block/snapshot.c Charlie Shepherd
@ 2013-08-29 15:14 ` Stefan Hajnoczi
0 siblings, 0 replies; 28+ messages in thread
From: Stefan Hajnoczi @ 2013-08-29 15:14 UTC (permalink / raw)
To: Charlie Shepherd; +Cc: kwolf, pbonzini, gabriel, qemu-devel
On Fri, Aug 09, 2013 at 07:43:57PM +0200, Charlie Shepherd wrote:
> +static int bdrv_snapshot_open(BlockDriverState *bs)
> +{
> + Coroutine *co;
> + struct SnapOp so = {
> + .bs = bs,
> + .ret = NOT_DONE,
> + };
> +
> + co = qemu_coroutine_create(bdrv_snapshot_open_entry);
> + qemu_coroutine_enter(co, &so);
> + while (so.ret == NOT_DONE) {
> + qemu_aio_wait();
> + }
> +
> + return so.ret;
> +}
This is orthogonal to snapshots and should be a generic .bdrv_open()
wrapper called block.c:bdrv_open() (the coroutine version should be
called bdrv_co_open()).
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [RFC v2 08/15] Convert bdrv_create and associated functions to be coroutine_fn
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
` (5 preceding siblings ...)
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 07/15] Call bdrv->open via a synchronous wrapper in block/snapshot.c Charlie Shepherd
@ 2013-08-09 17:43 ` Charlie Shepherd
2013-08-29 15:31 ` Stefan Hajnoczi
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 09/15] Add a synchronous wrapper bdrv_sync_rwco Charlie Shepherd
` (8 subsequent siblings)
15 siblings, 1 reply; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-09 17:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, gabriel, Charlie Shepherd, pbonzini
This patch converts bdrv_create, bdrv_create_file and bdrv_img_create to be
coroutine only functions. It adds a synchronous wrapper, bdrv_sync_create, for
any synchronous callers.
Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
---
block.c | 56 +++++++++++++++++++++++++--------------------------
include/block/block.h | 8 +++++---
2 files changed, 33 insertions(+), 31 deletions(-)
diff --git a/block.c b/block.c
index 1f9f653..51a6649 100644
--- a/block.c
+++ b/block.c
@@ -364,7 +364,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
typedef struct CreateCo {
BlockDriver *drv;
- char *filename;
+ const char *filename;
QEMUOptionParameter *options;
int ret;
} CreateCo;
@@ -372,48 +372,48 @@ typedef struct CreateCo {
static void coroutine_fn bdrv_create_co_entry(void *opaque)
{
CreateCo *cco = opaque;
- assert(cco->drv);
-
- cco->ret = cco->drv->bdrv_co_create(cco->filename, cco->options);
+ cco->ret = bdrv_create(cco->drv, cco->filename, cco->options);
}
-int bdrv_create(BlockDriver *drv, const char* filename,
+int coroutine_fn bdrv_create(BlockDriver *drv, const char* filename,
QEMUOptionParameter *options)
{
int ret;
+ char *dup_fn;
+ assert(drv);
+ if (!drv->bdrv_co_create) {
+ return -ENOTSUP;
+ }
+
+ dup_fn = g_strdup(filename);
+ ret = drv->bdrv_co_create(dup_fn, options);
+ g_free(dup_fn);
+ return ret;
+}
+
+
+int bdrv_sync_create(BlockDriver *drv, const char* filename,
+ QEMUOptionParameter *options)
+{
Coroutine *co;
CreateCo cco = {
.drv = drv,
- .filename = g_strdup(filename),
+ .filename = filename,
.options = options,
.ret = NOT_DONE,
};
- if (!drv->bdrv_co_create) {
- ret = -ENOTSUP;
- goto out;
- }
-
- if (qemu_in_coroutine()) {
- /* Fast-path if already in coroutine context */
- bdrv_create_co_entry(&cco);
- } else {
- co = qemu_coroutine_create(bdrv_create_co_entry);
- qemu_coroutine_enter(co, &cco);
- while (cco.ret == NOT_DONE) {
- qemu_aio_wait();
- }
+ co = qemu_coroutine_create(bdrv_create_co_entry);
+ qemu_coroutine_enter(co, &cco);
+ while (cco.ret == NOT_DONE) {
+ qemu_aio_wait();
}
- ret = cco.ret;
-
-out:
- g_free(cco.filename);
- return ret;
+ return cco.ret;
}
-int bdrv_create_file(const char* filename, QEMUOptionParameter *options)
+int coroutine_fn bdrv_create_file(const char* filename, QEMUOptionParameter *options)
{
BlockDriver *drv;
@@ -1035,7 +1035,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
drv->format_name);
}
- ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options);
+ ret = bdrv_sync_create(bdrv_qcow2, tmp_filename, create_options);
free_option_parameters(create_options);
if (ret < 0) {
goto fail;
@@ -4463,7 +4463,7 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie)
bs->total_time_ns[cookie->type] += get_clock() - cookie->start_time_ns;
}
-void bdrv_img_create(const char *filename, const char *fmt,
+void coroutine_fn bdrv_img_create(const char *filename, const char *fmt,
const char *base_filename, const char *base_fmt,
char *options, uint64_t img_size, int flags,
Error **errp, bool quiet)
diff --git a/include/block/block.h b/include/block/block.h
index 742fce5..c6a3aaf 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -116,9 +116,11 @@ BlockDriver *bdrv_find_protocol(const char *filename,
BlockDriver *bdrv_find_format(const char *format_name);
BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
bool readonly);
-int bdrv_create(BlockDriver *drv, const char* filename,
+int coroutine_fn bdrv_create(BlockDriver *drv, const char* filename,
QEMUOptionParameter *options);
-int bdrv_create_file(const char* filename, QEMUOptionParameter *options);
+int bdrv_sync_create(BlockDriver *drv, const char* filename,
+ QEMUOptionParameter *options);
+int coroutine_fn bdrv_create_file(const char* filename, QEMUOptionParameter *options);
BlockDriverState *bdrv_new(const char *device_name);
void bdrv_make_anon(BlockDriverState *bs);
void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
@@ -336,7 +338,7 @@ int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
int64_t pos, int size);
-void bdrv_img_create(const char *filename, const char *fmt,
+void coroutine_fn bdrv_img_create(const char *filename, const char *fmt,
const char *base_filename, const char *base_fmt,
char *options, uint64_t img_size, int flags,
Error **errp, bool quiet);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [RFC v2 08/15] Convert bdrv_create and associated functions to be coroutine_fn
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 08/15] Convert bdrv_create and associated functions to be coroutine_fn Charlie Shepherd
@ 2013-08-29 15:31 ` Stefan Hajnoczi
0 siblings, 0 replies; 28+ messages in thread
From: Stefan Hajnoczi @ 2013-08-29 15:31 UTC (permalink / raw)
To: Charlie Shepherd; +Cc: kwolf, pbonzini, gabriel, qemu-devel
On Fri, Aug 09, 2013 at 07:43:58PM +0200, Charlie Shepherd wrote:
> This patch converts bdrv_create, bdrv_create_file and bdrv_img_create to be
> coroutine only functions. It adds a synchronous wrapper, bdrv_sync_create, for
> any synchronous callers.
It would be more consistent to call the coroutine_fn bdrv_co_create()
and the synchronous function bdrv_create(). We don't have the
bdrv_sync_foo() naming convention and it's easily confused with the
bdrv_pwrite_sync() function which flushes the disk cache (fsync(2)).
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [RFC v2 09/15] Add a synchronous wrapper bdrv_sync_rwco
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
` (6 preceding siblings ...)
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 08/15] Convert bdrv_create and associated functions to be coroutine_fn Charlie Shepherd
@ 2013-08-09 17:43 ` Charlie Shepherd
2013-08-29 15:32 ` Stefan Hajnoczi
2013-08-09 17:44 ` [Qemu-devel] [RFC v2 10/15] Convert bdrv_read, bdrv_write and associated functions to coroutine functions Charlie Shepherd
` (7 subsequent siblings)
15 siblings, 1 reply; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-09 17:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, gabriel, Charlie Shepherd, pbonzini
A number of functions in block.c pass an RwCo struct to a coroutine entry point in order to
synchronise an asynchronous function. This patch factors this pattern out into a function.
Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
---
block.c | 37 +++++++++++++++++--------------------
1 file changed, 17 insertions(+), 20 deletions(-)
diff --git a/block.c b/block.c
index 51a6649..25edb44 100644
--- a/block.c
+++ b/block.c
@@ -2171,6 +2171,17 @@ typedef struct RwCo {
BdrvRequestFlags flags;
} RwCo;
+static int bdrv_sync_rwco(void coroutine_fn (*co_fn)(void *), RwCo *rwco)
+{
+ Coroutine *co;
+ co = qemu_coroutine_create(co_fn);
+ qemu_coroutine_enter(co, rwco);
+ while (rwco->ret == NOT_DONE) {
+ qemu_aio_wait();
+ }
+ return rwco->ret;
+}
+
static void coroutine_fn bdrv_rw_co_entry(void *opaque)
{
RwCo *rwco = opaque;
@@ -2219,14 +2230,10 @@ static int bdrv_rwv_co(BlockDriverState *bs, int64_t sector_num,
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
bdrv_rw_co_entry(&rwco);
+ return rwco.ret;
} else {
- co = qemu_coroutine_create(bdrv_rw_co_entry);
- qemu_coroutine_enter(co, &rwco);
- while (rwco.ret == NOT_DONE) {
- qemu_aio_wait();
- }
+ return bdrv_sync_rwco(bdrv_rw_co_entry, &rwco);
}
- return rwco.ret;
}
/*
@@ -4144,15 +4151,10 @@ int bdrv_flush(BlockDriverState *bs)
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
bdrv_flush_co_entry(&rwco);
+ return rwco.ret;
} else {
- co = qemu_coroutine_create(bdrv_flush_co_entry);
- qemu_coroutine_enter(co, &rwco);
- while (rwco.ret == NOT_DONE) {
- qemu_aio_wait();
- }
+ return bdrv_sync_rwco(bdrv_flush_co_entry, &rwco);
}
-
- return rwco.ret;
}
static void coroutine_fn bdrv_discard_co_entry(void *opaque)
@@ -4216,15 +4218,10 @@ int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
bdrv_discard_co_entry(&rwco);
+ return rwco.ret;
} else {
- co = qemu_coroutine_create(bdrv_discard_co_entry);
- qemu_coroutine_enter(co, &rwco);
- while (rwco.ret == NOT_DONE) {
- qemu_aio_wait();
- }
+ return bdrv_sync_rwco(bdrv_discard_co_entry, &rwco);
}
-
- return rwco.ret;
}
/**************************************************************/
--
1.8.3.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [RFC v2 09/15] Add a synchronous wrapper bdrv_sync_rwco
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 09/15] Add a synchronous wrapper bdrv_sync_rwco Charlie Shepherd
@ 2013-08-29 15:32 ` Stefan Hajnoczi
0 siblings, 0 replies; 28+ messages in thread
From: Stefan Hajnoczi @ 2013-08-29 15:32 UTC (permalink / raw)
To: Charlie Shepherd; +Cc: kwolf, pbonzini, gabriel, qemu-devel
On Fri, Aug 09, 2013 at 07:43:59PM +0200, Charlie Shepherd wrote:
> A number of functions in block.c pass an RwCo struct to a coroutine entry point in order to
> synchronise an asynchronous function. This patch factors this pattern out into a function.
>
> Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
> ---
> block.c | 37 +++++++++++++++++--------------------
> 1 file changed, 17 insertions(+), 20 deletions(-)
Nice :)
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [RFC v2 10/15] Convert bdrv_read, bdrv_write and associated functions to coroutine functions
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
` (7 preceding siblings ...)
2013-08-09 17:43 ` [Qemu-devel] [RFC v2 09/15] Add a synchronous wrapper bdrv_sync_rwco Charlie Shepherd
@ 2013-08-09 17:44 ` Charlie Shepherd
2013-08-09 17:44 ` [Qemu-devel] [RFC v2 11/15] Make bdrv_discard coroutine only and add bdrv_sync_discard Charlie Shepherd
` (6 subsequent siblings)
15 siblings, 0 replies; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-09 17:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, gabriel, Charlie Shepherd, pbonzini
Now that .bdrv_co_read and .bdrv_co_write are only called in a coroutine
context, convert their callers in block.c to be coroutine only, and add
bdrv_sync_* versions for synchronous callers.
Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
---
block.c | 106 +++++++++++++++++++++++++++++++-------------------
include/block/block.h | 18 +++++----
2 files changed, 77 insertions(+), 47 deletions(-)
diff --git a/block.c b/block.c
index 25edb44..c7b6f01 100644
--- a/block.c
+++ b/block.c
@@ -2182,39 +2182,15 @@ static int bdrv_sync_rwco(void coroutine_fn (*co_fn)(void *), RwCo *rwco)
return rwco->ret;
}
-static void coroutine_fn bdrv_rw_co_entry(void *opaque)
-{
- RwCo *rwco = opaque;
-
- if (!rwco->is_write) {
- rwco->ret = bdrv_co_do_readv(rwco->bs, rwco->sector_num,
- rwco->nb_sectors, rwco->qiov,
- rwco->flags);
- } else {
- rwco->ret = bdrv_co_do_writev(rwco->bs, rwco->sector_num,
- rwco->nb_sectors, rwco->qiov,
- rwco->flags);
- }
-}
-
/*
* Process a vectored synchronous request using coroutines
*/
-static int bdrv_rwv_co(BlockDriverState *bs, int64_t sector_num,
+static int coroutine_fn bdrv_rwv_co(BlockDriverState *bs, int64_t sector_num,
QEMUIOVector *qiov, bool is_write,
BdrvRequestFlags flags)
{
- Coroutine *co;
- RwCo rwco = {
- .bs = bs,
- .sector_num = sector_num,
- .nb_sectors = qiov->size >> BDRV_SECTOR_BITS,
- .qiov = qiov,
- .is_write = is_write,
- .ret = NOT_DONE,
- .flags = flags,
- };
assert((qiov->size & (BDRV_SECTOR_SIZE - 1)) == 0);
+ int nb_sectors = qiov->size >> BDRV_SECTOR_BITS;
/**
* In sync call context, when the vcpu is blocked, this throttling timer
@@ -2227,19 +2203,41 @@ static int bdrv_rwv_co(BlockDriverState *bs, int64_t sector_num,
bdrv_io_limits_disable(bs);
}
- if (qemu_in_coroutine()) {
- /* Fast-path if already in coroutine context */
- bdrv_rw_co_entry(&rwco);
- return rwco.ret;
+ if (!is_write) {
+ return bdrv_co_do_readv(bs, sector_num, nb_sectors, qiov, flags);
} else {
- return bdrv_sync_rwco(bdrv_rw_co_entry, &rwco);
+ return bdrv_co_do_writev(bs, sector_num, nb_sectors, qiov, flags);
}
}
+static void coroutine_fn bdrv_rw_co_entry(void *opaque)
+{
+ RwCo *rwco = opaque;
+ rwco->ret = bdrv_rwv_co(rwco->bs, rwco->sector_num, rwco->qiov, rwco->is_write, rwco->flags);
+}
+
+/*
+ * Process a vectored synchronous request synchronously
+ */
+static int bdrv_rwv_sync(BlockDriverState *bs, int64_t sector_num,
+ QEMUIOVector *qiov, bool is_write, BdrvRequestFlags flags)
+{
+ RwCo rwco = {
+ .bs = bs,
+ .sector_num = sector_num,
+ .qiov = qiov,
+ .is_write = is_write,
+ .ret = NOT_DONE,
+ .flags = flags,
+ };
+
+ return bdrv_sync_rwco(bdrv_rw_co_entry, &rwco);
+}
+
/*
* Process a synchronous request using coroutines
*/
-static int bdrv_rw_co(BlockDriverState *bs, int64_t sector_num, uint8_t *buf,
+static int coroutine_fn bdrv_rw_co(BlockDriverState *bs, int64_t sector_num, uint8_t *buf,
int nb_sectors, bool is_write, BdrvRequestFlags flags)
{
QEMUIOVector qiov;
@@ -2252,15 +2250,37 @@ static int bdrv_rw_co(BlockDriverState *bs, int64_t sector_num, uint8_t *buf,
return bdrv_rwv_co(bs, sector_num, &qiov, is_write, flags);
}
-/* return < 0 if error. See bdrv_write() for the return codes */
-int bdrv_read(BlockDriverState *bs, int64_t sector_num,
+/*
+ * Process a synchronous request using coroutines
+ */
+static int bdrv_rw_sync(BlockDriverState *bs, int64_t sector_num, uint8_t *buf,
+ int nb_sectors, bool is_write)
+{
+ QEMUIOVector qiov;
+ struct iovec iov = {
+ .iov_base = (void *)buf,
+ .iov_len = nb_sectors * BDRV_SECTOR_SIZE,
+ };
+
+ qemu_iovec_init_external(&qiov, &iov, 1);
+ return bdrv_rwv_sync(bs, sector_num, &qiov, is_write, 0);
+}
+
+int coroutine_fn bdrv_read(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors)
{
return bdrv_rw_co(bs, sector_num, buf, nb_sectors, false, 0);
}
+/* return < 0 if error. See bdrv_write() for the return codes */
+int bdrv_sync_read(BlockDriverState *bs, int64_t sector_num,
+ uint8_t *buf, int nb_sectors)
+{
+ return bdrv_rw_sync(bs, sector_num, buf, nb_sectors, false);
+}
+
/* Just like bdrv_read(), but with I/O throttling temporarily disabled */
-int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
+int coroutine_fn bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors)
{
bool enabled;
@@ -2279,13 +2299,19 @@ int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
-EINVAL Invalid sector number or nb_sectors
-EACCES Trying to write a read-only device
*/
-int bdrv_write(BlockDriverState *bs, int64_t sector_num,
+int bdrv_sync_write(BlockDriverState *bs, int64_t sector_num,
+ const uint8_t *buf, int nb_sectors)
+{
+ return bdrv_rw_sync(bs, sector_num, (uint8_t *)buf, nb_sectors, true);
+}
+
+int coroutine_fn bdrv_write(BlockDriverState *bs, int64_t sector_num,
const uint8_t *buf, int nb_sectors)
{
return bdrv_rw_co(bs, sector_num, (uint8_t *)buf, nb_sectors, true, 0);
}
-int bdrv_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov)
+int coroutine_fn bdrv_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov)
{
return bdrv_rwv_co(bs, sector_num, qiov, true, 0);
}
@@ -2296,7 +2322,7 @@ int bdrv_write_zeroes(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
BDRV_REQ_ZERO_WRITE);
}
-int bdrv_pread(BlockDriverState *bs, int64_t offset,
+int coroutine_fn bdrv_pread(BlockDriverState *bs, int64_t offset,
void *buf, int count1)
{
uint8_t tmp_buf[BDRV_SECTOR_SIZE];
@@ -2341,7 +2367,7 @@ int bdrv_pread(BlockDriverState *bs, int64_t offset,
return count1;
}
-int bdrv_pwritev(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov)
+int coroutine_fn bdrv_pwritev(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov)
{
uint8_t tmp_buf[BDRV_SECTOR_SIZE];
int len, nb_sectors, count;
@@ -2398,7 +2424,7 @@ int bdrv_pwritev(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov)
return qiov->size;
}
-int bdrv_pwrite(BlockDriverState *bs, int64_t offset,
+int coroutine_fn bdrv_pwrite(BlockDriverState *bs, int64_t offset,
const void *buf, int count1)
{
QEMUIOVector qiov;
diff --git a/include/block/block.h b/include/block/block.h
index c6a3aaf..a209102 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -153,20 +153,24 @@ void bdrv_dev_eject_request(BlockDriverState *bs, bool force);
bool bdrv_dev_has_removable_media(BlockDriverState *bs);
bool bdrv_dev_is_tray_open(BlockDriverState *bs);
bool bdrv_dev_is_medium_locked(BlockDriverState *bs);
-int bdrv_read(BlockDriverState *bs, int64_t sector_num,
+int coroutine_fn bdrv_read(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors);
-int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
+int bdrv_sync_read(BlockDriverState *bs, int64_t sector_num,
+ uint8_t *buf, int nb_sectors);
+int coroutine_fn bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors);
-int bdrv_write(BlockDriverState *bs, int64_t sector_num,
+int coroutine_fn bdrv_write(BlockDriverState *bs, int64_t sector_num,
+ const uint8_t *buf, int nb_sectors);
+int bdrv_sync_write(BlockDriverState *bs, int64_t sector_num,
const uint8_t *buf, int nb_sectors);
int bdrv_write_zeroes(BlockDriverState *bs, int64_t sector_num,
int nb_sectors);
-int bdrv_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov);
-int bdrv_pread(BlockDriverState *bs, int64_t offset,
+int coroutine_fn bdrv_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov);
+int coroutine_fn bdrv_pread(BlockDriverState *bs, int64_t offset,
void *buf, int count);
-int bdrv_pwrite(BlockDriverState *bs, int64_t offset,
+int coroutine_fn bdrv_pwrite(BlockDriverState *bs, int64_t offset,
const void *buf, int count);
-int bdrv_pwritev(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov);
+int coroutine_fn bdrv_pwritev(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov);
int bdrv_pwrite_sync(BlockDriverState *bs, int64_t offset,
const void *buf, int count);
int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num,
--
1.8.3.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [RFC v2 11/15] Make bdrv_discard coroutine only and add bdrv_sync_discard
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
` (8 preceding siblings ...)
2013-08-09 17:44 ` [Qemu-devel] [RFC v2 10/15] Convert bdrv_read, bdrv_write and associated functions to coroutine functions Charlie Shepherd
@ 2013-08-09 17:44 ` Charlie Shepherd
2013-08-09 17:44 ` [Qemu-devel] [RFC v2 12/15] Make bdrv_flush coroutine only and add bdrv_sync_flush Charlie Shepherd
` (5 subsequent siblings)
15 siblings, 0 replies; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-09 17:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, gabriel, Charlie Shepherd, pbonzini
This patch renames the coroutine only bdrv_co_discard to bdrv_discard and the original
bdrv_discard to bdrv_sync_discard. bdrv_sync_discard is synchronous only.
Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
---
block.c | 12 +++---------
include/block/block.h | 3 ++-
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/block.c b/block.c
index c7b6f01..6995d3f 100644
--- a/block.c
+++ b/block.c
@@ -4190,7 +4190,7 @@ static void coroutine_fn bdrv_discard_co_entry(void *opaque)
rwco->ret = bdrv_co_discard(rwco->bs, rwco->sector_num, rwco->nb_sectors);
}
-int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
+int coroutine_fn bdrv_discard(BlockDriverState *bs, int64_t sector_num,
int nb_sectors)
{
if (!bs->drv) {
@@ -4231,7 +4231,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
}
}
-int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
+int bdrv_sync_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
{
Coroutine *co;
RwCo rwco = {
@@ -4241,13 +4241,7 @@ int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
.ret = NOT_DONE,
};
- if (qemu_in_coroutine()) {
- /* Fast-path if already in coroutine context */
- bdrv_discard_co_entry(&rwco);
- return rwco.ret;
- } else {
- return bdrv_sync_rwco(bdrv_discard_co_entry, &rwco);
- }
+ return bdrv_sync_rwco(bdrv_discard_co_entry, &rwco);
}
/**************************************************************/
diff --git a/include/block/block.h b/include/block/block.h
index a209102..65d8864 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -279,7 +279,8 @@ int bdrv_flush_all(void);
void bdrv_close_all(void);
void bdrv_drain_all(void);
-int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
+int coroutine_fn bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
+int bdrv_sync_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
int bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
int bdrv_has_zero_init_1(BlockDriverState *bs);
int bdrv_has_zero_init(BlockDriverState *bs);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [RFC v2 12/15] Make bdrv_flush coroutine only and add bdrv_sync_flush
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
` (9 preceding siblings ...)
2013-08-09 17:44 ` [Qemu-devel] [RFC v2 11/15] Make bdrv_discard coroutine only and add bdrv_sync_discard Charlie Shepherd
@ 2013-08-09 17:44 ` Charlie Shepherd
2013-08-09 17:44 ` [Qemu-devel] [RFC v2 13/15] Introduce a run_handler function in qemu-img.c Charlie Shepherd
` (4 subsequent siblings)
15 siblings, 0 replies; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-09 17:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, gabriel, Charlie Shepherd, pbonzini
This patch renames the coroutine only bdrv_co_flush to bdrv_flush and the
original bdrv_flush to bdrv_sync_flush. bdrv_sync_flush is synchronous only.
This patch also converts a caller in block/mirror.c to coroutine_fn.
Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
---
block.c | 12 +++---------
block/mirror.c | 4 ++--
include/block/block.h | 4 ++--
3 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/block.c b/block.c
index 6995d3f..27339af 100644
--- a/block.c
+++ b/block.c
@@ -4078,7 +4078,7 @@ static void coroutine_fn bdrv_flush_co_entry(void *opaque)
rwco->ret = bdrv_co_flush(rwco->bs);
}
-int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
+int coroutine_fn bdrv_flush(BlockDriverState *bs)
{
int ret;
@@ -4166,7 +4166,7 @@ void bdrv_clear_incoming_migration_all(void)
}
}
-int bdrv_flush(BlockDriverState *bs)
+int bdrv_sync_flush(BlockDriverState *bs)
{
Coroutine *co;
RwCo rwco = {
@@ -4174,13 +4174,7 @@ int bdrv_flush(BlockDriverState *bs)
.ret = NOT_DONE,
};
- if (qemu_in_coroutine()) {
- /* Fast-path if already in coroutine context */
- bdrv_flush_co_entry(&rwco);
- return rwco.ret;
- } else {
- return bdrv_sync_rwco(bdrv_flush_co_entry, &rwco);
- }
+ return bdrv_sync_rwco(bdrv_flush_co_entry, &rwco);
}
static void coroutine_fn bdrv_discard_co_entry(void *opaque)
diff --git a/block/mirror.c b/block/mirror.c
index bed4a7e..3d5da7e 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -282,7 +282,7 @@ static void mirror_free_init(MirrorBlockJob *s)
}
}
-static void mirror_drain(MirrorBlockJob *s)
+static void coroutine_fn mirror_drain(MirrorBlockJob *s)
{
while (s->in_flight > 0) {
qemu_coroutine_yield();
@@ -390,7 +390,7 @@ static void coroutine_fn mirror_run(void *opaque)
should_complete = false;
if (s->in_flight == 0 && cnt == 0) {
trace_mirror_before_flush(s);
- ret = bdrv_flush(s->target);
+ ret = bdrv_co_flush(s->target);
if (ret < 0) {
if (mirror_error_action(s, false, -ret) == BDRV_ACTION_REPORT) {
goto immediate_exit;
diff --git a/include/block/block.h b/include/block/block.h
index 65d8864..370b4c6 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -273,8 +273,8 @@ void bdrv_invalidate_cache_all(void);
void bdrv_clear_incoming_migration_all(void);
/* Ensure contents are flushed to disk. */
-int bdrv_flush(BlockDriverState *bs);
-int coroutine_fn bdrv_co_flush(BlockDriverState *bs);
+int coroutine_fn bdrv_flush(BlockDriverState *bs);
+int bdrv_sync_flush(BlockDriverState *bs);
int bdrv_flush_all(void);
void bdrv_close_all(void);
void bdrv_drain_all(void);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [RFC v2 13/15] Introduce a run_handler function in qemu-img.c
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
` (10 preceding siblings ...)
2013-08-09 17:44 ` [Qemu-devel] [RFC v2 12/15] Make bdrv_flush coroutine only and add bdrv_sync_flush Charlie Shepherd
@ 2013-08-09 17:44 ` Charlie Shepherd
2013-08-09 17:44 ` [Qemu-devel] [RFC v2 14/15] Add coroutine annotations for qemu_co_rwlock_rdlock and qemu_co_rwlock_wrlock Charlie Shepherd
` (3 subsequent siblings)
15 siblings, 0 replies; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-09 17:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, gabriel, Charlie Shepherd, pbonzini
qemu-img doesn't run in a coroutine context, but uses block layer functions
which do need to run in a coroutine context. This patch converts qemu-img to
run the various qemu-img functions in a coroutine context correctly.
Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
---
qemu-img.c | 54 +++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 43 insertions(+), 11 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index b9a848d..615f273 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -324,7 +324,7 @@ static int add_old_style_options(const char *fmt, QEMUOptionParameter *list,
return 0;
}
-static int img_create(int argc, char **argv)
+static int coroutine_fn img_create(int argc, char **argv)
{
int c;
uint64_t img_size = -1;
@@ -522,7 +522,7 @@ static int collect_image_check(BlockDriverState *bs,
* 2 - Check completed, image is corrupted
* 3 - Check completed, image has leaked clusters, but is good otherwise
*/
-static int img_check(int argc, char **argv)
+static int coroutine_fn img_check(int argc, char **argv)
{
int c, ret;
OutputFormat output_format = OFORMAT_HUMAN;
@@ -657,7 +657,7 @@ fail:
return ret;
}
-static int img_commit(int argc, char **argv)
+static int coroutine_fn img_commit(int argc, char **argv)
{
int c, ret, flags;
const char *filename, *fmt, *cache;
@@ -854,7 +854,7 @@ static int64_t sectors_to_process(int64_t total, int64_t from)
* @param buffer: Allocated buffer for storing read data
* @param quiet: Flag for quiet mode
*/
-static int check_empty_sectors(BlockDriverState *bs, int64_t sect_num,
+static int coroutine_fn check_empty_sectors(BlockDriverState *bs, int64_t sect_num,
int sect_count, const char *filename,
uint8_t *buffer, bool quiet)
{
@@ -882,7 +882,7 @@ static int check_empty_sectors(BlockDriverState *bs, int64_t sect_num,
* 1 - Images differ
* >1 - Error occurred
*/
-static int img_compare(int argc, char **argv)
+static int coroutine_fn img_compare(int argc, char **argv)
{
const char *fmt1 = NULL, *fmt2 = NULL, *filename1, *filename2;
BlockDriverState *bs1, *bs2;
@@ -1114,7 +1114,7 @@ out3:
return ret;
}
-static int img_convert(int argc, char **argv)
+static int coroutine_fn img_convert(int argc, char **argv)
{
int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors;
int progress = 0, flags;
@@ -1704,7 +1704,7 @@ err:
return NULL;
}
-static int img_info(int argc, char **argv)
+static int coroutine_fn img_info(int argc, char **argv)
{
int c;
OutputFormat output_format = OFORMAT_HUMAN;
@@ -1785,7 +1785,7 @@ static int img_info(int argc, char **argv)
#define SNAPSHOT_APPLY 3
#define SNAPSHOT_DELETE 4
-static int img_snapshot(int argc, char **argv)
+static int coroutine_fn img_snapshot(int argc, char **argv)
{
BlockDriverState *bs;
QEMUSnapshotInfo sn;
@@ -1902,7 +1902,7 @@ static int img_snapshot(int argc, char **argv)
return 0;
}
-static int img_rebase(int argc, char **argv)
+static int coroutine_fn img_rebase(int argc, char **argv)
{
BlockDriverState *bs, *bs_old_backing = NULL, *bs_new_backing = NULL;
BlockDriver *old_backing_drv, *new_backing_drv;
@@ -2184,7 +2184,7 @@ out:
return 0;
}
-static int img_resize(int argc, char **argv)
+static int coroutine_fn img_resize(int argc, char **argv)
{
int c, ret, relative;
const char *filename, *fmt, *size;
@@ -2317,6 +2317,38 @@ static const img_cmd_t img_cmds[] = {
{ NULL, NULL, },
};
+struct HandlerCo {
+ int coroutine_fn (*handler)(int, char **);
+ int argc;
+ char **argv;
+ int ret;
+};
+
+
+static void coroutine_fn handler_entry(void *opaque)
+{
+ struct HandlerCo *hco = opaque;
+ hco->ret = hco->handler(hco->argc, hco->argv);
+ hco->handler = NULL;
+}
+
+static int run_handler(int coroutine_fn (*handler)(int, char **), int argc, char **argv)
+{
+ struct HandlerCo hco = {
+ .handler = handler,
+ .argc = argc,
+ .argv = argv,
+ };
+
+ Coroutine *co = qemu_coroutine_create(handler_entry);
+ qemu_coroutine_enter(co, &hco);
+ while (hco.handler) {
+ qemu_aio_wait();
+ }
+
+ return hco.ret;
+}
+
int main(int argc, char **argv)
{
const img_cmd_t *cmd;
@@ -2338,7 +2370,7 @@ int main(int argc, char **argv)
/* find the command */
for(cmd = img_cmds; cmd->name != NULL; cmd++) {
if (!strcmp(cmdname, cmd->name)) {
- return cmd->handler(argc, argv);
+ return run_handler(cmd->handler, argc, argv);
}
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [RFC v2 14/15] Add coroutine annotations for qemu_co_rwlock_rdlock and qemu_co_rwlock_wrlock
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
` (11 preceding siblings ...)
2013-08-09 17:44 ` [Qemu-devel] [RFC v2 13/15] Introduce a run_handler function in qemu-img.c Charlie Shepherd
@ 2013-08-09 17:44 ` Charlie Shepherd
2013-08-09 17:44 ` [Qemu-devel] [RFC v2 15/15] Add coroutine_fn annotations to nbd_co_* functions Charlie Shepherd
` (2 subsequent siblings)
15 siblings, 0 replies; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-09 17:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, gabriel, Charlie Shepherd, pbonzini
qemu_co_rwlock_rdlock and qemu_co_rwlock_wrlock can only run in a coroutine
context, so annotate them with coroutine_fn.
Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
---
include/block/coroutine.h | 4 ++--
qemu-coroutine-lock.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/block/coroutine.h b/include/block/coroutine.h
index 1aae6e9..5e73219 100644
--- a/include/block/coroutine.h
+++ b/include/block/coroutine.h
@@ -199,14 +199,14 @@ void qemu_co_rwlock_init(CoRwlock *lock);
* of a parallel writer, control is transferred to the caller of the current
* coroutine.
*/
-void qemu_co_rwlock_rdlock(CoRwlock *lock);
+void coroutine_fn qemu_co_rwlock_rdlock(CoRwlock *lock);
/**
* Write Locks the mutex. If the lock cannot be taken immediately because
* of a parallel reader, control is transferred to the caller of the current
* coroutine.
*/
-void qemu_co_rwlock_wrlock(CoRwlock *lock);
+void coroutine_fn qemu_co_rwlock_wrlock(CoRwlock *lock);
/**
* Unlocks the read/write lock and schedules the next coroutine that was
diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index aeb33b9..06ee5b9 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -161,7 +161,7 @@ void qemu_co_rwlock_init(CoRwlock *lock)
qemu_co_queue_init(&lock->queue);
}
-void qemu_co_rwlock_rdlock(CoRwlock *lock)
+void coroutine_fn qemu_co_rwlock_rdlock(CoRwlock *lock)
{
while (lock->writer) {
qemu_co_queue_wait(&lock->queue);
@@ -185,7 +185,7 @@ void qemu_co_rwlock_unlock(CoRwlock *lock)
}
}
-void qemu_co_rwlock_wrlock(CoRwlock *lock)
+void coroutine_fn qemu_co_rwlock_wrlock(CoRwlock *lock)
{
while (lock->writer || lock->reader) {
qemu_co_queue_wait(&lock->queue);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [RFC v2 15/15] Add coroutine_fn annotations to nbd_co_* functions.
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
` (12 preceding siblings ...)
2013-08-09 17:44 ` [Qemu-devel] [RFC v2 14/15] Add coroutine annotations for qemu_co_rwlock_rdlock and qemu_co_rwlock_wrlock Charlie Shepherd
@ 2013-08-09 17:44 ` Charlie Shepherd
2013-08-14 14:14 ` [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Stefan Hajnoczi
2013-08-29 15:34 ` Stefan Hajnoczi
15 siblings, 0 replies; 28+ messages in thread
From: Charlie Shepherd @ 2013-08-09 17:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, gabriel, Charlie Shepherd, pbonzini
nbd_co_* are all meant to be only be run in a coroutine context, annotate them accordingly.
Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
---
block/nbd.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index a0e3562..3e037ba 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -256,7 +256,7 @@ static int nbd_config(BDRVNBDState *s, QDict *options)
}
-static void nbd_coroutine_start(BDRVNBDState *s, struct nbd_request *request)
+static void coroutine_fn nbd_coroutine_start(BDRVNBDState *s, struct nbd_request *request)
{
int i;
@@ -334,7 +334,7 @@ static void nbd_restart_write(void *opaque)
qemu_coroutine_enter(s->send_coroutine, NULL);
}
-static int nbd_co_send_request(BDRVNBDState *s, struct nbd_request *request,
+static int coroutine_fn nbd_co_send_request(BDRVNBDState *s, struct nbd_request *request,
QEMUIOVector *qiov, int offset)
{
int rc, ret;
@@ -368,7 +368,7 @@ static int nbd_co_send_request(BDRVNBDState *s, struct nbd_request *request,
return rc;
}
-static void nbd_co_receive_reply(BDRVNBDState *s, struct nbd_request *request,
+static void coroutine_fn nbd_co_receive_reply(BDRVNBDState *s, struct nbd_request *request,
struct nbd_reply *reply,
QEMUIOVector *qiov, int offset)
{
@@ -394,7 +394,7 @@ static void nbd_co_receive_reply(BDRVNBDState *s, struct nbd_request *request,
}
}
-static void nbd_coroutine_end(BDRVNBDState *s, struct nbd_request *request)
+static void coroutine_fn nbd_coroutine_end(BDRVNBDState *s, struct nbd_request *request)
{
int i = HANDLE_TO_INDEX(s, request->handle);
s->recv_coroutine[i] = NULL;
@@ -463,7 +463,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
closesocket(s->sock);
}
-static int nbd_open(BlockDriverState *bs, QDict *options, int flags)
+static int coroutine_fn nbd_co_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVNBDState *s = bs->opaque;
int result;
@@ -485,7 +485,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags)
return result;
}
-static int nbd_co_readv_1(BlockDriverState *bs, int64_t sector_num,
+static int coroutine_fn nbd_co_readv_1(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov,
int offset)
{
@@ -510,7 +510,7 @@ static int nbd_co_readv_1(BlockDriverState *bs, int64_t sector_num,
}
-static int nbd_co_writev_1(BlockDriverState *bs, int64_t sector_num,
+static int coroutine_fn nbd_co_writev_1(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov,
int offset)
{
@@ -542,7 +542,7 @@ static int nbd_co_writev_1(BlockDriverState *bs, int64_t sector_num,
* remain aligned to 4K. */
#define NBD_MAX_SECTORS 2040
-static int nbd_co_readv(BlockDriverState *bs, int64_t sector_num,
+static int coroutine_fn nbd_co_readv(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov)
{
int offset = 0;
@@ -559,7 +559,7 @@ static int nbd_co_readv(BlockDriverState *bs, int64_t sector_num,
return nbd_co_readv_1(bs, sector_num, nb_sectors, qiov, offset);
}
-static int nbd_co_writev(BlockDriverState *bs, int64_t sector_num,
+static int coroutine_fn nbd_co_writev(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov)
{
int offset = 0;
@@ -576,7 +576,7 @@ static int nbd_co_writev(BlockDriverState *bs, int64_t sector_num,
return nbd_co_writev_1(bs, sector_num, nb_sectors, qiov, offset);
}
-static int nbd_co_flush(BlockDriverState *bs)
+static int coroutine_fn nbd_co_flush(BlockDriverState *bs)
{
BDRVNBDState *s = bs->opaque;
struct nbd_request request;
@@ -606,7 +606,7 @@ static int nbd_co_flush(BlockDriverState *bs)
return -reply.error;
}
-static int nbd_co_discard(BlockDriverState *bs, int64_t sector_num,
+static int coroutine_fn nbd_co_discard(BlockDriverState *bs, int64_t sector_num,
int nb_sectors)
{
BDRVNBDState *s = bs->opaque;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
` (13 preceding siblings ...)
2013-08-09 17:44 ` [Qemu-devel] [RFC v2 15/15] Add coroutine_fn annotations to nbd_co_* functions Charlie Shepherd
@ 2013-08-14 14:14 ` Stefan Hajnoczi
2013-08-29 15:34 ` Stefan Hajnoczi
15 siblings, 0 replies; 28+ messages in thread
From: Stefan Hajnoczi @ 2013-08-14 14:14 UTC (permalink / raw)
To: Charlie Shepherd; +Cc: kwolf, pbonzini, gabriel, qemu-devel
On Fri, Aug 09, 2013 at 07:43:51PM +0200, Charlie Shepherd wrote:
> Coroutine functions that can yield directly or indirectly should be annotated
> with a coroutine_fn annotation. Add an explanation to that effect in
> include/block/coroutine.h.
>
> Signed-off-by: Charlie Shepherd <charlie@ctshepherd.com>
> ---
> include/block/coroutine.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
A "Patch 00/NN" email that serves as a cover letter to explain the
background and motivation for a patch series usually helps reviews
understand the purpose of your patches. git-format-patch --cover-letter
does this and git-publish automatically adds a cover letter when there
is more than one patch.
No need to resend, but please write a cover letter in the future. It
will encourage reviews.
Stefan
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn
2013-08-09 17:43 [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Charlie Shepherd
` (14 preceding siblings ...)
2013-08-14 14:14 ` [Qemu-devel] [RFC v2 01/15] Add an explanation of when a function should be marked coroutine_fn Stefan Hajnoczi
@ 2013-08-29 15:34 ` Stefan Hajnoczi
15 siblings, 0 replies; 28+ messages in thread
From: Stefan Hajnoczi @ 2013-08-29 15:34 UTC (permalink / raw)
To: Charlie Shepherd; +Cc: kwolf, pbonzini, gabriel, qemu-devel
I have left some comments. At this point I think a v2 would be good so
that any ripple-effects on later patches can play out.
Stefan
^ permalink raw reply [flat|nested] 28+ messages in thread