On 23.08.19 18:48, Nir Soffer wrote:
> On Fri, Aug 23, 2019 at 4:58 PM Max Reitz <address@hidden
> <mailto:address@hidden>> 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?
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