[pve-devel] [PATCH pve-storage v3 3/3] lvmthin: disable autoactivation for new logical volumes

[Friedrich Weber]

On 30/06/2025 09:47, Friedrich Weber wrote:
> On 27/06/2025 10:14, Fabian Grünbichler wrote:
>>
>>> Friedrich Weber <f.weber at proxmox.com> hat am 23.06.2025 11:25 CEST geschrieben:
>>>
>>>  
>>> On 10/06/2025 17:00, Michael Köppl wrote:
>>>> On 4/29/25 13:36, Friedrich Weber wrote:
>>>>> When discovering a new volume group (VG), for example on boot, LVM
>>>>> triggers autoactivation. With the default settings, this activates all
>>>>> logical volumes (LVs) in the VG. Activating an LV creates a
>>>>> device-mapper device and a block device under /dev/mapper.
>>>>>
>>>>> Autoactivation is problematic for shared LVM storages, see #4997 [1].
>>>>> For the inherently local LVM-thin storage it is less problematic, but
>>>>> it still makes sense to avoid unnecessarily activating LVs and thus
>>>>> making them visible on the host at boot.
>>>>>
>>>>> Hence, disable autoactivation after creating new LVs. As lvcreate
>>>>> doesn't accept the --setautoactivation flag for thin LVs, this is done
>>>>> with an additional lvchange command. With this setting, LVM
>>>>> autoactivation will not activate these LVs, and the storage stack will
>>>>> take care of activating/deactivating LVs when needed.
>>>>>
>>>>> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=4997
>>>>>
>>>>> Signed-off-by: Friedrich Weber <f.weber at proxmox.com>
>>>>> ---
>>>>>
>>>>> Notes:
>>>>>     - would be great to get your opinion on whether we should consider
>>>>>       LVM-thin storages in this series or not.
>>>>>     
>>>>>     - passing --setautoactivation n to lvcreate for a thin volume says:
>>>>>     
>>>>>         Option --setautoactivation is unsupported with thins.
>>>>>     
>>>>>       But lvchange --setautoactivation seems to work on thin LVs, so the
>>>>>       fact that lvcreate doesn't accept it may be a bug. I reported it
>>>>>       upstream [1].
>>>>>     
>>>>>     new in v3
>>>>>     
>>>>>     [1] https://gitlab.com/lvmteam/lvm2/-/issues/32
>>>>
>>>> Since the upstream issue has not been addressed yet and the change to
>>>> LVM-thin does, AFAICT, not mitigate problems like in #4997 (or am I
>>>> missing something here?), but is mostly done to streamline behavior,
>>>> could the changes for LVM-thin be held back until it's clear that
>>>> lvcreate not supporting --setautoactivation for LVM-thin is not on purpose?
>>>
>>> Good point. I agree disabling autoactivation isn't as important for
>>> LVM-thin as it is for LVM-thick, though it's preferable also here that
>>> VM disks are not always active on the host, but only activated on-demand
>>> by our storage stack.
>>>
>>> From looking at the lvm2 commit introducing `--setautoactivation` [1]
>>> the omission of --setautoactivation for thin LVs doesn't seem
>>> intentional to me (maybe it was just forgotten to add to
>>> LVCREATE_ARGS?), but I can't be 100% sure either.
>>>
>>> The problem with holding back the change for LVM-thin is that we also
>>> need a way to update already-existing LVs, and the 8->9 bump is a good
>>> opportunity to do so via pve8to9.
>>>
>>> @Fabian, what do you think?
>>
>> it seems very likely this was by accident, and not by design.
>>
>> maybe opening an MR fixing it in addition to the issue gets
>> more upstream attention?
> 
> Good point, thanks. I opened a MR upstream:
> https://gitlab.com/lvmteam/lvm2/-/merge_requests/31

The MR was just merged, so looks like lvcreate refusing
--setautoactivation was indeed by accident. I guess we'll still have to
do  a separate lvchange --setautoactivation n here, because the patch
will only be available in lvm 2.03.34 at the earliest and I doubt this
is important enough to backport or to begin shipping our own LVM
packages. I can add a TODO PVE10 comment to check this again for PVE10,
though.