Am 27.10.2015 um 17:43 hat Alberto Garcia geschrieben:
> Hi, looks like we have a bug in the bdrv_reopen() code.
>
> It turns out that 'block-commit' fails if the 'top' node is not the
> active layer or its immediate backing file, and none of our test cases
> has detected that. I'm attaching one that reproduces the problem.
>
> What happens is that 'block-commit' reopens the overlay of the top
> node in read-write mode in order to update the backing file string. In
> addition to that, the 'base' image also needs to be reopened in r/w.
>
> Here's the relevant code from commit_start():
>
> if (!(orig_base_flags & BDRV_O_RDWR)) {
> reopen_queue = bdrv_reopen_queue(reopen_queue, base, NULL,
> orig_base_flags | BDRV_O_RDWR);
> }
> if (!(orig_overlay_flags & BDRV_O_RDWR)) {
> reopen_queue = bdrv_reopen_queue(reopen_queue, overlay_bs, NULL,
> orig_overlay_flags | BDRV_O_RDWR);
> }
> if (reopen_queue) {
> bdrv_reopen_multiple(reopen_queue, &local_err);
> /*...*/
> }
>
> 'base' is reopened first in r/w mode, then 'overlay_bs'. However it
> seems that the latter has the side effect or reopening 'base' again in
> read-only mode, therefore the job ends up failing with -EPERM.
>
> Just swapping the order of the bdrv_reopen_queue() calls is enough to
> fix the problem, but I'm sure this needs deeper changes in the
> bdrv_reopen() code instead.
I think this might be fixed by applying the rest of my bdrv_reopen()
patches and then converting read-only from a flag to an option. The
reason is that we really have three values here: ro, rw and inherit from
parent. This can be represented by options (false, true, missing QDict
entry), but not by flags.
I've applied your test case to my working branch so I won't forget about
this. Maybe I should really try to get the series into 2.5 then.
All that being said, having the same BDS twice in a reopen queue is
calling for trouble. I guess currently the last one wins, with all
changes made in the first call reverted even if the respective options
aren't touched by the second one. I wonder if we need to merge multiple
entries for the same BDS.
Kevin
