[Qemu-devel] [PATCHv5 00/11] cleanup/consolidate iovec functions

[Michael Tokarev]

This is my last attempt.  It is pointless to bother so much
people with so much trivial stuff again and again and again.

This is cleanup/consolidation of iovec-related low-level
routines in qemu.

The plan is to make library functions more understandable,
consistent and useful, and to drop numerous implementations
of the same thing.

The patch changes prototypes of several iov and qiov functions
to match each other, changes types of arguments for some
functions, _swaps_ some function arguments with each other,
and makes use of common code in r/w path.

The result of all these changes.

1. Most qiov-related (qemu_iovec_*) functions now accepts
 'offset' parameter to specify from which (byte) position to
 start operation.  This is added for _memset (removing
 _memset_skip), _from_buffer (allowing to copy a bounce-
 buffer to a middle of qiov).  Typical:

  size_t qemu_iovec_memset(QEMUIOVector *qiov, size_t offset,
                           int c, size_t bytes);

2. All functions that accepts this `offset' argument does
 it in a similar manner, following the
   iov, fromwhere, what, bytes
 pattern.  This is consistent with (updated) iov_send()
 and iov_recv() and friends, where `offset' and `bytes'
 arguments were _renamed_, with the following prototypes:

   ssize_t iov_send(sockfd, iov, iov_cnt size_t offset, size_t bytes)

 instead of

   int qemu_sendv(sockfd, iov, int len, int iov_offset)

 See how offset & bytes are used in the same way as for iov_*
 and qemu_iovec_*.   A few callers of these are verified and
 converted.

3. Always use iov_cnt (number of iov entries) together with
  iov, to be able to verify that we don't go past the array.
  iov_send (former qemu_sendv) and iov_recv accepts one extra
  argument now, as are all other derivates (qemu_co_sendv etc).

4. Used size_t instead of various variations for byte counts.
 Including qemu_iovec_copy which used uint64_t(!) type.

5. Function arguments are renamed to better match with their
 actual meaning.  Compare new and original prototype of
 qemu_sendv() above: old prototype with `len' does not
 tell if `len' refers to number of iov elements (as
 regular writev() call) or to number of data bytes.
 Ditto for several usages of `count' for some qemu_iovec_*,
 which is also replaced to `bytes'.

5. One implementation of the code remain. For example,
 qemu_iovec_from_buffer() uses iov_from_buf() directly,
 instead of repeating the same code.

The resulting function usage is much more consistent, the
functions themselves are nice and understandable, which
means they're easier to use and less error-prone.

This patchset also consolidates a few low-level send&recv
functions into one, since both versions were the same
(and were finally calling common function anyway).
This is done by exporting a common send_recv function
with one extra bool argument, and making current send&recv
to be just #defines.

And while at it all, also made some implementations shorter,
cleaner and much easier to read/understand, and add some
code comments.

The read&write consolidation has great potential for the
block layer, as has been demonstrated before.
Unification and generalization of qemu_iovec_* functions
will let to optimize/simplify some more code in block/*,
especially qemu_iovec_memset() and _from_buffer() (this
optimization/simplification is already used in qcow2.c
a bit).

The resulting thing should behave like current/existing
code, there should be no behavor changes, just some
shuffling and a bit of sanity checks.  It has been tested
slightly, by booting different guests out of different
image formats and doing some i/o, after each of the 11
patches, and it all works correctly so far.

Changes since v4:

- added unit tests for iov_from_buf(), iov_to_buf(), iov_memset()
  (in 3/11 "rewrite iov_* functions") and for iov_send_recv()
  (in 11/11 "rewrite iov_send_recv() and move it to iov.c")
  Note: the send_recv() test is unix-specific since it uses
  socketpair(PF_UNIX) and fork() to test actual send/receive.
- after running unit tests, fixed to bugs in iov_from_buf*()
  and iov_send_recv() introduced during countless rewrites
  of the code and rearranges of the patches to be even more
  review-friendly.

Changes since v3:

- rename qemu_sendv(), qemu_sendv_recvv() to iov_send(),
  iov_send_recv() and move them from qemu-common.h and
  cutils.c to iov.h and iov.c.
- add new argument, iov_cnt, to all send/recv-related
  functions (iov_send_recv(), qemu_co_sendv_recv() etc).
  This resulted in a bit more changes in other places,
  some of which, while simple, may still be non-obvious
  (block/nbd.c and block/sheepdog.c).
  Due to the rename above and to introduction of the
  new iov_cnt arg, the function signatures are changed
  so no old code using new function wrongly is possible.
- patch that changes iov_from_buf() & iov_to_buf() is
  split into two halves: prototype changes and rewrite.
- rewrite iov_send_recv() (former qemu_sendv_recvv())
  again, slightly, to use newly introduced iov_cnt arg.

Changes since v2:

- covered iov.[ch] with iov_*() functions which contained
  similar functionality
- changed tabs to spaces as per suggestion by Kevin
- explicitly allow to use large sizes for frombuf/tobuf
  functions and friends, stopping at the end of iovec
  in case more bytes requested when available, and
  _returning_ number of actual bytes processed, but
  made it extra clear what a return value will be.
- used ssize_t for sendv/recvv instead of int, to
  be consistent with all system headers and other
  places in qemu
- fix/clarify a case in my qemu_co_sendv_recvv()
  of handling zero reads(recvs) and writes(sends).
- covered qemu_iovec_to_buf() to have the same
  pattern as other too (was missing in v2), and
  use it in qcow2.c right away to simplify things
- spotted and removed wrong usage of qiov instead of
  plain iov in posix-aio-compat.c

What is _not_ covered:

- suggestion by pbonzini to not use macros for
  access methods to call sendv_recvv with an
  extra true/false argument: I still think the
  usage of macros here is better than anything
  else.
- maybe re-shuffling of all this stuff into
  different headers -- we already have iov.h,
  maybe rename it to qemu-iov.h for consistency
  and move all iov-related functions into there
  (and ditto for cutils.c => (qemu-)iov.c).

Michael Tokarev (11):
  virtio-serial-bus: use correct lengths in control_out() message
  change iov_* function prototypes to be more appropriate
  rewrite iov_* functions
  consolidate qemu_iovec_memset{,_skip}() into single function and use existing 
iov_memset()
  allow qemu_iovec_from_buffer() to specify offset from which to start copying
  consolidate qemu_iovec_copy() and qemu_iovec_concat() and make them consistent
  change qemu_iovec_to_buf() to match other to,from_buf functions
  rename qemu_sendv to iov_send, change proto and move declarations to iov.h
  export iov_send_recv() and use it in iov_send() and iov_recv()
  cleanup qemu_co_sendv(), qemu_co_recvv() and friends
  rewrite iov_send_recv() and move it to iov.c

 block.c                |   12 +-
 block/curl.c           |    6 +-
 block/iscsi.c          |    2 +-
 block/nbd.c            |   18 ++--
 block/qcow.c           |    4 +-
 block/qcow2.c          |   19 ++--
 block/qed.c            |   10 +-
 block/rbd.c            |    2 +-
 block/sheepdog.c       |    6 +-
 block/vdi.c            |    4 +-
 cutils.c               |  234 ++++++--------------------------------------
 hw/9pfs/virtio-9p.c    |    8 +-
 hw/rtl8139.c           |    2 +-
 hw/usb/core.c          |    6 +-
 hw/virtio-balloon.c    |    4 +-
 hw/virtio-net.c        |    4 +-
 hw/virtio-serial-bus.c |   10 +-
 iov.c                  |  192 ++++++++++++++++++++++++++----------
 iov.h                  |   77 +++++++++++++-
 linux-aio.c            |    4 +-
 net.c                  |    2 +-
 posix-aio-compat.c     |    8 +-
 qemu-common.h          |   56 +++++------
 qemu-coroutine-io.c    |   83 +++++-----------
 test-iov.c             |  256 ++++++++++++++++++++++++++++++++++++++++++++++++
 tests/Makefile         |    6 +-
 26 files changed, 617 insertions(+), 418 deletions(-)
 create mode 100644 test-iov.c

-- 
1.7.9.1