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

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> 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.

Ah! Put that description in the commit message and:


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> 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 :|
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK