diff options
| -rw-r--r-- | block/qcow2-refcount.c | 7 | ||||
| -rw-r--r-- | block/qcow2-snapshot.c | 61 |
2 files changed, 50 insertions, 18 deletions
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 9605367777..2db2ede3d1 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -700,6 +700,10 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs, l2_table = NULL; l1_table = NULL; l1_size2 = l1_size * sizeof(uint64_t); + + /* WARNING: qcow2_snapshot_goto relies on this function not using the + * l1_table_offset when it is the current s->l1_table_offset! Be careful + * when changing this! */ if (l1_table_offset != s->l1_table_offset) { if (l1_size2 != 0) { l1_table = g_malloc0(align_offset(l1_size2, 512)); @@ -819,7 +823,8 @@ fail: qcow2_cache_set_writethrough(bs, s->refcount_block_cache, old_refcount_writethrough); - if (l1_modified) { + /* Update L1 only if it isn't deleted anyway (addend = -1) */ + if (addend >= 0 && l1_modified) { for(i = 0; i < l1_size; i++) cpu_to_be64s(&l1_table[i]); if (bdrv_pwrite_sync(bs->file, l1_table_offset, l1_table, diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 1ee22eb28a..9fb3ff0253 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -387,6 +387,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) int i, snapshot_index; int cur_l1_bytes, sn_l1_bytes; int ret; + uint64_t *sn_l1_table = NULL; /* Search the snapshot */ snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_id); @@ -395,14 +396,6 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) } sn = &s->snapshots[snapshot_index]; - /* Decrease refcount of clusters of current L1 table. - * FIXME This is too early! */ - ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, - s->l1_size, -1); - if (ret < 0) { - goto fail; - } - /* * Make sure that the current L1 table is big enough to contain the whole * L1 table of the snapshot. If the snapshot L1 table is smaller, the @@ -416,33 +409,66 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) cur_l1_bytes = s->l1_size * sizeof(uint64_t); sn_l1_bytes = sn->l1_size * sizeof(uint64_t); - if (cur_l1_bytes > sn_l1_bytes) { - memset(s->l1_table + sn->l1_size, 0, cur_l1_bytes - sn_l1_bytes); - } - /* * Copy the snapshot L1 table to the current L1 table. * * Before overwriting the old current L1 table on disk, make sure to * increase all refcounts for the clusters referenced by the new one. + * Decrease the refcount referenced by the old one only when the L1 + * table is overwritten. */ - ret = bdrv_pread(bs->file, sn->l1_table_offset, s->l1_table, sn_l1_bytes); + sn_l1_table = g_malloc0(cur_l1_bytes); + + ret = bdrv_pread(bs->file, sn->l1_table_offset, sn_l1_table, sn_l1_bytes); + if (ret < 0) { + goto fail; + } + + ret = qcow2_update_snapshot_refcount(bs, sn->l1_table_offset, + sn->l1_size, 1); if (ret < 0) { goto fail; } - ret = bdrv_pwrite_sync(bs->file, s->l1_table_offset, s->l1_table, + ret = bdrv_pwrite_sync(bs->file, s->l1_table_offset, sn_l1_table, cur_l1_bytes); if (ret < 0) { goto fail; } + /* + * Decrease refcount of clusters of current L1 table. + * + * At this point, the in-memory s->l1_table points to the old L1 table, + * whereas on disk we already have the new one. + * + * qcow2_update_snapshot_refcount special cases the current L1 table to use + * the in-memory data instead of really using the offset to load a new one, + * which is why this works. + */ + ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, + s->l1_size, -1); + + /* + * Now update the in-memory L1 table to be in sync with the on-disk one. We + * need to do this even if updating refcounts failed. + */ for(i = 0;i < s->l1_size; i++) { - be64_to_cpus(&s->l1_table[i]); + s->l1_table[i] = be64_to_cpu(sn_l1_table[i]); } - /* FIXME This is too late! */ - ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 1); + if (ret < 0) { + goto fail; + } + + g_free(sn_l1_table); + sn_l1_table = NULL; + + /* + * Update QCOW_OFLAG_COPIED in the active L1 table (it may have changed + * when we decreased the refcount of the old snapshot. + */ + ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 0); if (ret < 0) { goto fail; } @@ -456,6 +482,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) return 0; fail: + g_free(sn_l1_table); return ret; } |