[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 07/11] block: Add QMP support for streaming t
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v9 07/11] block: Add QMP support for streaming to an intermediate layer |
Date: |
Tue, 3 May 2016 15:48:47 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 03.05.2016 um 15:33 hat Alberto Garcia geschrieben:
> On Tue 03 May 2016 03:23:24 PM CEST, Kevin Wolf wrote:
> >> c) we fix bdrv_reopen() so we can actually run both jobs at the same
> >> time. I'm wondering if pausing all block jobs between
> >> bdrv_reopen_prepare() and bdrv_reopen_commit() would do the
> >> trick. Opinions?
> >
> > I would have to read up the details of the problem again, but I think
> > with bdrv_drained_begin/end() we actually have the right tool now to fix
> > it properly. We may need to pull up the drain (bdrv_drain_all() today)
> > from bdrv_reopen_multiple() to its caller and just assert it in the
> > function itself, but there shouldn't be much more to it than that.
>
> I think that's not enough, see point 2) here:
>
> https://lists.gnu.org/archive/html/qemu-block/2015-12/msg00180.html
>
> "I've been taking a look at the bdrv_drained_begin/end() API, but as I
> understand it it prevents requests from a different AioContext.
> Since all BDS in the same chain share the same context it does not
> really help here."
Yes, that's the part I meant with pulling up the calls.
If I understand correctly, the problem is that first bdrv_reopen_queue()
queues a few BDSes for reopen, then bdrv_drain_all() completes all
running requests and can indirectly trigger a graph modification, and
then bdrv_reopen_multiple() uses the queue which doesn't match reality
any more.
The solution to that should be simply changing the order of things:
1. bdrv_drained_begin()
2. bdrv_reopen_queue()
3. bdrv_reopen_multiple()
* Instead of bdrv_drain_all(), assert that no requests are pending
* We don't run requests, so we can't complete a block job and
manipulate the graph any more
4. then bdrv_drained_end()
But you're right that this changed order is really the key and not
bdrv_drained_begin/end(). The latter are just cleaner than the existing
bdrv_drain_all(), but don't actually contribute much to the fix without
dataplane. I must have misremembered its importance.
Kevin
- Re: [Qemu-devel] [PATCH v9 07/11] block: Add QMP support for streaming to an intermediate layer, Alberto Garcia, 2016/05/03
- Re: [Qemu-devel] [PATCH v9 07/11] block: Add QMP support for streaming to an intermediate layer, Kevin Wolf, 2016/05/03
- Re: [Qemu-devel] [PATCH v9 07/11] block: Add QMP support for streaming to an intermediate layer, Alberto Garcia, 2016/05/03
- Re: [Qemu-devel] [PATCH v9 07/11] block: Add QMP support for streaming to an intermediate layer,
Kevin Wolf <=
- Re: [Qemu-devel] [PATCH v9 07/11] block: Add QMP support for streaming to an intermediate layer, Alberto Garcia, 2016/05/03
- Message not available
- Re: [Qemu-devel] [PATCH v9 07/11] block: Add QMP support for streaming to an intermediate layer, Kevin Wolf, 2016/05/12
- Message not available
- Re: [Qemu-devel] [PATCH v9 07/11] block: Add QMP support for streaming to an intermediate layer, Kevin Wolf, 2016/05/12
- Re: [Qemu-devel] [PATCH v9 07/11] block: Add QMP support for streaming to an intermediate layer, Alberto Garcia, 2016/05/17
- Re: [Qemu-devel] [PATCH v9 07/11] block: Add QMP support for streaming to an intermediate layer, Kevin Wolf, 2016/05/17
- Re: [Qemu-devel] [PATCH v9 07/11] block: Add QMP support for streaming to an intermediate layer, Alberto Garcia, 2016/05/17