On Tue, Aug 08, 2017 at 10:40:08AM +0200, Juan Quintela wrote: > "Daniel P. Berrange" <berra...@redhat.com> wrote: > > On Mon, Jul 17, 2017 at 03:42:24PM +0200, Juan Quintela wrote: > >> The functions waits until it is able to write the full iov. > >> > >> Signed-off-by: Juan Quintela <quint...@redhat.com> > >> > >> -- > >> > >> Add tests. > >> --- > >> include/io/channel.h | 46 +++++++++++++++++++++++++ > >> io/channel.c | 76 > >> ++++++++++++++++++++++++++++++++++++++++++ > >> migration/qemu-file-channel.c | 29 +--------------- > >> tests/io-channel-helpers.c | 55 ++++++++++++++++++++++++++++++ > >> tests/io-channel-helpers.h | 4 +++ > >> tests/test-io-channel-buffer.c | 55 ++++++++++++++++++++++++++++-- > >> 6 files changed, 234 insertions(+), 31 deletions(-) > > > > > > > >> diff --git a/io/channel.c b/io/channel.c > >> index cdf7454..82203ef 100644 > >> --- a/io/channel.c > >> +++ b/io/channel.c > >> @@ -22,6 +22,7 @@ > >> #include "io/channel.h" > >> #include "qapi/error.h" > >> #include "qemu/main-loop.h" > >> +#include "qemu/iov.h" > >> > >> bool qio_channel_has_feature(QIOChannel *ioc, > >> QIOChannelFeature feature) > >> @@ -85,6 +86,81 @@ ssize_t qio_channel_writev_full(QIOChannel *ioc, > >> } > >> > >> > >> + > >> +ssize_t qio_channel_readv_all(QIOChannel *ioc, > >> + const struct iovec *iov, > >> + size_t niov, > >> + Error **errp) > >> +{ > >> + ssize_t done = 0; > >> + struct iovec *local_iov = g_new(struct iovec, niov); > >> + struct iovec *local_iov_head = local_iov; > >> + unsigned int nlocal_iov = niov; > >> + > >> + nlocal_iov = iov_copy(local_iov, nlocal_iov, > >> + iov, niov, > >> + 0, iov_size(iov, niov)); > >> + > >> + while (nlocal_iov > 0) { > >> + ssize_t len; > >> + len = qio_channel_readv(ioc, local_iov, nlocal_iov, errp); > >> + if (len == QIO_CHANNEL_ERR_BLOCK) { > >> + qio_channel_wait(ioc, G_IO_OUT); > >> + continue; > >> + } > >> + if (len < 0) { > >> + error_setg_errno(errp, EIO, > >> + "Channel was not able to read full iov"); > >> + done = -1; > >> + goto cleanup; > >> + } > >> + > >> + iov_discard_front(&local_iov, &nlocal_iov, len); > >> + done += len; > >> + } > > > > If 'len == 0' (ie EOF from qio_channel_readv()) then this will busy > > loop. You need to break the loop on that condition and return whatever > > 'done' currently is. > > Done. > > >> +static void test_io_channel_buf2(void) > >> +{ > >> + QIOChannelBuffer *buf; > >> + QIOChannelTest *test; > >> + > >> + buf = qio_channel_buffer_new(0); > >> + > >> + test = qio_channel_test_new(); > >> + qio_channel_test_run_writer_all(test, QIO_CHANNEL(buf)); > >> + buf->offset = 0; > >> + qio_channel_test_run_reader(test, QIO_CHANNEL(buf)); > >> + qio_channel_test_validate(test); > >> + > >> + object_unref(OBJECT(buf)); > >> +} > >> + > >> +static void test_io_channel_buf3(void) > >> +{ > >> + QIOChannelBuffer *buf; > >> + QIOChannelTest *test; > >> + > >> + buf = qio_channel_buffer_new(0); > >> + > >> + test = qio_channel_test_new(); > >> + qio_channel_test_run_writer(test, QIO_CHANNEL(buf)); > >> + buf->offset = 0; > >> + qio_channel_test_run_reader_all(test, QIO_CHANNEL(buf)); > >> + qio_channel_test_validate(test); > >> + > >> + object_unref(OBJECT(buf)); > >> +} > >> + > >> +static void test_io_channel_buf4(void) > >> +{ > >> + QIOChannelBuffer *buf; > >> + QIOChannelTest *test; > >> + > >> + buf = qio_channel_buffer_new(0); > >> + > >> + test = qio_channel_test_new(); > >> + qio_channel_test_run_writer_all(test, QIO_CHANNEL(buf)); > >> + buf->offset = 0; > >> + qio_channel_test_run_reader_all(test, QIO_CHANNEL(buf)); > >> + qio_channel_test_validate(test); > >> + > >> + object_unref(OBJECT(buf)); > >> +} > >> > >> int main(int argc, char **argv) > >> { > >> @@ -46,6 +92,9 @@ int main(int argc, char **argv) > >> > >> g_test_init(&argc, &argv, NULL); > >> > >> - g_test_add_func("/io/channel/buf", test_io_channel_buf); > >> + g_test_add_func("/io/channel/buf1", test_io_channel_buf1); > >> + g_test_add_func("/io/channel/buf2", test_io_channel_buf2); > >> + g_test_add_func("/io/channel/buf3", test_io_channel_buf3); > >> + g_test_add_func("/io/channel/buf4", test_io_channel_buf4); > >> return g_test_run(); > >> } > > > > There's no need to add any of these additions to the test suite. Instead > > you can just change the existing io-channel-helpers.c functions > > test_io_thread_writer() and test_io_thread_reader(), to call > > qio_channel_writev_all() & qio_channel_readv_all() respectively. > > They are already done now, and the advantage of this was that I was able > to test that everything worked well against everything. That was good > to be able to check that all worked as expected.
The existing test_io_thread_reader/writer() methods that I mention are common code used by multiple tests - test-io-channel-buffer, test-io-chanel-socket, test-io-channel-file, test-io-channel-tls. I don't want to add code to test-io-channel-buffer that duplicates functionality that already exists, and is exercised by only one of the many IO channel implementation tests. 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 :|