qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Fam Zheng <famz@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org, mreitz@redhat.com,
	stefanha@redhat.com, den@openvz.org, pbonzini@redhat.com,
	jsnow@redhat.com
Subject: Re: [Qemu-devel] [PATCH] blk: fix aio context loss on media change
Date: Wed, 15 Mar 2017 22:42:59 +0800	[thread overview]
Message-ID: <20170315144259.GG3088@lemon.lan> (raw)
In-Reply-To: <735f5ad8-7a2a-8dcf-b806-fd17d00afac2@virtuozzo.com>

On Wed, 03/15 17:04, Vladimir Sementsov-Ogievskiy wrote:
> 15.03.2017 16:13, Fam Zheng wrote:
> > On Wed, 03/15 13:06, Kevin Wolf wrote:
> > > > > Stefan, Paolo, do you remember the details why we didn't even do a
> > > > > simple fix like the one below? I think there were some patches on the
> > > > > list, no?
> > > > ISTM the concern was mostly "what about other BB in the graph?"
> > > > 
> > > > Should the new op blocker API be used in this one (a new type of perm)?
> > > If we know what operation to block, that's an option. Would "change the
> > > node's AioContext" work for it?
> 
> we can start with empty cdrom, so there is no context at start

Practically, I agree this patch is fine for now because callers don't
arbitrarily move BDS from one context to another, only a small set of operations
exists - start/stop data plane, block job callbacks. The problem is that new
code in the future can cause trouble if different BBs in the same graph don't
agree on what context they can run on, especially with dynamical
reconfiguration. So the previous discussions were more about an API that works
generally.

> 
> > > 
> > > I think it would effectively mean that you need to attach the device
> > > first and then jobs etc. respect the AioContext, whereas the opposite
> > > order breaks because they don't have callbacks to adjust the AioContext
> > > after the fact.
> > > 
> > > This seems to match what's actually safe, so it might really be as easy
> > > as this.
> > Yes, this sounds good to me.
> > 
> > Vladimir, would you like to implement this? It would be good to have this fixed
> > in 2.9.
> 
> I don't sure about how to do it, so, I don't mind someone else to implement
> this..

OK, I can take a look at this tomorrow.

Fam

  reply	other threads:[~2017-03-15 14:43 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-14 17:11 [Qemu-devel] [PATCH] blk: fix aio context loss on media change Vladimir Sementsov-Ogievskiy
2017-03-15 11:03 ` Kevin Wolf
2017-03-15 11:14   ` Fam Zheng
2017-03-15 12:06     ` Kevin Wolf
2017-03-15 13:13       ` Fam Zheng
2017-03-15 14:04         ` Vladimir Sementsov-Ogievskiy
2017-03-15 14:42           ` Fam Zheng [this message]
2017-03-15 13:39   ` Paolo Bonzini
2017-03-15 14:30     ` Kevin Wolf
2017-03-15 14:43       ` Paolo Bonzini
2017-03-15 15:02         ` Kevin Wolf
2017-03-15 15:09           ` Paolo Bonzini
2017-03-15 15:30             ` 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=20170315144259.GG3088@lemon.lan \
    --to=famz@redhat.com \
    --cc=den@openvz.org \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@virtuozzo.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).