[PATCH v3 04/14] io: Add generic pwritev/preadv interface

Nikolay Borisov posted 14 patches 3 years, 3 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Daniel P. Berrangé" <berrange@redhat.com>, Juan Quintela <quintela@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>
[PATCH v3 04/14] io: Add generic pwritev/preadv interface
Posted by Nikolay Borisov 3 years, 3 months ago
Introduce basic pwriteve/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>
---
 include/io/channel.h | 49 ++++++++++++++++++++++++++++++++++++++++++++
 io/channel.c         | 26 +++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/include/io/channel.h b/include/io/channel.h
index c680ee748021..6b10bce8bbdf 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -124,6 +124,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);
@@ -504,6 +514,45 @@ int qio_channel_set_blocking(QIOChannel *ioc,
 int qio_channel_close(QIOChannel *ioc,
                       Error **errp);
 
+
+/**
+ * qio_channel_io_pwritev
+ * @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_io_pwritev(QIOChannel *ioc, const struct iovec *iov,
+                               size_t niov, off_t offset, Error **errp);
+
+
+/**
+ * qio_channel_io_preadv
+ * @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_io_preadv(QIOChannel *ioc, const struct iovec *iov,
+                             size_t niov, off_t offset, Error **errp);
 /**
  * qio_channel_shutdown:
  * @ioc: the channel object
diff --git a/io/channel.c b/io/channel.c
index 0640941ac573..f5ac9499a7ad 100644
--- a/io/channel.c
+++ b/io/channel.c
@@ -437,6 +437,32 @@ GSource *qio_channel_add_watch_source(QIOChannel *ioc,
 }
 
 
+ssize_t qio_channel_io_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;
+    }
+
+    return klass->io_pwritev(ioc, iov, niov, offset, errp);
+}
+
+ssize_t qio_channel_io_preadv(QIOChannel *ioc, const struct iovec *iov,
+                               size_t niov, off_t offset, Error **errp)
+{
+    QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
+
+    if (!klass->io_preadv) {
+        error_setg(errp, "Channel does not support preadv");
+        return -1;
+    }
+
+    return klass->io_preadv(ioc, iov, niov, offset, errp);
+}
+
 int qio_channel_shutdown(QIOChannel *ioc,
                          QIOChannelShutdown how,
                          Error **errp)
-- 
2.34.1
Re: [PATCH v3 04/14] io: Add generic pwritev/preadv interface
Posted by Daniel P. Berrangé 2 years, 12 months ago
On Fri, Oct 28, 2022 at 01:39:04PM +0300, Nikolay Borisov wrote:
> Introduce basic pwriteve/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>
> ---
>  include/io/channel.h | 49 ++++++++++++++++++++++++++++++++++++++++++++
>  io/channel.c         | 26 +++++++++++++++++++++++
>  2 files changed, 75 insertions(+)
> 
> diff --git a/include/io/channel.h b/include/io/channel.h
> index c680ee748021..6b10bce8bbdf 100644
> --- a/include/io/channel.h
> +++ b/include/io/channel.h
> @@ -124,6 +124,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);

nit-pick the alignment of the 2nd line of parameters onwards,
needs to be indented by 3 more spaces.


> +/**
> + * qio_channel_io_pwritev
> + * @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

Given this, we should have  "_full" suffix on these methods
too for consistent naming policy.

> + * handles as well as beginning the write at the passed @offset
> + *
> + */
> +ssize_t qio_channel_io_pwritev(QIOChannel *ioc, const struct iovec *iov,
> +                               size_t niov, off_t offset, Error **errp);
> +
> +
> +/**
> + * qio_channel_io_preadv
> + * @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_io_preadv(QIOChannel *ioc, const struct iovec *iov,
> +                             size_t niov, off_t offset, Error **errp);
>  /**
>   * qio_channel_shutdown:
>   * @ioc: the channel object
> diff --git a/io/channel.c b/io/channel.c
> index 0640941ac573..f5ac9499a7ad 100644
> --- a/io/channel.c
> +++ b/io/channel.c
> @@ -437,6 +437,32 @@ GSource *qio_channel_add_watch_source(QIOChannel *ioc,
>  }
>  
>  
> +ssize_t qio_channel_io_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;
> +    }

This also possibly benefits from a validation check

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

because the QIOChannelFile impl will support io_pwritev callback,
but not all the FDs it can use will support seeking.

Same check for preadv

> +
> +    return klass->io_pwritev(ioc, iov, niov, offset, errp);
> +}
> +
> +ssize_t qio_channel_io_preadv(QIOChannel *ioc, const struct iovec *iov,
> +                               size_t niov, off_t offset, Error **errp)
> +{
> +    QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
> +
> +    if (!klass->io_preadv) {
> +        error_setg(errp, "Channel does not support preadv");
> +        return -1;
> +    }
> +
> +    return klass->io_preadv(ioc, iov, niov, offset, errp);
> +}
> +
>  int qio_channel_shutdown(QIOChannel *ioc,
>                           QIOChannelShutdown how,
>                           Error **errp)
> -- 
> 2.34.1
> 

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 :|
Re: [PATCH v3 04/14] io: Add generic pwritev/preadv interface
Posted by Daniel P. Berrangé 2 years, 12 months ago
On Fri, Feb 10, 2023 at 04:26:22PM +0000, Daniel P. Berrangé wrote:
> On Fri, Oct 28, 2022 at 01:39:04PM +0300, Nikolay Borisov wrote:
> > Introduce basic pwriteve/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>
> > ---
> >  include/io/channel.h | 49 ++++++++++++++++++++++++++++++++++++++++++++
> >  io/channel.c         | 26 +++++++++++++++++++++++
> >  2 files changed, 75 insertions(+)
> > 
> > diff --git a/include/io/channel.h b/include/io/channel.h
> > index c680ee748021..6b10bce8bbdf 100644
> > --- a/include/io/channel.h
> > +++ b/include/io/channel.h
> > @@ -124,6 +124,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);
> 
> nit-pick the alignment of the 2nd line of parameters onwards,
> needs to be indented by 3 more spaces.
> 
> 
> > +/**
> > + * qio_channel_io_pwritev
> > + * @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
> 
> Given this, we should have  "_full" suffix on these methods
> too for consistent naming policy.
> 
> > + * handles as well as beginning the write at the passed @offset
> > + *
> > + */
> > +ssize_t qio_channel_io_pwritev(QIOChannel *ioc, const struct iovec *iov,
> > +                               size_t niov, off_t offset, Error **errp);

In addition to giving this method the '_full' suffix, we should also
add the qio_channel_io_pwritev entrypoint which takes a single
buffer instead of iovec. The migration code in QEMUFile that is
added in a later patch in this series doesn't actually want to
use iovecs in the first place. So having the non-iov entrypoints
in QIOChannel will simplify the migration code.

> > +
> > +
> > +/**
> > + * qio_channel_io_preadv
> > + * @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_io_preadv(QIOChannel *ioc, const struct iovec *iov,
> > +                             size_t niov, off_t offset, Error **errp);
> >  /**
> >   * qio_channel_shutdown:
> >   * @ioc: the channel object
> > diff --git a/io/channel.c b/io/channel.c
> > index 0640941ac573..f5ac9499a7ad 100644
> > --- a/io/channel.c
> > +++ b/io/channel.c
> > @@ -437,6 +437,32 @@ GSource *qio_channel_add_watch_source(QIOChannel *ioc,
> >  }
> >  
> >  
> > +ssize_t qio_channel_io_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;
> > +    }
> 
> This also possibly benefits from a validation check
> 
>     if (!qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_SEEKABLE)) {
>         error_setg_errno(errp, EINVAL,
>                          "Requested channel is not seekable")
>         return -1;
>     }
> 
> because the QIOChannelFile impl will support io_pwritev callback,
> but not all the FDs it can use will support seeking.
> 
> Same check for preadv
> 
> > +
> > +    return klass->io_pwritev(ioc, iov, niov, offset, errp);
> > +}
> > +
> > +ssize_t qio_channel_io_preadv(QIOChannel *ioc, const struct iovec *iov,
> > +                               size_t niov, off_t offset, Error **errp)
> > +{
> > +    QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
> > +
> > +    if (!klass->io_preadv) {
> > +        error_setg(errp, "Channel does not support preadv");
> > +        return -1;
> > +    }
> > +
> > +    return klass->io_preadv(ioc, iov, niov, offset, errp);
> > +}
> > +
> >  int qio_channel_shutdown(QIOChannel *ioc,
> >                           QIOChannelShutdown how,
> >                           Error **errp)
> > -- 
> > 2.34.1
> > 
> 
> 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 :|
> 
> 

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