| Message ID | 15af03087adc989794fc74ee671502a1881a62eb.1553619891.git.berto@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | freeze the backing chain earlier in stream_start() | expand |
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 <berto@igalia.com> > --- > 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 <vsementsov@virtuozzo.com>
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); }
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 <berto@igalia.com> --- block/stream.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)