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

[Daniel P . Berrangé]

On Fri, Jan 08, 2021 at 02:14:30PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 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.

I'm not sure we actually care about the floating point exponent
syntax suffixes. The test suite doesn't cover that format, only
the plain fractional parts.

It should be possible to avoid floating point entirely if we just
split on "." and then use int64 arithmetic to scale the two parts
before recombining them. 


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|