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 <hreitz@redhat.com>
Message-Id: <20220314162719.65384-2-hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>

1 files changed, 55 insertions, 1 deletions

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,