On Fri, Aug 23, 2019 at 8:53 PM Max Reitz <[email protected]> wrote: > On 23.08.19 18:48, Nir Soffer wrote: > > On Fri, Aug 23, 2019 at 4:58 PM Max Reitz <[email protected] > > <mailto:[email protected]>> 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
