Re: [PATCH 05/20] migration: rename 'pos' field in QEMUFile to 'bytes_processed'

[Daniel P . Berrangé]

On Thu, Jun 09, 2022 at 10:51:27AM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > This makes the field name align with the newly introduced method
> > names in the previous commit.
> 
> I think that's the method in the following commits?

Opps, yeah, I did re-arrange this series a few times to get the
most attractive diffs.

> tbh I'm not sure about this; 'pos' is still passed to writev_buffer
> and get_buffer to say where the data is - and that makes it a 'pos'
> still rather than a simple stats counter.

Note every QIOChannel backed impl of QEMUFile is ignoring the
'pos' field.

The only QEMUFile impl using 'pos' is the one during I/O is
the block device vmstate. A later patch is introducing a
QIOChannel impl for the vmstate, and to handle this it is
tracking a file offset itself internally to the QIOChannel
impl. So when we eliminate the QEMUFileOps callbacks later,
this 'pos' goes away.

So I guess my description here is a little ahead of itself.


> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  migration/qemu-file.c | 19 ++++++++++---------
> >  1 file changed, 10 insertions(+), 9 deletions(-)
> > 
> > diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> > index 03f0b13a55..b21da4c5bf 100644
> > --- a/migration/qemu-file.c
> > +++ b/migration/qemu-file.c
> > @@ -50,8 +50,9 @@ struct QEMUFile {
> >       */
> >      int64_t rate_limit_used;
> >  
> > -    int64_t pos; /* start of buffer when writing, end of buffer
> > -                    when reading */
> > +    /* The sum of bytes transferred on the wire */
> > +    int64_t total_transferred;
> > +
> >      int buf_index;
> >      int buf_size; /* 0 when writing */
> >      uint8_t buf[IO_BUF_SIZE];
> > @@ -241,14 +242,14 @@ void qemu_fflush(QEMUFile *f)
> >      }
> >      if (f->iovcnt > 0) {
> >          expect = iov_size(f->iov, f->iovcnt);
> > -        ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos,
> > +        ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, 
> > f->total_transferred,
> >                                      &local_error);
> >  
> >          qemu_iovec_release_ram(f);
> >      }
> >  
> >      if (ret >= 0) {
> > -        f->pos += ret;
> > +        f->total_transferred += ret;
> >      }
> >      /* We expect the QEMUFile write impl to send the full
> >       * data set we requested, so sanity check that.
> > @@ -357,11 +358,11 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
> >          return 0;
> >      }
> >  
> > -    len = f->ops->get_buffer(f->opaque, f->buf + pending, f->pos,
> > +    len = f->ops->get_buffer(f->opaque, f->buf + pending, 
> > f->total_transferred,
> >                               IO_BUF_SIZE - pending, &local_error);
> >      if (len > 0) {
> >          f->buf_size += len;
> > -        f->pos += len;
> > +        f->total_transferred += len;
> >      } else if (len == 0) {
> >          qemu_file_set_error_obj(f, -EIO, local_error);
> >      } else if (len != -EAGAIN) {
> > @@ -375,7 +376,7 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
> >  
> >  void qemu_update_position(QEMUFile *f, size_t size)
> >  {
> > -    f->pos += size;
> > +    f->total_transferred += size;
> >  }
> >  
> >  /** Closes the file
> > @@ -658,7 +659,7 @@ int qemu_get_byte(QEMUFile *f)
> >  
> >  int64_t qemu_ftell_fast(QEMUFile *f)
> >  {
> > -    int64_t ret = f->pos;
> > +    int64_t ret = f->total_transferred;
> >      int i;
> >  
> >      for (i = 0; i < f->iovcnt; i++) {
> > @@ -671,7 +672,7 @@ int64_t qemu_ftell_fast(QEMUFile *f)
> >  int64_t qemu_ftell(QEMUFile *f)
> >  {
> >      qemu_fflush(f);
> > -    return f->pos;
> > +    return f->total_transferred;
> >  }
> >  
> >  int qemu_file_rate_limit(QEMUFile *f)
> > -- 
> > 2.36.1
> > 
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 

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