On Wed 15 Apr 2015 06:09:18 PM CEST, Max Reitz wrote:
>> + orig_bs_flags = bdrv_get_flags(bs);
>> + if (!(orig_bs_flags & BDRV_O_RDWR)) {
>
> I feel like we don't want to do this if we're not streaming to an
> intermediate layer but to the top layer (because that means there is
> some reason for the BDS to be read-only beyond it just being a backing
> BDS).
I didn't think about this... that I can fix easily, but I wonder what's
the scenario where the top layer is read-only.
>> + /* Block all intermediate nodes between bs and base, because they
>> + * will disappear from the chain after this operation */
>
> Hm, do we really need to? There's a difference between "it doesn't
> make sense, but it works if you want to" and "it will
> break". Shouldn't it be enough that the intermediate nodes are all
> read-only anyway (hopefully)?
I don't think that the fact that intermediate nodes are read-only
changes anything, because some operations (this one, block-commit)
reopens them in r/w mode.
> But then again, it probably won't hurt and I don't really want to
> think about the implications of trying to run a block-commit or a
> separate block-stream on the chain...
I actually tried with scenarios such as A>B>C>D>E, streaming from B to E
and from A to C simultaneously, and it seems to work (at least I didn't
see any obvious problems), but I don't think we want to support that
because a) I don't see the use case and b) we're likely opening a can of
worms.
What we can do is stream from A to C and from D to E at the same time,
that would be allowed with these patches and I don't see any obvious
reason why it would fail.
Berto
