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

[Nikita]

On 6/16/22 2:20 PM, Daniel P. Berrangé wrote:
> 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 ?

Sorry, it's my bad. It was used in previous version. But after 
refactoring I threw this function out of implementation.

So it is in fact not used now.

> > 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 ?

qemu_ftell_fast() counts iovec length. When we try to read from QEMUFile 
using for exmaple qemu_peek_buffer f->buf will be filled with data so we 
need to consider buf_index.

> With regards,
> Daniel
Thank you for your review!