Re: [Qemu-block] [Qemu-devel] [PATCH 11/34] block: Allow references for backing files

[Paolo Bonzini]

On 27/05/2015 15:30, Kevin Wolf wrote:
> So it would be even more important to figure out what to do with
> bdrv_swap().
> 
> Paolo, you added that bunch of assertions to bdrv_swap() when you
> introduced it in commit 4ddc07ca. Did you just add them because they
> were true at the time, or is anything actually relying on these
> invariants?

When I added bdrv_swap, the idea was to bs_new was either being closed
afterwards,  or (for bdrv_append) it would be the backing file of bs_top.

In either case, the assertions would make sure that nothing would break
when further manipulating bs_new.

Are they being relied on?  Probably not, but some violations would be
just weird, such as a device attached to a non-topmost BDS, or a job
attached to the old version of a mirrored disk.

> What do we actually want to happen? Let's assume a small chain of
> backing files:
> 
>         +---------- A-BlockBackend
>         |
>         |    +----- B-BlockBackend
>         |    |
> base <- A <- B
> 
> After completing the commit operation, what we want to have for the BB
> of B is clear:
> 
>         +---------- B-BlockBackend
>         |
> base <- A
> 
> If we just remove the assertions that are currently present in
> bdrv_swap(), I think we'd end up with this:
> 
>              +----- A-BlockBackend
>              |
>         +---------- B-BlockBackend
>         |    |
> base <- A <- B
> 
> This is probably surprising for the user if they ever look at
> A-BlockBackend again. It's also surprising because (unlike the case
> without a BB for A) B is actually still referenced and therefore the
> file stays opened.
> 
> I suspect what we really want is this (which is not swapping):
> 
>         +---------- A-BlockBackend
>         |
>         +---------- B-BlockBackend
>         |
> base <- A
> 
> Both BBs point to the same BDS now and B is actually closed.

Correct.  Swapping was just a trick to do things underneath the device's
feet, effectively emulating BlockBackends.

Since I'm not very much up-to-date with the block device graph stuff, do
BDSes have a list of BlockBackends that point to them, so that they can
redirect all those BlockBackends to the backing file?

Paolo