Re: [Qemu-devel] [PATCH v10 08/16] block: Support streaming to an intermediate layer

[Kevin Wolf <kwolf@redhat.com>]

Am 12.10.2016 um 16:33 hat Alberto Garcia geschrieben:
> On Wed 12 Oct 2016 04:23:05 PM CEST, Kevin Wolf <kwolf@redhat.com> wrote:
> >> +    /* Block all intermediate nodes between bs and base, because they
> >> +     * will disappear from the chain after this operation */
> >> +    for (iter = backing_bs(bs); iter && iter != base; iter = backing_bs(iter)) {
> >> +        block_job_add_bdrv(&s->common, iter);
> >> +    }
> >
> > In patch 6, you had a comment that the top node must be blocked as
> > well because its backing file string will be updated. Isn't it the
> > same for streaming?
> 
> In the block-stream case, 'device' is always the top node, and creating
> the block job there already blocks all operations in it.
> 
> In the block-commit case, 'device' and 'top' are different parameters,
> that's why 'top' must be explicitly blocked. I actually don't think that
> we need to block 'device' at all, and we could even make the parameter
> optional. That would be for a future patch, though. Also, the backing
> file string is not updated in 'top', but in its overlay (unlike in
> block-stream, 'top' disappears after a block-commit job).

I see. So the block job for commit is owned by a node that is
potentially completely unrelated to the operation except that it holds
op blockers and because we use it to finde the overlay to change the
backing file pointers. This is _so_ broken. :-)

With your series (for the op blockers) and BdrvChild (for the operations
on the overlay) we should actually be much closer to finally remove
this. Good news.

Kevin