[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv3 0/9] cleanup/consolidate some iovec functions
From: |
Michael Tokarev |
Subject: |
Re: [Qemu-devel] [PATCHv3 0/9] cleanup/consolidate some iovec functions |
Date: |
Tue, 13 Mar 2012 20:09:03 +0400 |
User-agent: |
Mozilla/5.0 (X11; Linux i686 on x86_64; rv:8.0) Gecko/20120216 Icedove/8.0 |
I created a git tree with these patches,
git://git.corpit.ru/qemu.git branch mjt-iov
It includes one extra patch:
6856d2ef3e72753aaa3bcdfc274ee6ae6c112dc6 virtio-serial-bus: use correct
lengths in control_out() message
which has been discussed with Amit Shah and he suggested
me to include it in this series.
Thanks,
/mjt
On 12.03.2012 23:14, Michael Tokarev wrote:
> This is a little cleanup/consolidation for some 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:
>
> void 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) qemu_sendv()
> and qemu_recvv() and friends, where `offset' and `bytes'
> arguments were _renamed_, with the following prototypes:
>
> ssize_t qemu_sendv(sockfd, iov, 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. Used size_t instead of various variations for byte counts.
> Including qemu_iovec_copy which used uint64_t(!) type.
>
> 4. 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 exactly
> 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 change at all, just
> some shuffling. It has been tested slightly, by booting
> different guests out of different image formats and doing
> some i/o, after each of the 9 patches, and it all works
> correctly so far.
>
> The patchset depends on a tiny patch I sent to Amit Shah,
> "virtio-serial-bus: use correct lengths in control_out() message"
> but the only conflict without that patch is in
> hw/virtio-serial-bus.c and it is trivial to resolve it.
>
> 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 (9):
> refresh 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
> change prototypes of qemu_sendv() and qemu_recvv()
> export qemu_sendv_recvv() and use it in qemu_sendv() and qemu_recvv()
> cleanup qemu_co_sendv(), qemu_co_recvv() and friends
> rewrite and comment qemu_sendv_recvv()
>
> block.c | 12 +-
> block/curl.c | 6 +-
> block/iscsi.c | 2 +-
> block/nbd.c | 4 +-
> 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 | 261 +++++++++++++++++------------------------------
> hw/9pfs/virtio-9p.c | 8 +-
> hw/rtl8139.c | 2 +-
> hw/usb.c | 6 +-
> hw/virtio-balloon.c | 4 +-
> hw/virtio-net.c | 4 +-
> hw/virtio-serial-bus.c | 6 +-
> iov.c | 89 +++++++----------
> iov.h | 47 ++++++++-
> linux-aio.c | 4 +-
> net.c | 2 +-
> posix-aio-compat.c | 8 +-
> qemu-common.h | 69 +++++++------
> qemu-coroutine-io.c | 82 +++++-----------
> 24 files changed, 293 insertions(+), 368 deletions(-)
>
- [Qemu-devel] [PATCHv3 2/9] consolidate qemu_iovec_memset{, _skip}() into single function and use existing iov_memset(), (continued)
- [Qemu-devel] [PATCHv3 2/9] consolidate qemu_iovec_memset{, _skip}() into single function and use existing iov_memset(), Michael Tokarev, 2012/03/12
- [Qemu-devel] [PATCHv3 1/9] refresh iov_* functions, Michael Tokarev, 2012/03/12
- [Qemu-devel] [PATCHv3 4/9] consolidate qemu_iovec_copy() and qemu_iovec_concat() and make them consistent, Michael Tokarev, 2012/03/12
- [Qemu-devel] [PATCHv3 5/9] change qemu_iovec_to_buf() to match other to, from_buf functions, Michael Tokarev, 2012/03/12
- [Qemu-devel] [PATCHv3 8/9] cleanup qemu_co_sendv(), qemu_co_recvv() and friends, Michael Tokarev, 2012/03/12
- [Qemu-devel] [PATCHv3 7/9] export qemu_sendv_recvv() and use it in qemu_sendv() and qemu_recvv(), Michael Tokarev, 2012/03/12
- [Qemu-devel] [PATCHv3 3/9] allow qemu_iovec_from_buffer() to specify offset from which to start copying, Michael Tokarev, 2012/03/12
- [Qemu-devel] [PATCHv3 6/9] change prototypes of qemu_sendv() and qemu_recvv(), Michael Tokarev, 2012/03/12
- [Qemu-devel] [PATCHv3 9/9] rewrite and comment qemu_sendv_recvv(), Michael Tokarev, 2012/03/12
- Re: [Qemu-devel] [PATCHv3 0/9] cleanup/consolidate some iovec functions,
Michael Tokarev <=