Re: [PATCH v4 1/3] QIOChannel: Add io_writev_zerocopy & io_flush_zerocopy callbacks

[Peter Xu]

On Sat, Oct 09, 2021 at 04:56:11AM -0300, Leonardo Bras wrote:
> -int qio_channel_writev_full_all(QIOChannel *ioc,
> -                                const struct iovec *iov,
> -                                size_t niov,
> -                                int *fds, size_t nfds,
> -                                Error **errp)
> +int qio_channel_writev_full_all_flags(QIOChannel *ioc,
> +                                      const struct iovec *iov,
> +                                      size_t niov,
> +                                      int *fds, size_t nfds,
> +                                      int flags, Error **errp)
>  {
>      int ret = -1;
>      struct iovec *local_iov = g_new(struct iovec, niov);
> @@ -237,8 +250,8 @@ int qio_channel_writev_full_all(QIOChannel *ioc,
>  
>      while (nlocal_iov > 0) {
>          ssize_t len;
> -        len = qio_channel_writev_full(ioc, local_iov, nlocal_iov, fds, nfds,
> -                                      errp);
> +        len = qio_channel_writev_full_flags(ioc, local_iov, nlocal_iov, fds, 
> nfds,
> +                                          flags, errp);

IMHO we can keep qio_channel_writev_full() untouched, as it is the wrapper for
io_writev() hook right now (and it allows taking fds).  Then here instead of
adding a new flags into it, we can introduce qio_channel_writev_zerocopy_full()
and directly call here:

           if (flags & ZEROCOPY) {
               assert(fds == NULL && nfds == 0);
               qio_channel_writev_zerocopy_full(...[no fds passing in]);
           } else {
               qio_channel_writev_full(...[with all parameters]);
           }

Then qio_channel_writev_zerocopy_full() should be simplely the wrapper for the
new io_writev_zerocopy() hook.

>          if (len == QIO_CHANNEL_ERR_BLOCK) {
>              if (qemu_in_coroutine()) {
>                  qio_channel_yield(ioc, G_IO_OUT);
> @@ -474,6 +487,31 @@ off_t qio_channel_io_seek(QIOChannel *ioc,
>  }
>  
>  
> +ssize_t qio_channel_writev_zerocopy(QIOChannel *ioc,
> +                                    const struct iovec *iov,
> +                                    size_t niov,
> +                                    Error **errp)
> +{
> +    return qio_channel_writev_full_flags(ioc, iov, niov, NULL, 0,
> +                                         QIO_CHANNEL_WRITE_FLAG_ZEROCOPY,
> +                                         errp);
> +}

This function is never used, right? qio_channel_writev_all_flags() is used in
patch 3, with proper flags passed in.  Then IMHO we can drop this one.

The rest looks good, as long as with Eric's comment addressed.

Thanks,

> +
> +
> +int qio_channel_flush_zerocopy(QIOChannel *ioc,
> +                               Error **errp)
> +{
> +    QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
> +
> +    if (!klass->io_flush_zerocopy ||
> +        !qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_WRITE_ZEROCOPY)) {
> +        return 0;
> +    }
> +
> +    return klass->io_flush_zerocopy(ioc, errp);
> +}
> +
> +
>  static void qio_channel_restart_read(void *opaque)
>  {
>      QIOChannel *ioc = opaque;
> -- 
> 2.33.0
> 

-- 
Peter Xu