[Libguestfs] [PATCH] nbdcopy: Speed up raw images copy

[Eric Blake]

On 2/18/21 5:32 PM, Nir Soffer wrote:
> When exporting sparse raw image, qemu-nbd reports unallocated area as
> zero:
> 
> $ qemu-nbd --persistent --socket /tmp/nbd.sock --read-only --shared=10 \
>     --format raw empty-6g.raw --cache=none --aio=native
> 
> $ nbdinfo --map nbd+unix:///?socket=/tmp/nbd.sock
>          0  6442450944    2  zero
> 

More precisely, lseek(SEEK_HOLE) only tells us what areas of the file
read as zero; it does NOT tell us if that area of the file is also
allocated (for that, you need the FIEMAP ioctl, but that is a
performance hit).  Yes, it's a bit confusing that SEEK_HOLE can't
identify actual holes: when it was first introduced by Solaris, the only
way to have a sparse file was with holes that read as zero, and it was
only with later filesystems on Linux where file allocation was made
independent from read-as-zero, making SEEK_HOLE a bit of a misnomer.

It would be nice if the kernel could give us an easy way to learn which
portions of a file are allocated.

> When using qcow2 image, it reports unallocated areas as a hole:
> 
> $ qemu-nbd --persistent --socket /tmp/nbd.sock --read-only --shared=10 \
>     --format qcow2 empty-6g.qcow2 --cache=none --aio=native
> 
> $ nbdinfo --map nbd+unix:///?socket=/tmp/nbd.sock
>          0  6442450944    3  hole,zero
> 
> Since nbdcopy is ignoring the ZERO flag and using only the HOLE flag,
> coping raw images is extremely slow:
> 

> +++ b/copy/multi-thread-copying.c
> @@ -157,8 +157,8 @@ worker_thread (void *indexp)
>        char *data;
>        size_t len;
>  
> -      if (exts.ptr[i].hole) {
> -        /* The source is a hole so we can proceed directly to
> +      if (exts.ptr[i].zero) {
> +        /* The source is zero so we can proceed directly to
>           * skipping, trimming or writing zeroes at the destination.

> +++ b/copy/nbd-ops.c
> @@ -190,8 +190,14 @@ add_extent (void *vp, const char *metacontext,
>  
>      e.offset = offset;
>      e.length = entries[i];
> -    /* Note we deliberately don't care about the ZERO flag. */
> -    e.hole = (entries[i+1] & LIBNBD_STATE_HOLE) != 0;
> +
> +    /*
> +     * Note we deliberately don't care about the HOLE flag. There is no need to
> +     * read extent that reads as zeroes. We will convert to it to a hole or
> +     * allocated extents based on the command line arguments.
> +     */
> +    e.zero = (entries[i+1] & LIBNBD_STATE_ZERO) != 0;

But this patch is absolutely correct: we care less about the allocation
pattern and more whether we know the data reads as zeroes.  So checking
for holes is not the right metric, and this patch is correct.

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