Re: [PATCH v3 11/17] migration/qemu-file: Fix qemu_ftell() for non-writable file

[Daniel P . Berrangé]

On Thu, Jun 16, 2022 at 01:28:05PM +0300, nikita.lapshin@openvz.org wrote:
> From: Nikita Lapshin <nikita.lapshin@openvz.org>
> 
> qemu_ftell() will return wrong value for non-writable QEMUFile.
> This happens due to call qemu_fflush() inside qemu_ftell(), this
> function won't flush if file is readable.

Well the return value isn't necessarily wrong today - it really
depends what semantics each callers desires.

Can you say what particular caller needs these semantics changed
and the impact on them from current behaviour ?

> Signed-off-by: Nikita Lapshin <nikita.lapshin@openvz.org>
> ---
>  migration/qemu-file.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 1479cddad9..53ccef80ac 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -663,7 +663,8 @@ int64_t qemu_ftell_fast(QEMUFile *f)
>  int64_t qemu_ftell(QEMUFile *f)
>  {
>      qemu_fflush(f);
> -    return f->pos;
> +    /* Consider that qemu_fflush() won't work if file is non-writable */
> +    return f->pos + f->buf_index;
>  }

IIUC, this is more or less trying to make 'qemu_ftell' be
equivalent to 'qemu_ftell_fast' semantics in the non-writable
case. But that makes me wonder if whichever calls has problems,
shouldn't be just changed to use  qemu_ftell_fast instead ?


With 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 :|