Re: [Qemu-devel] [PATCH] block: posix: Always allocate the first block

[Nir Soffer]

On Fri, Aug 23, 2019 at 8:53 PM Max Reitz  wrote:

> On 23.08.19 18:48, Nir Soffer wrote:
> > On Fri, Aug 23, 2019 at 4:58 PM Max Reitz  > > wrote:
>
> [...]
>
> > If you have a format layer that truncates the image to a fixed size
> and
> > does not write anything into the first block itself (say because it
> uses
> > a footer), then (with O_DIRECT) allocate_first_block() will fail
> > (silently, because while it does return an error value, it is never
> > checked and there is no comment that explains why we don’t check it)
> >
> >
> > The motivation is that this is an optimization for the special case of
> using
> > empty image, so it does not worth failing image creation.
> > I will add a comment about that.
>
> Thanks!
>
> [...]
>
> > Thanks for the example.
> >
> > I will need time to play with blockdev and understand the flows when
> image
> > are created. Do you think is would be useful to fix now only image
> creation
> > via qemu-img, and handle blockdev later?
>
> Well, it isn’t about blockdev, it’s simply about the fact that this
> function doesn’t work for O_DIRECT files.  I showed how to reproduce the
> issue without blockdev (namely block_resize).  Sure, that is an edge
> case, but it is a completely valid case.
>
> Also, it seems to me the fix is rather simple.  Just something like:
>
> static int allocate_first_block(int fd, int64_t max_size)
> {
> int write_size = MIN(max_size, MAX_BLOCKSIZE);
> void *buf;
> ssize_t n;
>
> /* Round down to power of two */
> assert(write_size > 0);
> write_size = 1 << (31 - clz32(write_size));
>
> buf = qemu_memalign(MAX(getpagesize(), write_size), write_size);
> memset(buf, 0, write_size);
>
> do {
> n = pwrite(fd, buf, write_size, 0);
> } while (n < 0 && errno == EINTR);
>
> qemu_vfree(buf);
>
> return n < 0 ? -errno : 0;
> }
>
> Wouldn’t that work?
>

Sure, it should work.

But I think we can make this simpler, always writing MIN(max_size,
MAX_BLOCKSIZE).

vdsm is enforcing now 4k alignment, and there is no way to create images
with unaligned
size. Maybe qemu should adapt this rule?

Nir

Re: [Qemu-devel] [PATCH] block: posix: Always allocate the first block

[Max Reitz]

On 23.08.19 18:48, Nir Soffer wrote:
> On Fri, Aug 23, 2019 at 4:58 PM Max Reitz  > wrote:

[...]

> If you have a format layer that truncates the image to a fixed size and
> does not write anything into the first block itself (say because it uses
> a footer), then (with O_DIRECT) allocate_first_block() will fail
> (silently, because while it does return an error value, it is never
> checked and there is no comment that explains why we don’t check it)
> 
> 
> The motivation is that this is an optimization for the special case of using
> empty image, so it does not worth failing image creation.
> I will add a comment about that.

Thanks!

[...]

> Thanks for the example.
> 
> I will need time to play with blockdev and understand the flows when image
> are created. Do you think is would be useful to fix now only image creation
> via qemu-img, and handle blockdev later?

Well, it isn’t about blockdev, it’s simply about the fact that this
function doesn’t work for O_DIRECT files.  I showed how to reproduce the
issue without blockdev (namely block_resize).  Sure, that is an edge
case, but it is a completely valid case.

Also, it seems to me the fix is rather simple.  Just something like:

static int allocate_first_block(int fd, int64_t max_size)
{
int write_size = MIN(max_size, MAX_BLOCKSIZE);
void *buf;
ssize_t n;

/* Round down to power of two */
assert(write_size > 0);
write_size = 1 << (31 - clz32(write_size));

buf = qemu_memalign(MAX(getpagesize(), write_size), write_size);
memset(buf, 0, write_size);

do {
n = pwrite(fd, buf, write_size, 0);
} while (n < 0 && errno == EINTR);

qemu_vfree(buf);

return n < 0 ? -errno : 0;
}

Wouldn’t that work?

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] block: posix: Always allocate the first block

[Max Reitz]

On 23.08.19 18:30, Nir Soffer wrote:
> On Fri, Aug 23, 2019 at 4:58 PM Max Reitz  > wrote:
> 

[...]

> Exactly.  But if posix_fallocate() works, it should have allocated the
> first block.
> 
> 
> Only if the file system does not support fallocate(). posix_fallocate()
> first try
> fallocate(), and fall back to manual preallocation:
> https://code.woboq.org/userspace/glibc/sysdeps/unix/sysv/linux/posix_fallocate.c.html#27

I still don’t understand.  Your example does show that the first block
is not allocated by fallocate(), but I still don’t understand the
connection to not having fallocate() support.

If it doesn’t have fallocate() support and posix_fallocate() does fall
back, the result should be that posix_fallocate() manually allocates
data, which should be completely sufficient.

So in fact, it seems to me that the opposite is true: It seems that when
allocating blocks on XFS with fallocate(), that simply won’t be enough
to cause alignment errors.  So it doesn’t seem to be about fallback
code, but precisely the normal XFS code that fully supports fallocate.

(Just running your example on a local file on XFS shows the same result.)

So that seems to me why the additional allocation is necessary.  I think
that should be noted in a comment – if I’m right (I may well not be).

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] block: posix: Always allocate the first block

[Nir Soffer]

On Fri, Aug 23, 2019 at 4:58 PM Max Reitz  wrote:

> On 22.08.19 21:01, Nir Soffer wrote:
> > On Thu, Aug 22, 2019 at 9:11 PM Max Reitz  > > wrote:
> >
> > On 22.08.19 18:39, Nir Soffer wrote:
> > > On Thu, Aug 22, 2019 at 5:28 PM Max Reitz  > 
> > > >> wrote:
> > >
> > > On 16.08.19 23:21, 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.
> > > >
> > > > When using preallocation=off, we always allocate at least one
> > > filesystem
> > > > block:
> > > >
> > > > $ ./qemu-img create -f raw test.raw 1g
> > > > Formatting 'test.raw', fmt=raw size=1073741824
> > > >
> > > > $ ls -lhs test.raw
> > > > 4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48
> > test.raw
> > > >
> > > > I did quick performance tests for these flows:
> > > > - Provisioning a VM with a new raw image.
> > > > - Copying disks with qemu-img convert to new raw target image
> > > >
> > > > I installed Fedora 29 server on raw sparse image, measuring
> > the time
> > > > from clicking "Begin installation" until the "Reboot" button
> > appears:
> > > >
> > > > Before(s)  After(s) Diff(%)
> > > > ---
> > > >  356389+8.4
> > > >
> > > > I ran this only once, so we cannot tell much from these
> results.
> > >
> > > So you’d expect it to be fast but it was slower?  Well, you
> > only ran it
> > > once and it isn’t really a precise benchmark...
> > >
> > > > The second test was cloning the installation image with
> qemu-img
> > > > convert, doing 10 runs:
> > > >
> > > > for i in $(seq 10); do
> > > > rm -f dst.raw
> > > > sleep 10
> > > > time ./qemu-img convert -f raw -O raw -t none -T none
> > > src.raw dst.raw
> > > > done
> > > >
> > > > Here is a table comparing the total time spent:
> > > >
> > > > TypeBefore(s)   After(s)Diff(%)
> > > > ---
> > > > real  530.028469.123  -11.4
> > > > user   17.204 10.768  -37.4
> > > > sys17.881  7.011  -60.7
> > > >
> > > > Here we see very clear improvement in CPU usage.
> > > >
> > > > Signed-off-by: Nir Soffer  > 
> > > >>
> > > > ---
> > > >  block/file-posix.c | 25 +
> > > >  tests/qemu-iotests/150.out |  1 +
> > > >  tests/qemu-iotests/160 |  4 
> > > >  tests/qemu-iotests/175 | 19 +--
> > > >  tests/qemu-iotests/175.out |  8 
> > > >  tests/qemu-iotests/221.out | 12 
> > > >  tests/qemu-iotests/253.out | 12 
> > > >  7 files changed, 63 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/block/file-posix.c b/block/file-posix.c
> > > > index b9c33c8f6c..3964dd2021 100644
> > > > --- a/block/file-posix.c
> > > > +++ b/block/file-posix.c
> > > > @@ -1755,6 +1755,27 @@ static int handle_aiocb_discard(void
> > *opaque)
> > > >  return ret;
> > > >  }
> > > >
> > > > +/*
> > > > + * Help alignment detection by allocating the first block.
> > > > + *
> > > > + * When reading with direct I/O from unallocated area on
> > Gluster
> > > backed by XFS,
> > > > + * reading succeeds regardless of request length. In this
> > case we
> > > fallback to
> > > > + * safe aligment which is not optimal. Allocating the first
> > block
> > > avoids this
> > > > + * fallback.
> > > > + *
> > > > + * Returns: 0 on success, -errno on failure.
> > > > + */
> > > > +static int allocate_first_block(int fd)
> > > > +{
> > > > +ssize_t n;
> > >  

Re: [Qemu-devel] [PATCH] block: posix: Always allocate the first block

[Nir Soffer]

On Fri, Aug 23, 2019 at 4:58 PM Max Reitz  wrote:

> On 22.08.19 21:01, Nir Soffer wrote:
>
...

> > > > @@ -1794,6 +1815,8 @@ static int handle_aiocb_truncate(void
> > *opaque)
> > > >  /* posix_fallocate() doesn't set errno. */
> > > >  error_setg_errno(errp, -result,
> > > >   "Could not preallocate new
> > data");
> > > > +} else if (current_length == 0) {
> > > > +allocate_first_block(fd);
> > >
> > > Should posix_fallocate() not take care of precisely this?
> > >
> > >
> > > Only if the filesystem does not support fallocate() (e.g. NFS <
> 4.2).
> > >
> > > In this case posix_fallocate() is doing:
> > >
> > >   for (offset += (len - 1) % increment; len > 0; offset +=
> increment)
> > > {
> > >   len -= increment;
> > >   if (offset < st.st_size)
> > > {
> > >   unsigned char c;
> > >   ssize_t rsize = __pread (fd, , 1, offset);
> > >   if (rsize < 0)
> > > return errno;
> > >   /* If there is a non-zero byte, the block must have been
> > >  allocated already.  */
> > >   else if (rsize == 1 && c != 0)
> > > continue;
> > > }
> > >   if (__pwrite (fd, "", 1, offset) != 1)
> > > return errno;
> > > }
> > >
> > >
> >
> https://code.woboq.org/userspace/glibc/sysdeps/posix/posix_fallocate.c.html#96
> > >
> > > So opening a file with O_DIRECT will break preallocation=falloc on
> > such
> > > filesystems,
> >
> > But won’t the function above just fail with EINVAL?
> > allocate_first_block() is executed only in case of success.
> >
> >
> > Sure, but if posix_fallocate() fails, we fail qemu-img create/convert.
>
> Exactly.  But if posix_fallocate() works, it should have allocated the
> first block.
>

Only if the file system does not support fallocate(). posix_fallocate()
first try
fallocate(), and fall back to manual preallocation:
https://code.woboq.org/userspace/glibc/sysdeps/unix/sysv/linux/posix_fallocate.c.html#27

Here is an example using fallocate --posix:

$ sudo mount -t glusterfs gluster1:/gv0 /tmp/gv0

(gv0 is gluster volume backed by XFS on top of VDO device with 4k sector
size)

$ fallocate -l 1g --posix empty.raw
$ dd if=empty.raw bs=1 count=1 of=/dev/null iflag=direct status=none

$ dd if=/dev/zero bs=1 count=1 of=empty.raw conv=notrunc status=none
$ dd if=empty.raw bs=1 count=1 of=/dev/null iflag=direct status=none
dd: error reading 'empty.raw': Invalid argument

$ dd if=empty.raw bs=512 count=1 of=/dev/null iflag=direct status=none
dd: error reading 'empty.raw': Invalid argument

$ dd if=empty.raw bs=4096 count=1 of=/dev/null iflag=direct status=none

Here is example using gluster storage with sector size of 512 bytes.

$ sudo mount -t glusterfs gluster1:/gv1 /tmp/gv1

$ fallocate -l 1g --posix empty.raw
$ dd if=empty.raw bs=1 count=1 of=/dev/null iflag=direct status=none

$ dd if=/dev/zero bs=1 count=1 of=empty.raw conv=notrunc status=none

$ dd if=empty.raw bs=1 count=1 of=/dev/null iflag=direct status=none
dd: error reading 'empty.raw': Invalid argument

$ dd if=empty.raw bs=512 count=1 of=/dev/null iflag=direct status=none

So we must allocated using write() after calling posix_fallocate().

Nir

Re: [Qemu-devel] [PATCH] block: posix: Always allocate the first block

[Max Reitz]

On 22.08.19 21:01, Nir Soffer wrote:
> On Thu, Aug 22, 2019 at 9:11 PM Max Reitz  > wrote:
> 
> On 22.08.19 18:39, Nir Soffer wrote:
> > On Thu, Aug 22, 2019 at 5:28 PM Max Reitz  
> > >> wrote:
> >
> >     On 16.08.19 23:21, 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.
> >     >
> >     > When using preallocation=off, we always allocate at least one
> >     filesystem
> >     > block:
> >     >
> >     >     $ ./qemu-img create -f raw test.raw 1g
> >     >     Formatting 'test.raw', fmt=raw size=1073741824
> >     >
> >     >     $ ls -lhs test.raw
> >     >     4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48
> test.raw
> >     >
> >     > I did quick performance tests for these flows:
> >     > - Provisioning a VM with a new raw image.
> >     > - Copying disks with qemu-img convert to new raw target image
> >     >
> >     > I installed Fedora 29 server on raw sparse image, measuring
> the time
> >     > from clicking "Begin installation" until the "Reboot" button
> appears:
> >     >
> >     > Before(s)  After(s)     Diff(%)
> >     > ---
> >     >      356        389        +8.4
> >     >
> >     > I ran this only once, so we cannot tell much from these results.
> >
> >     So you’d expect it to be fast but it was slower?  Well, you
> only ran it
> >     once and it isn’t really a precise benchmark...
> >
> >     > The second test was cloning the installation image with qemu-img
> >     > convert, doing 10 runs:
> >     >
> >     >     for i in $(seq 10); do
> >     >         rm -f dst.raw
> >     >         sleep 10
> >     >         time ./qemu-img convert -f raw -O raw -t none -T none
> >     src.raw dst.raw
> >     >     done
> >     >
> >     > 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
> >     >
> >     > Here we see very clear improvement in CPU usage.
> >     >
> >     > Signed-off-by: Nir Soffer  
> >     >>
> >     > ---
> >     >  block/file-posix.c         | 25 +
> >     >  tests/qemu-iotests/150.out |  1 +
> >     >  tests/qemu-iotests/160     |  4 
> >     >  tests/qemu-iotests/175     | 19 +--
> >     >  tests/qemu-iotests/175.out |  8 
> >     >  tests/qemu-iotests/221.out | 12 
> >     >  tests/qemu-iotests/253.out | 12 
> >     >  7 files changed, 63 insertions(+), 18 deletions(-)
> >     >
> >     > diff --git a/block/file-posix.c b/block/file-posix.c
> >     > index b9c33c8f6c..3964dd2021 100644
> >     > --- a/block/file-posix.c
> >     > +++ b/block/file-posix.c
> >     > @@ -1755,6 +1755,27 @@ static int handle_aiocb_discard(void
> *opaque)
> >     >      return ret;
> >     >  }
> >     > 
> >     > +/*
> >     > + * Help alignment detection by allocating the first block.
> >     > + *
> >     > + * When reading with direct I/O from unallocated area on
> Gluster
> >     backed by XFS,
> >     > + * reading succeeds regardless of request length. In this
> case we
> >     fallback to
> >     > + * safe aligment which is not optimal. Allocating the first
> block
> >     avoids this
> >     > + * fallback.
> >     > + *
> >     > + * Returns: 0 on success, -errno on failure.
> >     > + */
> >     > +static int allocate_first_block(int fd)
> >     > +{
> >     > +    ssize_t n;
> >     > +
> >     > +    do {
> >     > +        n = pwrite(fd, "\0", 1, 0);
> >
> >     This breaks when fd has been opened with O_DIRECT.
> >
> >
> > It seems that we always open images without O_DIRECT when creating
> an image
> > in qemu-img create, 

Re: [Qemu-devel] [PATCH] block: posix: Always allocate the first block

[Nir Soffer]

On Thu, Aug 22, 2019 at 9:11 PM Max Reitz  wrote:

> On 22.08.19 18:39, Nir Soffer wrote:
> > On Thu, Aug 22, 2019 at 5:28 PM Max Reitz  > > wrote:
> >
> > On 16.08.19 23:21, 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.
> > >
> > > When using preallocation=off, we always allocate at least one
> > filesystem
> > > block:
> > >
> > > $ ./qemu-img create -f raw test.raw 1g
> > > Formatting 'test.raw', fmt=raw size=1073741824
> > >
> > > $ ls -lhs test.raw
> > > 4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48 test.raw
> > >
> > > I did quick performance tests for these flows:
> > > - Provisioning a VM with a new raw image.
> > > - Copying disks with qemu-img convert to new raw target image
> > >
> > > I installed Fedora 29 server on raw sparse image, measuring the
> time
> > > from clicking "Begin installation" until the "Reboot" button
> appears:
> > >
> > > Before(s)  After(s) Diff(%)
> > > ---
> > >  356389+8.4
> > >
> > > I ran this only once, so we cannot tell much from these results.
> >
> > So you’d expect it to be fast but it was slower?  Well, you only ran
> it
> > once and it isn’t really a precise benchmark...
> >
> > > The second test was cloning the installation image with qemu-img
> > > convert, doing 10 runs:
> > >
> > > for i in $(seq 10); do
> > > rm -f dst.raw
> > > sleep 10
> > > time ./qemu-img convert -f raw -O raw -t none -T none
> > src.raw dst.raw
> > > done
> > >
> > > Here is a table comparing the total time spent:
> > >
> > > TypeBefore(s)   After(s)Diff(%)
> > > ---
> > > real  530.028469.123  -11.4
> > > user   17.204 10.768  -37.4
> > > sys17.881  7.011  -60.7
> > >
> > > Here we see very clear improvement in CPU usage.
> > >
> > > Signed-off-by: Nir Soffer  > >
> > > ---
> > >  block/file-posix.c | 25 +
> > >  tests/qemu-iotests/150.out |  1 +
> > >  tests/qemu-iotests/160 |  4 
> > >  tests/qemu-iotests/175 | 19 +--
> > >  tests/qemu-iotests/175.out |  8 
> > >  tests/qemu-iotests/221.out | 12 
> > >  tests/qemu-iotests/253.out | 12 
> > >  7 files changed, 63 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/block/file-posix.c b/block/file-posix.c
> > > index b9c33c8f6c..3964dd2021 100644
> > > --- a/block/file-posix.c
> > > +++ b/block/file-posix.c
> > > @@ -1755,6 +1755,27 @@ static int handle_aiocb_discard(void
> *opaque)
> > >  return ret;
> > >  }
> > >
> > > +/*
> > > + * Help alignment detection by allocating the first block.
> > > + *
> > > + * When reading with direct I/O from unallocated area on Gluster
> > backed by XFS,
> > > + * reading succeeds regardless of request length. In this case we
> > fallback to
> > > + * safe aligment which is not optimal. Allocating the first block
> > avoids this
> > > + * fallback.
> > > + *
> > > + * Returns: 0 on success, -errno on failure.
> > > + */
> > > +static int allocate_first_block(int fd)
> > > +{
> > > +ssize_t n;
> > > +
> > > +do {
> > > +n = pwrite(fd, "\0", 1, 0);
> >
> > This breaks when fd has been opened with O_DIRECT.
> >
> >
> > It seems that we always open images without O_DIRECT when creating an
> image
> > in qemu-img create, or when creating a target image in qemu-img convert.
>
> Yes.  But you don’t call this function directly from image creation code
> but instead from the truncation function.  (The former also calls the
> latter, but truncating is also an operation on its own.)
>
> [...]
>
> > (Which happens when you open some file with cache.direct=on, and then
> > use e.g. QMP’s block_resize.)
> >
> >
> > What would be a command triggering this? I can add a test.
>
> block_resize, as I’ve said:
>
> $ ./qemu-img create -f raw empty.img 0
>

This is extreme edge case - why would someone create such image?


> $ x86_64-softmmu/qemu-system-x86_64 \
> -qmp stdio \
> -blockdev 

Re: [Qemu-devel] [PATCH] block: posix: Always allocate the first block

[Max Reitz]

On 22.08.19 18:39, Nir Soffer wrote:
> On Thu, Aug 22, 2019 at 5:28 PM Max Reitz  > wrote:
> 
> On 16.08.19 23:21, 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.
> >
> > When using preallocation=off, we always allocate at least one
> filesystem
> > block:
> >
> >     $ ./qemu-img create -f raw test.raw 1g
> >     Formatting 'test.raw', fmt=raw size=1073741824
> >
> >     $ ls -lhs test.raw
> >     4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48 test.raw
> >
> > I did quick performance tests for these flows:
> > - Provisioning a VM with a new raw image.
> > - Copying disks with qemu-img convert to new raw target image
> >
> > I installed Fedora 29 server on raw sparse image, measuring the time
> > from clicking "Begin installation" until the "Reboot" button appears:
> >
> > Before(s)  After(s)     Diff(%)
> > ---
> >      356        389        +8.4
> >
> > I ran this only once, so we cannot tell much from these results.
> 
> So you’d expect it to be fast but it was slower?  Well, you only ran it
> once and it isn’t really a precise benchmark...
> 
> > The second test was cloning the installation image with qemu-img
> > convert, doing 10 runs:
> >
> >     for i in $(seq 10); do
> >         rm -f dst.raw
> >         sleep 10
> >         time ./qemu-img convert -f raw -O raw -t none -T none
> src.raw dst.raw
> >     done
> >
> > 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
> >
> > Here we see very clear improvement in CPU usage.
> >
> > Signed-off-by: Nir Soffer  >
> > ---
> >  block/file-posix.c         | 25 +
> >  tests/qemu-iotests/150.out |  1 +
> >  tests/qemu-iotests/160     |  4 
> >  tests/qemu-iotests/175     | 19 +--
> >  tests/qemu-iotests/175.out |  8 
> >  tests/qemu-iotests/221.out | 12 
> >  tests/qemu-iotests/253.out | 12 
> >  7 files changed, 63 insertions(+), 18 deletions(-)
> >
> > diff --git a/block/file-posix.c b/block/file-posix.c
> > index b9c33c8f6c..3964dd2021 100644
> > --- a/block/file-posix.c
> > +++ b/block/file-posix.c
> > @@ -1755,6 +1755,27 @@ static int handle_aiocb_discard(void *opaque)
> >      return ret;
> >  }
> > 
> > +/*
> > + * Help alignment detection by allocating the first block.
> > + *
> > + * When reading with direct I/O from unallocated area on Gluster
> backed by XFS,
> > + * reading succeeds regardless of request length. In this case we
> fallback to
> > + * safe aligment which is not optimal. Allocating the first block
> avoids this
> > + * fallback.
> > + *
> > + * Returns: 0 on success, -errno on failure.
> > + */
> > +static int allocate_first_block(int fd)
> > +{
> > +    ssize_t n;
> > +
> > +    do {
> > +        n = pwrite(fd, "\0", 1, 0);
> 
> This breaks when fd has been opened with O_DIRECT.
> 
> 
> It seems that we always open images without O_DIRECT when creating an image
> in qemu-img create, or when creating a target image in qemu-img convert.

Yes.  But you don’t call this function directly from image creation code
but instead from the truncation function.  (The former also calls the
latter, but truncating is also an operation on its own.)

[...]

> (Which happens when you open some file with cache.direct=on, and then
> use e.g. QMP’s block_resize.)
> 
> 
> What would be a command triggering this? I can add a test.

block_resize, as I’ve said:

$ ./qemu-img create -f raw empty.img 0
$ x86_64-softmmu/qemu-system-x86_64 \
-qmp stdio \
-blockdev file,node-name=file,filename=empty.img,cache.direct=on \
 < 
> It isn’t that bad because eventually you simply ignore the error.  But
> it still makes me wonder whether we shouldn’t write like the biggest
> power of two that does not exceed the new file length or MAX_BLOCKSIZE.
> 
> 
> It makes sense if there is a way to cause qemu-img to use O_DIRECT when
> 

Re: [Qemu-devel] [PATCH] block: posix: Always allocate the first block

[Nir Soffer]

On Thu, Aug 22, 2019 at 5:28 PM Max Reitz  wrote:

> On 16.08.19 23:21, 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.
> >
> > When using preallocation=off, we always allocate at least one filesystem
> > block:
> >
> > $ ./qemu-img create -f raw test.raw 1g
> > Formatting 'test.raw', fmt=raw size=1073741824
> >
> > $ ls -lhs test.raw
> > 4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48 test.raw
> >
> > I did quick performance tests for these flows:
> > - Provisioning a VM with a new raw image.
> > - Copying disks with qemu-img convert to new raw target image
> >
> > I installed Fedora 29 server on raw sparse image, measuring the time
> > from clicking "Begin installation" until the "Reboot" button appears:
> >
> > Before(s)  After(s) Diff(%)
> > ---
> >  356389+8.4
> >
> > I ran this only once, so we cannot tell much from these results.
>
> So you’d expect it to be fast but it was slower?  Well, you only ran it
> once and it isn’t really a precise benchmark...
>
> > The second test was cloning the installation image with qemu-img
> > convert, doing 10 runs:
> >
> > for i in $(seq 10); do
> > rm -f dst.raw
> > sleep 10
> > time ./qemu-img convert -f raw -O raw -t none -T none src.raw
> dst.raw
> > done
> >
> > Here is a table comparing the total time spent:
> >
> > TypeBefore(s)   After(s)Diff(%)
> > ---
> > real  530.028469.123  -11.4
> > user   17.204 10.768  -37.4
> > sys17.881  7.011  -60.7
> >
> > Here we see very clear improvement in CPU usage.
> >
> > Signed-off-by: Nir Soffer 
> > ---
> >  block/file-posix.c | 25 +
> >  tests/qemu-iotests/150.out |  1 +
> >  tests/qemu-iotests/160 |  4 
> >  tests/qemu-iotests/175 | 19 +--
> >  tests/qemu-iotests/175.out |  8 
> >  tests/qemu-iotests/221.out | 12 
> >  tests/qemu-iotests/253.out | 12 
> >  7 files changed, 63 insertions(+), 18 deletions(-)
> >
> > diff --git a/block/file-posix.c b/block/file-posix.c
> > index b9c33c8f6c..3964dd2021 100644
> > --- a/block/file-posix.c
> > +++ b/block/file-posix.c
> > @@ -1755,6 +1755,27 @@ static int handle_aiocb_discard(void *opaque)
> >  return ret;
> >  }
> >
> > +/*
> > + * Help alignment detection by allocating the first block.
> > + *
> > + * When reading with direct I/O from unallocated area on Gluster backed
> by XFS,
> > + * reading succeeds regardless of request length. In this case we
> fallback to
> > + * safe aligment which is not optimal. Allocating the first block
> avoids this
> > + * fallback.
> > + *
> > + * Returns: 0 on success, -errno on failure.
> > + */
> > +static int allocate_first_block(int fd)
> > +{
> > +ssize_t n;
> > +
> > +do {
> > +n = pwrite(fd, "\0", 1, 0);
>
> This breaks when fd has been opened with O_DIRECT.
>

It seems that we always open images without O_DIRECT when creating an image
in qemu-img create, or when creating a target image in qemu-img convert.

Here is a trace of qemu-img create:

$ strace -f -tt -o /tmp/create.trace ./qemu-img create -f raw -o
preallocation=falloc /tmp/gv1/src.raw 1g
Formatting '/tmp/gv1/src.raw', fmt=raw size=1073741824 preallocation=falloc

1. open #1

17686 18:58:23.681921 openat(AT_FDCWD, "/tmp/gv1/src.raw",
O_RDONLY|O_NONBLOCK|O_CLOEXEC) = 9
17686 18:58:23.683753 fstat(9, {st_mode=S_IFREG|0600, st_size=1073741824,
...}) = 0
17686 18:58:23.683829 close(9)  = 0

2. open #2

17686 18:58:23.684146 openat(AT_FDCWD, "/tmp/gv1/src.raw",
O_RDONLY|O_NONBLOCK|O_CLOEXEC) = 9
17686 18:58:23.684227 fstat(9, {st_mode=S_IFREG|0600, st_size=1073741824,
...}) = 0
17686 18:58:23.684256 close(9)  = 0

3. open #3

17686 18:58:23.684339 openat(AT_FDCWD, "/tmp/gv1/src.raw",
O_RDWR|O_CREAT|O_CLOEXEC, 0644) = 9
...
17688 18:58:23.690178 fstat(9, {st_mode=S_IFREG|0600, st_size=1073741824,
...}) = 0
17688 18:58:23.690217 ftruncate(9, 0 
...
17688 18:58:23.700266 <... ftruncate resumed>) = 0
...
17688 18:58:23.700595 fstat(9,  
...
17688 18:58:23.700619 <... fstat resumed>{st_mode=S_IFREG|0600, st_size=0,
...}) = 0
...
17688 18:58:23.700651 fallocate(9, 0, 0, 1073741824 
...
17688 18:58:23.728141 <... fallocate resumed>) = 0
...
17688 18:58:23.728196 pwrite64(9, "\0", 1, 0) = 1
...
17686 18:58:23.738391 close(9)  = 0

Here is convert trace:

$ strace -f -tt -o /tmp/convert.trace 

Re: [Qemu-devel] [PATCH] block: posix: Always allocate the first block

[Max Reitz]

On 16.08.19 23:21, 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.
> 
> When using preallocation=off, we always allocate at least one filesystem
> block:
> 
> $ ./qemu-img create -f raw test.raw 1g
> Formatting 'test.raw', fmt=raw size=1073741824
> 
> $ ls -lhs test.raw
> 4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48 test.raw
> 
> I did quick performance tests for these flows:
> - Provisioning a VM with a new raw image.
> - Copying disks with qemu-img convert to new raw target image
> 
> I installed Fedora 29 server on raw sparse image, measuring the time
> from clicking "Begin installation" until the "Reboot" button appears:
> 
> Before(s)  After(s) Diff(%)
> ---
>  356389+8.4
> 
> I ran this only once, so we cannot tell much from these results.

So you’d expect it to be fast but it was slower?  Well, you only ran it
once and it isn’t really a precise benchmark...

> The second test was cloning the installation image with qemu-img
> convert, doing 10 runs:
> 
> for i in $(seq 10); do
> rm -f dst.raw
> sleep 10
> time ./qemu-img convert -f raw -O raw -t none -T none src.raw dst.raw
> done
> 
> Here is a table comparing the total time spent:
> 
> TypeBefore(s)   After(s)Diff(%)
> ---
> real  530.028469.123  -11.4
> user   17.204 10.768  -37.4
> sys17.881  7.011  -60.7
> 
> Here we see very clear improvement in CPU usage.
> 
> Signed-off-by: Nir Soffer 
> ---
>  block/file-posix.c | 25 +
>  tests/qemu-iotests/150.out |  1 +
>  tests/qemu-iotests/160 |  4 
>  tests/qemu-iotests/175 | 19 +--
>  tests/qemu-iotests/175.out |  8 
>  tests/qemu-iotests/221.out | 12 
>  tests/qemu-iotests/253.out | 12 
>  7 files changed, 63 insertions(+), 18 deletions(-)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index b9c33c8f6c..3964dd2021 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -1755,6 +1755,27 @@ static int handle_aiocb_discard(void *opaque)
>  return ret;
>  }
>  
> +/*
> + * Help alignment detection by allocating the first block.
> + *
> + * When reading with direct I/O from unallocated area on Gluster backed by 
> XFS,
> + * reading succeeds regardless of request length. In this case we fallback to
> + * safe aligment which is not optimal. Allocating the first block avoids this
> + * fallback.
> + *
> + * Returns: 0 on success, -errno on failure.
> + */
> +static int allocate_first_block(int fd)
> +{
> +ssize_t n;
> +
> +do {
> +n = pwrite(fd, "\0", 1, 0);

This breaks when fd has been opened with O_DIRECT.

(Which happens when you open some file with cache.direct=on, and then
use e.g. QMP’s block_resize.)

It isn’t that bad because eventually you simply ignore the error.  But
it still makes me wonder whether we shouldn’t write like the biggest
power of two that does not exceed the new file length or MAX_BLOCKSIZE.

> +} while (n == -1 && errno == EINTR);
> +
> +return (n == -1) ? -errno : 0;
> +}
> +
>  static int handle_aiocb_truncate(void *opaque)
>  {
>  RawPosixAIOData *aiocb = opaque;
> @@ -1794,6 +1815,8 @@ static int handle_aiocb_truncate(void *opaque)
>  /* posix_fallocate() doesn't set errno. */
>  error_setg_errno(errp, -result,
>   "Could not preallocate new data");
> +} else if (current_length == 0) {
> +allocate_first_block(fd);

Should posix_fallocate() not take care of precisely this?

>  }
>  } else {
>  result = 0;

[...]

> diff --git a/tests/qemu-iotests/160 b/tests/qemu-iotests/160
> index df89d3864b..ad2d054a47 100755
> --- a/tests/qemu-iotests/160
> +++ b/tests/qemu-iotests/160
> @@ -57,6 +57,10 @@ for skip in $TEST_SKIP_BLOCKS; do
>  $QEMU_IMG dd if="$TEST_IMG" of="$TEST_IMG.out" skip="$skip" -O "$IMGFMT" 
> \
>  2> /dev/null
>  TEST_IMG="$TEST_IMG.out" _check_test_img
> +
> +# We always write the first byte of an image.
> +printf "\0" > "$TEST_IMG.out.dd"
> +
>  dd if="$TEST_IMG" of="$TEST_IMG.out.dd" skip="$skip" status=none

Won’t this dd completely overwrite $TEST_IMG.out.dd (especially given
the lack of conv=notrunc)?

>  
>  echo
> diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
> index 51e62c8276..c6a3a7bb1e 100755
> --- 

Re: [Qemu-devel] [PATCH] block: posix: Always allocate the first block

[Nir Soffer]

Max, did you have time to look at this?

On Sat, Aug 17, 2019 at 12:21 AM 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.
>
> When using preallocation=off, we always allocate at least one filesystem
> block:
>
> $ ./qemu-img create -f raw test.raw 1g
> Formatting 'test.raw', fmt=raw size=1073741824
>
> $ ls -lhs test.raw
> 4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48 test.raw
>
> I did quick performance tests for these flows:
> - Provisioning a VM with a new raw image.
> - Copying disks with qemu-img convert to new raw target image
>
> I installed Fedora 29 server on raw sparse image, measuring the time
> from clicking "Begin installation" until the "Reboot" button appears:
>
> Before(s)  After(s) Diff(%)
> ---
>  356389+8.4
>
> I ran this only once, so we cannot tell much from these results.
>
> The second test was cloning the installation image with qemu-img
> convert, doing 10 runs:
>
> for i in $(seq 10); do
> rm -f dst.raw
> sleep 10
> time ./qemu-img convert -f raw -O raw -t none -T none src.raw
> dst.raw
> done
>
> Here is a table comparing the total time spent:
>
> TypeBefore(s)   After(s)Diff(%)
> ---
> real  530.028469.123  -11.4
> user   17.204 10.768  -37.4
> sys17.881  7.011  -60.7
>
> Here we see very clear improvement in CPU usage.
>
> Signed-off-by: Nir Soffer 
> ---
>  block/file-posix.c | 25 +
>  tests/qemu-iotests/150.out |  1 +
>  tests/qemu-iotests/160 |  4 
>  tests/qemu-iotests/175 | 19 +--
>  tests/qemu-iotests/175.out |  8 
>  tests/qemu-iotests/221.out | 12 
>  tests/qemu-iotests/253.out | 12 
>  7 files changed, 63 insertions(+), 18 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index b9c33c8f6c..3964dd2021 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -1755,6 +1755,27 @@ static int handle_aiocb_discard(void *opaque)
>  return ret;
>  }
>
> +/*
> + * Help alignment detection by allocating the first block.
> + *
> + * When reading with direct I/O from unallocated area on Gluster backed
> by XFS,
> + * reading succeeds regardless of request length. In this case we
> fallback to
> + * safe aligment which is not optimal. Allocating the first block avoids
> this
> + * fallback.
> + *
> + * Returns: 0 on success, -errno on failure.
> + */
> +static int allocate_first_block(int fd)
> +{
> +ssize_t n;
> +
> +do {
> +n = pwrite(fd, "\0", 1, 0);
> +} while (n == -1 && errno == EINTR);
> +
> +return (n == -1) ? -errno : 0;
> +}
> +
>  static int handle_aiocb_truncate(void *opaque)
>  {
>  RawPosixAIOData *aiocb = opaque;
> @@ -1794,6 +1815,8 @@ static int handle_aiocb_truncate(void *opaque)
>  /* posix_fallocate() doesn't set errno. */
>  error_setg_errno(errp, -result,
>   "Could not preallocate new data");
> +} else if (current_length == 0) {
> +allocate_first_block(fd);
>  }
>  } else {
>  result = 0;
> @@ -1855,6 +1878,8 @@ static int handle_aiocb_truncate(void *opaque)
>  if (ftruncate(fd, offset) != 0) {
>  result = -errno;
>  error_setg_errno(errp, -result, "Could not resize file");
> +} else if (current_length == 0 && offset > current_length) {
> +allocate_first_block(fd);
>  }
>  return result;
>  default:
> diff --git a/tests/qemu-iotests/150.out b/tests/qemu-iotests/150.out
> index 2a54e8dcfa..3cdc7727a5 100644
> --- a/tests/qemu-iotests/150.out
> +++ b/tests/qemu-iotests/150.out
> @@ -3,6 +3,7 @@ QA output created by 150
>  === Mapping sparse conversion ===
>
>  Offset  Length  File
> +0   0x1000  TEST_DIR/t.IMGFMT
>
>  === Mapping non-sparse conversion ===
>
> diff --git a/tests/qemu-iotests/160 b/tests/qemu-iotests/160
> index df89d3864b..ad2d054a47 100755
> --- a/tests/qemu-iotests/160
> +++ b/tests/qemu-iotests/160
> @@ -57,6 +57,10 @@ for skip in $TEST_SKIP_BLOCKS; do
>  $QEMU_IMG dd if="$TEST_IMG" of="$TEST_IMG.out" skip="$skip" -O
> "$IMGFMT" \
>  2> /dev/null
>  TEST_IMG="$TEST_IMG.out" _check_test_img
> +
> +# We always write the first byte of an image.
> +printf "\0" > "$TEST_IMG.out.dd"
> +
>  dd 

[Qemu-devel] [PATCH] block: posix: Always allocate the first block

[Nir Soffer]

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.

When using preallocation=off, we always allocate at least one filesystem
block:

$ ./qemu-img create -f raw test.raw 1g
Formatting 'test.raw', fmt=raw size=1073741824

$ ls -lhs test.raw
4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48 test.raw

I did quick performance tests for these flows:
- Provisioning a VM with a new raw image.
- Copying disks with qemu-img convert to new raw target image

I installed Fedora 29 server on raw sparse image, measuring the time
from clicking "Begin installation" until the "Reboot" button appears:

Before(s)  After(s) Diff(%)
---
 356389+8.4

I ran this only once, so we cannot tell much from these results.

The second test was cloning the installation image with qemu-img
convert, doing 10 runs:

for i in $(seq 10); do
rm -f dst.raw
sleep 10
time ./qemu-img convert -f raw -O raw -t none -T none src.raw dst.raw
done

Here is a table comparing the total time spent:

TypeBefore(s)   After(s)Diff(%)
---
real  530.028469.123  -11.4
user   17.204 10.768  -37.4
sys17.881  7.011  -60.7

Here we see very clear improvement in CPU usage.

Signed-off-by: Nir Soffer 
---
 block/file-posix.c | 25 +
 tests/qemu-iotests/150.out |  1 +
 tests/qemu-iotests/160 |  4 
 tests/qemu-iotests/175 | 19 +--
 tests/qemu-iotests/175.out |  8 
 tests/qemu-iotests/221.out | 12 
 tests/qemu-iotests/253.out | 12 
 7 files changed, 63 insertions(+), 18 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index b9c33c8f6c..3964dd2021 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1755,6 +1755,27 @@ static int handle_aiocb_discard(void *opaque)
 return ret;
 }
 
+/*
+ * Help alignment detection by allocating the first block.
+ *
+ * When reading with direct I/O from unallocated area on Gluster backed by XFS,
+ * reading succeeds regardless of request length. In this case we fallback to
+ * safe aligment which is not optimal. Allocating the first block avoids this
+ * fallback.
+ *
+ * Returns: 0 on success, -errno on failure.
+ */
+static int allocate_first_block(int fd)
+{
+ssize_t n;
+
+do {
+n = pwrite(fd, "\0", 1, 0);
+} while (n == -1 && errno == EINTR);
+
+return (n == -1) ? -errno : 0;
+}
+
 static int handle_aiocb_truncate(void *opaque)
 {
 RawPosixAIOData *aiocb = opaque;
@@ -1794,6 +1815,8 @@ static int handle_aiocb_truncate(void *opaque)
 /* posix_fallocate() doesn't set errno. */
 error_setg_errno(errp, -result,
  "Could not preallocate new data");
+} else if (current_length == 0) {
+allocate_first_block(fd);
 }
 } else {
 result = 0;
@@ -1855,6 +1878,8 @@ static int handle_aiocb_truncate(void *opaque)
 if (ftruncate(fd, offset) != 0) {
 result = -errno;
 error_setg_errno(errp, -result, "Could not resize file");
+} else if (current_length == 0 && offset > current_length) {
+allocate_first_block(fd);
 }
 return result;
 default:
diff --git a/tests/qemu-iotests/150.out b/tests/qemu-iotests/150.out
index 2a54e8dcfa..3cdc7727a5 100644
--- a/tests/qemu-iotests/150.out
+++ b/tests/qemu-iotests/150.out
@@ -3,6 +3,7 @@ QA output created by 150
 === Mapping sparse conversion ===
 
 Offset  Length  File
+0   0x1000  TEST_DIR/t.IMGFMT
 
 === Mapping non-sparse conversion ===
 
diff --git a/tests/qemu-iotests/160 b/tests/qemu-iotests/160
index df89d3864b..ad2d054a47 100755
--- a/tests/qemu-iotests/160
+++ b/tests/qemu-iotests/160
@@ -57,6 +57,10 @@ for skip in $TEST_SKIP_BLOCKS; do
 $QEMU_IMG dd if="$TEST_IMG" of="$TEST_IMG.out" skip="$skip" -O "$IMGFMT" \
 2> /dev/null
 TEST_IMG="$TEST_IMG.out" _check_test_img
+
+# We always write the first byte of an image.
+printf "\0" > "$TEST_IMG.out.dd"
+
 dd if="$TEST_IMG" of="$TEST_IMG.out.dd" skip="$skip" status=none
 
 echo
diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
index 51e62c8276..c6a3a7bb1e 100755
--- a/tests/qemu-iotests/175
+++ b/tests/qemu-iotests/175
@@ -37,14 +37,16 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 # the file size.  This function hides the resulting difference in the