[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH v2] v2v: rhv-upload-plugin: Defer imageio connection



On 1/21/21 6:16 AM, Nir Soffer wrote:
> On Thu, Jan 21, 2021 at 1:57 PM Nir Soffer <nirsof gmail com> wrote:
> ...
> 
> How I tested:
> 
> I could not use the example python plugin with the added delay, since
> os.preadv() is not available in python 3.6 (added in 3.7).
> 
> I could not use the delay filter since it supports only integer delay.

You are correct that it currently doesn't take floating point delays,
but it does take an 'ms' suffix to sleep by milliseconds instead of seconds.

 With
> fedora 32 image with 231 extents, qemu-img calls block_status 960 times
> during import and this is too slow to test.
> 
> So I used this hack:
> 
> $ git diff
> diff --git a/plugins/file/file.c b/plugins/file/file.c
> index 1651079a..99a95af0 100644
> --- a/plugins/file/file.c
> +++ b/plugins/file/file.c
> @@ -785,6 +785,8 @@ do_extents (void *handle, uint32_t count, uint64_t offset,
>    const bool req_one = flags & NBDKIT_FLAG_REQ_ONE;
>    uint64_t end = offset + count;
> 
> +  usleep(300000);

That looks like it could be achieved by '--filter delay-extents=300ms'.


> I also tried to import using imageio client, using this pipeline:
> 
>     imageio nbd client -> qemu-nbd -> overlay.qcow2 -> nbdkit
> 
> On imageio side, we always try to get extents for 2g, and we don't use
> NBD_CMD_FLAG_REQ_ONE. But on nbdkit we see that all extents
> calls use req_one=1.
> 
> Looks like the issue is this code in qemu nbd driver:
> 
> 1676 static int coroutine_fn nbd_client_co_block_status(
> 1677         BlockDriverState *bs, bool want_zero, int64_t offset,
> int64_t bytes,
> 1678         int64_t *pnum, int64_t *map, BlockDriverState **file)
> 1679 {
> ...
> 1685     NBDRequest request = {
> 1686         .type = NBD_CMD_BLOCK_STATUS,
> 1687         .from = offset,
> 1688         .len = MIN(QEMU_ALIGN_DOWN(INT_MAX, bs->bl.request_alignment),
> 1689                    MIN(bytes, s->info.size - offset)),
> 1690         .flags = NBD_CMD_FLAG_REQ_ONE,
> 1691     };

Yes, qemu is not yet optimized to remember more than just the current
extent.

> 
> Also qemu-img convert is getting extents twice:
> 
> 1. One call per extents before starting the copy:
> 
> nbdkit: file.0: debug: file: extents count=196608 offset=851968 req_one=1
> nbdkit: file.1: debug: file: extents count=196608 offset=1376256 req_one=1
> nbdkit: file.3: debug: file: extents count=393216 offset=1638400 req_one=1
> nbdkit: file.4: debug: file: extents count=458752 offset=2686976 req_one=1
> nbdkit: file.5: debug: file: extents count=983040 offset=3211264 req_one=1
> ...
> nbdkit: file.14: debug: file: extents count=524288 offset=6441402368 req_one=1
> 
> 2. One call per extent before each pread(), during copy:

Yes, that's probably another thing that could be improved in qemu.

> Total 960 calls per one import.
> 
> A tiny delay in getting extents can slow down the entire import.

Known issue in qemu-img; that's partly why libnbd's nbdcopy is currently
faster.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]