Re: Block alignment of qcow2 compress driver

[Hanna Reitz <hreitz@redhat.com>]

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

>> Like, when I set the NBD
>> block driver’s alignment to 512[2], the following still succeeds:
> Did you just patch that line in the code or is there a qemu-nbd
> option/image-opts to do this?

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).

Hanna

> Rich.
>
>> [1] https://gitlab.com/qemu-project/qemu/-/blob/master/block/qcow2.c#L4662
>> [2] https://gitlab.com/qemu-project/qemu/-/blob/master/block/nbd.c#L1918

[3]
diff --git a/block/filter-compress.c b/block/filter-compress.c
index d5be538619..5a11d77231 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -114,7 +114,7 @@ static void compress_refresh_limits(BlockDriverState 
*bs, Error **errp)
          return;
      }

-    bs->bl.request_alignment = bdi.cluster_size;
+    bs->bl.request_alignment = 4096;
  }

[4]
diff --git a/block/nbd.c b/block/nbd.c
index 63dbfa807d..8608055800 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1915,7 +1915,7 @@ static void nbd_refresh_limits(BlockDriverState 
*bs, Error **errp)
                 s->info.base_allocation) ? 1 : BDRV_SECTOR_SIZE;
      }

-    bs->bl.request_alignment = min;
+    bs->bl.request_alignment = 512;
      bs->bl.max_pdiscard = QEMU_ALIGN_DOWN(INT_MAX, min);
      bs->bl.max_pwrite_zeroes = max;
      bs->bl.max_transfer = max;

[5] https://gitlab.com/qemu-project/qemu/-/blob/master/nbd/server.c#L2355