[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH reformat] block: move I/O request processing to
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH reformat] block: move I/O request processing to block/io.c |
Date: |
Wed, 04 Feb 2015 11:01:38 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
On 02/04/2015 10:51 AM, Eric Blake wrote:
> From: Stefan Hajnoczi <address@hidden>
>
> The block.c file has grown to over 6000 lines. It is time to split this
> file so there are fewer conflicts and the code is easier to maintain.
>
> Extract I/O request processing code:
> * Read
> * Write
> * Flush
> * Discard
> * ioctl
> * Tracked requests and queuing
> * Throttling and copy-on-read
>
> The patch simply moves code from block.c into block/io.c.
>
> No code changes are made except adding the following block_int.h
> functions so they can be called across block.c and block/io.c:
> bdrv_drain_one(), bdrv_set_dirty(), bdrv_reset_dirty().
>
> I/O request processing needs to set up BlockDriver coroutine and AIO
> emulation function pointers, so add bdrv_setup_io_funcs(bdrv) interface
> that block.c calls.
>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>
> This patch produces identical results to Stefan's email, but is
> MUCH more readable (hint: git config diff.algorithm patience)
>
> block.c | 1980 +-------------------------------------------
> block/Makefile.objs | 1 +
> block/io.c | 1997
> +++++++++++++++++++++++++++++++++++++++++++++
> include/block/block_int.h | 14 +
> 4 files changed, 2015 insertions(+), 1977 deletions(-)
> create mode 100644 block/io.c
And here's how I reviewed it:
$ git format-patch --stdout -1 > patch
$ diff -u <(sed -n 's/^-//p' patch) <(sed -n 's/^\+//p' patch)
> --- /dev/fd/63 2015-02-04 10:59:08.221371351 -0700
> +++ /dev/fd/62 2015-02-04 10:59:08.221371351 -0700
> @@ -1,5 +1,39 @@
> ---
> --- a/block.c
> +++ b/block.c
> + bdrv_setup_io_funcs(bdrv);
> +void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors)
> +void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector, int
> nr_sectors)
> +++ b/block/Makefile.objs
> +block-obj-y += io.o
> +++ b/block/io.c
> +/*
> + * QEMU System Emulator block driver
> + *
> + * Copyright (c) 2003 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> + * of this software and associated documentation files (the "Software"), to
> deal
> + * in the Software without restriction, including without limitation the
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "trace.h"
> +#include "block/block_int.h"
> +
> +#define NOT_DONE 0x7fffffff /* used while emulated sync operation in
> progress */
> +
> static BlockAIOCB *bdrv_aio_readv_em(BlockDriverState *bs,
> int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
> BlockCompletionFunc *cb, void *opaque);
> @@ -30,10 +64,24 @@
> static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
> int64_t sector_num, int nb_sectors, BdrvRequestFlags flags);
>
> -static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
> - int nr_sectors);
> -static void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
> - int nr_sectors);
> +void bdrv_setup_io_funcs(BlockDriver *bdrv)
> +{
> + /* Block drivers without coroutine functions need emulation */
> + if (!bdrv->bdrv_co_readv) {
> + bdrv->bdrv_co_readv = bdrv_co_readv_em;
> + bdrv->bdrv_co_writev = bdrv_co_writev_em;
> +
> + /* bdrv_co_readv_em()/brdv_co_writev_em() work in terms of aio, so if
> + * the block driver lacks aio we need to emulate that too.
> + */
> + if (!bdrv->bdrv_aio_readv) {
> + /* add AIO emulation layer */
> + bdrv->bdrv_aio_readv = bdrv_aio_readv_em;
> + bdrv->bdrv_aio_writev = bdrv_aio_writev_em;
> + }
> + }
> +}
> +
> /* throttling disk I/O limits */
> void bdrv_set_io_limits(BlockDriverState *bs,
> ThrottleConfig *cfg)
> @@ -132,20 +180,6 @@
> qemu_co_queue_next(&bs->throttled_reqs[is_write]);
> }
>
> - /* Block drivers without coroutine functions need emulation */
> - if (!bdrv->bdrv_co_readv) {
> - bdrv->bdrv_co_readv = bdrv_co_readv_em;
> - bdrv->bdrv_co_writev = bdrv_co_writev_em;
> -
> - /* bdrv_co_readv_em()/brdv_co_writev_em() work in terms of aio, so if
> - * the block driver lacks aio we need to emulate that too.
> - */
> - if (!bdrv->bdrv_aio_readv) {
> - /* add AIO emulation layer */
> - bdrv->bdrv_aio_readv = bdrv_aio_readv_em;
> - bdrv->bdrv_aio_writev = bdrv_aio_writev_em;
> - }
> - }
> /**
> * The copy-on-read flag is actually a reference count so multiple users may
> * use the feature without worrying about clobbering its previous state.
> @@ -183,7 +217,7 @@
> return false;
> }
>
> -static bool bdrv_drain_one(BlockDriverState *bs)
> +bool bdrv_drain_one(BlockDriverState *bs)
> {
> bool bs_busy;
>
> @@ -286,19 +320,6 @@
> }
> }
>
> -static int bdrv_get_cluster_size(BlockDriverState *bs)
> -{
> - BlockDriverInfo bdi;
> - int ret;
> -
> - ret = bdrv_get_info(bs, &bdi);
> - if (ret < 0 || bdi.cluster_size == 0) {
> - return bs->request_alignment;
> - } else {
> - return bdi.cluster_size;
> - }
> -}
> -
> static bool tracked_request_overlaps(BdrvTrackedRequest *req,
> int64_t offset, unsigned int bytes)
> {
> @@ -357,7 +378,6 @@
> return waited;
> }
>
> -
> static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
> size_t size)
> {
> @@ -598,6 +618,22 @@
> return 0;
> }
>
> +int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
> + const uint8_t *buf, int nb_sectors)
> +{
> + BlockDriver *drv = bs->drv;
> + if (!drv)
> + return -ENOMEDIUM;
> + if (!drv->bdrv_write_compressed)
> + return -ENOTSUP;
> + if (bdrv_check_request(bs, sector_num, nb_sectors))
> + return -EIO;
> +
> + assert(QLIST_EMPTY(&bs->dirty_bitmaps));
> +
> + return drv->bdrv_write_compressed(bs, sector_num, buf, nb_sectors);
> +}
> +
> static int coroutine_fn bdrv_co_do_copy_on_readv(BlockDriverState *bs,
> int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
> {
> @@ -669,6 +705,19 @@
> return ret;
> }
>
> +static int bdrv_get_cluster_size(BlockDriverState *bs)
> +{
> + BlockDriverInfo bdi;
> + int ret;
> +
> + ret = bdrv_get_info(bs, &bdi);
> + if (ret < 0 || bdi.cluster_size == 0) {
> + return bs->request_alignment;
> + } else {
> + return bdi.cluster_size;
> + }
> +}
> +
> /*
> * Forwards an already correctly aligned request to the BlockDriver. This
> * handles copy on read and zeroing after EOF; any other features must be
> @@ -1161,22 +1210,6 @@
> BDRV_REQ_ZERO_WRITE | flags);
> }
>
> -int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
> - const uint8_t *buf, int nb_sectors)
> -{
> - BlockDriver *drv = bs->drv;
> - if (!drv)
> - return -ENOMEDIUM;
> - if (!drv->bdrv_write_compressed)
> - return -ENOTSUP;
> - if (bdrv_check_request(bs, sector_num, nb_sectors))
> - return -EIO;
> -
> - assert(QLIST_EMPTY(&bs->dirty_bitmaps));
> -
> - return drv->bdrv_write_compressed(bs, sector_num, buf, nb_sectors);
> -}
> -
> /**************************************************************/
> /* async I/Os */
>
> @@ -1211,7 +1244,6 @@
> cb, opaque, true);
> }
>
> -
> typedef struct MultiwriteCB {
> int error;
> int num_requests;
> @@ -1501,7 +1533,6 @@
> return bdrv_aio_rw_vector(bs, sector_num, qiov, nb_sectors, cb, opaque,
> 1);
> }
>
> -
> typedef struct BlockAIOCBCoroutine {
> BlockAIOCB common;
> BlockRequest req;
> @@ -1620,7 +1651,6 @@
>
> return &acb->common;
> }
> -
> void *qemu_aio_get(const AIOCBInfo *aiocb_info, BlockDriverState *bs,
> BlockCompletionFunc *cb, void *opaque)
> {
> @@ -1937,10 +1967,6 @@
> return NULL;
> }
>
> -static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
> - int nr_sectors)
> -static void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
> - int nr_sectors)
> void bdrv_add_before_write_notifier(BlockDriverState *bs,
> NotifierWithReturn *notifier)
> {
> @@ -1976,8 +2002,18 @@
> bdrv_flush_io_queue(bs->file);
> }
> }
> +++ b/include/block/block_int.h
> +/**
> + * bdrv_setup_io_funcs:
> + *
> + * Prepare a #BlockDriver for I/O request processing by populating
> + * unimplemented coroutine and AIO interfaces with generic wrapper functions
> + * that fall back to implemented interfaces.
> + */
> +void bdrv_setup_io_funcs(BlockDriver *bdrv);
> +bool bdrv_drain_one(BlockDriverState *bs);
> +
> +void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int
> nr_sectors);
> +void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
> + int nr_sectors);
>
> --- a/block/Makefile.objs
> --- /dev/null
> --- a/include/block/block_int.h
> --
>
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature