Re: [PATCH] migration: add FEATURE_SEEKABLE to QIOChannelBlock

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

On Tue, Apr 15, 2025 at 04:14:08PM +0530, Prasad Pandit wrote:
> Hi,
> 
> On Tue, 15 Apr 2025 at 15:51, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > It is actually NOT really connected to lseek, and as such
> 
> * For file and fd channels _FEATURE_SEEKABLE is set  when/if  lseek(2) works.
>     -> https://gitlab.com/qemu-project/qemu/-/commit/401e311ff72e0a62c834bfe466de68a82cfd90cb

Yes I know, checking lseek is a proxy that determines whether
preadv will work or not. This is basically validating that
the file/fd is not a FIFO/pipe/character device. It must be
a block device or regular file.

> > QIO_CHANNEL_FEATURE_SEEKABLE is probably a bad choice of name
> > in retrospect.
> 
> * That's plausible. Because while reading code,  _SEEKABLE indicates
> (or hints) that the underlying stream allows random access at a given
> offset. Even pread(2)/pwrite(2) manuals say that -> file referenced by
> fd should be capable of seeking. So correlation is that, since
> QIO_CHANNEL is an abstraction layer, it supports different
> streams/channels underneath, maybe some of those underneath streams
> support seeking (random access) and some don't. Hence we set
> _FEATURE_SEEKABLE when the underlying channel/stream supports it.
> 
> > In QIOChanel API specification, having QIO_CHANNEL_FEATURE_SEEKABLE
> > set is a pre-requisite for use of qio_channel_{pread,preadv,pwrite,pwritev}.
> 
> * If *_FEATURE_SEEKABLE is not connected to lseek(2) or seeking, what
> is its right interpretation? Why is it a pre-requisite for use of
> qio_channel_{pread,preadv,pwrite,pwritev} functions?

Because that's what the QEMU API specification declares

 * Not all implementations will support this facility, so may report
 * an error. To avoid errors, the caller may check for the feature
 * flag QIO_CHANNEL_FEATURE_SEEKABLE prior to calling this method.

and what the QEMU API impl defines

  ssize_t qio_channel_pwritev(QIOChannel *ioc, const struct iovec *iov,
                              size_t niov, off_t offset, Error **errp)
  {
      QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);

      if (!klass->io_pwritev) {
          error_setg(errp, "Channel does not support pwritev");
          return -1;
      }

      if (!qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_SEEKABLE)) {
          error_setg_errno(errp, EINVAL, "Requested channel is not seekable");
          return -1;
      }

      return klass->io_pwritev(ioc, iov, niov, offset, errp);
  }


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