[lvm-devel] [PATCH] lvm2app: Add thin and thin pool lv creation V2
M. Mohan Kumar
mohan at in.ibm.com
Mon May 6 15:48:54 UTC 2013
Tony Asleson <tasleson at redhat.com> writes:
> On 05/06/2013 08:26 AM, M. Mohan Kumar wrote:
>> Tony Asleson <tasleson at redhat.com> writes:
>>
>>> + *
>>> + */
>>> +lv_create_params_t lvm_lv_params_create_thin_pool(vg_t vg,
>>> + const char *pool_name, uint64_t size, uint32_t chunk_size,
>>> + uint64_t meta_size, lvm_thin_discards_t discard);
>>> +
>>> +#define lvm_lv_params_create_thin_pool_default(vg, pool_name, size) \
>>> + lvm_lv_params_create_thin_pool((vg), (pool_name), (size), 0, 0, \
>>> + LVM_THIN_DISCARDS_PASSDOWN)
>>> +
>>> +
>>> +/**
>>> + * Set the value of zero skip for the lv_create_params
>>> + *
>>> + * \param params
>>> + * lv_create_prams to change
>>> + *
>>> + * \param skip
>>> + * Value of skip zero to be set (0 == zero, 1 == skip zero)
>>> + *
>>> + * \return
>>> + * 0 on success, else -1
>>> + */
>>> +int lvm_lv_params_skip_zero_set(lv_create_params_t params, int skip);
>>
>> Are we going to provide as many helper functions to get/set parameters
>> in lv_create_params structure? For example if an user does wants a new
>> LV to be created in deactivation state, should we provide a helper
>> something like this?
>>
>> int lvm_lv_params_activation_set(lv_create_params_t params, int
>> activate);
>>
>> In this case lvm-api wont be flooded with lots of helper functions?
>> Instead of individual helper function how about a common helper function
>> to take care of this?
>>
>> int lvm_lv_params_set(lv_create_params_t params, enum lv_create_param_type,
>> type, void *data);
>>
>> lvm_lv_params_set function will validate 'data' based on the
>> lv_create_param_type and will return error if 'data' does not match
>> given enum (resource)?
>>
>> enum lv_create_param_type {
>> LV_CR_INVALID,
>> LV_CR_ZERO,
>> LV_CR_ACTIVATION,
>>
>> LV_CR_MAX_PARAM,
>> };
>>
>> In above zero case, this function will be invoked like this:
>>
>> lvm_lv_params_set(params, LV_CR_ZERO, &zero);
>>
>> lvm_lv_params_set will do:
>> switch (type) {
>> case LV_CR_ZERO:
>> memcpy (&zero_skip, data, sizeof(int));
>> break;
>> case LV_CR_ACTIVATION:
>> memcpy (&activation, data, sizeof(int));
>> if (activation < CHANGE_AY && activation >
>> CHANGE_AAY) {
>> log_error();
>> return -1;
>> }
>> }
>>
>>
>
> Yeah, this is probably a better approach to minimize API explosion. The
> only thing I would like to add is a size_t data_len field. This way we
> can accommodate variable size data for same operation.
>
> Does that sound reasonable? I guess if we only support simple fixed
> type sizes it would be redundant.
>
+1
More information about the lvm-devel
mailing list