Re: [pve-devel] [PATCH storage] Revert "workaround zfs create -V error for unaligned sizes"

[Aaron Lauterer <a.lauterer@proxmox.com>]

On 6/14/23 14:13, Fiona Ebner wrote:
> Am 14.06.23 um 13:44 schrieb Aaron Lauterer:
>> On 6/14/23 13:38, Thomas Lamprecht wrote:
>>> Am 14/06/2023 um 13:28 schrieb Aaron Lauterer:
>>>> This reverts commit cdef3abb25984c369571626b38f97f92a0a2fd15.
>>>>
>>>> The bug should be fixed by now [0]. The reproducer doesn't cause any
>>>> issues in my tests.
>>>>
>>>> [0] https://github.com/openzfs/zfs/issues/8541
>>>
>>> hmm, torn on this one; 1 MB aligned images sound not to bad for
>>> various things,
>>> and the extra size is rather negligible most of the time so we can
>>> mostly lose
>>> here, otoh. it should be callers decision if storage works fine now..
>>>
>>>>
>>>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>>>> ---
>>>> AFAICT this has an affect on EFI disks which after this revert will be
>>>> 528k and not 1M. Similar as if we would store it as a .raw file.
>>>>
>>>
>>> that sounds like it _could_ break stuff..
>>>
>>> @fiona: what was the state with local storage migration and those disk
>>> size
>>> mismatches? Or anything else coming to your mind?
>>
>> I did a few tests in the meantime. An EFI disk on a directory based
>> storage will be 528 K and can be moved to a ZFS storage with this patch.
>> Without it, it will fail, similar to RBD which needs a 1M min size IIRC.
> 
> Yes, drive mirror will fail if the source and target volume don't have
> the exact same size: https://bugzilla.proxmox.com/show_bug.cgi?id=3227
> so this would be an improvement. Although the proper fix for that bug
> would need to be made in drive mirror (e.g. by adding an option to allow
> larger target image).
> 
> Offline storage export/import for ZFS is currently limited to ZFS<->ZFS
> anyways.
> 
> I'm not aware of any issues, but the alignment has been there for a
> while now ;)
> 
> There's also similar padding in volume_resize(). Does ZFS also round up
> automatically there now or do we need to keep that?

Doesn't look like it:
zfs create tank01/sizetest -V 35K
zfs set volsize=73K tank01/sizetest
cannot set property for 'tank01/sizetest': 'volsize' must be a multiple of 
volume block size (8K)


With that in mind, it is probably better to let things be as they are?