Ensure DRBD sync before resizing volumes on thick provisoned linstor SR #105

klmp200 · 2025-10-28T09:31:37Z

When resizing a VDI on a thick provisioned linstor SR, we need to wait for DRBD to be up to date.
This is dependent on previous operations that we can't know.
If we wait too much, we stop and return an error to the user letting him know that he needs to try later.

Also fix an issue when resizing raw images.

drivers/linstorvolumemanager.py

Millefeuille42 · 2025-11-05T14:41:42Z

drivers/LinstorSR.py

+        def compute_vdi_sizes(vdi: LinstorVDI) -> Tuple[int, int]:
+            if vdi.vdi_type == vhdutil.VDI_TYPE_RAW:
+                return vdi.size, LinstorVolumeManager.round_up_volume_size(size)
+
+            old_volume_size = vdi.utilisation
+            if vdi.sr._provisioning == 'thin':
+                # VDI is currently deflated, so keep it deflated.
+                return old_volume_size, old_volume_size
+
+            return old_volume_size, LinstorVhdUtil.compute_volume_size(size, vdi.vdi_type)
+
+        def resize_vdi(vdi: LinstorVDI, old_volume_size: int, new_volume_size) -> None:
+            if vdi.vdi_type == vhdutil.VDI_TYPE_RAW:
+                vdi._linstor.resize_volume(vdi.uuid, new_volume_size)
+                return
+
+            if new_volume_size != old_volume_size:
+                vdi.sr._vhdutil.inflate(
+                    vdi.sr._journaler, vdi.uuid, vdi.path,
+                    new_volume_size, old_volume_size
+                )
+            vdi.sr._vhdutil.set_size_virt_fast(vdi.path, size)
+


Not really against it but the refactor seems a bit out of scope here.

I've put this refactor in it's dedicated commit

Refactoring is always a good idea, but personally, I don't think we should use nested functions that are only called once. A comment above the blocks that seem complex to you (even with numbering) seems more appropriate here.

This has been replaced by comments

Wescoeur · 2025-11-13T00:09:53Z

drivers/LinstorSR.py

+        def compute_vdi_sizes(vdi: LinstorVDI) -> Tuple[int, int]:
+            if vdi.vdi_type == vhdutil.VDI_TYPE_RAW:
+                return vdi.size, LinstorVolumeManager.round_up_volume_size(size)
+
+            old_volume_size = vdi.utilisation
+            if vdi.sr._provisioning == 'thin':
+                # VDI is currently deflated, so keep it deflated.
+                return old_volume_size, old_volume_size
+
+            return old_volume_size, LinstorVhdUtil.compute_volume_size(size, vdi.vdi_type)
+
+        def resize_vdi(vdi: LinstorVDI, old_volume_size: int, new_volume_size) -> None:
+            if vdi.vdi_type == vhdutil.VDI_TYPE_RAW:
+                vdi._linstor.resize_volume(vdi.uuid, new_volume_size)
+                return
+
+            if new_volume_size != old_volume_size:
+                vdi.sr._vhdutil.inflate(
+                    vdi.sr._journaler, vdi.uuid, vdi.path,
+                    new_volume_size, old_volume_size
+                )
+            vdi.sr._vhdutil.set_size_virt_fast(vdi.path, size)
+


Refactoring is always a good idea, but personally, I don't think we should use nested functions that are only called once. A comment above the blocks that seem complex to you (even with numbering) seems more appropriate here.

Wescoeur · 2025-11-13T00:18:45Z

drivers/linstorvolumemanager.py

+        # We can't resize anything until DRBD is up to date
+        # We wait here for 5min max and raise an easy to understand error for the user
+        # 5min is an arbitrary time, it's impossible to get a fit all situation value
+        # and it's currently impossible to know how much time we have to wait
+        # This is mostly an issue for thick provisioning, thin isn't affected


I think we'll need to discuss comment formatting for the new stack, especially regarding period at the end of sentences. I'm a fan of this for pydocstyle, but also for comments. :)

I've added proper punctuation

Wescoeur · 2025-11-13T00:19:57Z

drivers/linstorvolumemanager.py

+        try:
+            self._linstor.resource_dfn_wait_synced(volume_name, wait_interval=1.0, timeout=60*5)
+        except linstor.LinstorTimeoutError:
+            raise linstor.LinstorTimeoutError(


Suggested change

raise linstor.LinstorTimeoutError(

raise LinstorVolumeManagerError(

Wescoeur · 2025-11-13T00:23:06Z

drivers/linstorvolumemanager.py

+        result = self._linstor.volume_dfn_modify(
+            rsc_name=volume_name,
+            volume_nr=0,
+            size=new_size
+        )


I think we need to keep the initial call before waiting for the sync to complete. The idea would then be to display a message saying that we need to wait, which would create a sort of block/symmetry with your message suggestion "DRBD is up to date, syncing took...".

Experience proves that we do need to wait, especially for early steps. That's why we had the previous retry mechanism.

Either we keep the previous process which is a bit bruteforce or we just wait before but we can't keep both.
I don't see the benefit of calling the function if we know that it's gonna crash anyway.

I think we should discuss this :)

I think we need to discuss this, I'm not sure this would be benificial

fix resize typo for linstor on raw VDI Signed-off-by: Antoine Bartuccio <antoine.bartuccio@vates.tech>

Avoid most common drbd sync errors during resize with a cleaner wait mechanism and friendly user error Signed-off-by: Antoine Bartuccio <antoine.bartuccio@vates.tech>

klmp200 force-pushed the linstor_resize_retry branch from d5c806e to 545ac97 Compare October 28, 2025 09:31

klmp200 requested review from Nambrok and Wescoeur October 28, 2025 09:31

klmp200 requested a review from Millefeuille42 November 5, 2025 08:43

Millefeuille42 reviewed Nov 5, 2025

View reviewed changes

klmp200 force-pushed the linstor_resize_retry branch from 545ac97 to 6fc3638 Compare November 7, 2025 08:53

klmp200 requested a review from Millefeuille42 November 7, 2025 08:53

Millefeuille42 approved these changes Nov 7, 2025

View reviewed changes

Wescoeur requested changes Nov 13, 2025

View reviewed changes

klmp200 force-pushed the linstor_resize_retry branch from 6fc3638 to 24dece5 Compare November 14, 2025 09:18

klmp200 requested a review from Wescoeur November 14, 2025 09:18

klmp200 force-pushed the linstor_resize_retry branch from 24dece5 to ec1f735 Compare November 14, 2025 09:25

klmp200 added 2 commits November 14, 2025 15:17

fix(linstor): typo on resize for linstor raw VDI

8a12be0

fix resize typo for linstor on raw VDI Signed-off-by: Antoine Bartuccio <antoine.bartuccio@vates.tech>

fix(linstor): ensure that drbd is synced up before resizing

1b531eb

Avoid most common drbd sync errors during resize with a cleaner wait mechanism and friendly user error Signed-off-by: Antoine Bartuccio <antoine.bartuccio@vates.tech>

klmp200 force-pushed the linstor_resize_retry branch from ec1f735 to 1b531eb Compare November 14, 2025 14:18

	raise linstor.LinstorTimeoutError(
	raise LinstorVolumeManagerError(

Ensure DRBD sync before resizing volumes on thick provisoned linstor SR #105

Are you sure you want to change the base?

Ensure DRBD sync before resizing volumes on thick provisoned linstor SR #105

Uh oh!

Conversation

klmp200 commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

klmp200 commented Oct 28, 2025 •

edited

Loading