[PATCH V2 0/2] Add thin lv and thin pool creation support

[PATCH V2 0/2] Add thin lv and thin pool creation support

[M. Mohan Kumar]

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Add thin lv and thin pool creation support to lvm2app.

Changes from previous version:
* Add support to specify data block size and low water mark thresold for
newly created thin pool.
* Added support for specifying lowwatermark parameter to thin pool
creation in lvcreate command

M. Mohan Kumar (2):
  lvm2app: Add thin and thin pool lv creation
  master - thin: Add low water mark parameter to pool creation

 lib/metadata/lv_manip.c          |   2 +-
 lib/metadata/metadata-exported.h |   1 +
 liblvm/lvm2app.h                 |  53 +++++++++++++++++++
 liblvm/lvm_lv.c                  | 107 +++++++++++++++++++++++++++++++++++++++
 tools/args.h                     |   1 +
 tools/commands.h                 |   6 ++-
 tools/lvconvert.c                |   1 +
 tools/lvcreate.c                 |   7 ++-
 tools/toollib.c                  |   4 ++
 tools/toollib.h                  |   1 +
 10 files changed, 179 insertions(+), 4 deletions(-)

-- 
1.7.11.7

[PATCH V2 1/2] lvm2app: Add thin and thin pool lv creation

[M. Mohan Kumar]

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Add thin and thin pool lv creation support to lvm library

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 lib/metadata/lv_manip.c          |   2 +-
 lib/metadata/metadata-exported.h |   1 +
 liblvm/lvm2app.h                 |  53 +++++++++++++++++++
 liblvm/lvm_lv.c                  | 107 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 162 insertions(+), 1 deletion(-)

diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c
index c1437eb..4dd5ebf 100644
--- a/lib/metadata/lv_manip.c
+++ b/lib/metadata/lv_manip.c
@@ -4518,7 +4518,7 @@ static struct logical_volume *_lv_create_an_lv(struct volume_group *vg, struct l
 		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;
 	} else if (seg_is_thin_volume(lp)) {
 		pool_lv = first_seg(lv)->pool_lv;
 
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index ffec129..40519f5 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -617,6 +617,7 @@ struct lvcreate_params {
 	uint64_t voriginsize; /* snapshot */
 	uint32_t poolmetadataextents; /* thin pool */
 	uint64_t poolmetadatasize; /* thin pool */
+	uint64_t low_water_mark;  /* thin_pool */
 	struct dm_list *pvh; /* all */
 
 	uint32_t permission; /* all */
diff --git a/liblvm/lvm2app.h b/liblvm/lvm2app.h
index 93a78c3..1b621a8 100644
--- a/liblvm/lvm2app.h
+++ b/liblvm/lvm2app.h
@@ -1400,6 +1400,59 @@ 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
+ *
+ * \param   chunk_size
+ * data block size of the pool
+ * DM_THIN_MIN_DATA_BLOCK_SIZE < chunk_size < DM_THIN_MAX_DATA_BLOCK_SIZE
+ *
+ * \param  low_water_mark_blocks
+ * When number of free blocks in the pool reaches below than this thresold a dm
+ * event is sent.
+ *
+ * \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,
+		     uint32_t chunk_size, uint64_t low_water_mark_blocks);
+
+/**
+ * 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 ************************/
 
 /**
diff --git a/liblvm/lvm_lv.c b/liblvm/lvm_lv.c
index 91948a6..6091b3d 100644
--- a/liblvm/lvm_lv.c
+++ b/liblvm/lvm_lv.c
@@ -350,3 +350,110 @@ lv_t lvm_lv_snapshot(const lv_t lv, const char *snap_name, uint64_t max_snap_siz
 		return NULL;
 	return (lv_t) lvl->lv;
 }
+
+/* Set defaults for thin pool specific LV parameters */
+static void _lv_set_pool_params(struct lvcreate_params *lp,
+				vg_t vg, const char *pool,
+				   uint64_t extents)
+{
+	_lv_set_default_params(lp, vg, NULL, extents);
+
+	lp->pool = pool;
+
+	lp->create_thin_pool = 1;
+	lp->segtype = get_segtype_from_string(vg->cmd, "thin-pool");
+	lp->stripes = 1;
+	lp->poolmetadatasize = extents * vg->extent_size / lp->chunk_size *
+		(SECTOR_SIZE / UINT64_C(64));
+	lp->poolmetadataextents = extents_from_size(vg->cmd, lp->poolmetadatasize,
+						   vg->extent_size);
+}
+
+lv_t lvm_lv_thinpool(const vg_t vg, const char *pool, uint64_t size,
+		     uint32_t chunk_size, uint64_t low_water_mark_blocks)
+{
+	struct lvcreate_params lp = { 0 };
+	uint64_t extents = 0;
+	struct lv_list *lvl = NULL;
+
+	if (vg_read_error(vg))
+		return NULL;
+	if (!vg_check_write_mode(vg))
+		return NULL;
+
+	if (!(extents = extents_from_size(vg->cmd, size / SECTOR_SIZE,
+					  vg->extent_size))) {
+		log_error("Unable to create LV thin pool without size.");
+		return NULL;
+	}
+
+	if (chunk_size)
+		lp.chunk_size = chunk_size;
+	else
+		lp.chunk_size = DEFAULT_THIN_POOL_CHUNK_SIZE * 2;
+
+	if (lp.chunk_size < DM_THIN_MIN_DATA_BLOCK_SIZE ||
+	    lp.chunk_size > DM_THIN_MAX_DATA_BLOCK_SIZE) {
+		log_error("Invalid chunk_size");
+		return NULL;
+	}
+
+	_lv_set_pool_params(&lp, vg, pool, extents);
+
+	lp.low_water_mark = low_water_mark_blocks;
+	if (!lp.segtype)
+		return_NULL;
+	if (!lv_create_single(vg, &lp))
+		return_NULL;
+	if (!(lvl = find_lv_in_vg(vg, pool)))
+		return NULL;
+	return (lv_t) lvl->lv;
+}
+
+/* Set defaults for thin LV specific parameters */
+static void _lv_set_thin_params(struct lvcreate_params *lp,
+				vg_t vg, const char *pool,
+				const char *lvname,
+				uint64_t extents)
+{
+	_lv_set_default_params(lp, vg, lvname, extents);
+
+	lp->thin = 1;
+	lp->pool = pool;
+	lp->segtype = get_segtype_from_string(vg->cmd, "thin");
+
+	lp->voriginsize = extents * vg->extent_size;
+	lp->voriginextents = extents_from_size(vg->cmd, lp->voriginsize,
+						   vg->extent_size);
+
+	lp->stripes = 1;
+}
+
+lv_t lvm_lv_thin(const vg_t vg, const char *pool,
+		 const char *lvname, uint64_t size)
+{
+	struct lvcreate_params lp = { 0 };
+	uint64_t extents = 0;
+	struct lv_list *lvl = NULL;
+
+	if (vg_read_error(vg))
+		return NULL;
+	if (!vg_check_write_mode(vg))
+		return NULL;
+
+	if (!(extents = extents_from_size(vg->cmd, size / SECTOR_SIZE,
+					  vg->extent_size))) {
+		log_error("Unable to create thin LV without size.");
+		return NULL;
+	}
+
+	_lv_set_thin_params(&lp, vg, pool, lvname, extents);
+
+	if (!lp.segtype)
+		return_NULL;
+	if (!lv_create_single(vg, &lp))
+		return_NULL;
+	if (!(lvl = find_lv_in_vg(vg, pool)))
+		return NULL;
+	return (lv_t) lvl->lv;
+}
-- 
1.7.11.7

[PATCH V2 2/2] master - thin: Add lowwatermark parameter to pool creation

[M. Mohan Kumar]

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Add --lowwatermark parameter to thin-pool creation. It enables sending
dmevent when the number of free blocks in the pool reaches below this
number.

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 tools/args.h      | 1 +
 tools/commands.h  | 6 ++++--
 tools/lvconvert.c | 1 +
 tools/lvcreate.c  | 7 ++++++-
 tools/toollib.c   | 4 ++++
 tools/toollib.h   | 1 +
 6 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/tools/args.h b/tools/args.h
index d4d6c40..93657cc 100644
--- a/tools/args.h
+++ b/tools/args.h
@@ -77,6 +77,7 @@ arg(force_long_ARG, '\0', "force", NULL, ARG_COUNTABLE)
 arg(stripes_long_ARG, '\0', "stripes", int_arg, 0)
 arg(sysinit_ARG, '\0', "sysinit", NULL, 0)
 arg(thinpool_ARG, '\0', "thinpool", string_arg, 0)
+arg(lowwatermark_ARG, '\0', "lowwatermark", int_arg, 0)
 
 /* Allow some variations */
 arg(resizable_ARG, '\0', "resizable", yes_no_arg, 0)
diff --git a/tools/commands.h b/tools/commands.h
index 986539e..f3b86b0 100644
--- a/tools/commands.h
+++ b/tools/commands.h
@@ -185,7 +185,8 @@ xx(lvcreate,
    "\t[-r|--readahead ReadAheadSectors|auto|none]\n"
    "\t[-R|--regionsize MirrorLogRegionSize]\n"
    "\t[-T|--thin  [-c|--chunksize  ChunkSize]\n"
-   "\t  [--discards {ignore|nopassdown|passdown}]\n"
+   "\t  [--discards {ignore|nopassdown|passdown}] \n"
+   "\t  [--lowwatermark low_water_mark thresold] \n"
    "\t  [--poolmetadatasize MetadataSize[bBsSkKmMgG]]]\n"
    "\t[--thinpool ThinPoolLogicalVolume{Name|Path}]\n"
    "\t[-t|--test]\n"
@@ -232,7 +233,8 @@ xx(lvcreate,
    monitor_ARG, name_ARG, nosync_ARG, noudevsync_ARG, permission_ARG,
    persistent_ARG, readahead_ARG, regionsize_ARG, size_ARG, snapshot_ARG,
    stripes_ARG, stripesize_ARG, test_ARG, thin_ARG, thinpool_ARG, type_ARG,
-   virtualoriginsize_ARG, poolmetadatasize_ARG, virtualsize_ARG, zero_ARG)
+   virtualoriginsize_ARG, poolmetadatasize_ARG, virtualsize_ARG, zero_ARG,
+   lowwatermark_ARG)
 
 xx(lvdisplay,
    "Display information about a logical volume",
diff --git a/tools/lvconvert.c b/tools/lvconvert.c
index 5bda00f..9c0ed99 100644
--- a/tools/lvconvert.c
+++ b/tools/lvconvert.c
@@ -352,6 +352,7 @@ static int _read_params(struct lvconvert_params *lp, struct cmd_context *cmd,
 				     &lp->chunk_size,
 				     &lp->discards,
 				     &lp->poolmetadata_size,
+				     NULL,
 				     &lp->zero))
 			return_0;
 
diff --git a/tools/lvcreate.c b/tools/lvcreate.c
index 78292f7..41358f8 100644
--- a/tools/lvcreate.c
+++ b/tools/lvcreate.c
@@ -799,7 +799,8 @@ static int _lvcreate_params(struct lvcreate_params *lp,
 	    !get_stripe_params(cmd, &lp->stripes, &lp->stripe_size) ||
 	    (lp->create_thin_pool &&
 	     !get_pool_params(cmd, &lp->chunk_size, &lp->discards,
-			      &lp->poolmetadatasize, &lp->zero)) ||
+			      &lp->poolmetadatasize, &lp->low_water_mark,
+			      &lp->zero)) ||
 	    !_read_mirror_params(lp, cmd) ||
 	    !_read_raid_params(lp, cmd))
 		return_0;
@@ -926,6 +927,10 @@ static int _check_thin_parameters(struct volume_group *vg, struct lvcreate_param
 			log_error("--zero may only be specified when allocating the thin pool.");
 			return 0;
 		}
+		if (arg_count(vg->cmd, lowwatermark_ARG)) {
+			log_error("--lowwatermark may only be specified when allocating the thin pool.");
+			return 0;
+		}
 	}
 
 	if (lp->create_thin_pool && lp->pool) {
diff --git a/tools/toollib.c b/tools/toollib.c
index 669d772..798d112 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -1525,6 +1525,7 @@ int get_pool_params(struct cmd_context *cmd,
 		    uint32_t *chunk_size,
 		    thin_discards_t *discards,
 		    uint64_t *pool_metadata_size,
+		    uint64_t *lowwatermark,
 		    int *zero)
 {
 	const char *dstr;
@@ -1577,6 +1578,9 @@ int get_pool_params(struct cmd_context *cmd,
 	}
 	*pool_metadata_size = arg_uint64_value(cmd, poolmetadatasize_ARG, UINT64_C(0));
 
+	if (lowwatermark)
+		*lowwatermark = arg_uint64_value(cmd, lowwatermark_ARG, UINT64_C(0));
+
 	return 1;
 }
 
diff --git a/tools/toollib.h b/tools/toollib.h
index 80c01fd..1b7a03e 100644
--- a/tools/toollib.h
+++ b/tools/toollib.h
@@ -115,6 +115,7 @@ int get_pool_params(struct cmd_context *cmd,
 		    uint32_t *chunk_size,
 		    thin_discards_t *discards,
 		    uint64_t *pool_metadata_size,
+		    uint64_t *lowwatermark,
 		    int *zero);
 int update_pool_params(struct cmd_context *cmd, unsigned attr,
 		       uint32_t data_extents, uint32_t extent_size,
-- 
1.7.11.7

[PATCH V2 0/2] Add thin lv and thin pool creation support

[Tony Asleson]

On 01/31/2013 08:25 AM, M. Mohan Kumar wrote:
> From: "M. Mohan Kumar" <mohan@in.ibm.com>
> 
> Add thin lv and thin pool creation support to lvm2app.
> 
> Changes from previous version:
> * Add support to specify data block size and low water mark thresold for
> newly created thin pool.
> * Added support for specifying lowwatermark parameter to thin pool
> creation in lvcreate command

OK, so the block size if specified as 0 results in a default value being
selected as before, which is what I believe we want.  The default low
water mark was 0, which is what we believe disables event creation.
This doesn't appear to be a safe default behavior?

One option would be to change the type to a signed integer and change
the meaning from number of blocks free to percentage free and then treat
-1 as pick a sane default.  Thus 0 would be no events and 30 would
indicate when 30% blocks free is remaining to generate an event.  We
could also use an unsigned value and use 100 as please pick a sane
default as this value and values close to it really wouldn't make much
sense.

Once we have defined what the default values are, I can modify my python
bindings patch to use these default values.

Thanks,
Tony

[PATCH V2 0/2] Add thin lv and thin pool creation support

[Zdenek Kabelac]

Dne 31.1.2013 22:04, Tony Asleson napsal(a):
> On 01/31/2013 08:25 AM, M. Mohan Kumar wrote:
>> From: "M. Mohan Kumar" <mohan@in.ibm.com>
>>
>> Add thin lv and thin pool creation support to lvm2app.
>>
>> Changes from previous version:
>> * Add support to specify data block size and low water mark thresold for
>> newly created thin pool.
>> * Added support for specifying lowwatermark parameter to thin pool
>> creation in lvcreate command
>
> OK, so the block size if specified as 0 results in a default value being
> selected as before, which is what I believe we want.  The default low
> water mark was 0, which is what we believe disables event creation.
> This doesn't appear to be a safe default behavior?
>
> One option would be to change the type to a signed integer and change
> the meaning from number of blocks free to percentage free and then treat
> -1 as pick a sane default.  Thus 0 would be no events and 30 would
> indicate when 30% blocks free is remaining to generate an event.  We
> could also use an unsigned value and use 100 as please pick a sane
> default as this value and values close to it really wouldn't make much
> sense.
>
> Once we have defined what the default values are, I can modify my python
> bindings patch to use these default values.
>


Currently the lowwatermark setting are unused by dm driver - since we
need to first resolve policies.

Reason why they are not yet support (and dmeventd only does pooling) is,
that we have to make sure during all activations, we are not starting thinpool
which is past the threshold - and there are many ways how to deal with this state.

Also - for now - there is no  per-pool  threshold setting -  IMHO this should 
be improved - and it's the same problem lvm2 has with i.e. mirror/raid,
where many setting are currently set only via  lvm.conf - while there is no 
option for LV override - this should be probably resolved at global level.

Zdenek

[PATCH V2 0/2] Add thin lv and thin pool creation support

[M. Mohan Kumar]

Zdenek Kabelac <zkabelac@redhat.com> writes:

> Dne 31.1.2013 22:04, Tony Asleson napsal(a):
>> On 01/31/2013 08:25 AM, M. Mohan Kumar wrote:
>>> From: "M. Mohan Kumar" <mohan@in.ibm.com>
>>>
>>> Add thin lv and thin pool creation support to lvm2app.
>>>
>>> Changes from previous version:
>>> * Add support to specify data block size and low water mark thresold for
>>> newly created thin pool.
>>> * Added support for specifying lowwatermark parameter to thin pool
>>> creation in lvcreate command
>>
>> OK, so the block size if specified as 0 results in a default value being
>> selected as before, which is what I believe we want.  The default low
>> water mark was 0, which is what we believe disables event creation.
>> This doesn't appear to be a safe default behavior?
>>
>> One option would be to change the type to a signed integer and change
>> the meaning from number of blocks free to percentage free and then treat
>> -1 as pick a sane default.  Thus 0 would be no events and 30 would
>> indicate when 30% blocks free is remaining to generate an event.  We
>> could also use an unsigned value and use 100 as please pick a sane
>> default as this value and values close to it really wouldn't make much
>> sense.
>>
>> Once we have defined what the default values are, I can modify my python
>> bindings patch to use these default values.
>>
>
>
Hi Zdenek,

> Currently the lowwatermark setting are unused by dm driver - since we
> need to first resolve policies.

dm-thin target checks for current free block vs low water threshold
blocks and sends dm event

dm-thin.c:alloc_data_block()
        if (free_blocks <= pool->low_water_blocks &&
        !pool->low_water_triggered) {
                DMWARN("%s: reached low water mark, sending event.",
                       dm_device_name(pool->pool_md));

And when I tested my api to create a thin pool & thin LV (with a certain
low water mark threshold) and when number of free blocks in that pool
reached lower than that thresold there was a event generated. I am
attaching part of my dmesg output here.
device-mapper: thin: 253:8: reached low water mark, sending event.

>
> Reason why they are not yet support (and dmeventd only does pooling) is,
> that we have to make sure during all activations, we are not starting thinpool
> which is past the threshold - and there are many ways how to deal with this state.
>
Thats right, but IMHO its not related to APIs for creating thin lv and
thin pool.

> Also - for now - there is no  per-pool  threshold setting -  IMHO this should 
> be improved - and it's the same problem lvm2 has with i.e. mirror/raid,
> where many setting are currently set only via  lvm.conf - while there is no 
> option for LV override - this should be probably resolved at global level.
>
IIUC this thresold is per pool setting as per the code that I pasted above.

[PATCH V2 0/2] Add thin lv and thin pool creation support

[M. Mohan Kumar]

Tony Asleson <tasleson@redhat.com> writes:

> On 01/31/2013 08:25 AM, M. Mohan Kumar wrote:
>> From: "M. Mohan Kumar" <mohan@in.ibm.com>
>> 
>> Add thin lv and thin pool creation support to lvm2app.
>> 
>> Changes from previous version:
>> * Add support to specify data block size and low water mark thresold for
>> newly created thin pool.
>> * Added support for specifying lowwatermark parameter to thin pool
>> creation in lvcreate command
>
> OK, so the block size if specified as 0 results in a default value being
> selected as before, which is what I believe we want.  The default low
> water mark was 0, which is what we believe disables event creation.
> This doesn't appear to be a safe default behavior?
>
> One option would be to change the type to a signed integer and change
> the meaning from number of blocks free to percentage free and then treat
> -1 as pick a sane default.  Thus 0 would be no events and 30 would
> indicate when 30% blocks free is remaining to generate an event.  We
> could also use an unsigned value and use 100 as please pick a sane
> default as this value and values close to it really wouldn't make much
> sense.

IMHO specifying the % in threshold is a good idea. You suggest to assume some
default threshold for newly created thin pool if user is not passing
anything. May be we can document this in lvm2app.h mentioning if '0'
passed as percentage it will be treated as the default thresold and 100%
means no event will be generated. What do you think about this policy?

>
> Once we have defined what the default values are, I can modify my python
> bindings patch to use these default values.
>
> Thanks,
> Tony

[PATCH V2 0/2] Add thin lv and thin pool creation support

[Tony Asleson]

On 02/01/2013 12:08 AM, M. Mohan Kumar wrote:
> Tony Asleson <tasleson@redhat.com> writes:
>> One option would be to change the type to a signed integer and change
>> the meaning from number of blocks free to percentage free and then treat
>> -1 as pick a sane default.  Thus 0 would be no events and 30 would
>> indicate when 30% blocks free is remaining to generate an event.  We
>> could also use an unsigned value and use 100 as please pick a sane
>> default as this value and values close to it really wouldn't make much
>> sense.
> 
> IMHO specifying the % in threshold is a good idea. You suggest to assume some
> default threshold for newly created thin pool if user is not passing
> anything. May be we can document this in lvm2app.h mentioning if '0'
> passed as percentage it will be treated as the default thresold and 100%
> means no event will be generated. What do you think about this policy?

This would be fine too.  As long as we clearly document what the value
represents and what the special values which denote disable and select
default we should be good.

Do we know what a reasonable threshold value should be?

Thanks,
Tony

[PATCH V2 0/2] Add thin lv and thin pool creation support

[M. Mohan Kumar]

Tony Asleson <tasleson@redhat.com> writes:

> On 02/01/2013 12:08 AM, M. Mohan Kumar wrote:
>> Tony Asleson <tasleson@redhat.com> writes:
>>> One option would be to change the type to a signed integer and change
>>> the meaning from number of blocks free to percentage free and then treat
>>> -1 as pick a sane default.  Thus 0 would be no events and 30 would
>>> indicate when 30% blocks free is remaining to generate an event.  We
>>> could also use an unsigned value and use 100 as please pick a sane
>>> default as this value and values close to it really wouldn't make much
>>> sense.
>> 
>> IMHO specifying the % in threshold is a good idea. You suggest to assume some
>> default threshold for newly created thin pool if user is not passing
>> anything. May be we can document this in lvm2app.h mentioning if '0'
>> passed as percentage it will be treated as the default thresold and 100%
>> means no event will be generated. What do you think about this policy?
>
> This would be fine too.  As long as we clearly document what the value
> represents and what the special values which denote disable and select
> default we should be good.
>
> Do we know what a reasonable threshold value should be?

I am not sure about the threshold value, should it bring it to dm folks?

[PATCH V2 0/2] Add thin lv and thin pool creation support

[M. Mohan Kumar]

Tony Asleson <tasleson@redhat.com> writes:

> On 02/01/2013 12:08 AM, M. Mohan Kumar wrote:
>> Tony Asleson <tasleson@redhat.com> writes:
>>> One option would be to change the type to a signed integer and change
>>> the meaning from number of blocks free to percentage free and then treat
>>> -1 as pick a sane default.  Thus 0 would be no events and 30 would
>>> indicate when 30% blocks free is remaining to generate an event.  We
>>> could also use an unsigned value and use 100 as please pick a sane
>>> default as this value and values close to it really wouldn't make much
>>> sense.
>> 
>> IMHO specifying the % in threshold is a good idea. You suggest to assume some
>> default threshold for newly created thin pool if user is not passing
>> anything. May be we can document this in lvm2app.h mentioning if '0'
>> passed as percentage it will be treated as the default thresold and 100%
>> means no event will be generated. What do you think about this policy?
>
> This would be fine too.  As long as we clearly document what the value
> represents and what the special values which denote disable and select
> default we should be good.
>
> Do we know what a reasonable threshold value should be?

Oops I meant, should we discuss with dm folks about default threshold?

[PATCH V2 0/2] Add thin lv and thin pool creation support

[Tony Asleson]

On 02/01/2013 11:28 AM, M. Mohan Kumar wrote:
> Tony Asleson <tasleson@redhat.com> writes:
>> Do we know what a reasonable threshold value should be?
> 
> Oops I meant, should we discuss with dm folks about default threshold?

Sure, please do.  I was hoping that some dm folks were lurking on this
list and would shout out :-)

Regards,
Tony