Re: [Qemu-devel] [PATCH v2 1/2] block: posix: Always allocate the first block

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1518 bytes --]

On 8/25/19 5:03 PM, Nir Soffer wrote:
> When creating an image with preallocation "off" or "falloc", the first
> block of the image is typically not allocated. When using Gluster
> storage backed by XFS filesystem, reading this block using direct I/O
> succeeds regardless of request length, fooling alignment detection.
> 
> In this case we fallback to a safe value (4096) instead of the optimal
> value (512), which may lead to unneeded data copying when aligning
> requests.  Allocating the first block avoids the fallback.
> 

> Here is a table comparing the total time spent:
> 
> Type    Before(s)   After(s)    Diff(%)
> ---------------------------------------
> real      530.028    469.123      -11.4
> user       17.204     10.768      -37.4
> sys        17.881      7.011      -60.7
> 
> We can see very clear improvement in CPU usage.

Nice justification.


> +/*
> + * Help alignment probing by allocating the first block.
> + *

> +    do {
> +        n = pwrite(fd, buf, write_size, 0);
> +    } while (n == -1 && errno == EINTR);
> +
> +    qemu_vfree(buf);

qemu_vfree() can corrupt errno...

> +
> +    return (n == -1) ? -errno : 0;

...which means you may be returning an unexpected value here.

Either we should patch qemu_vfree() to guarantee that errno is
preserved, or you locally capture errno before calling it here.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]