Re: [Qemu-devel] [PATCH] qemu-img: make convert async

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4143 bytes --]

On Tue, Feb 21, 2017 at 01:29:51PM +0100, Peter Lieven wrote:
> the convert process is currently completely implemented with sync operations.
> That means it reads one buffer and then writes it. No parallelism and each sync
> request takes as long as it takes until it is completed.
> 
> This can be a big performance hit when the convert process reads and writes
> to devices which do not benefit from kernel readahead or pagecache.
> In our environment we heavily have the following two use cases when using
> qemu-img convert.
> 
> a) reading from NFS and writing to iSCSI for deploying templates
> b) reading from iSCSI and writing to NFS for backups
> 
> In both processes we use libiscsi and libnfs so we have no kernel cache.
> 
> This patch changes the convert process to work with parallel running coroutines
> which can significantly improve performance for network storage devices:
> 
> qemu-img (master)
>  nfs -> iscsi 22.8 secs
>  nfs -> ram   11.7 secs
>  ram -> iscsi 12.3 secs
> 
> qemu-img-async (8 coroutines, in-order write disabled)
>  nfs -> iscsi 11.0 secs
>  nfs -> ram   10.4 secs
>  ram -> iscsi  9.0 secs
> 
> This patches introduces 2 new cmdline parameters. The -m parameter to specify
> the number of coroutines running in parallel (defaults to 8). And the -W paremeter to
> allow qemu-img to write to the target out of order rather than sequential. This improves
> performance as the writes do not have to wait for each other to complete.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>    RFC->V1: - add documentation
>             - add missing coroutine_fn annotation [Stefan]
>             - add a comment why it is safe to call coroutine_enter [Stefan]
>             - check -m paramater for values < 1 [Stefan]
>             - disallow -W parameter with compression [Stefan]
> 
> RFC V3->V4: - avoid to prepare a request queue upfront [Kevin]
>             - do not ignore the BLK_BACKING_FILE status [Kevin]
>             - redesign the interface to the read and write routines [Kevin]
>     
> RFC V2->V3: - updated stats in the commit msg from a host with a better network card
>             - only wake up the coroutine that is acutally waiting for a write to complete.
>               this was not only overhead, but also breaking at least linux AIO.
>             - fix coding style complaints
>             - rename some variables and structs
>     
> RFC V1->V2: - using coroutine as worker "threads". [Max]
>             - keeping the request queue as otherwise it happens
>               that we wait on BLK_ZERO chunks while keeping the write order.
>               it also avoids redundant calls to get_block_status and helps
>               to skip some conditions for fully allocated imaged (!s->min_sparse)
> 
>  qemu-img-cmds.hx |   4 +-
>  qemu-img.c       | 276 ++++++++++++++++++++++++++++++++++++++++---------------
>  qemu-img.texi    |  16 +++-
>  3 files changed, 220 insertions(+), 76 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

> @@ -326,6 +332,14 @@ skipped. This is useful for formats such as @code{rbd} if the target
>  volume has already been created with site specific options that cannot
>  be supplied through qemu-img.
>  
> +With option @code{-W} specified, it is allowed to write out of order to the target.
> +This is option improves performance, but is only recommened for preallocated devices

Minor nit.  Suggested rewording:

Out of order writes can be enabled with @code{-W} to improve
performance.  This is only recommended for preallocated devices

> +like host devices or other raw block devices. Out of order write does not work
> +in combination with creating compressed images.
> +
> +@var{num_coroutines} specifies how many coroutines work in parallel during
> +the convert process (defaults to 8).
> +
>  @item dd [-f @var{fmt}] [-O @var{output_fmt}] [bs=@var{block_size}] [count=@var{blocks}] [skip=@var{blocks}] if=@var{input} of=@var{output}
>  
>  Dd copies from @var{input} file to @var{output} file converting it from
> -- 
> 1.9.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]