[lvm-devel] [PATCH 1/2] lvm2app: Add thin and thin pool lv creation

[M. Mohan Kumar]

Zdenek Kabelac <zkabelac at redhat.com> writes:

> Dne 12.2.2013 18:24, M. Mohan Kumar napsal(a):
>>   		first_seg(lv)->chunk_size = lp->chunk_size;
>>   		first_seg(lv)->discards = lp->discards;
>>   		/* FIXME: use lowwatermark  via lvm.conf global for all thinpools ? */
>> -		first_seg(lv)->low_water_mark = 0;
>> +		first_seg(lv)->low_water_mark = lp->low_water_mark;
>
> low_water_mark is not supported yet in the lvm2 stack,
> and there is not yet clear definition how is it going to be used.
>

IMHO there is no harm in adding support to specify low water mark during
thin pool creation. 

> For now dmeventd is based on 10sec polling interval.
>

Is it not possible to use the 'thin-pool' threshold dm-event instead of
10-sec polling in dmeventd? 

> However there needs to be made clean definition for various error path,
> and we have to also support 'clean' shutdown for those paths according
> to defined policy.
>

Could you please explain it further?

>> +lv_t lvm_lv_thin(const vg_t vg, const char *pool,
>> +		 const char *lvname, uint64_t size)
>> +{
>> +	struct lvcreate_params lp = { 0 };
>
> I think the lvm2app needs some extension here and rather
> make the 'lvcreate_params' structure a visible object with some methods,
> and use just one universal function call to create all types
> of LVs - so we can better match the internal lvm2 processing.
> i.e. there is going to be support for creation of thin
> lvs with external origins - so adding  lvm API call for every
> possible use case of thin pool is IMHO less clean then maintenance
> of 'lvcreate_params' object separately from lvm_lv_create() call.
>

I agree for a need of a single lv create function capable of creating
any LV type and I already posted a RFC version of that patch a month
ago.

But providing a simpler interface to create a specific type of LV may
also be a valid usecase, where an user does not want to
customize/specify each parameters in LV creation.

IIUC by 'support for creation of thin lvs with external origins' you
mean creating a thin snapshot LV from a linear LV. In this case
lvm_lv_snapshot function needs to be changed, while this function
lvm_lv_thin creates only thin Lvs (not thin snapshots)