oVirt / vdsm

The Virtual Desktop Server Manager
GNU General Public License v2.0
160 stars 201 forks source link

volume: allow extend cow preallocated #379

Closed aesteve-rh closed 1 year ago

aesteve-rh commented 1 year ago

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 aesteve@redhat.com

aesteve-rh commented 1 year ago

/ost

aesteve-rh commented 1 year ago

Verified in my local setup doing:

  1. Create a new 10GiB Preallocated disk with Enable Incremental Backup
  2. Virtual Size and Actual Size are 10GiB
  3. Edit disk and extend by 5GiB
  4. Results: a) Without the fix: Actual Size is 15GiB, but Virtual Size remains at 10GiB b) With the fix: both Actual Size and Virtual Size are expanded to 15GiB
aesteve-rh commented 1 year ago

/ost

nirs commented 1 year ago

Verified in my local setup doing:

  1. Create a new 10GiB Preallocated disk with Enable Incremental Backup
  2. Virtual Size and Actual Size are 10GiB
  3. Edit disk and extend by 5GiB
  4. Results: a) Without the fix: Actual Size is 15GiB, but Virtual Size remains at 10GiB

This is wrong - extending cow disk must increase the virtual size, but the actual size does not have to change, since cow volume grow as needed. We can argue that when using preallocated volume extending must also preallocated the disk size, looks like the existing ocde already does this.

b) With the fix: both Actual Size and Virtual Size are expanded to 15GiB

Sound correct.

But I don't see how this issue can cause a VM to pause. If the virtual size did not change (a), the vm does not see the new virtual size so it wil not try to write after the end of the actual lv, and will not pause.

ahadas commented 1 year ago

@nirs the part you comnented on was supposed to be: Without the fix: Virtual Size is 15GiB, but Actual Size remains at 10GiB

nirs commented 1 year ago

@nirs the part you comnented on was supposed to be: Without the fix: Virtual Size is 15GiB, but Actual Size remains at 10GiB

But this is fine, the disk will be extended as needed while writing.

nirs commented 1 year ago

Looking at the code path changed, this affects HSM.extendVolumeSize, which extend the amount of space allocated (e.g. file size or lv size). This virtual size is changed in HSM.updateVolumeSize.

Bad API on vdsm side, instead of one API that does the right thing we have 2 APIs that engine must call in some cases, but too late to fix this now.

So the fix is just a nice to have allocation fix, keep preallocated volumes preallocated after user extend the size on the engine side.

If this fix an issue of pausing vms, it means thin provisioning code is broken and should be fixed. This change can hide the thin proviioning issue by preallocating, but if the thin provioning is broken, the vm will still pause when the disk will become full, becuase qcow2 metadata need extra space.

Testing extension of preallocated volumes without this fix will tell if more work is needed for thin volumes.

ahadas commented 1 year ago

So the fix ... keep preallocated volumes preallocated after user extend the size on the engine side.

right, without this fix our tools show customers that something is wrong in their storage domains after extending preallocated qcow disks on block storage, as they expect virtual size == actual size for preallocated disks

nirs commented 1 year ago

OK, so the issue is not pausing vms, but incorrect allocation reported by the tools.