Re: [RFC PATCH] curl: Allow reading after EOF

[Kevin Wolf <kwolf@redhat.com>]

Am 17.03.2021 um 17:12 hat Daniel P. Berrangé geschrieben:
> On Wed, Mar 17, 2021 at 04:17:34PM +0100, Kevin Wolf wrote:
> > This makes the curl driver more consistent with file-posix in that it
> > doesn't return errors any more for reading after the end of the remote
> > file. Instead, zeros are returned for these areas.
> > 
> > This inconsistency was reported in:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1935061
> > 
> > Note that the image used in this bug report has a corrupted snapshot
> > table, which means that the qcow2 driver tries to do a zero-length read
> > after EOF on its image file.
> > 
> > The old behaviour of the curl driver can hardly be called a bug, but the
> > inconsistency turned out to be confusing.
> > 
> > Reported-by: Alice Frosi <afrosi@redhat.com>
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> > 
> > It is not entirely clear to me if this is something we want to do. If we
> > do care about consistency between protocol drivers, something like this
> > should probably be done in block/io.c eventually - but that would
> > require converting bs->total_sectors to byte granularity first.
> > 
> > Any opinions on what the most desirable semantics would be and whether
> > we should patch individual drivers until we can have a generic solution?
> 
> What valid scenarios are there for wanting to read beyond the bounds
> of the protocol driver storage ? Why was file-posix allowing this
> so far ?
> 
> If I've given file-posix a 10 GB plain file or device and something
> requests a read from the 11 GB offset, IMHO, that is a sign of serious
> error somewhere and possible impending doom.
> 
> For writable storage, I would think that read + write should be
> symmetric, by which I mean if a read() at a particular offset
> succeeds, then I would also expect a write() at the same offset to
> succeed, and have its data later returned by a read().
> 
> We generally can't write at an offset beyond the storage (unless we
> are intending to auto-enlarge a plain file), so I think we shouldn't
> allow reads either.

It is definitely related to format drivers that grow their image files.
I think the reason for allowing this may have been that with O_DIRECT,
you need aligned requests and when format drivers write just a few
bytes, we actually do a RMW - and you don't want to get an error during
the read part just because the image file will only be resized by the
write.

Since curl is a read-only protocol driver (at the moment, I actually
have an experimental branch that adds write support so we can run
iotests for http), this reason doesn't really apply. At the moment, it
would be just for consistency.

Kevin