| Message ID | 20210831110238.299458-2-leobras@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | QIOChannel flags + multifd zerocopy | expand |
On Tue, Aug 31, 2021 at 08:02:37AM -0300, Leonardo Bras wrote: > Some syscalls used for writting, such as sendmsg(), accept flags that > can modify their behavior, even allowing the usage of features such as > MSG_ZEROCOPY. > > Change qio_channel_write*() interface to allow passing down flags, > allowing a more flexible use of IOChannel. > > At first, it's use is enabled for QIOChannelSocket, but can be easily > extended to any other QIOChannel implementation. As a followup to this patch, I wonder if we can also get performance improvements by implementing MSG_MORE, and using that in preference to corking/uncorking to better indicate that back-to-back short messages may behave better when grouped together over the wire. At least the NBD code could make use of it (going off of my experience with the libnbd project demonstrating a performance boost when we added MSG_MORE support there). > > Signed-off-by: Leonardo Bras <leobras@redhat.com> > --- > chardev/char-io.c | 2 +- > hw/remote/mpqemu-link.c | 2 +- > include/io/channel.h | 56 ++++++++++++++++++++--------- > io/channel-buffer.c | 1 + > io/channel-command.c | 1 + > io/channel-file.c | 1 + > io/channel-socket.c | 4 ++- > io/channel-tls.c | 1 + > io/channel-websock.c | 1 + > io/channel.c | 53 ++++++++++++++------------- > migration/rdma.c | 1 + > scsi/pr-manager-helper.c | 2 +- > tests/unit/test-io-channel-socket.c | 1 + > 13 files changed, 81 insertions(+), 45 deletions(-) > > diff --git a/chardev/char-io.c b/chardev/char-io.c > index 8ced184160..4ea7b1ee2a 100644 > --- a/chardev/char-io.c > +++ b/chardev/char-io.c > @@ -122,7 +122,7 @@ int io_channel_send_full(QIOChannel *ioc, > > ret = qio_channel_writev_full( > ioc, &iov, 1, > - fds, nfds, NULL); > + fds, 0, nfds, NULL); 0 before nfds here... > if (ret == QIO_CHANNEL_ERR_BLOCK) { > if (offset) { > return offset; > diff --git a/hw/remote/mpqemu-link.c b/hw/remote/mpqemu-link.c > index 7e841820e5..0d13321ef0 100644 > --- a/hw/remote/mpqemu-link.c > +++ b/hw/remote/mpqemu-link.c > @@ -69,7 +69,7 @@ bool mpqemu_msg_send(MPQemuMsg *msg, QIOChannel *ioc, Error **errp) > } > > if (!qio_channel_writev_full_all(ioc, send, G_N_ELEMENTS(send), > - fds, nfds, errp)) { > + fds, nfds, 0, errp)) { Thanks for fixing the broken indentation. ...but after nfds here, so one is wrong; up to this point in a linear review, I can't tell which was intended... > +++ b/include/io/channel.h > @@ -104,6 +104,7 @@ struct QIOChannelClass { > size_t niov, > int *fds, > size_t nfds, > + int flags, > Error **errp); ...and finally I see that in general, you wanted to add the argument after. Looks like the change to char-io.c is buggy. (You can use scripts/git.orderfile as a way to force your patch to list the .h file first, to make it easier for linear review). > ssize_t (*io_readv)(QIOChannel *ioc, > const struct iovec *iov, > @@ -260,6 +261,7 @@ ssize_t qio_channel_writev_full(QIOChannel *ioc, > size_t niov, > int *fds, > size_t nfds, > + int flags, > Error **errp); > > /** > @@ -325,6 +327,7 @@ int qio_channel_readv_all(QIOChannel *ioc, > * @ioc: the channel object > * @iov: the array of memory regions to write data from > * @niov: the length of the @iov array > + * @flags: optional sending flags > * @errp: pointer to a NULL-initialized error object > * > * Write data to the IO channel, reading it from the > @@ -339,10 +342,14 @@ int qio_channel_readv_all(QIOChannel *ioc, > * > * Returns: 0 if all bytes were written, or -1 on error > */ > -int qio_channel_writev_all(QIOChannel *ioc, > - const struct iovec *iov, > - size_t niov, > - Error **erp); > +int qio_channel_writev_all_flags(QIOChannel *ioc, > + const struct iovec *iov, > + size_t niov, > + int flags, > + Error **errp); You changed the function name here, but not in the comment beforehand. > + > +#define qio_channel_writev_all(ioc, iov, niov, errp) \ > + qio_channel_writev_all_flags(ioc, iov, niov, 0, errp) In most cases, you were merely adding a new function to minimize churn to existing callers while making the old name a macro,... > @@ -853,6 +876,7 @@ int qio_channel_writev_full_all(QIOChannel *ioc, > const struct iovec *iov, > size_t niov, > int *fds, size_t nfds, > + int flags, > Error **errp); ...but this one you changed in-place. Any reason? It might be nice to mention how you chose which functions to wrap (to minimize churn to existing clients) vs. modify signatures. > > #endif /* QIO_CHANNEL_H */ > diff --git a/io/channel-buffer.c b/io/channel-buffer.c > index baa4e2b089..bf52011be2 100644 > --- a/io/channel-buffer.c > +++ b/io/channel-buffer.c > @@ -81,6 +81,7 @@ static ssize_t qio_channel_buffer_writev(QIOChannel *ioc, > size_t niov, > int *fds, > size_t nfds, > + int flags, > Error **errp) > { > QIOChannelBuffer *bioc = QIO_CHANNEL_BUFFER(ioc); Would it be wise to check that flags only contains values we can honor in this (and all other) implementations of qio backends? Do we need some way for a backend to advertise to the core qio code which flags it is willing to accept? > +++ b/io/channel-socket.c > @@ -525,6 +525,7 @@ static ssize_t qio_channel_socket_writev(QIOChannel *ioc, > size_t niov, > int *fds, > size_t nfds, > + int flags, > Error **errp) > { > QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc); > @@ -558,7 +559,7 @@ static ssize_t qio_channel_socket_writev(QIOChannel *ioc, > } > > retry: > - ret = sendmsg(sioc->fd, &msg, 0); > + ret = sendmsg(sioc->fd, &msg, flags); Because of this line, we are forcing our use of flags to be exactly the same set of MSG_* flags understood by sendmsg(), which feels a bit fragile. Wouldn't it be safer to define our own set of QIO_MSG_ flags, and map those into whatever flag values the underlying backends can support? After all, one thing I learned on libnbd is that MSG_MORE is not universally portable, but the goal of qio is to abstract away things so that the rest of the code doesn't have to do #ifdef tests everywhere, but instead let the qio code deal with it (whether to ignore an unsupported flag because it is only an optimization hint, or to return an error because we depend on the behavior change the flag would cause if supported, or...). And that goes back to my question of whether backends should have a way to inform the qio core which flags they can support.
Thanks for the feedback Eric! On Wed, Sep 1, 2021 at 5:54 PM Eric Blake <eblake@redhat.com> wrote: > > On Tue, Aug 31, 2021 at 08:02:37AM -0300, Leonardo Bras wrote: > > Some syscalls used for writting, such as sendmsg(), accept flags that > > can modify their behavior, even allowing the usage of features such as > > MSG_ZEROCOPY. > > > > Change qio_channel_write*() interface to allow passing down flags, > > allowing a more flexible use of IOChannel. > > > > At first, it's use is enabled for QIOChannelSocket, but can be easily > > extended to any other QIOChannel implementation. > > As a followup to this patch, I wonder if we can also get performance > improvements by implementing MSG_MORE, and using that in preference to > corking/uncorking to better indicate that back-to-back short messages > may behave better when grouped together over the wire. At least the > NBD code could make use of it (going off of my experience with the > libnbd project demonstrating a performance boost when we added > MSG_MORE support there). That's interesting! We could use this patchset for testing that out, as I believe it's easy to add those flags to the sendmsg() we want to have it enabled. > > > > > Signed-off-by: Leonardo Bras <leobras@redhat.com> > > --- > > chardev/char-io.c | 2 +- > > hw/remote/mpqemu-link.c | 2 +- > > include/io/channel.h | 56 ++++++++++++++++++++--------- > > io/channel-buffer.c | 1 + > > io/channel-command.c | 1 + > > io/channel-file.c | 1 + > > io/channel-socket.c | 4 ++- > > io/channel-tls.c | 1 + > > io/channel-websock.c | 1 + > > io/channel.c | 53 ++++++++++++++------------- > > migration/rdma.c | 1 + > > scsi/pr-manager-helper.c | 2 +- > > tests/unit/test-io-channel-socket.c | 1 + > > 13 files changed, 81 insertions(+), 45 deletions(-) > > > > diff --git a/chardev/char-io.c b/chardev/char-io.c > > index 8ced184160..4ea7b1ee2a 100644 > > --- a/chardev/char-io.c > > +++ b/chardev/char-io.c > > @@ -122,7 +122,7 @@ int io_channel_send_full(QIOChannel *ioc, > > > > ret = qio_channel_writev_full( > > ioc, &iov, 1, > > - fds, nfds, NULL); > > + fds, 0, nfds, NULL); > > 0 before nfds here... Good catch! I will fix that for v2! > > > if (ret == QIO_CHANNEL_ERR_BLOCK) { > > if (offset) { > > return offset; > > diff --git a/hw/remote/mpqemu-link.c b/hw/remote/mpqemu-link.c > > index 7e841820e5..0d13321ef0 100644 > > --- a/hw/remote/mpqemu-link.c > > +++ b/hw/remote/mpqemu-link.c > > @@ -69,7 +69,7 @@ bool mpqemu_msg_send(MPQemuMsg *msg, QIOChannel *ioc, Error **errp) > > } > > > > if (!qio_channel_writev_full_all(ioc, send, G_N_ELEMENTS(send), > > - fds, nfds, errp)) { > > + fds, nfds, 0, errp)) { > > Thanks for fixing the broken indentation. I kept questioning myself if I was breaking something here :) > > ...but after nfds here, so one is wrong; up to this point in a linear > review, I can't tell which was intended... > > > +++ b/include/io/channel.h > > @@ -104,6 +104,7 @@ struct QIOChannelClass { > > size_t niov, > > int *fds, > > size_t nfds, > > + int flags, > > Error **errp); > > ...and finally I see that in general, you wanted to add the argument > after. Looks like the change to char-io.c is buggy. Yeap! > > (You can use scripts/git.orderfile as a way to force your patch to > list the .h file first, to make it easier for linear review). Nice tip! just added to my qemu gitconfig :) > > > ssize_t (*io_readv)(QIOChannel *ioc, > > const struct iovec *iov, > > @@ -260,6 +261,7 @@ ssize_t qio_channel_writev_full(QIOChannel *ioc, > > size_t niov, > > int *fds, > > size_t nfds, > > + int flags, > > Error **errp); > > > > /** > > @@ -325,6 +327,7 @@ int qio_channel_readv_all(QIOChannel *ioc, > > * @ioc: the channel object > > * @iov: the array of memory regions to write data from > > * @niov: the length of the @iov array > > + * @flags: optional sending flags > > * @errp: pointer to a NULL-initialized error object > > * > > * Write data to the IO channel, reading it from the > > @@ -339,10 +342,14 @@ int qio_channel_readv_all(QIOChannel *ioc, > > * > > * Returns: 0 if all bytes were written, or -1 on error > > */ > > -int qio_channel_writev_all(QIOChannel *ioc, > > - const struct iovec *iov, > > - size_t niov, > > - Error **erp); > > +int qio_channel_writev_all_flags(QIOChannel *ioc, > > + const struct iovec *iov, > > + size_t niov, > > + int flags, > > + Error **errp); > > You changed the function name here, but not in the comment beforehand. > Will fix this for v2, thanks ! > > + > > +#define qio_channel_writev_all(ioc, iov, niov, errp) \ > > + qio_channel_writev_all_flags(ioc, iov, niov, 0, errp) > > In most cases, you were merely adding a new function to minimize churn > to existing callers while making the old name a macro,... > > > @@ -853,6 +876,7 @@ int qio_channel_writev_full_all(QIOChannel *ioc, > > const struct iovec *iov, > > size_t niov, > > int *fds, size_t nfds, > > + int flags, > > Error **errp); > > ...but this one you changed in-place. Any reason? It might be nice > to mention how you chose which functions to wrap (to minimize churn to > existing clients) vs. modify signatures. It's the first one I did change, TBH. It just had a few uses. mostly in the same file scope, and a single use on mpqemu-link.c, so I thought it would be ok to just change it. But yeah, it makes sense to also add a macro to this one as well, and create another function to keep them all looking the same. > > > > > #endif /* QIO_CHANNEL_H */ > > diff --git a/io/channel-buffer.c b/io/channel-buffer.c > > index baa4e2b089..bf52011be2 100644 > > --- a/io/channel-buffer.c > > +++ b/io/channel-buffer.c > > @@ -81,6 +81,7 @@ static ssize_t qio_channel_buffer_writev(QIOChannel *ioc, > > size_t niov, > > int *fds, > > size_t nfds, > > + int flags, > > Error **errp) > > { > > QIOChannelBuffer *bioc = QIO_CHANNEL_BUFFER(ioc); > > Would it be wise to check that flags only contains values we can honor > in this (and all other) implementations of qio backends? Do we need > some way for a backend to advertise to the core qio code which flags > it is willing to accept? That's a good idea, maybe we can do as you suggest below, choose a set of features we are willing to support and then translate it depending on the implementation. Then this would only be testing for a mask. > > > +++ b/io/channel-socket.c > > @@ -525,6 +525,7 @@ static ssize_t qio_channel_socket_writev(QIOChannel *ioc, > > size_t niov, > > int *fds, > > size_t nfds, > > + int flags, > > Error **errp) > > { > > QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc); > > @@ -558,7 +559,7 @@ static ssize_t qio_channel_socket_writev(QIOChannel *ioc, > > } > > > > retry: > > - ret = sendmsg(sioc->fd, &msg, 0); > > + ret = sendmsg(sioc->fd, &msg, flags); > > Because of this line, we are forcing our use of flags to be exactly > the same set of MSG_* flags understood by sendmsg(), which feels a bit > fragile. Wouldn't it be safer to define our own set of QIO_MSG_ > flags, and map those into whatever flag values the underlying backends > can support? After all, one thing I learned on libnbd is that > MSG_MORE is not universally portable, but the goal of qio is to > abstract away things so that the rest of the code doesn't have to do > #ifdef tests everywhere, but instead let the qio code deal with it > (whether to ignore an unsupported flag because it is only an > optimization hint, or to return an error because we depend on the > behavior change the flag would cause if supported, or...). And that > goes back to my question of whether backends should have a way to > inform the qio core which flags they can support. I think you are correct and having our own QIO_MSG_* would make sense here. This could allow us to filter incorrect flags easily, and also have well documented what each implementation supports, by their own masks. Thanks! Leonardo Bras > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org >
diff --git a/chardev/char-io.c b/chardev/char-io.c index 8ced184160..4ea7b1ee2a 100644 --- a/chardev/char-io.c +++ b/chardev/char-io.c @@ -122,7 +122,7 @@ int io_channel_send_full(QIOChannel *ioc, ret = qio_channel_writev_full( ioc, &iov, 1, - fds, nfds, NULL); + fds, 0, nfds, NULL); if (ret == QIO_CHANNEL_ERR_BLOCK) { if (offset) { return offset; diff --git a/hw/remote/mpqemu-link.c b/hw/remote/mpqemu-link.c index 7e841820e5..0d13321ef0 100644 --- a/hw/remote/mpqemu-link.c +++ b/hw/remote/mpqemu-link.c @@ -69,7 +69,7 @@ bool mpqemu_msg_send(MPQemuMsg *msg, QIOChannel *ioc, Error **errp) } if (!qio_channel_writev_full_all(ioc, send, G_N_ELEMENTS(send), - fds, nfds, errp)) { + fds, nfds, 0, errp)) { ret = true; } else { trace_mpqemu_send_io_error(msg->cmd, msg->size, nfds); diff --git a/include/io/channel.h b/include/io/channel.h index 88988979f8..dada9ebaaf 100644 --- a/include/io/channel.h +++ b/include/io/channel.h @@ -104,6 +104,7 @@ struct QIOChannelClass { size_t niov, int *fds, size_t nfds, + int flags, Error **errp); ssize_t (*io_readv)(QIOChannel *ioc, const struct iovec *iov, @@ -260,6 +261,7 @@ ssize_t qio_channel_writev_full(QIOChannel *ioc, size_t niov, int *fds, size_t nfds, + int flags, Error **errp); /** @@ -325,6 +327,7 @@ int qio_channel_readv_all(QIOChannel *ioc, * @ioc: the channel object * @iov: the array of memory regions to write data from * @niov: the length of the @iov array + * @flags: optional sending flags * @errp: pointer to a NULL-initialized error object * * Write data to the IO channel, reading it from the @@ -339,10 +342,14 @@ int qio_channel_readv_all(QIOChannel *ioc, * * Returns: 0 if all bytes were written, or -1 on error */ -int qio_channel_writev_all(QIOChannel *ioc, - const struct iovec *iov, - size_t niov, - Error **erp); +int qio_channel_writev_all_flags(QIOChannel *ioc, + const struct iovec *iov, + size_t niov, + int flags, + Error **errp); + +#define qio_channel_writev_all(ioc, iov, niov, errp) \ + qio_channel_writev_all_flags(ioc, iov, niov, 0, errp) /** * qio_channel_readv: @@ -364,15 +371,21 @@ ssize_t qio_channel_readv(QIOChannel *ioc, * @ioc: the channel object * @iov: the array of memory regions to write data from * @niov: the length of the @iov array + * @flags: optional sending flags * @errp: pointer to a NULL-initialized error object * * Behaves as qio_channel_writev_full() but does not support * sending of file handles. */ -ssize_t qio_channel_writev(QIOChannel *ioc, - const struct iovec *iov, - size_t niov, - Error **errp); +ssize_t qio_channel_writev_flags(QIOChannel *ioc, + const struct iovec *iov, + size_t niov, + int flags, + Error **errp); + +#define qio_channel_writev(ioc, iov, niov, errp) \ + qio_channel_writev_flags(ioc, iov, niov, 0, errp) + /** * qio_channel_read: @@ -395,16 +408,21 @@ ssize_t qio_channel_read(QIOChannel *ioc, * @ioc: the channel object * @buf: the memory regions to send data from * @buflen: the length of @buf + * @flags: optional sending flags * @errp: pointer to a NULL-initialized error object * * Behaves as qio_channel_writev_full() but does not support * sending of file handles, and only supports writing from a * single memory region. */ -ssize_t qio_channel_write(QIOChannel *ioc, - const char *buf, - size_t buflen, - Error **errp); +ssize_t qio_channel_write_flags(QIOChannel *ioc, + const char *buf, + size_t buflen, + int flags, + Error **errp); + +#define qio_channel_write(ioc, buf, buflen, errp) \ + qio_channel_write_flags(ioc, buf, buflen, 0, errp) /** * qio_channel_read_all_eof: @@ -453,6 +471,7 @@ int qio_channel_read_all(QIOChannel *ioc, * @ioc: the channel object * @buf: the memory region to write data into * @buflen: the number of bytes to @buf + * @flags: optional sending flags * @errp: pointer to a NULL-initialized error object * * Writes @buflen bytes from @buf, possibly blocking or (if the @@ -462,10 +481,14 @@ int qio_channel_read_all(QIOChannel *ioc, * * Returns: 0 if all bytes were written, or -1 on error */ -int qio_channel_write_all(QIOChannel *ioc, - const char *buf, - size_t buflen, - Error **errp); +int qio_channel_write_all_flags(QIOChannel *ioc, + const char *buf, + size_t buflen, + int flags, + Error **errp); + +#define qio_channel_write_all(ioc, buf, buflen, errp) \ + qio_channel_write_all_flags(ioc, buf, buflen, 0, errp) /** * qio_channel_set_blocking: @@ -853,6 +876,7 @@ int qio_channel_writev_full_all(QIOChannel *ioc, const struct iovec *iov, size_t niov, int *fds, size_t nfds, + int flags, Error **errp); #endif /* QIO_CHANNEL_H */ diff --git a/io/channel-buffer.c b/io/channel-buffer.c index baa4e2b089..bf52011be2 100644 --- a/io/channel-buffer.c +++ b/io/channel-buffer.c @@ -81,6 +81,7 @@ static ssize_t qio_channel_buffer_writev(QIOChannel *ioc, size_t niov, int *fds, size_t nfds, + int flags, Error **errp) { QIOChannelBuffer *bioc = QIO_CHANNEL_BUFFER(ioc); diff --git a/io/channel-command.c b/io/channel-command.c index b2a9e27138..5ff1691bad 100644 --- a/io/channel-command.c +++ b/io/channel-command.c @@ -258,6 +258,7 @@ static ssize_t qio_channel_command_writev(QIOChannel *ioc, size_t niov, int *fds, size_t nfds, + int flags, Error **errp) { QIOChannelCommand *cioc = QIO_CHANNEL_COMMAND(ioc); diff --git a/io/channel-file.c b/io/channel-file.c index c4bf799a80..348a48545e 100644 --- a/io/channel-file.c +++ b/io/channel-file.c @@ -114,6 +114,7 @@ static ssize_t qio_channel_file_writev(QIOChannel *ioc, size_t niov, int *fds, size_t nfds, + int flags, Error **errp) { QIOChannelFile *fioc = QIO_CHANNEL_FILE(ioc); diff --git a/io/channel-socket.c b/io/channel-socket.c index 606ec97cf7..e377e7303d 100644 --- a/io/channel-socket.c +++ b/io/channel-socket.c @@ -525,6 +525,7 @@ static ssize_t qio_channel_socket_writev(QIOChannel *ioc, size_t niov, int *fds, size_t nfds, + int flags, Error **errp) { QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc); @@ -558,7 +559,7 @@ static ssize_t qio_channel_socket_writev(QIOChannel *ioc, } retry: - ret = sendmsg(sioc->fd, &msg, 0); + ret = sendmsg(sioc->fd, &msg, flags); if (ret <= 0) { if (errno == EAGAIN) { return QIO_CHANNEL_ERR_BLOCK; @@ -620,6 +621,7 @@ static ssize_t qio_channel_socket_writev(QIOChannel *ioc, size_t niov, int *fds, size_t nfds, + int flags, Error **errp) { QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc); diff --git a/io/channel-tls.c b/io/channel-tls.c index 2ae1b92fc0..4ce890a538 100644 --- a/io/channel-tls.c +++ b/io/channel-tls.c @@ -301,6 +301,7 @@ static ssize_t qio_channel_tls_writev(QIOChannel *ioc, size_t niov, int *fds, size_t nfds, + int flags, Error **errp) { QIOChannelTLS *tioc = QIO_CHANNEL_TLS(ioc); diff --git a/io/channel-websock.c b/io/channel-websock.c index 70889bb54d..035dd6075b 100644 --- a/io/channel-websock.c +++ b/io/channel-websock.c @@ -1127,6 +1127,7 @@ static ssize_t qio_channel_websock_writev(QIOChannel *ioc, size_t niov, int *fds, size_t nfds, + int flags, Error **errp) { QIOChannelWebsock *wioc = QIO_CHANNEL_WEBSOCK(ioc); diff --git a/io/channel.c b/io/channel.c index e8b019dc36..ee3cb83d4d 100644 --- a/io/channel.c +++ b/io/channel.c @@ -72,6 +72,7 @@ ssize_t qio_channel_writev_full(QIOChannel *ioc, size_t niov, int *fds, size_t nfds, + int flags, Error **errp) { QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc); @@ -83,7 +84,7 @@ ssize_t qio_channel_writev_full(QIOChannel *ioc, return -1; } - return klass->io_writev(ioc, iov, niov, fds, nfds, errp); + return klass->io_writev(ioc, iov, niov, fds, nfds, flags, errp); } @@ -212,18 +213,20 @@ int qio_channel_readv_full_all(QIOChannel *ioc, return ret; } -int qio_channel_writev_all(QIOChannel *ioc, - const struct iovec *iov, - size_t niov, - Error **errp) +int qio_channel_writev_all_flags(QIOChannel *ioc, + const struct iovec *iov, + size_t niov, + int flags, + Error **errp) { - return qio_channel_writev_full_all(ioc, iov, niov, NULL, 0, errp); + return qio_channel_writev_full_all(ioc, iov, niov, NULL, 0, flags, errp); } int qio_channel_writev_full_all(QIOChannel *ioc, const struct iovec *iov, size_t niov, int *fds, size_t nfds, + int flags, Error **errp) { int ret = -1; @@ -238,7 +241,7 @@ 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); + flags, errp); if (len == QIO_CHANNEL_ERR_BLOCK) { if (qemu_in_coroutine()) { qio_channel_yield(ioc, G_IO_OUT); @@ -272,15 +275,15 @@ ssize_t qio_channel_readv(QIOChannel *ioc, } -ssize_t qio_channel_writev(QIOChannel *ioc, - const struct iovec *iov, - size_t niov, - Error **errp) +ssize_t qio_channel_writev_flags(QIOChannel *ioc, + const struct iovec *iov, + size_t niov, + int flags, + Error **errp) { - return qio_channel_writev_full(ioc, iov, niov, NULL, 0, errp); + return qio_channel_writev_full(ioc, iov, niov, NULL, 0, flags, errp); } - ssize_t qio_channel_read(QIOChannel *ioc, char *buf, size_t buflen, @@ -291,16 +294,16 @@ ssize_t qio_channel_read(QIOChannel *ioc, } -ssize_t qio_channel_write(QIOChannel *ioc, - const char *buf, - size_t buflen, - Error **errp) +ssize_t qio_channel_write_flags(QIOChannel *ioc, + const char *buf, + size_t buflen, + int flags, + Error **errp) { struct iovec iov = { .iov_base = (char *)buf, .iov_len = buflen }; - return qio_channel_writev_full(ioc, &iov, 1, NULL, 0, errp); + return qio_channel_writev_full(ioc, &iov, 1, NULL, 0, flags, errp); } - int qio_channel_read_all_eof(QIOChannel *ioc, char *buf, size_t buflen, @@ -321,16 +324,16 @@ int qio_channel_read_all(QIOChannel *ioc, } -int qio_channel_write_all(QIOChannel *ioc, - const char *buf, - size_t buflen, - Error **errp) +int qio_channel_write_all_flags(QIOChannel *ioc, + const char *buf, + size_t buflen, + int flags, + Error **errp) { struct iovec iov = { .iov_base = (char *)buf, .iov_len = buflen }; - return qio_channel_writev_all(ioc, &iov, 1, errp); + return qio_channel_writev_all_flags(ioc, &iov, 1, flags, errp); } - int qio_channel_set_blocking(QIOChannel *ioc, bool enabled, Error **errp) diff --git a/migration/rdma.c b/migration/rdma.c index 5c2d113aa9..bc0558b70e 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -2713,6 +2713,7 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc, size_t niov, int *fds, size_t nfds, + int flags, Error **errp) { QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc); diff --git a/scsi/pr-manager-helper.c b/scsi/pr-manager-helper.c index 451c7631b7..3be52a98d5 100644 --- a/scsi/pr-manager-helper.c +++ b/scsi/pr-manager-helper.c @@ -77,7 +77,7 @@ static int pr_manager_helper_write(PRManagerHelper *pr_mgr, iov.iov_base = (void *)buf; iov.iov_len = sz; n_written = qio_channel_writev_full(QIO_CHANNEL(pr_mgr->ioc), &iov, 1, - nfds ? &fd : NULL, nfds, errp); + nfds ? &fd : NULL, nfds, 0, errp); if (n_written <= 0) { assert(n_written != QIO_CHANNEL_ERR_BLOCK); diff --git a/tests/unit/test-io-channel-socket.c b/tests/unit/test-io-channel-socket.c index c49eec1f03..6713886d02 100644 --- a/tests/unit/test-io-channel-socket.c +++ b/tests/unit/test-io-channel-socket.c @@ -444,6 +444,7 @@ static void test_io_channel_unix_fd_pass(void) G_N_ELEMENTS(iosend), fdsend, G_N_ELEMENTS(fdsend), + 0, &error_abort); qio_channel_readv_full(dst,
Some syscalls used for writting, such as sendmsg(), accept flags that can modify their behavior, even allowing the usage of features such as MSG_ZEROCOPY. Change qio_channel_write*() interface to allow passing down flags, allowing a more flexible use of IOChannel. At first, it's use is enabled for QIOChannelSocket, but can be easily extended to any other QIOChannel implementation. Signed-off-by: Leonardo Bras <leobras@redhat.com> --- chardev/char-io.c | 2 +- hw/remote/mpqemu-link.c | 2 +- include/io/channel.h | 56 ++++++++++++++++++++--------- io/channel-buffer.c | 1 + io/channel-command.c | 1 + io/channel-file.c | 1 + io/channel-socket.c | 4 ++- io/channel-tls.c | 1 + io/channel-websock.c | 1 + io/channel.c | 53 ++++++++++++++------------- migration/rdma.c | 1 + scsi/pr-manager-helper.c | 2 +- tests/unit/test-io-channel-socket.c | 1 + 13 files changed, 81 insertions(+), 45 deletions(-)