From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49937) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cQz8k-0002Ik-It for qemu-devel@nongnu.org; Tue, 10 Jan 2017 11:19:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cQz8i-0000WB-1z for qemu-devel@nongnu.org; Tue, 10 Jan 2017 11:19:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46712) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cQz8h-0000Vl-T7 for qemu-devel@nongnu.org; Tue, 10 Jan 2017 11:19:11 -0500 From: Markus Armbruster References: <1479108340-3453-1-git-send-email-caoj.fnst@cn.fujitsu.com> <20170109234446-mutt-send-email-mst@kernel.org> <87bmvftejz.fsf@dusky.pond.sub.org> <20170110162151-mutt-send-email-mst@kernel.org> Date: Tue, 10 Jan 2017 17:19:07 +0100 In-Reply-To: <20170110162151-mutt-send-email-mst@kernel.org> (Michael S. Tsirkin's message of "Tue, 10 Jan 2017 16:38:00 +0200") Message-ID: <87pojuc2h0.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v7 00/10] Convert msix_init() to error List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Jiri Pirko , Jason Wang , Cao jin , qemu-devel@nongnu.org, Dmitry Fleytman , Alex Williamson , Hannes Reinecke , Marcel Apfelbaum , Paolo Bonzini , Gerd Hoffmann "Michael S. Tsirkin" writes: > On Tue, Jan 10, 2017 at 11:06:08AM +0100, Markus Armbruster wrote: >> "Michael S. Tsirkin" writes: >> >> > On Mon, Nov 14, 2016 at 03:25:30PM +0800, Cao jin wrote: >> >> v7 changelog: >> >> 1. fix the segfaut bug in patch 2. So drop the all the R-b of it, >> >> please take a look, there is detailed description in the patch. >> >> 2. add the R-b from Hannes Reinecke >> >> >> >> Test: >> >> 1. make check: pass >> >> 2. After applied all the patch, command line test for all the >> >> affected devices, just make sure device realize process is ok, >> >> no crash, but no further use of device. >> > >> > Consider the megasas device for example, don't you >> > need to test that the change actually does what >> > it's intended to do? >> >> For better or worse, that's a higher bar than we commonly require for >> refactorings. >> >> [...] > > Well the patch says that it's addressing a TODO. If no one can > be bothered to test the functionality, maybe we shouldn't bother > with the change. > > Generally this patchset is at v7. It brings a very limited benefit to > the project. It better be perfect otherwise I don't see why bother. We obviously disagree on the benefit. Before this series, error reporting is *broken* for QMP. Fixing that is definitely not "why bother" material. I certainly don't object to tightening our testing habits. I was merely pointing out that you're doing that. Testing patches that modernize interfaces often isn't easy for the person doing the work. Fortunately, megasas has a maintainer: Hannes. Who gave his R-by. For me, that would suffice, but you may see things differently. Would a Tested-by from him satisfy you? If not, what would satisfy you?