Re: [PATCH v2 10/29] io: Add generic pwritev/preadv interface

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

On Mon, Oct 23, 2023 at 05:35:49PM -0300, Fabiano Rosas wrote:
> From: Nikolay Borisov <nborisov@suse.com>
> 
> Introduce basic pwritev/preadv support in the generic channel layer.
> Specific implementation will follow for the file channel as this is
> required in order to support migration streams with fixed location of
> each ram page.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  include/io/channel.h | 82 ++++++++++++++++++++++++++++++++++++++++++++
>  io/channel.c         | 58 +++++++++++++++++++++++++++++++
>  2 files changed, 140 insertions(+)
> 
> diff --git a/include/io/channel.h b/include/io/channel.h
> index fcb19fd672..a8181d576a 100644
> --- a/include/io/channel.h
> +++ b/include/io/channel.h
> @@ -131,6 +131,16 @@ struct QIOChannelClass {
>                             Error **errp);
>  
>      /* Optional callbacks */
> +    ssize_t (*io_pwritev)(QIOChannel *ioc,
> +                          const struct iovec *iov,
> +                          size_t niov,
> +                          off_t offset,
> +                          Error **errp);
> +    ssize_t (*io_preadv)(QIOChannel *ioc,
> +                         const struct iovec *iov,
> +                         size_t niov,
> +                         off_t offset,
> +                         Error **errp);
>      int (*io_shutdown)(QIOChannel *ioc,
>                         QIOChannelShutdown how,
>                         Error **errp);
> @@ -529,6 +539,78 @@ void qio_channel_set_follow_coroutine_ctx(QIOChannel *ioc, bool enabled);
>  int qio_channel_close(QIOChannel *ioc,
>                        Error **errp);
>  
> +/**
> + * qio_channel_pwritev_full
> + * @ioc: the channel object
> + * @iov: the array of memory regions to write data from
> + * @niov: the length of the @iov array
> + * @offset: offset in the channel where writes should begin
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * 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.
> + *
> + * Behaves as qio_channel_writev_full, apart from not supporting
> + * sending of file handles as well as beginning the write at the
> + * passed @offset
> + *
> + */
> +ssize_t qio_channel_pwritev_full(QIOChannel *ioc, const struct iovec *iov,
> +                                 size_t niov, off_t offset, Error **errp);

In terms of naming this should be  just "_pwritev".

We don't support FD passing, so the "_full" suffix is not
appropriate

> +
> +/**
> + * qio_channel_pwritev
> + * @ioc: the channel object
> + * @buf: the memory region to write data into
> + * @buflen: the number of bytes to @buf
> + * @offset: offset in the channel where writes should begin
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * 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.
> + *
> + */
> +ssize_t qio_channel_pwritev(QIOChannel *ioc, char *buf, size_t buflen,
> +                            off_t offset, Error **errp);

This isn't passing a vector of buffers, so it should be just
"pwrite", not "pwritev".

> +
> +/**
> + * qio_channel_preadv_full
> + * @ioc: the channel object
> + * @iov: the array of memory regions to read data into
> + * @niov: the length of the @iov array
> + * @offset: offset in the channel where writes should begin
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * 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.
> + *
> + * Behaves as qio_channel_readv_full, apart from not supporting
> + * receiving of file handles as well as beginning the read at the
> + * passed @offset
> + *
> + */
> +ssize_t qio_channel_preadv_full(QIOChannel *ioc, const struct iovec *iov,
> +                                size_t niov, off_t offset, Error **errp);

"preadv"


> +
> +/**
> + * qio_channel_preadv
> + * @ioc: the channel object
> + * @buf: the memory region to write data into
> + * @buflen: the number of bytes to @buf
> + * @offset: offset in the channel where writes should begin
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * 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.
> + *
> + */
> +ssize_t qio_channel_preadv(QIOChannel *ioc, char *buf, size_t buflen,
> +                           off_t offset, Error **errp);

"pread"


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