oVirt · nirs · Nov 28, 2022 · Nov 17, 2022 · Nov 22, 2022 · Nov 22, 2022
@@ -125,6 +125,27 @@ def remove_bitmap(vol_path, bitmap):
             vol_path=vol_path)
 
 
+def prune_bitmaps(base_path, top_path):
+    """
+    Prune all the stale bitmaps from the base volume path.
+    A bitmap is considered stale if it appears only in the base volume,
+    and is missing or invalid in the top volume.
+
+    Args:
+        base_path (str): Path to the base volume
+        top_path (str): Path to the top volume
+    """
+    base_bitmaps = _query_bitmaps(base_path)
+    valid_top_bitmaps = _query_bitmaps(top_path, filter=_valid)
+
+    stale_bitmaps = [
+        name for name in base_bitmaps if name not in valid_top_bitmaps]
+    if stale_bitmaps:
+        log.warning("Prune stale bitmaps %s from %r", stale_bitmaps, base_path)
+        for bitmap in stale_bitmaps:
+            remove_bitmap(base_path, bitmap)
+
+
 def clear_bitmaps(vol_path):
     """
     Remove all the bitmaps from the given volume path

@@ -22,6 +22,7 @@
 from vdsm.common import properties
 from vdsm.common.units import MiB
 
+from vdsm.storage import bitmaps
 from vdsm.storage import constants as sc
 from vdsm.storage import exception as se
 from vdsm.storage import guarded
@@ -158,6 +159,15 @@ def prepare(subchain):
     log.info("Preparing subchain %s for merge", subchain)
     with guarded.context(subchain.locks):
         with subchain.prepare():
+            # There may be stale bitmaps in base if they were left
+            # after failed delete bitmap operation. These bitmaps may
+            # cause the bitmaps merge operation to fail with ENOSPC.
+            # As there is not a reliable way to calculate the size of
+            # these bitmaps, and they are invalid and can never be used
+            # for incremental backup, we prune them before the commit.
+            bitmaps.prune_bitmaps(subchain.base_vol.getVolumePath(),
+                                  subchain.top_vol.getVolumePath())
+
             _update_base_capacity(subchain.base_vol,
                                   subchain.top_vol)
             _extend_base_allocation(subchain.base_vol,

@@ -7,6 +7,8 @@
 
 from collections import namedtuple
 
+from . import qemuio
+
 from vdsm.common import cmdutils
 from vdsm.common import exception
 from vdsm.common.units import MiB
@@ -234,6 +236,78 @@ def test_remove_non_existing_bitmap_succeed(tmp_mount, vol_chain):
     bitmaps.remove_bitmap(vol_chain.top_vol, 'bitmap')
 
 
+def test_prune_stale_bitmaps(tmp_mount, vol_chain):
+    # Add valid bitmap to volumes
+    bitmap = 'valid-bitmap'
+    qemuimg.bitmap_add(vol_chain.base_vol, bitmap).run()
+    qemuimg.bitmap_add(vol_chain.top_vol, bitmap).run()
+
+    # Add stale bitmaps to base volume
+    for i in range(5):
+        qemuimg.bitmap_add(vol_chain.top_vol, f"stale-bitmap-{i}").run()
+
+    bitmaps.prune_bitmaps(vol_chain.base_vol, vol_chain.top_vol)
+
+    info = qemuimg.info(vol_chain.base_vol)
+    assert info['format-specific']['data']['bitmaps'] == [
+        {
+            "flags": ['auto'],
+            "name": bitmap,
+            "granularity": 65536
+        },
+    ]
+
+
+def test_prune_disabled_bitmaps(tmp_mount, vol_chain):
+    # Add valid bitmap to volumes
+    bitmap = 'valid-bitmap'
+    qemuimg.bitmap_add(vol_chain.base_vol, bitmap).run()
+    qemuimg.bitmap_add(vol_chain.top_vol, bitmap).run()
+
+    # Add disabled bitmaps to top volume
+    for i in range(5):
+        bitmap_disabled = f"disabled-bitmap-{i}"
+        qemuimg.bitmap_add(vol_chain.base_vol, bitmap_disabled).run()
+        qemuimg.bitmap_add(
+            vol_chain.top_vol, bitmap_disabled, enable=False).run()
+
+    bitmaps.prune_bitmaps(vol_chain.base_vol, vol_chain.top_vol)
+
+    info = qemuimg.info(vol_chain.base_vol)
+    assert info['format-specific']['data']['bitmaps'] == [
+        {
+            "flags": ['auto'],
+            "name": bitmap,
+            "granularity": 65536
+        },
+    ]
+
+
+def test_prune_in_use_bitmaps(tmp_mount, vol_chain):
+    # Add inconsistent "in-use" bitmaps to volumes
+    for i in range(5):
+        bitmap_in_use = f"in-use-bitmap-{i}"
+        qemuimg.bitmap_add(vol_chain.base_vol, bitmap_in_use).run()
+        qemuimg.bitmap_add(vol_chain.to
8000
p_vol, bitmap_in_use).run()
+    qemuio.abort(vol_chain.top_vol)
+
+    # Add valid bitmap to volumes
+    bitmap = 'valid-bitmap'
+    qemuimg.bitmap_add(vol_chain.base_vol, bitmap).run()
+    qemuimg.bitmap_add(vol_chain.top_vol, bitmap).run()
+
+    bitmaps.prune_bitmaps(vol_chain.base_vol, vol_chain.top_vol)
+
+    info = qemuimg.info(vol_chain.base_vol)
+    assert info['format-specific']['data']['bitmaps'] == [
+        {
+            "flags": ['auto'],
+            "name": bitmap,
+            "granularity": 65536
+        },
+    ]
+
+
 def test_clear_bitmaps(tmp_mount, vol_chain):
     bitmap_1 = 'bitmap_1'
     bitmap_2 = 'bitmap_2'

@@ -13,29 +13,36 @@
 
 import pytest
 
+from vdsm import jobs
 from vdsm import utils
 from vdsm.config import config
 from vdsm.common.units import MiB, GiB
 from vdsm.storage import blockSD
 from vdsm.storage import clusterlock
 from vdsm.storage import constants as sc
 from vdsm.storage import exception as se
+from vdsm.storage import guarded
 from vdsm.storage import image
 from vdsm.storage import lvm
 from vdsm.storage import qemuimg
+from vdsm.storage import merge
 from vdsm.storage import sanlock_direct
 from vdsm.storage import sd
 from vdsm.storage import sp
 from vdsm.storage.sdc import sdCache
+from vdsm.storage.sdm.api import merge as api_merge
 from vdsm.storage.spbackends import StoragePoolDiskBackend
 
 from . import qemuio
 from . marks import requires_root
 
+from fakelib import FakeNotifier
+from fakelib import FakeScheduler
 from storage.storagefakelib import fake_spm
 from storage.storagefakelib import fake_vg
 from storage.storagefakelib import FakeDomainMonitor
 from storage.storagefakelib import FakeTaskManager
+from storage.storagefakelib import fake_guarded_context
 
 TESTDIR = os.path.dirname(__file__)
 CHUNK_SIZE_MB = config.getint("irs", "volume_utilization_chunk_mb")
@@ -1244,6 +1251,124 @@ def test_create_volume_with_new_bitmap(
     ]
 
 
+@requires_root
+@pytest.mark.root
+@pytest.mark.parametrize("stale_bitmaps", [
+    6,   # required size < lv size
+    11,  # required size = lv size
+    15,  # required size > lv size
+])
+def test_merge_with_bitmap(
+        tmp_storage, tmp_repo, fake_access, fake_rescan, tmp_db, fake_task,
+        fake_sanlock, monkeypatch, stale_bitmaps, caplog):
+    sd_id = str(uuid.uuid4())
+
+    dev = tmp_storage.create_device(10 * GiB)
+    lvm.createVG(sd_id, [dev], blockSD.STORAGE_UNREADY_DOMAIN_TAG, 128)
+    vg = lvm.getVG(sd_id)
+
+    dom = blockSD.BlockStorageDomain.create(
+        sdUUID=sd_id,
+        domainName="domain",
+        domClass=sd.DATA_DOMAIN,
+        vgUUID=vg.uuid,
+        version=4,
+        storageType=sd.ISCSI_DOMAIN)
+
+    sdCache.knownSDs[sd_id] = blockSD.findDomain
+    sdCache.manuallyAddDomain(dom)
+
+    dom.refresh()
+    dom.attach(tmp_repo.pool_id)
+
+    img_id = str(uuid.uuid4())
+    base_vol_uuid = str(uuid.uuid4())
+    top_vol_uuid = str(uuid.uuid4())
+
+    # Create base volume.
+    dom.createVolume(
+        imgUUID=img_id,
+        capacity=128 * MiB,
+        volFormat=sc.COW_FORMAT,
+        preallocate=sc.SPARSE_VOL,
+        diskType='DATA',
+        volUUID=base_vol_uuid,
+        desc="Test volume",
+        srcImgUUID=sc.BLANK_UUID,
+        srcVolUUID=sc.BLANK_UUID)
+
+    base_vol = dom.produceVolume(img_id, base_vol_uuid)
+
+    # Create top volume with bitmaps.
+    dom.createVolume(
+        imgUUID=img_id,
+        capacity=128 * MiB,
+        volFormat=sc.COW_FORMAT,
+        preallocate=sc.SPARSE_VOL,
+        diskType='DATA',
+        volUUID=top_vol_uuid,
+        desc="Test top volume",
+        srcImgUUID=base_vol.imgUUID,
+        srcVolUUID=base_vol.volUUID)
+
+    top_vol = dom.produceVolume(img_id, top_vol_uuid)
+
+    # Create leaf volume.
+    dom.createVolume(
+        imgUUID=img_id,
+        capacity=128 * MiB,
+        volFormat=sc.COW_FORMAT,
+        preallocate=sc.SPARSE_VOL,
+        diskType='DATA',
+        volUUID=str(uuid.uuid4()),
+        desc="Test top volume",
+        srcImgUUID=top_vol.imgUUID,
+        srcVolUUID=top_vol.volUUID)
+
+    host_id = 0
+    subchain_info = dict(sd_id=base_vol.sdUUID,
+                         img_id=base_vol.imgUUID,
+                         base_id=base_vol.volUUID,
+                         top_id=top_vol.volUUID,
+                         base_generation=0)
+    subchain = merge.SubchainInfo(subchain_info, host_id)
+
+    # Nearly fill the volume with data.
+    top_vol.prepare()
+    qemuio.write_pattern(
+        top_vol.getVolumePath(),
+        qemuimg.FORMAT.QCOW2,
+        offset=0,
+        len=126 * MiB)
+    # Add good bitmap to top.
+    qemuimg.bitmap_add(base_vol.getVolumePath(), "good-bitmap").run()
+    top_vol.teardown(sd_id, top_vol_uuid)
+
+    # Add bitmaps to the base volume after creating top volume.
+    base_vol.prepare()
+    for i in range(stale_bitmaps):
+        qemuimg.bitmap_add(base_vol.getVolumePath(), f"stale-bitmap{i}").run()
+    base_vol.teardown(sd_id, base_vol_uuid)
+
+    with monkeypatch.context() as mc:
+        # TODO: check why this context fake is required. Otherwise, we need to
+        # acquire the host id, which causes other tests to fail.
+        mc.setattr(guarded, 'context', fake_guarded_context())
+        # Prepare for merge, should expand the base volume.
+        merge.prepare(subchain)
+        # Run the actual merge.
+        jobs.start(FakeScheduler(), FakeNotifier())
+        caplog.clear()
+        job = api_merge.Job(str(uuid.uuid4()), subchain, merge_bitmaps=True)
+        job.run()
+        merge.finalize(subchain)
+
+    # Merge ended correctly.
+    for record in caplog.records:
+        assert record.levelname != "WARNING"
+    assert job.status == jobs.STATUS.DONE
+
+
 @requires_root
 @pytest.mark.root
 def test_fail_clone_bitmaps_v3_domain(