From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59319) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHbK5-0003Cq-QK for qemu-devel@nongnu.org; Fri, 08 Jan 2016 12:59:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aHbK2-0002TV-GG for qemu-devel@nongnu.org; Fri, 08 Jan 2016 12:59:37 -0500 Received: from mx2.parallels.com ([199.115.105.18]:60287) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHbK2-0002TI-AG for qemu-devel@nongnu.org; Fri, 08 Jan 2016 12:59:34 -0500 References: <1449240275-26196-1-git-send-email-den@openvz.org> <1449240275-26196-2-git-send-email-den@openvz.org> <567B11BA.4090901@redhat.com> <568F9D24.6090803@openvz.org> <568FE068.9040203@redhat.com> <568FE672.3040401@openvz.org> <568FF7CD.3070901@redhat.com> From: "Denis V. Lunev" Message-ID: <568FF8FE.4050306@openvz.org> Date: Fri, 8 Jan 2016 20:59:26 +0300 MIME-Version: 1.0 In-Reply-To: <568FF7CD.3070901@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/5] migration: split hmp_savevm to do_savevm and hmp_savevm wrapper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Amit Shah , Markus Armbruster , qemu-devel@nongnu.org, quintela@redhat.com On 01/08/2016 08:54 PM, Eric Blake wrote: > On 01/08/2016 09:40 AM, Denis V. Lunev wrote: > >>>>> Markus' series to add a prefixing notation would be better to use here >>>>> (although I didn't check if he caught this one in that series already): >>>>> https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg03495.html >>>> this series is not yet merged. I think that we could do this refactoring >>>> later on. >>>> This thing could be considered independent. Anyway, this series has its >>>> own value >>>> and it takes a lot of time to push it in. Could we do error setting >>>> improvement later on? >>> I don't care who rebases on top of the other, but maybe Markus will have >>> an opinion when he gets back online next week. >>> >> why we have to wait with this set due to this reason? > One of you will have to rebase on the other - either you wait for > Markus' error_prepend to go in and you use it, or your patch goes in and > Markus updates his error_prepend patch to cover your additional instance > that will be benefitted by it. I don't care which, and the timing is > really up to the maintainers and how fast they send pull requests. > >> The code with error_prepend and current code are BOTH >> correct. One is a bit shorter then other. Yes, it would >> be nice to switch to it, but why this should be done in >> this set? > Exactly, we're saying the same things. > >>>>>> + if (local_err != NULL) { >>>>> I would have just written 'if (local_err) {'; but that's minor style. >>>> from my point of view explicit != NULL exposes that local_err is a >>>> pointer rather than a boolean value. >>> But the code base already overwhelmingly relies on C's implicit >>> conversion of pointer to a boolean context, as it requires less typing; >>> being verbose doesn't make the code base any easier to read. However, >>> since HACKING doesn't say one way or the other, I won't make you change. >>> >> I do not understand your last words. >> >> I am not agitating you with one approach or another. This >> is a reason why I am writing code this way. The code written >> this way looks better to me. This code is NEW and this does >> not contradict any written rule in coding style policy. >> >> If the code is working and correct, can we just move on with it? > Once again, we are saying the same thing. I pointed out a cosmetic > issue, but one where I do not have a strong enough leg to stand on to > force you to change your style, so what you did is fine as is. > ok. perfect to be on the same page :) I'll promise to switch to error_prepend code when it will be merged. I hope that v4 of the set is good enough to proceed. Den