All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Fam Zheng <famz@redhat.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/2] scsi: add block job opblockers for scsi-block
Date: Mon, 12 Feb 2018 15:48:24 +0100	[thread overview]
Message-ID: <20180212144824.GI5103@localhost.localdomain> (raw)
In-Reply-To: <3d6031b0-7d75-c3ab-e466-ce3d87d630df@redhat.com>

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

  reply	other threads:[~2018-02-12 14:48 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-07 16:36 [Qemu-devel] [PATCH 0/2] scsi: add block job opblockers for scsi-block Paolo Bonzini
2018-02-07 16:36 ` [Qemu-devel] [PATCH 1/2] scsi: add unrealize method for SCSI devices Paolo Bonzini
2018-02-08  1:35   ` Fam Zheng
2018-02-07 16:36 ` [Qemu-devel] [PATCH 2/2] scsi: add block job opblockers for scsi-block Paolo Bonzini
2018-02-08  1:35   ` Fam Zheng
2018-02-08 10:42     ` Paolo Bonzini
2018-02-12 13:52       ` Kevin Wolf
2018-02-12 14:00         ` Paolo Bonzini
2018-02-12 14:30           ` Kevin Wolf
2018-02-12 14:32             ` Paolo Bonzini
2018-02-12 14:48               ` Kevin Wolf [this message]
2018-02-12 14:50                 ` Paolo Bonzini
2018-03-12 11:10                   ` [Qemu-devel] [Qemu-block] " Paolo Bonzini
2018-03-12 11:58                     ` Kevin Wolf
2018-04-05 11:59                       ` Paolo Bonzini
2018-04-05 12:43                         ` Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180212144824.GI5103@localhost.localdomain \
    --to=kwolf@redhat.com \
    --cc=famz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.