Re: [ptxdist] [PATCH v3] lvm2: move lvmdump and fsadm into separate options

["Sven Püschel" <s.pueschel@pengutronix.de>]

Hi,

On 7/15/26 12:55 PM, Michael Olbrich wrote:
> On Tue, Jul 14, 2026 at 10:14:50AM +0200, Sven Püschel wrote:
>> Move lvmdump and the fsadm command into a separate option, as both
>> commands are bash scripts and therefore need bash as a dependency.
>> As these commands may not be used, we don't add bash as a dependency to
>> the much bigger LVM2_LVM_TOOLS option but instead add distinct options
>> for these commands.
>>
>> Signed-off-by: Sven Püschel <s.pueschel@pengutronix.de>
>>
>> ---
>> v3
>> - added new options instead of adding bash to LVM2_LVM_TOOLS
>> - added RUNTIME flag to the bash dependency
>> v2
>> - added
>> ---
>>   rules/lvm2.in   | 11 +++++++++++
>>   rules/lvm2.make |  7 ++++++-
>>   2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/rules/lvm2.in b/rules/lvm2.in
>> index 3213d9c31..6eae0a2de 100644
>> --- a/rules/lvm2.in
>> +++ b/rules/lvm2.in
>> @@ -8,6 +8,7 @@ menuconfig LVM2
>>   	select READLINE
>>   	select NCURSES
>>   	select SYSTEMD			if LVM2_SYSTEMD
>> +	select BASH			if (LVM2_FSADM || LVM2_LVMDUMP) && RUNTIME
>>   	prompt "lvm2                          "
>>   	help
>>   	  LVM2 refers to a new userspace toolset that provide logical volume management
>> @@ -32,6 +33,16 @@ config LVM2_STARTSCRIPT
>>   	depends on INITMETHOD_BBINIT
>>   	prompt "install /etc/init.d/lvm2"
>>   
>> +config LVM2_FSADM
> I think you need to select LVM2_LVM_TOOLS here. The scripts probably don't
> make sense without the actual binaries and the targetinstall stage will
> fail as well.

These are guarded by the same `if·LVM2_LVM_TOOLS` that is also used for 
`LVM2_STARTSCRIPT`. The diff just hides this fact.

Therefore they cannot be selected if LVM2_LVM_TOOLS is disabled.

Btw. the makefile doesn't have the nested if, as it should be handled by 
the kconfig.

>
>> +	bool
>> +	default y
> No 'default y' I think. These are tools typically used during development.
> And automatically enabling bash during an update is not so nice.

My reasoning here was potential backwards compability (e.g. when you had 
all tools installed previously you don't want to accidentially kick them 
out during an update) and the fact that `LVM2_LVM_TOOLS` also has a 
`default y`.

But I don't mind dropping this, if it's preferred to avoid accidentially 
pulling bash as dependency on an update.

Sincerely
     Sven

>
>> +	prompt "install the fsadm command"
>> +
>> +
>> +config LVM2_LVMDUMP
>> +	bool
>> +	default y
> Same here.
>
> Michael
>
>> +	prompt "install the lvmdump command"
>>   endif
>>   
>>   config LVM2_LIBDEVMAPPER_EVENT
>> diff --git a/rules/lvm2.make b/rules/lvm2.make
>> index 57773a1f7..b5aea9873 100644
>> --- a/rules/lvm2.make
>> +++ b/rules/lvm2.make
>> @@ -110,10 +110,15 @@ $(STATEDIR)/lvm2.targetinstall:
>>   
>>   	@$(call install_copy, lvm2, 0, 0, 0755, -, /usr/sbin/dmsetup)
>>   
>> -ifdef PTXCONF_LVM2_LVM_TOOLS
>> +ifdef PTXCONF_LVM2_FSADM
>>   	@$(call install_copy, lvm2, 0, 0, 0755, -, /usr/sbin/fsadm)
>> +endif
>> +
>> +ifdef PTXCONF_LVM2_LVMDUMP
>>   	@$(call install_copy, lvm2, 0, 0, 0755, -, /usr/sbin/lvmdump)
>> +endif
>>   
>> +ifdef PTXCONF_LVM2_LVM_TOOLS
>>   	@$(call install_copy, lvm2, 0, 0, 0755, -, /usr/sbin/lvm)
>>   	@$(call install_link, lvm2, lvm, /usr/sbin/lvchange)
>>   	@$(call install_link, lvm2, lvm, /usr/sbin/lvconvert)
>> -- 
>> 2.47.3
>>
>>
>>