From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43419) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1elFPX-0005C6-5R for qemu-devel@nongnu.org; Mon, 12 Feb 2018 09:48:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1elFPW-0005zz-6c for qemu-devel@nongnu.org; Mon, 12 Feb 2018 09:48:51 -0500 Date: Mon, 12 Feb 2018 15:48:24 +0100 From: Kevin Wolf Message-ID: <20180212144824.GI5103@localhost.localdomain> References: <20180207163622.29935-1-pbonzini@redhat.com> <20180207163622.29935-3-pbonzini@redhat.com> <20180208013517.GB24289@lemon.usersys.redhat.com> <5ac0b76e-213a-60c3-c010-87512d3a564a@redhat.com> <20180212135206.GG5103@localhost.localdomain> <81af35f9-4934-0c21-e3b1-b27c2fa7fdd9@redhat.com> <20180212143040.GH5103@localhost.localdomain> <3d6031b0-7d75-c3ab-e466-ce3d87d630df@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3d6031b0-7d75-c3ab-e466-ce3d87d630df@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/2] scsi: add block job opblockers for scsi-block List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Fam Zheng , qemu-devel@nongnu.org, qemu-block@nongnu.org Am 12.02.2018 um 15:32 hat Paolo Bonzini geschrieben: > On 12/02/2018 15:30, Kevin Wolf wrote: > >>> We shouldn't be adding new instances of BLOCK_OP_* at all. I couldn't > >>> find the time yet to remove the existing ones, but any new protections > >>> should be using the permission system. > >> I agree. But does this include not fixing bugs wherever clients are > >> using the old op blockers? > > I'm not saying that we shouldn't fix the bug, just that we should fix it > > properly with the best infrastructure we have. > > > > The old op blockers are "fixing" the problem at the symptom level, and > > you have to check for each high-level operation if it does something > > problematic internally. You have to repeat this analysis every time you > > add a new operation or modifiy an existing one (which noone ever does). > > The risk that this breaks sooner or later is pretty high. > > > > The new permission system, on the other hand, directly addresses the > > root cause, and any new feature that uses dirty bitmaps will then > > automatically get the protection, too. > > > > So in fact, I would say that the bug isn't really fixed (but at best > > papered over) until we add a proper fix on the permission level. > > Okay, we are in agreement about this and you expressed very well why I > (at the gut feeling level) didn't like the old op blockers. But you > bypassed the real question, which is: should I send a pull request for > these two patches or not? :) I didn't spell it out that explicitly, but this is essentially a NACK. I'd very much prefer if you could replace it with the proper solution. Of course, we can always make exceptions when there is a good reason, but with 2.12 still two months away, I doubt we have one. Kevin