Am 05.08.2013 um 20:44 hat Charlie Shepherd geschrieben:
> This patch follows on from the previous one and converts some block layer
> functions to be
> explicitly annotated with coroutine_fn instead of yielding depending upon
> calling context.
> ---
> block.c | 235
> ++++++++++++++++++++++++++------------------------
> block/mirror.c | 4 +-
> include/block/block.h | 37 ++++----
> 3 files changed, 148 insertions(+), 128 deletions(-)
>
> diff --git a/block.c b/block.c
> index aaa122c..e7011f9 100644
> --- a/block.c
> +++ b/block.c
> @@ -164,7 +164,7 @@ bool bdrv_io_limits_enabled(BlockDriverState *bs)
> || io_limits->iops[BLOCK_IO_LIMIT_TOTAL];
> }
>
> -static void bdrv_io_limits_intercept(BlockDriverState *bs,
> +static void coroutine_fn bdrv_io_limits_intercept(BlockDriverState *bs,
> bool is_write, int nb_sectors)
> {
> int64_t wait_time = -1;
> @@ -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;
Like Gabriel said, this should be a separate patch. Keeping it in this
series is probably easiest.
> @@ -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_create_sync(BlockDriver *drv, const char* filename,
> + QEMUOptionParameter *options)
bdrv_foo_sync is an unfortunate naming convention, because
bdrv_pwrite_sync already exists and means something totally different:
It's a write directly followed by a disk flush.
> diff --git a/include/block/block.h b/include/block/block.h
> index dd8eca1..57d8ba2 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -125,9 +125,9 @@ void bdrv_append(BlockDriverState *bs_new,
> BlockDriverState *bs_top);
> void bdrv_delete(BlockDriverState *bs);
> int bdrv_parse_cache_flags(const char *mode, int *flags);
> int bdrv_parse_discard_flags(const char *mode, int *flags);
> -int bdrv_file_open(BlockDriverState **pbs, const char *filename,
> +int coroutine_fn bdrv_file_open(BlockDriverState **pbs, const char *filename,
> QDict *options, int flags);
> -int bdrv_open_backing_file(BlockDriverState *bs, QDict *options);
> +int coroutine_fn bdrv_open_backing_file(BlockDriverState *bs, QDict
> *options);
> int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
> int flags, BlockDriver *drv);
> BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
> @@ -150,18 +150,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_read_sync(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_write_sync(BlockDriverState *bs, int64_t sector_num,
> const uint8_t *buf, 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_pread_sync(BlockDriverState *bs, int64_t offset,
> void *buf, int count);
I haven't checked everything, but bdrv_pread_sync is an example of a
declaration without any user and without implementation.
Proper patch splitting will make review of such things easier, so I'll
defer thorough review until then.
Kevin
