Re: [pbs-devel] [PATCH proxmox-backup] mapped loop device: use read loop instead of read_exact

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Thu, Jun 29, 2023 at 12:32:13PM +0200, Fabian Grünbichler wrote:
> since read_exact does not support short reads, which can easily happen if the
> mapped image's EOF is not aligned with the request size.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>     reported on the forum:
>     
>     https://forum.proxmox.com/threads/problem-backing-up-using-backup-client.129347
>     
>     did a quick test reading from a mapped image full of random data, observed
>     no performance difference..

Do you get one if we just drop the loop logic and *actually* just
`read()` once? IMO this is more in line with what a read syscall
*should* be doing.
Further, we use a `CachedChunkReader` under it which actually does a
read loop anyway, so AFAICT this *can't* make a difference.

> 
>  pbs-fuse-loop/src/fuse_loop.rs | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/pbs-fuse-loop/src/fuse_loop.rs b/pbs-fuse-loop/src/fuse_loop.rs
> index 3d0ef123..7e780799 100644
> --- a/pbs-fuse-loop/src/fuse_loop.rs
> +++ b/pbs-fuse-loop/src/fuse_loop.rs
> @@ -188,13 +188,20 @@ impl<R: AsyncRead + AsyncSeek + Unpin> FuseLoopSession<R> {
>                              match self.reader.seek(SeekFrom::Start(req.offset)).await {
>                                  Ok(_) => {
>                                      let mut buf = vec![0u8; req.size];
> -                                    match self.reader.read_exact(&mut buf).await {
> -                                        Ok(_) => {
> -                                            req.reply(&buf)
> -                                        },
> -                                        Err(e) => {
> -                                            req.io_fail(e)
> +                                    let mut read = 0;
> +                                    let mut res = Ok(());
> +                                    while read < req.size && res.is_ok() {
> +                                        match self.reader.read(&mut buf).await {
> +                                            Ok(0) => { break; },
> +                                            Ok(n) => { read += n; },
> +                                            Err(e) => { res = Err(e); },
>                                          }
> +                                    };
> +                                    if let Err(e) = res {
> +                                        req.io_fail(e)
> +                                    } else {
> +                                        buf.truncate(read);
> +                                        req.reply(&buf)
>                                      }
>                                  },
>                                  Err(e) => {
> -- 
> 2.39.2