[pve-devel] [PATCH storage] fix-4272: btrfs: add rename feature

[Aaron Lauterer]

On  2024-07-03  14:32, Maximiliano Sandoval wrote:
> Aaron Lauterer <a.lauterer at proxmox.com> writes:
> 
>> Works overall. I did not test edge cases like working around the BTRFS plugin to
>> have qcow2 files instead of the raw files in a subvol.
> 
> Thanks for testing!
> 
[...]
>> I am not too familiar with how BTRFS and our plugin works, but looking at other
>> functions, like 'alloc_image' or 'clone_image' it seems that we do have checks
>> regarding the format in place. If it is not a 'raw' or 'subvol' we call the
>> SUPER from the main Plugin.pm instead of continuing in the BTRFS specific
>> implementations that use subvols.
>>
>> This seems to be missing here, but it might be that it is fine if the "move" is
>> working in any way.
> 
> While it is true that others methods call the SUPER method, I didn't
> find any other implementation using SUPER::rename_volume.

Not surprising, as this behavior seems to be a BTRFS specific one for 
the edge case that we are dealing with a regular file (non .raw) instead 
of how the plugin usually wants it. AFAICT this is a safety mechanism, 
just in case.

For example (IIRC): on BTRFS the layout if usually:
images/{vmid}/vm-{vmid}-disk-X/disk.raw

where the `vm-{vmid}-disk-X` part is a BTRFS subvolume. But it would 
also be possible that someone (manually) builds it like a regular 
directory based storage plugin:

images/{vmid}/vm-{vmid}-disk-X.qcow2

where everything is just a path/file. How well does the rename_volume 
code handle this? Or should it call the SUPER method?