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

[Fabiano Rosas]

Daniel P. Berrangé <berrange@redhat.com> writes:

> 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

I'll fix all instances.

Thanks