Skip to content

Commit

Permalink
storage: Use offset on (cold) merge instead of mesure required size.
Browse files Browse the repository at this point in the history
When we commit a snapshot into a base volume, we first measure do a
measure and then extend the base image with the required size.
But this is not a valid assumption.

The required size output from measure gives you the size that would be
required for a NEW volume, but not the size required to commit the top
volume into the base volume.

While the final required since on a new volume might be lower than the
size of the base volume. It might be required to extend the base volume
to fit the comitted data.

See for example: https://gitlab.com/qemu-project/qemu/-/issues/2369

Base vol: 13421772800 bytes / end offset 13421772800
Top vol:   6174015488 bytes / end offset  3488350208

Measure gives us required size: 3491168256

But if we commit the top volume, the base volume grows to 13504806912
bytes.

Therefor we check the end offset of each volume, and the sum of that is
the required size.
We might allocate to much here, but the finalize shrinks the LV again to
the end offset of the commited base image. So only some additional space
during commit is needed.

Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
  • Loading branch information
dupondje committed May 31, 2024
1 parent 1b1a210 commit 5ca7949
Showing 1 changed file with 14 additions and 2 deletions.
16 changes: 14 additions & 2 deletions lib/vdsm/storage/merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,17 @@ def _extend_base_allocation(base_vol, top_vol):
if not (base_vol.is_block() and base_vol.getFormat() == sc.COW_FORMAT):
return

# Get the latest offset
log.debug("Checking top=%r", top_vol.volUUID)
top_check = qemuimg.check(top_vol.getVolumePath(), qemuimg.FORMAT.QCOW2)
log.debug("Checking base=%r", base_vol.volUUID)
base_check = qemuimg.check(base_vol.getVolumePath(), qemuimg.FORMAT.QCOW2)

log.debug("Offset result: top: %s - base %s",
top_check["offset"], base_check["offset"])

# Measure the subchain from top to base. This gives us the required
# allocation for merging top into base.
# bitmap space
log.debug("Measuring sub chain top=%r base=%r",
top_vol.volUUID, base_vol.volUUID)
measure = qemuimg.measure(
Expand All @@ -213,7 +222,10 @@ def _extend_base_allocation(base_vol, top_vol):
# When merging we always copy the bitmaps from the top to base. Measure
# gives us the size of the bitmaps in top *and* base, so this may allocate
# more than needed, but bitmaps are small so it should be good enough.
required_size = measure["required"] + measure.get("bitmaps", 0)
# But but the data itself we sum the offset of both images to make sure we
# allocate enough space. This as measure does not give us a reliable size.
required_size = top_check["offset"] + base_check["offset"] + \
measure.get("bitmaps", 0)

# If the top volume is leaf, the base volume will become leaf after the
# merge, so it needs more space.
Expand Down

0 comments on commit 5ca7949

Please sign in to comment.