From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Hanna Reitz Date: Mon, 14 Mar 2022 17:27:18 +0100 Subject: [PATCH] block/vmdk: Fix reopening bs->file VMDK disk data is stored in extents, which may or may not be separate from bs->file. VmdkExtent.file points to where they are stored. Each that is stored in bs->file will simply reuse the exact pointer value of bs->file. (That is why vmdk_free_extents() will unref VmdkExtent.file (e->file) only if e->file != bs->file.) Reopen operations can change bs->file (they will replace the whole BdrvChild object, not just the BDS stored in that BdrvChild), and then we will need to change all .file pointers of all such VmdkExtents to point to the new BdrvChild. In vmdk_reopen_prepare(), we have to check which VmdkExtents are affected, and in vmdk_reopen_commit(), we can modify them. We have to split this because: - The new BdrvChild is created only after prepare, so we can change VmdkExtent.file only in commit - In commit, there no longer is any (valid) reference to the old BdrvChild object, so there would be nothing to compare VmdkExtent.file against to see whether it was equal to bs->file before reopening (There is BDRVReopenState.old_file_bs, but the old bs->file BdrvChild's .bs pointer will be NULL-ed when the new BdrvChild is created, and so we cannot compare VmdkExtent.file->bs against BDRVReopenState.old_file_bs) Signed-off-by: Hanna Reitz Message-Id: <20220314162719.65384-2-hreitz@redhat.com> Signed-off-by: Kevin Wolf (cherry-picked from commit 6d17e2879854d7d0e623c06a9286085e97bf3545) Signed-off-by: Fabian Ebner --- block/vmdk.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/block/vmdk.c b/block/vmdk.c index 37c0946066..38e5ab3806 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -178,6 +178,10 @@ typedef struct BDRVVmdkState { char *create_type; } BDRVVmdkState; +typedef struct BDRVVmdkReopenState { + bool *extents_using_bs_file; +} BDRVVmdkReopenState; + typedef struct VmdkMetaData { unsigned int l1_index; unsigned int l2_index; @@ -400,15 +404,63 @@ static int vmdk_is_cid_valid(BlockDriverState *bs) return 1; } -/* We have nothing to do for VMDK reopen, stubs just return success */ static int vmdk_reopen_prepare(BDRVReopenState *state, BlockReopenQueue *queue, Error **errp) { + BDRVVmdkState *s; + BDRVVmdkReopenState *rs; + int i; + assert(state != NULL); assert(state->bs != NULL); + assert(state->opaque == NULL); + + s = state->bs->opaque; + + rs = g_new0(BDRVVmdkReopenState, 1); + state->opaque = rs; + + /* + * Check whether there are any extents stored in bs->file; if bs->file + * changes, we will need to update their .file pointers to follow suit + */ + rs->extents_using_bs_file = g_new(bool, s->num_extents); + for (i = 0; i < s->num_extents; i++) { + rs->extents_using_bs_file[i] = s->extents[i].file == state->bs->file; + } + return 0; } +static void vmdk_reopen_clean(BDRVReopenState *state) +{ + BDRVVmdkReopenState *rs = state->opaque; + + g_free(rs->extents_using_bs_file); + g_free(rs); + state->opaque = NULL; +} + +static void vmdk_reopen_commit(BDRVReopenState *state) +{ + BDRVVmdkState *s = state->bs->opaque; + BDRVVmdkReopenState *rs = state->opaque; + int i; + + for (i = 0; i < s->num_extents; i++) { + if (rs->extents_using_bs_file[i]) { + s->extents[i].file = state->bs->file; + } + } + + vmdk_reopen_clean(state); +} + +static void vmdk_reopen_abort(BDRVReopenState *state) +{ + vmdk_reopen_clean(state); +} + static int vmdk_parent_open(BlockDriverState *bs) { char *p_name; @@ -3072,6 +3124,8 @@ static BlockDriver bdrv_vmdk = { .bdrv_open = vmdk_open, .bdrv_co_check = vmdk_co_check, .bdrv_reopen_prepare = vmdk_reopen_prepare, + .bdrv_reopen_commit = vmdk_reopen_commit, + .bdrv_reopen_abort = vmdk_reopen_abort, .bdrv_child_perm = bdrv_default_perms, .bdrv_co_preadv = vmdk_co_preadv, .bdrv_co_pwritev = vmdk_co_pwritev,