From 087aa59c1c0e5e930234816a5865d6643fb0978b Mon Sep 17 00:00:00 2001 From: Albert Esteve Date: Mon, 20 Feb 2023 14:22:48 +0100 Subject: [PATCH 1/4] blocksd_test: add test to extend volume Add test to extend volume size. Preallocated volumes (both raw and cow) should extend to prevent VMs from pausing when reaching the volume truesize limit. Test is used to reproduce the issue in https://bugzilla.redhat.com/2170689 Signed-off-by: Albert Esteve --- tests/storage/blocksd_test.py | 47 +++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/storage/blocksd_test.py b/tests/storage/blocksd_test.py index e3fb933e0d..f802c2551f 100644 --- a/tests/storage/blocksd_test.py +++ b/tests/storage/blocksd_test.py @@ -1857,6 +1857,53 @@ def test_reduce_volume_skipped(domain_factory, fake_task, fake_sanlock): assert dom.getVolumeSize(img_uuid, vol_id).apparentsize == vol_capacity +@requires_root +@pytest.mark.root +@pytest.mark.xfail(reason='Cow volumes are not extended.') +def test_extend_volume(domain_factory, fake_task, fake_sanlock): + """ + Test added to verify fix for https://bugzilla.redhat.com/2170689. + COW preallocated volumes should be extended when demanded. Otherwise, + VMs on preallocated disks will pause when reaching the volume truesize. + + To avoid slowness of creating loop devices and storage domains for every + test, avoid parametrized test, just create as many volumes as needed + to test, and check them in one execution. + """ + vol_formats = [ + sc.COW_FORMAT, + sc.RAW_FORMAT + ] + sd_uuid = str(uuid.uuid4()) + dom = domain_factory.create_domain(sd_uuid=sd_uuid, version=5) + + img_uuid = str(uuid.uuid4()) + vol_info = [(str(uuid.uuid4()), fmt) for fmt in vol_formats] + vol_capacity = 3 * GiB + new_capacity = 5 * GiB + + for vol_uuid, vol_format in vol_info: + dom.createVolume( + imgUUID=img_uuid, + capacity=vol_capacity, + volFormat=vol_format, + preallocate=sc.PREALLOCATED_VOL, + diskType=sc.DATA_DISKTYPE, + volUUID=vol_uuid, + desc="Base volume", + srcImgUUID=sc.BLANK_UUID, + srcVolUUID=sc.BLANK_UUID) + + # Produce and extend volume to the new capacity. + vol = dom.produceVolume(img_uuid, vol_uuid) + vol.extendSize(new_capacity) + + # Check that volume size has changed. + vol_size = dom.getVolumeSize(img_uuid, vol_uuid) + assert vol_size.truesize == new_capacity + assert vol_size.apparentsize == new_capacity + + LVM_TAG_CHARS = string.ascii_letters + "0123456789_+.-/=!:#" LVM_TAGS = [ From 5e4389a55faad315a92f1b275d86162c2c1467ea Mon Sep 17 00:00:00 2001 From: Albert Esteve Date: Mon, 20 Feb 2023 14:25:23 +0100 Subject: [PATCH 2/4] volume: allow extend cow preallocated Currently we assume in the code that cow volumes do not need to be extended in any case. However, all preallocated volumes, both cow and raw, should be extended when requested. Otherwise, cow preallocated volumes have bigger capacity than their truesize. Thus, when the size of the disk reaches the truesize, the VM will pause, which is an undesired effect. To avoid this, we relax this assumption so that only cow sparse volumes are skipped on extend calls. Bug-Url: https://bugzilla.redhat.com/2170689 Signed-off-by: Albert Esteve --- lib/vdsm/storage/volume.py | 4 ++-- tests/storage/blocksd_test.py | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/vdsm/storage/volume.py b/lib/vdsm/storage/volume.py index dfe0c2c824..268194abff 100644 --- a/lib/vdsm/storage/volume.py +++ b/lib/vdsm/storage/volume.py @@ -1381,11 +1381,11 @@ def extendSize(self, new_capacity): raise se.VolumeNonWritable(self.volUUID) volFormat = self.getFormat() - if volFormat == sc.COW_FORMAT: + if volFormat == sc.COW_FORMAT and self.getType() == sc.SPARSE_VOL: self.log.debug("skipping cow size extension for volume %s to " "capacity %s", self.volUUID, new_capacity) return - elif volFormat != sc.RAW_FORMAT: + if volFormat not in [sc.RAW_FORMAT, sc.COW_FORMAT]: raise se.IncorrectFormat(self.volUUID) # Note: This function previously prohibited extending non-leaf volumes. diff --git a/tests/storage/blocksd_test.py b/tests/storage/blocksd_test.py index f802c2551f..2785fea5de 100644 --- a/tests/storage/blocksd_test.py +++ b/tests/storage/blocksd_test.py @@ -1859,7 +1859,6 @@ def test_reduce_volume_skipped(domain_factory, fake_task, fake_sanlock): @requires_root @pytest.mark.root -@pytest.mark.xfail(reason='Cow volumes are not extended.') def test_extend_volume(domain_factory, fake_task, fake_sanlock): """ Test added to verify fix for https://bugzilla.redhat.com/2170689. From 5eb8b8b3f21ccf6a1d5e90a8f0fab27090e75650 Mon Sep 17 00:00:00 2001 From: Albert Esteve Date: Mon, 20 Feb 2023 16:08:07 +0100 Subject: [PATCH 3/4] volume: rename _extendSizeRaw Rename '_extendSizeRaw' to just '_extendSize', as COW preallocated volumes can also reach this call to extend LVs. Signed-off-by: Albert Esteve --- lib/vdsm/storage/blockVolume.py | 2 +- lib/vdsm/storage/fileVolume.py | 2 +- lib/vdsm/storage/volume.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/vdsm/storage/blockVolume.py b/lib/vdsm/storage/blockVolume.py index 15ddb14a59..149926221a 100644 --- a/lib/vdsm/storage/blockVolume.py +++ b/lib/vdsm/storage/blockVolume.py @@ -718,7 +718,7 @@ def setParentTag(self, puuid): def getMetaSlot(self): return self._manifest.getMetaSlot() - def _extendSizeRaw(self, new_capacity): + def _extendSize(self, new_capacity): # Since this method relies on lvm.extendLV (lvextend) when the # requested size is equal or smaller than the current size, the # request is siliently ignored. diff --git a/lib/vdsm/storage/fileVolume.py b/lib/vdsm/storage/fileVolume.py index 8be37d903e..960b50f1f8 100644 --- a/lib/vdsm/storage/fileVolume.py +++ b/lib/vdsm/storage/fileVolume.py @@ -734,7 +734,7 @@ def getLeaseVolumePath(self, vol_path=None): # pylint: disable=no-member return self._manifest.getLeaseVolumePath(vol_path) - def _extendSizeRaw(self, new_capacity): + def _extendSize(self, new_capacity): volPath = self.getVolumePath() cur_capacity = self.oop.os.stat(volPath).st_size diff --git a/lib/vdsm/storage/volume.py b/lib/vdsm/storage/volume.py index 268194abff..3f930ec62f 100644 --- a/lib/vdsm/storage/volume.py +++ b/lib/vdsm/storage/volume.py @@ -1416,7 +1416,7 @@ def extendSize(self, new_capacity): "Extend size for volume: " + self.volUUID, "volume", "Volume", "extendSizeFinalize", [self.sdUUID, self.imgUUID, self.volUUID])) - self._extendSizeRaw(new_capacity) + self._extendSize(new_capacity) self.syncMetadata() # update the metadata @@ -1594,7 +1594,7 @@ def newVolumeLease(cls, metaId, sdUUID, volUUID): def getImageVolumes(cls, sdUUID, imgUUID): return cls.manifestClass.getImageVolumes(sdUUID, imgUUID) - def _extendSizeRaw(self, newSize): + def _extendSize(self, newSize): raise NotImplementedError # Used only for block volume From b3c98eb94b6b8a1b047b7568e1685f11b8ce5537 Mon Sep 17 00:00:00 2001 From: Albert Esteve Date: Mon, 20 Feb 2023 16:19:28 +0100 Subject: [PATCH 4/4] blocksd_test: add test for skip extend Add a test to ensure that extendSize is correctly skipped for cow sparse volumes. Signed-off-by: Albert Esteve --- tests/storage/blocksd_test.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/storage/blocksd_test.py b/tests/storage/blocksd_test.py index 2785fea5de..7f1176b41b 100644 --- a/tests/storage/blocksd_test.py +++ b/tests/storage/blocksd_test.py @@ -1903,6 +1903,38 @@ def test_extend_volume(domain_factory, fake_task, fake_sanlock): assert vol_size.apparentsize == new_capacity +@requires_root +@pytest.mark.root +def test_extend_volume_skipped(domain_factory, fake_task, fake_sanlock): + sd_uuid = str(uuid.uuid4()) + dom = domain_factory.create_domain(sd_uuid=sd_uuid, version=5) + + img_uuid = str(uuid.uuid4()) + vol_id = str(uuid.uuid4()) + vol_capacity = 10 * GiB + new_capacity = 15 * GiB + + dom.createVolume( + imgUUID=img_uuid, + capacity=vol_capacity, + volFormat=sc.COW_FORMAT, + preallocate=sc.SPARSE_VOL, + diskType=sc.DATA_DISKTYPE, + volUUID=vol_id, + desc="Base volume", + srcImgUUID=sc.BLANK_UUID, + srcVolUUID=sc.BLANK_UUID) + + # Produce and extend volume to the new capacity, but we skip extends for + # cow sparse volumes. + vol = dom.produceVolume(img_uuid, vol_id) + vol.extendSize(new_capacity) + + # Check that volume size has not changed. + assert dom.getVolumeSize(img_uuid, vol_id).truesize <= vol_capacity + assert dom.getVolumeSize(img_uuid, vol_id).apparentsize <= vol_capacity + + LVM_TAG_CHARS = string.ascii_letters + "0123456789_+.-/=!:#" LVM_TAGS = [