[lvm-devel] [PATCH] Change process_each_lv_in_vg() iterator to only process non-hidden LVs.

[Milan Broz]

On 10/15/2009 04:07 PM, Dave Wysochanski wrote:
> Unless the "--all" flag is given, the iterator functions should not be
> processing hidden LVs.  I have checked all the tools and this seems to
> be how they all behave.  Somehow before I missed the fact I could re-use
> the "--all" flag to qualify the hidden LV check (perhaps it's because
> the all flag is not documented well).  If there are things I've overlooked
> I can deal with them in another patch.  I am working on patches to clarify
> and document the usage of "--all".

Ack for the intention (see my previous patch) but nack to use of all_ARG :-)

We should really document --all first, missing in man pages
(lvs, lvdisplay, lvscan, pvs, pvdisplay, vgs)


> --- a/tools/toollib.c
> +++ b/tools/toollib.c
> @@ -122,6 +122,9 @@ int process_each_lv_in_vg(struct cmd_context *cmd,
>  		if (lvl->lv->status & SNAPSHOT)
>  			continue;
>  
> +		if (!lv_is_visible(lvl->lv) && !arg_count(cmd, all_ARG))
> +			continue;
> +

This function is used used in process_each_lv().

If you do this and command uses all_ARG, caller can activate hidden
volumes directly (see lvchange).

But you cannot remove hidden volumes now (see lvremove, it has no all arg).

I think that

 - not visible (hidden) volumes must be always be activated through some
visible LV or other operation
(I have special hidden keystore volume and I never want user to activate it
explicitly for example.)

 - we should allow user to remove hidden volume (if it is unreferenced),
(like hidden orphaned mirror log when something went wrong)

- also see my previous patch for vgchange, it should be cleaned similar way
(adding exceptions for every type of hidden volume is not ideal,
lv_is_visible() seems to be ok here)

Milan