Re: [PATCH 4/4] block: introduce BDRV_MAX_LENGTH

[Vladimir Sementsov-Ogievskiy]

07.01.2021 12:58, Richard W.M. Jones wrote:
> On Fri, Dec 04, 2020 at 01:27:13AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > Finally to be safe with calculations, to not calculate different
> > maximums for different nodes (depending on cluster size and
> > request_alignment), let's simply set QEMU_ALIGN_DOWN(INT64_MAX, 2^30)
> > as absolute maximum bytes length for Qemu. Actually, it's not much less
> > than INT64_MAX.
>
> > +/*
> > + * We want allow aligning requests and disk length up to any 32bit alignment
> > + * and don't afraid of overflow.
> > + * To achieve it, and in the same time use some pretty number as maximum disk
> > + * size, let's define maximum "length" (a limit for any offset/bytes request 
> > and
> > + * for disk size) to be the greatest power of 2 less than INT64_MAX.
> > + */
> > +#define BDRV_MAX_ALIGNMENT (1L << 30)
> > +#define BDRV_MAX_LENGTH (QEMU_ALIGN_DOWN(INT64_MAX, BDRV_MAX_ALIGNMENT))
>
> This change broke nbdkit tests.
>
> We test that qemu can handle a qemu NBD export of size 2^63 - 512, the
> largest size that (experimentally) we found qemu could safely handle.
> eg:
>
>    
> https://github.com/libguestfs/nbdkit/blob/master/tests/test-memory-largest-for-qemu.sh
>
> Before this commit:
>
>    $ nbdkit memory $(( 2**63 - 512 )) --run './qemu-img info "$uri"'
>    image: nbd://localhost:10809
>    file format: raw
>    virtual size: 8 EiB (9223372036854775296 bytes)
>    disk size: unavailable
>
> After this commit:
>
>    $ nbdkit memory $(( 2**63 - 512 )) --run './qemu-img info "$uri"'
>    qemu-img: Could not open 'nbd://localhost:10809': Could not refresh total 
> sector count: File too large
>
> Can I confirm that this limit is now the new official one and we
> should adjust nbdkit tests?  Or was this change unintentional given
> that qemu seemed happy to handle 2^63 - 512 disks before?
>
> Note that nbdkit & libnbd support up to 2^63 - 1 bytes (we are not
> limited to whole sectors).  Also the Linux kernel will let you create
> a /dev/nbdX device of size 2^63 - 1.

Hi Rich! The change is intentional.

I think the benefit of having clean limit, allowing us to align up bytes to 
some alignment (which is a common operation) exceeds the loss of 1G at the end 
of 2**63 range. We get simpler and safer code. And anyway, new limit is not 
much worse than 2**63. If at some point we have a problem with it being too 
restrictive, it's than very likely that 2**63 would be too small as well, which 
will require so much work that our a bit more restrictive limit is unlikely to 
increase the difficulty.


--
Best regards,
Vladimir