[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH for-4.0 2/3] block: freeze the backing chain ear
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-block] [PATCH for-4.0 2/3] block: freeze the backing chain earlier in stream_start() |
Date: |
Thu, 28 Mar 2019 10:01:42 +0000 |
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 <address@hidden>
> ---
> 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 <address@hidden>
--
Best regards,
Vladimir