From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47183) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S6kMI-0006WP-8F for qemu-devel@nongnu.org; Sun, 11 Mar 2012 11:06:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S6kMG-0005vq-4I for qemu-devel@nongnu.org; Sun, 11 Mar 2012 11:06:53 -0400 Received: from mail-we0-f173.google.com ([74.125.82.173]:37642) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S6kMF-0005vb-Ng for qemu-devel@nongnu.org; Sun, 11 Mar 2012 11:06:52 -0400 Received: by werp12 with SMTP id p12so3038804wer.4 for ; Sun, 11 Mar 2012 08:06:49 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <4F5CBF86.7000709@redhat.com> Date: Sun, 11 Mar 2012 16:06:46 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1331430564-32745-1-git-send-email-mjt@msgid.tls.msk.ru> In-Reply-To: <1331430564-32745-1-git-send-email-mjt@msgid.tls.msk.ru> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv2 0/7] cleanup/consolidate some iovec functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev Cc: qemu-devel@nongnu.org Il 11/03/2012 02:49, Michael Tokarev ha scritto: > 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. > > 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,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: > > int 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 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'. > > 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 isn't done in this series) > > Michael Tokarev (7): > Consolidate qemu_iovec_memset{,_skip}() into single, simplified function > 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 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 | 8 +- > block/curl.c | 4 +- > block/nbd.c | 4 +- > block/qcow.c | 2 +- > block/qcow2.c | 14 ++-- > block/qed.c | 10 +- > block/sheepdog.c | 6 +- > block/vdi.c | 2 +- > cutils.c | 275 +++++++++++++++++++++------------------------------ > hw/9pfs/virtio-9p.c | 8 +- > linux-aio.c | 4 +- > posix-aio-compat.c | 2 +- > qemu-common.h | 64 +++++++----- > qemu-coroutine-io.c | 83 ++++----------- > 14 files changed, 205 insertions(+), 281 deletions(-) > Looks good, except that I don't like #defines. (I also don't like exporting qemu_sendv_recvv, but I can live with it. :)) I commented on the single patches. I'm happy to take 4-7 via the NBD tree. Paolo