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

[Vladimir Sementsov-Ogievskiy]

07.01.2021 15:20, Richard W.M. Jones wrote:
> On Thu, Jan 07, 2021 at 10:56:12AM +0000, Richard W.M. Jones wrote:
> > On Thu, Jan 07, 2021 at 09:58:17AM +0000, 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.
> >
> > Actually that's not the only problem.  It appears that we're unable to
> > read or write the last sector of this disk:
> >
> > $ nbdkit memory $(( 2**63 - 2**30 )) --run 'build/qemu-io -r -f raw "$uri" -c "r -v 
> > $(( 2**63 - 2**30 - 512 )) 512" '
> > read failed: Input/output error
> >
> > $ nbdkit memory $(( 2**63 - 2**30 )) --run 'build/qemu-io -f raw "$uri" -c "w -P 3 
> > $(( 2**63 - 2**30 - 512 )) 512" '
> > write failed: Input/output error
> >
> > You can play around with the constants.  I found it's possible to read
> > and write the non-aligned 512 bytes starting at 2^63-2^30-513.  Could
> > be a fencepost error somewhere in qemu?
>
> Actually this is a pre-existing bug in qemu.
>
> What happens is qemu-io calls qemu_strtosz("9223372035781033472")
> which returns 0x7fffffffc0000000 and no error.  That answer is plain
> flat out wrong.  The reason for that is qemu_strtosz uses floating
> point for the calculation(!) so is limited to 53 bits of precision and
> silently truncates.

Hmm.. Seems it wants to support floats, like 1.5G.. Don't seem to be very 
useful, but why not. I think we should call qemu_strtou64 first, and only if it 
fails or leaves '.', 'e' or 'E' (anything else?) as end character we should do 
all this floating point logic.

> It happened we were just lucky that our earlier test with
> 2^63 - 1024 worked.
>
> Rich.


--
Best regards,
Vladimir