lvm2app: Adding lv creation support
cover lvm2app: Adding lv creation support
From: "M. Mohan Kumar" <[email protected]>
Hello,
We added Block Device(BD) backend capability to GlusterFS last year. BD backend takes volume group as the input for GlusterFS volume and exports all logical volumes under it as regular files to the GlusterFS client. Now we are planning to export thin LVs as also regular files through GlusterFS. In order to support thin LVs, we need thin lv creation support in the lvm-devel.
I am posting RFC patches for adding various lv target creation support and thin(pool) lv creation support in lvm library.
First approach (Add thin and thin pool lv creation) adds two interfaces lvm_lv_thinpool and lvm_lv_thin (similar to lvm_vg_create_lv_linear()) to create thin-pool LV and thin LV respectively.
Second approach (Add LV creation support) adds interface lvm_vg_create_lv() that can be used to create LV of any target type (stripe, mirror, raid, thin etc). As part of this interface a new structure lv_params_t is addded which is similar to lvcreate_params exposing all parameters (such as chunk size, stripe size etc) to the end user. Advantage with this approach is that it gives the user to control properties of new LVs such as if the minor number of new lv should be persistent, allocation policy, permission mode, stripe size etc. But problem with this approach is that lots of code (parameter parsing, setting default value for LV properties etc) from tools is duplicated and it needs to be made as a library so that both tools and lvm-devel can consume them.
In the long run, lvm-devel may need to provide finer control to the user when creating LVs. In that case second approach is more suitable.
M. Mohan Kumar (2):
lvm2app: Add thin and thin pool lv creation
lvm2app: Add LV creation support
liblvm/lvm2app.h | 102 +++++++ liblvm/lvm_lv.c | 835 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 937 insertions(+)
Signature · 1 line
1.7.11.7
[RFC/PATCH] lvm2app: Add thin and thin pool lv creation
From: "M. Mohan Kumar" <[email protected]>
Add thin and thin pool lv creation support to lvm library
--- liblvm/lvm2app.h | 44 ++++++++++++++++++++++++++ liblvm/lvm_lv.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+)
liblvm/lvm2app.h +44 -0
@@ -1400,6 +1400,50 @@ int lvm_lv_resize(const lv_t lv, uint64_t new_size);*/ lv_t lvm_lv_snapshot (const lv_t lv,const char *snap_name,uint64_t max_snap_size); + +/** +* Create a thinpool in a given VG +* +* \param vg +* Volume Group handle. +* +* \param pool_name +* Name of the pool. +* +* \param size +* size of the pool +* +* \return +* Valid lv pointer on success, else NULL on error. +* +*/ +lv_t lvm_lv_thinpool (const vg_t vg,const char *pool_name,uint64_t size); + +/** +* Create a thin LV in a given VG & thin pool +* +* \param vg +* Volume Group handle. +* +* \param pool_name +* Name of the pool. +* +* \param lvname +* Name of the LV to create +* +* \param size +* Size of logical volume +* +* \return +* Valid lv pointer on success, else NULL on error. +* +*/ + +lv_t lvm_lv_thin (const vg_t vg,const char *pool_name,const char *lvname, +uint64_t size); + + +/************************** physical volume handling ************************/ /**
liblvm/lvm_lv.c +94 -0
97 lines not on this page · raw message
Signature · 1 line
1.7.11.7
[RFC/PATCH] lvm2app: Add thin and thin pool lv creation
Review
Quoted 2 lines · M. Mohan Kumar
(quoted diff) +2
-0+lv_t lvm_lv_thinpool(const vg_t vg, const char *pool_name, uint64_t size); +
We need to specify 'low_water_mark' when creating a thin pool. Otherwise a thin pool is created with 0 as low_water_mark and it will never trigger a dm event so that an userspace daemon can do the resizing of pool.
I am looking at how to pass this low water mark data to dm create. So please do not merge this patch till we have capability to mention low_water_mark argument for thin pool creation.
[RFC/PATCH] lvm2app: Add LV creation support
In reply to M. Mohan KumarFrom: "M. Mohan Kumar" <[email protected]>
This patch adds lv creation API to liblvm. Using this API one create any logical volume with requested target type (like mirror, raid-x etc).
Advantage with this approach is that it gives the user to control properties of new LVs such as if the minor number of new lv should be persistent, allocation policy, permission mode, stripe size etc.
TODO:
* Lots of code duplicated from tools part of lvm, make the code generic and make it as library so that tools and library can use them
--- liblvm/lvm2app.h | 58 +++++ liblvm/lvm_lv.c | 741 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 799 insertions(+)
liblvm/lvm2app.h +58 -0
@@ -97,6 +97,7 @@ struct volume_group;struct logical_volume ;struct lv_segment ;struct pv_segment ; +typedef struct lv_params lv_params_t ;/** * \class lvm_t
@@ -1024,6 +1025,25 @@ int lvm_vg_set_property(const vg_t vg, const char *name,lv_t lvm_vg_create_lv_linear (vg_t vg,const char *name,uint64_t size);/** +* Create a logical volume as per lv_params_t request. +* This function commits the change to disk and does _not_ require calling +* lvm_vg_write(). +* NOTE: The commit behavior of this function is subject to change +* as the API is developed. +* +* \param vg +* VG handle obtained from lvm_vg_create() or lvm_vg_open(). +* +* \param params +* LV parameter +* +* \return +* non-NULL handle to an LV object created, or NULL if creation fails. +* +*/ +lv_t lvm_vg_create_lv (vg_t vg,lv_params_t *params); + +/** * Return a list of lvseg handles for a given LV handle. * * \memberof lv_t
@@ -1708,6 +1728,44 @@ typedef int32_t percent_t;*/ float lvm_percent_to_float (percent_t v); +#define LV_TYPE_INVALID 0x00000000 +#define LV_STRIPE 0x00000001 +#define LV_MIRROR 0x00000010 +#define LV_THIN 0x00000020 +#define LV_THINPOOL 0x00000040 +#define LV_SNAPSHOT 0x00000080 +#define LV_RAID1 0x00000100 +#define LV_RAID4 0x00000200 +#define LV_RAID5 0x00000400 +#define LV_RAID6 0x00000800 +#define LV_ERROR 0x00001000 + +typedef struct lv_params { +int type; +char *name; +lv_t origin; +char *pool_name; +uint32_t stripes;/* striped */ +uint32_t stripe_size;/* striped */ +uint32_t chunk_size;/* snapshot */ +uint32_t region_size;/* mirror */ +uint32_t mirrors;/* mirror */ +uint64_t size; +uint64_t virtualsize;/* snapshot */ +uint64_t poolmetadatasize;/* thin pool */ +uint32_t permission;/* all */ +uint32_t read_ahead;/* all */ +int alloc;/* all */ +int thind; +uint32_t zero; +uint32_t mirrorlog; +uint32_t nosync; +int activate; +int major; +int minor; +int persistent; +}lv_params_t ; + #ifdef __cplusplus }#endif
liblvm/lvm_lv.c +741 -0
753 lines not on this page · raw message
Signature · 1 line
1.7.11.7
cover lvm2app: Adding lv creation support
In reply to M. Mohan KumarQuoted 11 lines · M. Mohan Kumar
From: "M. Mohan Kumar" <[email protected]>
Hello,
We added Block Device(BD) backend capability to GlusterFS last year. BD backend takes volume group as the input for GlusterFS volume and exports all logical volumes under it as regular files to the GlusterFS client. Now we are planning to export thin LVs as also regular files through GlusterFS. In order to support thin LVs, we need thin lv creation support in the lvm-devel.
Quoted 6 lines
I am posting RFC patches for adding various lv target creation support and thin(pool) lv creation support in lvm library.
First approach (Add thin and thin pool lv creation) adds two interfaces
lvm_lv_thinpoolandlvm_lv_thin(similar tolvm_vg_create_lv_linear()) to create thin-pool LV and thin LV respectively.
Thanks for the patches!
This patch looks pretty straight forward. Ideally we would not have the function do the write and commit to match the other functions. Did you happen to take a peek to what that would involve?
Quoted 13 lines
Second approach (Add LV creation support) adds interface
lvm_vg_create_lv()that can be used to create LV of any target type (stripe, mirror, raid, thin etc). As part of this interface a new structurelv_params_tis addded which is similar tolvcreate_paramsexposing all parameters (such as chunk size, stripe size etc) to the end user. Advantage with this approach is that it gives the user to control properties of new LVs such as if the minor number of new lv should be persistent, allocation policy, permission mode, stripe size etc. But problem with this approach is that lots of code (parameter parsing, setting default value for LV properties etc) from tools is duplicated and it needs to be made as a library so that both tools and lvm-devel can consume them.
Quoted 2 lines
In the long run, lvm-devel may need to provide finer control to the user when creating LVs. In that case second approach is more suitable.
I agree with everything you state here and I also agree with your comments about moving the code into a common spot instead of copying it.
I see that you do have some slight differences in the copied functions,
but those are required.
Exposing parameter passing with the use of structures is typically discouraged in a shared C API. The struct padding can be different from client application and library compile based on compiler settings. If we go this route we should at the very least do a pragma pack to mitigate this. In addition once you do this the structure forever cannot change. We could address these issues by creating an opaque data type and then adding some functions to set/get the values from the opaque handle. Kind of tedious, but it is a common approach.
Not sure what else we could do to offset the vast number of arguments that are available.
Regards,
Tony
cover lvm2app: Adding lv creation support
Quoted 19 lines · Tony Asleson
Quoted 11 lines · M. Mohan Kumar
From: "M. Mohan Kumar" <[email protected]>
Hello,
We added Block Device(BD) backend capability to GlusterFS last year. BD backend takes volume group as the input for GlusterFS volume and exports all logical volumes under it as regular files to the GlusterFS client. Now we are planning to export thin LVs as also regular files through GlusterFS. In order to support thin LVs, we need thin lv creation support in the lvm-devel.
Quoted 6 lines
I am posting RFC patches for adding various lv target creation support and thin(pool) lv creation support in lvm library.
First approach (Add thin and thin pool lv creation) adds two interfaces
lvm_lv_thinpoolandlvm_lv_thin(similar tolvm_vg_create_lv_linear()) to create thin-pool LV and thin LV respectively.
Quoted 6 lines
Thanks for the patches!
This patch looks pretty straight forward. Ideally we would not have the function do the write and commit to match the other functions. Did you happen to take a peek to what that would involve?
This function is based on existing function lvm_vg_create_lv_linear. This existing function does not call lvm_vg_write. or am I missing something here?
Quoted 13 lines
Quoted 13 lines
Second approach (Add LV creation support) adds interface
lvm_vg_create_lv()that can be used to create LV of any target type (stripe, mirror, raid, thin etc). As part of this interface a new structurelv_params_tis addded which is similar tolvcreate_paramsexposing all parameters (such as chunk size, stripe size etc) to the end user. Advantage with this approach is that it gives the user to control properties of new LVs such as if the minor number of new lv should be persistent, allocation policy, permission mode, stripe size etc. But problem with this approach is that lots of code (parameter parsing, setting default value for LV properties etc) from tools is duplicated and it needs to be made as a library so that both tools and lvm-devel can consume them.
Quoted 8 lines
Quoted 2 lines
In the long run, lvm-devel may need to provide finer control to the user when creating LVs. In that case second approach is more suitable.
I agree with everything you state here and I also agree with your comments about moving the code into a common spot instead of copying it.
I see that you do have some slight differences in the copied functions,
but those are required.
I need to modify some of the validation as it was not needed in case of parameter passed by user.
Quoted 9 lines
Exposing parameter passing with the use of structures is typically discouraged in a shared C API. The struct padding can be different from client application and library compile based on compiler settings. If we go this route we should at the very least do a pragma pack to mitigate this. In addition once you do this the structure forever cannot change. We could address these issues by creating an opaque data type and then adding some functions to set/get the values from the opaque handle. Kind of tedious, but it is a common approach.
Quoted 3 lines
Not sure what else we could do to offset the vast number of arguments that are available.
I agree, can we try something like this:lv_set_param(mirror, value_for_mirror, opaque structure); lv_set_param(region_size, value_for_region_size, opaque structure);
cover lvm2app: Adding lv creation support
Quoted 8 lines · M. Mohan Kumar
Quoted 4 lines · Tony Asleson
This patch looks pretty straight forward. Ideally we would not have the function do the write and commit to match the other functions. Did you happen to take a peek to what that would involve?
This function is based on existing function
lvm_vg_create_lv_linear. This existing function does not calllvm_vg_write. or am I missing something here?
The existing lvm_vg_create_lv_linear indirectly does a vg_write and vg_commit. Look at function _lv_create_an_lv in lib/metadata/lv_manip.c
I took a look at this and it won't be easy. I should have looked at lvm_vg_create_linear as Dave has a nice comment on the function about the difficulty in changing this behavior.
Quoted 2 lines
I need to modify some of the validation as it was not needed in case of parameter passed by user.
I think before we integrate a patch like this we should address the code duplication part. Otherwise we are making the code base incrementally messier rather than incrementally better :-) At the moment I'm not sure where common code like this should be placed.
Anyone have a suggestion?
Quoted 9 lines
Quoted 9 lines
Exposing parameter passing with the use of structures is typically discouraged in a shared C API. The struct padding can be different from client application and library compile based on compiler settings. If we go this route we should at the very least do a pragma pack to mitigate this. In addition once you do this the structure forever cannot change. We could address these issues by creating an opaque data type and then adding some functions to set/get the values from the opaque handle. Kind of tedious, but it is a common approach.
Quoted 7 lines
Quoted 3 lines
Not sure what else we could do to offset the vast number of arguments that are available.
I agree, can we try something like this:
lv_set_param(mirror, value_for_mirror, opaque structure);lv_set_param(region_size, value_for_region_size, opaque structure);
This would work, but I'm not sure what you were thinking about for different types? What happens when value_for* is a non numeric? Is mirror & region_size enumerated values? Python or even C++ could handle this easily, but without throwing out type safety I'm not sure about C.
My initial simple thought was something more like:
int lv_param_mirror_set(opaque_data_type handle, int value); int lv_param_mirror_get(opaque_data_type handle);
Historically I have seen the opaque structure pointer first in the argument list (kind of like the this pointer), but it will work fine either way :-)
Thanks,
-Tony