26.03.2019 20:07, Alberto Garcia wrote: > Commit 6585493369819a48d34a86d57ec6b97cb5cd9bc0 added code to freeze > the backing chain from 'top' to 'base' for the duration of the > block-stream job. > > The problem is that the freezing happens too late in stream_start(): > during the bdrv_reopen_set_read_only() call earlier in that function > another job can jump in and remove the base image. If that happens we > have an invalid chain and QEMU crashes. > > This patch puts the bdrv_freeze_backing_chain() call at the beginning > of the function. > > Signed-off-by: Alberto Garcia <[email protected]> > --- > block/stream.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/block/stream.c b/block/stream.c > index 6253c86fae..6502468f88 100644 > --- a/block/stream.c > +++ b/block/stream.c > @@ -238,11 +238,16 @@ void stream_start(const char *job_id, BlockDriverState > *bs, > BlockDriverState *iter; > bool bs_read_only; > > + if (bdrv_freeze_backing_chain(bs, base, errp) < 0) { > + return; > + } > + > /* Make sure that the image is opened in read-write mode */ > bs_read_only = bdrv_is_read_only(bs); > if (bs_read_only) { > if (bdrv_reopen_set_read_only(bs, false, errp) != 0) { > - return; > + bs_read_only = false; > + goto fail; > } > } > > @@ -269,11 +274,6 @@ void stream_start(const char *job_id, BlockDriverState > *bs, > &error_abort); > } > > - if (bdrv_freeze_backing_chain(bs, base, errp) < 0) { > - job_early_fail(&s->common.job); > - goto fail; > - } > - > s->base = base; > s->backing_file_str = g_strdup(backing_file_str); > s->bs_read_only = bs_read_only; > @@ -285,6 +285,7 @@ void stream_start(const char *job_id, BlockDriverState > *bs, > return; > > fail: > + bdrv_unfreeze_backing_chain(bs, base); > if (bs_read_only) { > bdrv_reopen_set_read_only(bs, true, NULL); > } >
A bit nicer, if rollback in reverse order, unfreeze at last. Anyway, Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]> -- Best regards, Vladimir
