Re: Block alignment of qcow2 compress driver

[Hanna Reitz]

On 28.01.22 13:18, Richard W.M. Jones wrote:
> On Fri, Jan 28, 2022 at 12:57:47PM +0100, Hanna Reitz wrote:
> > On 28.01.22 12:48, Richard W.M. Jones wrote:
> > > On Fri, Jan 28, 2022 at 12:39:11PM +0100, Hanna Reitz wrote:
> > > > So I actually don’t know why it works for you.  OTOH, I don’t
> > > > understand why the block size affects you over NBD, because I would
> > > > have expected qemu to internally auto-align requests when they are
> > > > not aligned (in bdrv_co_pwritev_part()).
> > > I checked it again and my hack definitely fixes nbdcopy.  But maybe
> > > that's expected if qemu-nbd is auto-aligning requests?  (I'm only
> > > accessing the block layer through qemu-nbd, not with qemu-io)
> > It’s not just qemu-io, with your diff[3] I get the same EINVAL over
> > NBD, too:
> >
> > $ ./qemu-img create -f qcow2 test.qcow2 64M
> > Formatting 'test.qcow2', fmt=qcow2 cluster_size=65536
> > extended_l2=off compression_type=zlib size=67108864
> > lazy_refcounts=off refcount_bits=16
> >
> > $ ./qemu-nbd --fork --image-opts \
> > driver=compress,file.driver=qcow2,file.file.driver=file,file.file.filename=test.qcow2
> >
> > $ ./qemu-io -c 'write 0 32k' -f raw nbd://localhost
> > write failed: Invalid argument
> Strange - is that error being generated by qemu's nbd client code?

It’s generated by qcow2, namely the exact place I pointed out (as [1]).  
I can see that when I put an fprintf there.

> Here's my test not involving qemu's client code:
>
> $ qemu-nbd --version
> qemu-nbd 6.2.0 (qemu-6.2.0-2.fc36)
>
> $ qemu-img create -f qcow2 output.qcow2 1M
> Formatting 'output.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off 
> compression_type=zlib size=1048576 lazy_refcounts=off refcount_bits=16
>
> $ qemu-nbd --fork --image-opts 
> driver=compress,file.driver=qcow2,file.file.driver=file,file.file.filename=output.qcow2
>
> $ nbdsh -u nbd://localhost
> nbd> h.get_strict_mode()
> 31
> nbd> h.set_strict_mode(31 & ~nbd.STRICT_ALIGN)
> nbd> h.get_strict_mode()
> 15
> nbd> h.pwrite(b'1'*1024, 0)
> nbd> exit
>
> So an unaligned 1K write works (after disabling libnbd's client-side
> alignment checks).

It does work when having the NBD client side simply ignore the request 
alignment (my diff [4]), but it doesn’t work (for me) when only reducing 
the compress filter’s request alignment (my diff [3]).

> > I just changed that line of code [2], as shown in [4].  I suppose
> > the better thing to do would be to have an option for the NBD server
> > to force-change the announced request alignment, because it can
> > expect the qemu block layer code to auto-align requests through
> > RMW.  Doing it in the client is wrong, because the NBD server might
> > want to detect that the client sends unaligned requests and reject
> > them (though ours doesn’t, it just traces such events[5] – note that
> > it’s explicitly noted there that qemu will auto-align requests).
> I know I said I didn't care about performance (in this case), but is
> there in fact a penalty to sending unaligned requests to the qcow2
> layer?  Or perhaps it cannot compress them?

In qcow2, only the whole cluster can be compressed, so writing 
compressed data means having to write the whole cluster.  qcow2 could 
implement the padding by itself, but we decided to just leave the burden 
of only writing full clusters (with the COMPRESSED write flag) on the 
callers.

Things like qemu-img convert and blockdev-backup just adhere to that by 
design; and the compress driver makes sure to set its request alignment 
accordingly so that requests to it will always be aligned to the cluster 
size (either by its user, or by the qemu block layer which performs the 
padding automatically).

Hanna