Add support for Managed Block Disks to OVA export - #1133
Conversation
d0aa0a1 to
3749bb8
Compare
|
Greetings from LINBIT (another storage provider, also using oVirt's MBS feature) - More testing done on the LINSTOR and DRBD side. I've tested this PR with successful exports of MBS-backed VMs (single-disk, 2-disk, 3-disk, mixed iSCSI+MBS). Looks great from over here 👍 |
Hey @ryan-ronnander, thanks for testing this! I did some more testing and on our integration I got a leftover attached disk when I was exporting a live VM (stays forever in locked state, can't touch it unless you dig into the DB). I think I'm missing a detach command somewhere, will debug this and update the PR unless it's something in our integration side. |
3749bb8 to
24e26f0
Compare
|
Pushed a fix for the live MBS teardown (it was really missing detach+delete on live VMs due to it going through the "image" path which doesn't seem to need it in that case). Also noticed we were not calling Should be good now, ran some tests, will also do Ceph RBD tests tomorrow and run the OST as well just in case. |
Re-tested the latest revision on LINSTOR/DRBD, using the "running-VM path" this time (my earlier testing was all cold/offline):
Solid from over here 👍 |
24e26f0 to
c3c2a5e
Compare
|
Hi @ryan-ronnander, thank you again for the reviews and testing! I've added a fix for a freeze/thaw bug that I think was pre-existing and affects snapshots of VMs with MBS disks only. When a snapshot does not include memory (like OVA export requests via internal command) and there are only MBS or legacy Cinder disks, oVirt would issue a filesystem freeze via qemu guest agent, but will not issue a corresponding thaw after making the snapshot. This leaves the VM with a frozen fs leading to hanging writes. Not sure if the circumstances of hitting this are that niche (working guest agent, MBS disks only, no memory in the snapshot) or something else I'm doing is hitting it consistently, but anyways I've added a small rework on how snapshots call into freeze/thaw so this no longer happens. |
peter-boden
left a comment
peter-boden
left a comment
There was a problem hiding this comment.
PreparImage instead of PrepareImage in commit f569b9c
2 small comments.
|
|
||
| String devicePath; | ||
| try { | ||
| devicePath = connectAndAttachManagedBlockVolume(mbs, sdId, volumeId); |
There was a problem hiding this comment.
connectAndAttachManagedBlockVolume(..) calls deleteManagedBlockVolume(mbs, volumeId) and then throws an EngingeException. This exception is caught below, and then if the image is inactive we call deleteManagedBlockVolume(mbs, volumeId) again.
Maybe it doesn't cause issues, but just wanted to point out you have a pontential double delete.
There was a problem hiding this comment.
Ouch, the inner delete is unconditional (apart from being done on connection error) and might actually delete the active volume :(
I'll fix & retest it
There was a problem hiding this comment.
Fixed the commit message typo;
No longer deleting MBS disk in the internal connectAndAttachManagedBlockVolume() (had a case where it could delete the live disk, on top of doing double deletes).
Also added handling for attach failures (before it could leave MBS discs connected indefinitely).
Also added unit tests for the various failure modes for MBS as well as the common success paths (both for MBS and image disks).
|
Anytime! I did another round of testing with a few of the in-flight PRs. I built an OL10 guest with the qemu agent on a LINSTOR MBS disk and took a memory-less snapshot:
|
c3c2a5e to
ba6997f
Compare
Add Managed Block Storage disk support when exporting to OVA (both VM and VM Template). Force Sparse IMAGE COW format for those as well (they get converted by the existing logic). For VMs, create a temp volume from the transient snapshot and attach it in lieu of calling PrepareImage. Pass the disk name explicitly to pack_ova.py as well, since it will be different from the base image due to the temp volume. For Templates, also scan MBS disks so they can get included in the OVA along with the IMAGE disks. Add unit tests for CreateOvaCommand. Signed-off-by: Viktor Ivanov <viktor.ivanov@storpool.com>
Use ftruncate() on the pre-allocated OVA tar file to properly enable qemu-img sparse output. Previously the loopback device did not treat the empty regions as actual holes, leading to qemu-img not creating sparse output for raw sources such as Managed Block Storage disks. Examine the output to find the actual data regions and reclaim the unused space. This is especially needed for MBS disks since qemu-img measure always returns the full worst-case size for them, resulting in huge OVA files. Pre-allocate space disk by disk (instead of all at once in the beginning) to benefit from the possible reclaimed portions. Signed-off-by: Viktor Ivanov <viktor.ivanov@storpool.com>
Add audit log type for initiating VM thaw (THAW_VM_INITIATED). Add initiation audit logs in both FreezeVmCommand and ThawVmCommand. Signed-off-by: Viktor Ivanov <viktor.ivanov@storpool.com>
Rework freeze/thaw mechanic when doing VM snapshots with managed block storage disks. Fixes a bug where MBS-only VMs without memory included in the snapshot would be left in frozen state. The freeze/thaw is now performed by running FreezeVmCommand and ThawVmCommand as internal actions, so both operations are audit logged consistently across the snapshot flows. Add a separate VmNeedsThaw flag to make sure VM that is always attempted regardless of reported freeze outcome. Signed-off-by: Viktor Ivanov <viktor.ivanov@storpool.com>
ba6997f to
86e1156
Compare
# Conflicts: # backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/ManagedBlockStorageDomainValidator.java
3 participants
Changes introduced with this PR
Add support for Managed Block Storage disks during OVA export, both from a VM and a VM Template. Instead of PrepareImage, use the already existing attach mechanism.
Rework OVA packing script to reclaim the tail end of the unused space in the sparse COW images that get converted directly into the OVA file. This is especially useful for MBS disks since they are RAW and
qemu-img measure, which is used to pre-measure the needed space for the OVA, always returns the full disk size.Are you the owner of the code you are sending in, or do you have permission of the owner?
Yes