Re: [PATCH 1/2] migration: simplify error reporting after channel read

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Aug 04, 2025 at 03:48:59PM +0530, Prasad Pandit wrote:
> On Sat, 2 Aug 2025 at 00:06, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > The code handling the return value of qio_channel_read proceses
> > len == 0 (EOF) separately from len < 1  (error), but in both
> > cases ends up calling qemu_file_set_error_obj() with -EIO as the
> > errno. This logic can be merged into one codepath to simplify it.
> >
> >              } else {
> >                  qio_channel_wait(f->ioc, G_IO_IN);
> >              }
> > -        } else if (len < 0) {
> > -            len = -EIO;
> >          }
> >      } while (len == QIO_CHANNEL_ERR_BLOCK);
> >
> >      if (len > 0) {
> >          f->buf_size += len;
> > -    } else if (len == 0) {
> > -        qemu_file_set_error_obj(f, -EIO, local_error);
> >      } else {
> > -        qemu_file_set_error_obj(f, len, local_error);
> > +        qemu_file_set_error_obj(f, -EIO, local_error);
> >      }
> 
> * But should  _file_set_error_obj(...  -EIO) be called for len == 0
> (EOF) case? ie. function is trying to read from a file, at some point
> it is bound to reach EOF.  '-EIO' indicates an I/O error, reaching EOF
> could not be an error. Maybe we could just return zero(0) ?  (just
> checking)

The migration protocol knows whether it is expecting more data or not.

If we want more data, then a call to qemu_fill_buffer must successfully
read at least 1 byte.

If we don't want more data, then we would not have triggered any call
to qemu_fill_buffer.

Thus, a call to qemu_fill_buffer which gets EOF is an error scenario.

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