From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56109) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S787O-0004U5-CV for qemu-devel@nongnu.org; Mon, 12 Mar 2012 12:29:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S787M-0000zT-FC for qemu-devel@nongnu.org; Mon, 12 Mar 2012 12:29:05 -0400 Received: from isrv.corpit.ru ([86.62.121.231]:39725) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S787M-0000xN-4R for qemu-devel@nongnu.org; Mon, 12 Mar 2012 12:29:04 -0400 Message-ID: <4F5E244C.3010701@msgid.tls.msk.ru> Date: Mon, 12 Mar 2012 20:29:00 +0400 From: Michael Tokarev MIME-Version: 1.0 References: <1331430564-32745-1-git-send-email-mjt@msgid.tls.msk.ru> <1331430564-32745-7-git-send-email-mjt@msgid.tls.msk.ru> <4F5CBE4F.1040007@redhat.com> <4F5CC41E.1080106@msgid.tls.msk.ru> <4F5DFA66.80900@redhat.com> In-Reply-To: <4F5DFA66.80900@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv2 6/7] cleanup qemu_co_sendv(), qemu_co_recvv() and friends List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On 12.03.2012 17:30, Paolo Bonzini wrote: > Il 11/03/2012 16:26, Michael Tokarev ha scritto: >> Note that - I still hope - in the end there will be no sendv or >> recv calls at all, only common sendv_recvv with is_write passed >> as an argument from upper layer. It will be easier to remove >> that #define - just two lines of code instead of minimum 5 :) > > This is what I don't really like in the second part of these patches. > You are doing changes for the sake of other changes which are not > upstream yet, for which there is no clear vision, and for which there is > no clear benefit. I already posted the example of this. I can complete whole thing and send it all in one huge chunk if you prefer that :) > While I agree that there is a lot of duplicated code in block.c and > block/*, I don't think that what we need is more parameters to the > functions. We have places where we need to know the request flags, for > example, but the methods are already quite unwieldy and have a lot of > arguments. So I'm not sure that this kind of unification buys anything. Which places are these? Also, how about dropping nr_sectors? If you need flags, well, the extra argument being added can really be used for that if necessary. > On top of this, your patches unify things that are similar but not quite > identical, and you do not provide hints in the commit messages that you > did so. For example, qemu_co_recvv has handling for receiving 0, but > qemu_co_sendv does not. This is a bug, which I dind't notice, it shouldn't be there. Somehow I overlooked this difference, but I really wondered how they're differ! By using common code here it becomes much more obvious (whith the bug actually fixed). I'll take a look at other similar things here and in my block layer changes. > Can you please separate the changes to make the argument order uniform? > Those should be easy to get in. While at it I found another "library", iov.[ch], which has another implementation of the same thing. So that's where it all should have been started. I'll resend a v3 covering iov_* functions too. Thank you! /mjt