[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] block: move I/O request processing to block/
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2] block: move I/O request processing to block/io.c |
Date: |
Tue, 17 Feb 2015 09:12:45 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
On 02/17/2015 07:39 AM, Stefan Hajnoczi wrote:
> 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
> * Zero writes and making the image empty
> * Flush
> * Discard
> * ioctl
> * Tracked requests and queuing
> * Throttling and copy-on-read
> * Block status and allocated functions
> * Refreshing block limits
> * Reading/writing vmstate
> * qemu_blockalign() and friends
>
> 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_set_dirty(), bdrv_reset_dirty().
I spotted a couple of other changes that you may not have intended (or
if you did, you should also call them out here).
>
> 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 is based on the last block pull request from Fri 13th February
> 2015.
> This is necessary since the patch is prone to conflicts in block.c - we need
> to
> use the latest code!
>
> If anyone wants to move more stuff, please consider sending follow-up patches
> instead because it is painful to rebase this.
>
> v2:
> * Move bdrv_drain_all() [Kevin]
> * Move bdrv_make_zero() [Kevin]
> * Move bdrv_flush_all() [Kevin]
> * Move bdrv_is_allocated/get_block_status family of functions [Kevin]
> * Move bdrv_refresh_limits() [Kevin]
> * Move load/save_vmstate() [Kevin]
> * Move qemu_blockalign() and friends [Kevin]
> -void bdrv_drain_all(void)
> -{
> - /* Always run first iteration so any pending completion BHs run */
> - bool busy = true;
> - BlockDriverState *bs;
> -
> - while (busy) {
> - busy = false;
> -
> - QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
> - AioContext *aio_context = bdrv_get_aio_context(bs);
> -
> +void bdrv_drain_all(void)
> +{
> + /* Always run first iteration so any pending completion BHs run */
> + bool busy = true;
> + BlockDriverState *bs = NULL;
> +
> + while (busy) {
> + busy = false;
> +
> + while ((bs = bdrv_next(bs))) {
> + AioContext *aio_context = bdrv_get_aio_context(bs);
> +
You switched iteration from QTAILQ_FOREACH to a while(bdrv_next()) loop,
here, and in bdrv_flush_all.
I think the change is safe, but it would be smarter to split it into a
separate patch and/or document it in the commit message as intentional.
But I also agree that we want this patch in sooner rather than later to
minimize conflict churn. So:
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