From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42280) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7UHs-0008TN-6c for qemu-devel@nongnu.org; Tue, 13 Mar 2012 12:09:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S7UHh-0005gb-Kc for qemu-devel@nongnu.org; Tue, 13 Mar 2012 12:09:23 -0400 Received: from isrv.corpit.ru ([86.62.121.231]:42061) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7UHh-0005Zn-9W for qemu-devel@nongnu.org; Tue, 13 Mar 2012 12:09:13 -0400 Message-ID: <4F5F711F.4070304@msgid.tls.msk.ru> Date: Tue, 13 Mar 2012 20:09:03 +0400 From: Michael Tokarev MIME-Version: 1.0 References: <1331579663-29950-1-git-send-email-mjt@msgid.tls.msk.ru> In-Reply-To: <1331579663-29950-1-git-send-email-mjt@msgid.tls.msk.ru> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv3 0/9] cleanup/consolidate some iovec functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev Cc: Paolo Bonzini , qemu-devel@nongnu.org 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(-) >