[linux-lvm] [PATCH v2] lvs: add -o lv_usable

[linux-lvm] [PATCH v2] lvs: add -o lv_usable

[Zhao Heming]

report LV is usable for upper layer.

leave issues
- this patch doesn't contain dm table comparison. So if the disk
  is removed then re-inserted, but the re-inserted disk
  major:minor is changed, the code doesn't have ability to detect.
- raid10: removing any 2 disks will think as array broken.

Signed-off-by: Zhao Heming <heming.zhao@suse.com>
---
v2:
- remove dm table parsing code in _lv_is_usable()
- add new status bit NOT_USABLE_LV. 
  note, I chose the first available bit 0x0000000080000000
- _lvusable_disp() uses lv_is_usable() to return usable status

---
 lib/metadata/metadata-exported.h |   3 +
 lib/metadata/metadata.c          | 110 ++++++++++++++++++++++++++++++-
 lib/report/columns.h             |   1 +
 lib/report/properties.c          |   2 +
 lib/report/report.c              |  10 +++
 lib/report/values.h              |   1 +
 6 files changed, 124 insertions(+), 3 deletions(-)

diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 670656a0f..dca9317a9 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -88,6 +88,8 @@
 #define PV_MOVED_VG		UINT64_C(0x4000000000000000)	/* PV - Moved to a new VG */
 #define PARTIAL_LV		UINT64_C(0x0000000001000000)	/* LV - derived flag, not
 							   written out in metadata*/
+#define NOT_USABLE_LV	UINT64_C(0x0000000080000000)	/* LV - derived flag, not
+							   written out in metadata*/
 
 #define WRITECACHE_ORIGIN	UINT64_C(0x0000000002000000)
 #define INTEGRITY_METADATA	UINT64_C(0x0000000004000000)    /* LV - Internal use only */
@@ -214,6 +216,7 @@
 
 #define lv_is_locked(lv)	(((lv)->status & LOCKED) ? 1 : 0)
 #define lv_is_partial(lv)	(((lv)->status & PARTIAL_LV) ? 1 : 0)
+#define lv_is_usable(lv)	(((lv)->status & NOT_USABLE_LV) ? 0 : 1)
 #define lv_is_virtual(lv)	(((lv)->status & VIRTUAL) ? 1 : 0)
 #define lv_is_merging(lv)	(((lv)->status & MERGING) ? 1 : 0)
 #define lv_is_merging_origin(lv) (lv_is_merging(lv) && (lv)->snapshot)
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 8b8c491c0..181a6441e 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -2043,17 +2043,118 @@ static int _lv_mark_if_partial_collect(struct logical_volume *lv, void *data)
 	return 1;
 }
 
+/*
+ * Return LV is still work or not when underlying dev is removed
+ *
+ * RAID:
+ * - raid 0: if there is any disk loose, return false
+ * - raid1,10,4/5,6: below case think as available, return true:
+ *   - raid 1: at least 1 disk live
+ *   - raid 10: loose 1 disk
+ *   - raid 4/5: loose 1 disk
+ *   - raid 6: loose 2 disk
+ *
+ * LINEAR:
+ * - if there is any disk loose, return false
+ *
+ * MIRROR:
+ * - mirror type won't be in 'not available' status, return true directly.
+ * - the failing rule
+ *   - 3-way mirror convert to 2-way mirror
+ *   - 2-way mirror to linear device
+ *
+ * For all other LV type (e.g. thin, cache, integrity, vdo etc):
+ * - return false if there is any underlying dev loose.
+ */
+static bool _lv_is_usable(const struct logical_volume *lv)
+{
+	int s, missing_pv = 0, exist_pv = 0;
+	bool ret = true;
+	struct lv_segment *seg = NULL;
+	struct physical_volume *pv = NULL;
+
+	/* see comment, return directly */
+	if (seg_is_mirror(first_seg(lv))) {
+		ret = true;
+		goto out;
+	}
+
+	dm_list_iterate_items(seg, &lv->segments) {
+		for (s = 0; s < seg->area_count; ++s) {
+			if (seg_type(seg, s) == AREA_LV) {
+				if (seg_lv(seg, s)->status & PARTIAL_LV)
+					missing_pv++;
+				else
+					exist_pv++;
+			} else if (seg_type(seg, s) == AREA_PV) {
+				pv = seg_pv(seg, s);
+				if (!(pv->dev) && is_missing_pv(pv))
+					missing_pv++;
+				else
+					exist_pv++;
+			}
+		}
+	}
+
+	seg = first_seg(lv);
+	if (seg_is_linear(seg)) {
+		ret = missing_pv ? false : true;
+		goto out;
+	}
+	if (seg_is_any_raid0(seg)) {
+		ret = missing_pv ? false : true;
+		goto out;
+	}
+	if (seg_is_raid1(seg)) {
+		ret = exist_pv ? true : false;
+		goto out;
+	}
+	/*
+	 * There are raid10 near/offset/far copy modes. 
+	 * It's silly to think false when missing_pv great than 1.
+	 */
+	if (seg_is_any_raid10(seg)) {
+		ret = (missing_pv > 1) ? false : true;
+		goto out;
+	}
+	if (seg_is_raid4(seg)) {
+		ret = (missing_pv > 1) ? false : true;
+		goto out;
+	}
+	if (seg_is_any_raid5(seg)) {
+		ret = (missing_pv > 1) ? false : true;
+		goto out;
+	}
+	if (seg_is_any_raid6(seg)) {
+		ret = (missing_pv > 2) ? false : true;
+		goto out;
+	}
+
+	/*
+	 * if code go there, the LV type must be thin, cache, integrity, vdo etc
+	 * return false if there is any underlying dev loose
+	 */
+	ret = missing_pv ? false : true;
+
+out:
+	return ret;
+}
+
 static int _lv_mark_if_partial_single(struct logical_volume *lv, void *data)
 {
 	unsigned s;
 	struct _lv_mark_if_partial_baton baton = { .partial = 0 };
-	struct lv_segment *lvseg;
+	struct lv_segment *lvseg = NULL;
+	struct physical_volume *pv = NULL;
 
 	dm_list_iterate_items(lvseg, &lv->segments) {
 		for (s = 0; s < lvseg->area_count; ++s) {
 			if (seg_type(lvseg, s) == AREA_PV) {
-				if (is_missing_pv(seg_pv(lvseg, s)))
+				pv = seg_pv(lvseg, s);
+				if (!(pv->dev) && is_missing_pv(pv)) {
 					lv->status |= PARTIAL_LV;
+					lv->status |= NOT_USABLE_LV;
+				}
 			}
 		}
 	}
@@ -2061,8 +2162,11 @@ static int _lv_mark_if_partial_single(struct logical_volume *lv, void *data)
 	if (!_lv_each_dependency(lv, _lv_mark_if_partial_collect, &baton))
 		return_0;
 
-	if (baton.partial)
+	if (baton.partial) {
 		lv->status |= PARTIAL_LV;
+		if (!_lv_is_usable(lv))
+			lv->status |= NOT_USABLE_LV;
+	}
 
 	return 1;
 }
diff --git a/lib/report/columns.h b/lib/report/columns.h
index 426a32c50..357c42530 100644
--- a/lib/report/columns.h
+++ b/lib/report/columns.h
@@ -145,6 +145,7 @@ FIELD(LVSSTATUS, lv, STR_LIST, "KCacheSettings", lvid, 18, kernel_cache_settings
 FIELD(LVSSTATUS, lv, STR, "KCachePolicy", lvid, 18, kernel_cache_policy, kernel_cache_policy, "Cache policy used in kernel.", 0)
 FIELD(LVSSTATUS, lv, NUM, "KMFmt", lvid, 0, kernelmetadataformat, kernel_metadata_format, "Cache metadata format used in kernel.", 0)
 FIELD(LVSSTATUS, lv, STR, "Health", lvid, 15, lvhealthstatus, lv_health_status, "LV health status.", 0)
+FIELD(LVSSTATUS, lv, STR, "Usable", lvid, 15, lvusable, lv_usable, "whether lvm believes the uppser layer can successfully do io to the entire LV.", 0)
 FIELD(LVSSTATUS, lv, STR, "KDiscards", lvid, 0, kdiscards, kernel_discards, "For thin pools, how discards are handled in kernel.", 0)
 FIELD(LVSSTATUS, lv, BIN, "CheckNeeded", lvid, 15, lvcheckneeded, lv_check_needed, "For thin pools and cache volumes, whether metadata check is needed.", 0)
 FIELD(LVSSTATUS, lv, BIN, "MergeFailed", lvid, 15, lvmergefailed, lv_merge_failed, "Set if snapshot merge failed.", 0)
diff --git a/lib/report/properties.c b/lib/report/properties.c
index d4ac8c47e..e3d64a5d6 100644
--- a/lib/report/properties.c
+++ b/lib/report/properties.c
@@ -296,6 +296,8 @@ GET_PV_NUM_PROPERTY_FN(pv_ba_size, SECTOR_SIZE * pv->ba_size)
 #define _lv_device_open_get prop_not_implemented_get
 #define _lv_health_status_set prop_not_implemented_set
 #define _lv_health_status_get prop_not_implemented_get
+#define _lv_usable_set prop_not_implemented_set
+#define _lv_usable_get prop_not_implemented_get
 #define _lv_skip_activation_set prop_not_implemented_set
 #define _lv_skip_activation_get prop_not_implemented_get
 #define _lv_check_needed_set prop_not_implemented_set
diff --git a/lib/report/report.c b/lib/report/report.c
index 73a150a7e..51b5b98f5 100644
--- a/lib/report/report.c
+++ b/lib/report/report.c
@@ -3903,6 +3903,16 @@ static int _lvhealthstatus_disp(struct dm_report *rh, struct dm_pool *mem,
 	return _field_string(rh, field, health);
 }
 
+static int _lvusable_disp(struct dm_report *rh, struct dm_pool *mem,
+				struct dm_report_field *field,
+				const void *data, void *private)
+{
+	const struct lv_with_info_and_seg_status *lvdm = (const struct lv_with_info_and_seg_status *) data;
+	const struct logical_volume *lv = lvdm->lv;
+
+	return _field_string(rh, field, lv_is_usable(lv) ? "usable" : "not usable");
+}
+
 static int _lvcheckneeded_disp(struct dm_report *rh, struct dm_pool *mem,
 			       struct dm_report_field *field,
 			       const void *data, void *private)
diff --git a/lib/report/values.h b/lib/report/values.h
index 9b98c229e..53f285db6 100644
--- a/lib/report/values.h
+++ b/lib/report/values.h
@@ -102,6 +102,7 @@ FIELD_RESERVED_VALUE(NAMED | RANGE | FUZZY | DYNAMIC, lv_time_removed, lv_time_r
 FIELD_RESERVED_VALUE(NOFLAG, cache_policy, cache_policy_undef, "", "", "", "undefined")
 FIELD_RESERVED_VALUE(NOFLAG, seg_monitor, seg_monitor_undef, "", "", "", "undefined")
 FIELD_RESERVED_VALUE(NOFLAG, lv_health_status, health_undef, "", "", "", "undefined")
+FIELD_RESERVED_VALUE(NOFLAG, lv_usable, usable_undef, "", "", "", "undefined")
 FIELD_RESERVED_VALUE(NOFLAG, kernel_discards, seg_kernel_discards_undef, "", "", "", "undefined")
 FIELD_RESERVED_VALUE(NOFLAG, vdo_write_policy, vdo_write_policy_undef, "", "", "", "undefined")
 /* TODO the following 2 need STR_LIST support for reserved values
-- 
2.27.0

Re: [linux-lvm] [PATCH v2] lvs: add -o lv_usable

[Zdenek Kabelac]

Dne 09. 09. 20 v 18:47 Zhao Heming napsal(a):
> report LV is usable for upper layer.
> 
> leave issues
> - this patch doesn't contain dm table comparison. So if the disk
>    is removed then re-inserted, but the re-inserted disk
>    major:minor is changed, the code doesn't have ability to detect.
> - raid10: removing any 2 disks will think as array broken.
> 
> Signed-off-by: Zhao Heming <heming.zhao@suse.com>
> ---
> v2:
> - remove dm table parsing code in _lv_is_usable()
> - add new status bit NOT_USABLE_LV.
>    note, I chose the first available bit 0x0000000080000000
> - _lvusable_disp() uses lv_is_usable() to return usable status
> 
	dm_list_iterate_items(lvseg, &lv->segments) {
>   		for (s = 0; s < lvseg->area_count; ++s) {
>   			if (seg_type(lvseg, s) == AREA_PV) {
> -				if (is_missing_pv(seg_pv(lvseg, s)))
> +				pv = seg_pv(lvseg, s);
> +				if (!(pv->dev) && is_missing_pv(pv)) {
>   					lv->status |= PARTIAL_LV;
> +					lv->status |= NOT_USABLE_LV;
> +				}
>   			}
>   		}
>   	}

Hi

As it can be seen here - there is big intersection with meaning of
PARTIAL_LV.

And the question is - what does it mean in the context of various segment
types.

I believe we need to discuss with Heinz - whether we want to mark
Raid LVs partial in case they are actually 'only leg-pertial' and should
be actually activatable without partial activation  - which is ATM abused for 
this purpose.

ATM I'm not sure we want to introduce new flags, which has only slight
deviation from current partial flag - which should deserve closer look
of its meaning.

We'll try to find something with Heinz to agree with.


Zdenek

Re: [linux-lvm] [PATCH v2] lvs: add -o lv_usable

[heming.zhao]

On 9/10/20 1:17 AM, Zdenek Kabelac wrote:
> Dne 09. 09. 20 v 18:47 Zhao Heming napsal(a):
>> report LV is usable for upper layer.
>>
>> leave issues
>> - this patch doesn't contain dm table comparison. So if the disk
>> �� is removed then re-inserted, but the re-inserted disk
>> �� major:minor is changed, the code doesn't have ability to detect.
>> - raid10: removing any 2 disks will think as array broken.
>>
>> Signed-off-by: Zhao Heming <heming.zhao@suse.com>
>> ---
>> v2:
>> - remove dm table parsing code in _lv_is_usable()
>> - add new status bit NOT_USABLE_LV.
>> �� note, I chose the first available bit 0x0000000080000000
>> - _lvusable_disp() uses lv_is_usable() to return usable status
>>
>  ����dm_list_iterate_items(lvseg, &lv->segments) {
>> ��������� for (s = 0; s < lvseg->area_count; ++s) {
>> ������������� if (seg_type(lvseg, s) == AREA_PV) {
>> -��������������� if (is_missing_pv(seg_pv(lvseg, s)))
>> +��������������� pv = seg_pv(lvseg, s);
>> +��������������� if (!(pv->dev) && is_missing_pv(pv)) {
>> ��������������������� lv->status |= PARTIAL_LV;
>> +������������������� lv->status |= NOT_USABLE_LV;
>> +��������������� }
>> ������������� }
>> ��������� }
>> ����� }
> 
> Hi
> 
> As it can be seen here - there is big intersection with meaning of
> PARTIAL_LV.
> 
> And the question is - what does it mean in the context of various segment
> types.
> 
> I believe we need to discuss with Heinz - whether we want to mark
> Raid LVs partial in case they are actually 'only leg-pertial' and should
> be actually activatable without partial activation� - which is ATM abused for this purpose.
> 
> ATM I'm not sure we want to introduce new flags, which has only slight
> deviation from current partial flag - which should deserve closer look
> of its meaning.
> 
> We'll try to find something with Heinz to agree with.
> 
Ok, wait for feedback from Heinz.

I agree with you. the PARTIAL_LV is more closer to the new bit NOT_USABLE_LV.
There is another bit MISSING_PV, which is set when pv is missing or the pv is not workable.

 From my understanding, we could reuse the PARTIAL_LV to show different meaning according to different context. For example, in raid env, the top layer LV will be set PARTIAL_LV when the raid array not usable (e.g. raid0 missing a disk). Other cases, within raid limit, top layer raid LV won't be set. if following the rule, there will no need to set the new bit NOT_USABLE_LV.

Heming

Re: [linux-lvm] [PATCH v2] lvs: add -o lv_usable

[Heinz Mauelshagen]

On 9/10/20 8:34 AM, heming.zhao wrote:
> On 9/10/20 1:17 AM, Zdenek Kabelac wrote:
>> Dne 09. 09. 20 v 18:47 Zhao Heming napsal(a):
>>> report LV is usable for upper layer.
>>>
>>> leave issues
>>> - this patch doesn't contain dm table comparison. So if the disk
>>> �� is removed then re-inserted, but the re-inserted disk
>>> �� major:minor is changed, the code doesn't have ability to detect.
>>> - raid10: removing any 2 disks will think as array broken.
>>>
>>> Signed-off-by: Zhao Heming <heming.zhao@suse.com>
>>> ---
>>> v2:
>>> - remove dm table parsing code in _lv_is_usable()
>>> - add new status bit NOT_USABLE_LV.
>>> �� note, I chose the first available bit 0x0000000080000000
>>> - _lvusable_disp() uses lv_is_usable() to return usable status
>>>
>> �����dm_list_iterate_items(lvseg, &lv->segments) {
>>> ��������� for (s = 0; s < lvseg->area_count; ++s) {
>>> ������������� if (seg_type(lvseg, s) == AREA_PV) {
>>> -��������������� if (is_missing_pv(seg_pv(lvseg, s)))
>>> +��������������� pv = seg_pv(lvseg, s);
>>> +��������������� if (!(pv->dev) && is_missing_pv(pv)) {
>>> ��������������������� lv->status |= PARTIAL_LV;
>>> +������������������� lv->status |= NOT_USABLE_LV;
>>> +��������������� }
>>> ������������� }
>>> ��������� }
>>> ����� }
>>
>> Hi
>>
>> As it can be seen here - there is big intersection with meaning of
>> PARTIAL_LV.


The semantics of a usable LV is fuzzy by definition, because for 
instance a multi-segment PARTIAL_LV
linear LV with a subset if its segments missing is still accessible 
relative to the remaining segments
thus doesn't make it unusable.�� As a result, LVs failing t o activate 
would be the 'unusable ones'.
The later is given for RAID when it's, e.g.� missing more than its 
maximum number of parity devices
for striped RAID layouts.� So PARTIAL_LV is sufficient to tell that any 
LV is still partially usable.


>>
>> And the question is - what does it mean in the context of various 
>> segment
>> types.
>>
>> I believe we need to discuss with Heinz - whether we want to mark
>> Raid LVs partial in case they are actually 'only leg-pertial' and should
>> be actually activatable without partial activation� - which is ATM 
>> abused for this purpose.

Degraded RAID layouts are always usable unless more than its parity 
devices or all its mirrors failed because of missing PVs.� Hence such 
activatable RaidLVs are not partial at the LV but at the SubLV Level.

>>
>> ATM I'm not sure we want to introduce new flags, which has only slight
>> deviation from current partial flag - which should deserve closer look
>> of its meaning.
>>
>> We'll try to find something with Heinz to agree with.
>>
> Ok, wait for feedback from Heinz.

What are we missing if we define any SubLV partial state with 
PARTIAL_LV/not activatable and
leave it to the specific segment type handlers of the mappings on top of 
such SubLVs
to define their respective PARTIAL_LV state or reject activation. E.g. a 
fully usable RAID6 with a maximimum of 2 missing legs with those missing 
legs either being partial and RAID6 I/O addressing a missing segment 
-or- thise leg SubLVs not having been activated _not_ setting PATIAL_LV 
on the RAID6 LV ('lvs -oname,attr,devices' will show state details on 
the LV tree nodes).

Let's discuss this first before adding MISSING_PV to the picture...

FWIW:
raid0 mappings with a subset of missing segments may not be of much use 
but will provide data still.

Heinz


>
> I agree with you. the PARTIAL_LV is more closer to the new bit 
> NOT_USABLE_LV.
> There is another bit MISSING_PV, which is set when pv is missing or 
> the pv is not workable.

> From my understanding, we could reuse the PARTIAL_LV to show different 
> meaning according to different context. For example, in raid env, the 
> top layer LV will be set PARTIAL_LV when the raid array not usable 
> (e.g. raid0 missing a disk). Other cases, within raid limit, top layer 
> raid LV won't be set. if following the rule, there will no need to set 
> the new bit NOT_USABLE_LV.
>
> Heming
>
>
> _______________________________________________
> linux-lvm mailing list
> linux-lvm@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-lvm
> read the LVM HOW-TO at http://tldp.org/HOWTO/LVM-HOWTO/

Re: [linux-lvm] [PATCH v2] lvs: add -o lv_usable

[heming.zhao]

On 9/17/20 6:18 PM, Heinz Mauelshagen wrote:
> On 9/10/20 8:34 AM, heming.zhao wrote:
>> On 9/10/20 1:17 AM, Zdenek Kabelac wrote:
>>> Dne 09. 09. 20 v 18:47 Zhao Heming napsal(a):
>>>> report LV is usable for upper layer.
>>>>
>>>> leave issues
>>>> - this patch doesn't contain dm table comparison. So if the disk
>>>>    is removed then re-inserted, but the re-inserted disk
>>>>    major:minor is changed, the code doesn't have ability to detect.
>>>> - raid10: removing any 2 disks will think as array broken.
>>>>
>>>> Signed-off-by: Zhao Heming <heming.zhao@suse.com>
>>>> ---
>>>> v2:
>>>> - remove dm table parsing code in _lv_is_usable()
>>>> - add new status bit NOT_USABLE_LV.
>>>>    note, I chose the first available bit 0x0000000080000000
>>>> - _lvusable_disp() uses lv_is_usable() to return usable status
>>>>
>>>      dm_list_iterate_items(lvseg, &lv->segments) {
>>>>           for (s = 0; s < lvseg->area_count; ++s) {
>>>>               if (seg_type(lvseg, s) == AREA_PV) {
>>>> -                if (is_missing_pv(seg_pv(lvseg, s)))
>>>> +                pv = seg_pv(lvseg, s);
>>>> +                if (!(pv->dev) && is_missing_pv(pv)) {
>>>>                       lv->status |= PARTIAL_LV;
>>>> +                    lv->status |= NOT_USABLE_LV;
>>>> +                }
>>>>               }
>>>>           }
>>>>       }
>>>
>>> Hi
>>>
>>> As it can be seen here - there is big intersection with meaning of
>>> PARTIAL_LV.
> 
> 
> The semantics of a usable LV is fuzzy by definition, because for instance a multi-segment PARTIAL_LV
> linear LV with a subset if its segments missing is still accessible relative to the remaining segments
> thus doesn't make it unusable.   As a result, LVs failing t o activate would be the 'unusable ones'.
> The later is given for RAID when it's, e.g.  missing more than its maximum number of parity devices
> for striped RAID layouts.  So PARTIAL_LV is sufficient to tell that any LV is still partially usable.
> 
> 
the usable or unusable is up to from which side to see. I prefer to see from top to bottom, upper
software (e.g. FS, VM) see the virtual disk (e.g. which is made up of RaidLV) unusable when missing beyond
max limit parity devices.  Or linear LV missing any one underlying devices.
The reason is that there are only few system/kernel level issue which can make lvm not to work. e.g.
device-mapper layer doesn't work, lvm internal bugs. The missing devices won't block lvm issue io, and this
time kernel (lower dm layer) will report io error to lvm.
So we could make the usable definition:
- whether lvm believes the uppser layer can successfully do io to the entire LV

>>>
>>> And the question is - what does it mean in the context of various segment
>>> types.
>>>
>>> I believe we need to discuss with Heinz - whether we want to mark
>>> Raid LVs partial in case they are actually 'only leg-pertial' and should
>>> be actually activatable without partial activation  - which is ATM abused for this purpose.
> 
> Degraded RAID layouts are always usable unless more than its parity devices or all its mirrors failed because of missing PVs.  Hence such activatable RaidLVs are not partial at the LV but at the SubLV Level.
> 
agree.

>>>
>>> ATM I'm not sure we want to introduce new flags, which has only slight
>>> deviation from current partial flag - which should deserve closer look
>>> of its meaning.
>>>
>>> We'll try to find something with Heinz to agree with.
>>>
>> Ok, wait for feedback from Heinz.
> 
> What are we missing if we define any SubLV partial state with PARTIAL_LV/not activatable and
> leave it to the specific segment type handlers of the mappings on top of such SubLVs
> to define their respective PARTIAL_LV state or reject activation. E.g. a fully usable RAID6 with a maximimum of 2 missing legs with those missing legs either being partial and RAID6 I/O addressing a missing segment -or- thise leg SubLVs not having been activated _not_ setting PATIAL_LV on the RAID6 LV ('lvs -oname,attr,devices' will show state details on the LV tree nodes).
> 
> Let's discuss this first before adding MISSING_PV to the picture...
> 
not to active PARTIAL_LV means the SubLV doesn't work? by suspend dm-table?
It looks there is no big different between marking PARTIAL_LV flag and suspend dm-table.
For me, keep exist logic is more acceptable.

> FWIW:
> raid0 mappings with a subset of missing segments may not be of much use but will provide data still.
> 
this situation will make upper layer software work abnormally. if a upper layer software can directly manage
subset of raid0LV, (in my opinion) there is no reason to set up raid0.

> Heinz
> 
> 
>>
>> I agree with you. the PARTIAL_LV is more closer to the new bit NOT_USABLE_LV.
>> There is another bit MISSING_PV, which is set when pv is missing or the pv is not workable.
> 
>> From my understanding, we could reuse the PARTIAL_LV to show different meaning according to different context. For example, in raid env, the top layer LV will be set PARTIAL_LV when the raid array not usable (e.g. raid0 missing a disk). Other cases, within raid limit, top layer raid LV won't be set. if following the rule, there will no need to set the new bit NOT_USABLE_LV.
>>
>> Heming
>>
>>
>> _______________________________________________
>> linux-lvm mailing list
>> linux-lvm@redhat.com
>> https://www.redhat.com/mailman/listinfo/linux-lvm
>> read the LVM HOW-TO at http://tldp.org/HOWTO/LVM-HOWTO/
> 

Re: [linux-lvm] [PATCH v2] lvs: add -o lv_usable

[Heinz Mauelshagen]

On 9/18/20 9:07 AM, heming.zhao wrote:
> On 9/17/20 6:18 PM, Heinz Mauelshagen wrote:
>> On 9/10/20 8:34 AM, heming.zhao wrote:
>>> On 9/10/20 1:17 AM, Zdenek Kabelac wrote:
>>>> Dne 09. 09. 20 v 18:47 Zhao Heming napsal(a):
>>>>> report LV is usable for upper layer.
>>>>>
>>>>> leave issues
>>>>> - this patch doesn't contain dm table comparison. So if the disk
>>>>>    is removed then re-inserted, but the re-inserted disk
>>>>>    major:minor is changed, the code doesn't have ability to detect.
>>>>> - raid10: removing any 2 disks will think as array broken.
>>>>>
>>>>> Signed-off-by: Zhao Heming <heming.zhao@suse.com>
>>>>> ---
>>>>> v2:
>>>>> - remove dm table parsing code in _lv_is_usable()
>>>>> - add new status bit NOT_USABLE_LV.
>>>>>    note, I chose the first available bit 0x0000000080000000
>>>>> - _lvusable_disp() uses lv_is_usable() to return usable status
>>>>>
>>>>      dm_list_iterate_items(lvseg, &lv->segments) {
>>>>>           for (s = 0; s < lvseg->area_count; ++s) {
>>>>>               if (seg_type(lvseg, s) == AREA_PV) {
>>>>> -                if (is_missing_pv(seg_pv(lvseg, s)))
>>>>> +                pv = seg_pv(lvseg, s);
>>>>> +                if (!(pv->dev) && 
>>>>> is_missing_pv(pv)) {
>>>>>                       lv->status |= PARTIAL_LV;
>>>>> +                    lv->status |= NOT_USABLE_LV;
>>>>> +                }
>>>>>               }
>>>>>           }
>>>>>       }
>>>>
>>>> Hi
>>>>
>>>> As it can be seen here - there is big intersection with meaning of
>>>> PARTIAL_LV.
>>
>>
>> The semantics of a usable LV is fuzzy by definition, because for 
>> instance a multi-segment PARTIAL_LV
>> linear LV with a subset if its segments missing is still accessible 
>> relative to the remaining segments
>> thus doesn't make it unusable.   As a result, LVs failing t o 
>> activate would be the 'unusable ones'.
>> The later is given for RAID when it's, e.g.  missing more than its 
>> maximum number of parity devices
>> for striped RAID layouts.  So PARTIAL_LV is sufficient to tell that 
>> any LV is still partially usable.
>>
>>
> the usable or unusable is up to from which side to see. I prefer to 
> see from top to bottom, upper
> software (e.g. FS, VM) see the virtual disk (e.g. which is made up of 
> RaidLV) unusable when missing beyond
> max limit parity devices.

We are on the same plate looking downstack.

> Or linear LV missing any one underlying devices.
> The reason is that there are only few system/kernel level issue which 
> can make lvm not to work. e.g.
> device-mapper layer doesn't work, lvm internal bugs. The missing 
> devices won't block lvm issue io, and this
> time kernel (lower dm layer) will report io error to lvm.
> So we could make the usable definition:
> - whether lvm believes the uppser layer can successfully do io to the 
> entire LV


...which is the semantics of PARTIAL_LV state flag when it is set (i.e. 
parts of the LV are accessible fine
and other parts will cause I/O errors). So fully usable is the adequate 
of 'activated && !PARTIAL_LV'.


>
>>>>
>>>> And the question is - what does it mean in the context of various 
>>>> segment
>>>> types.
>>>>
>>>> I believe we need to discuss with Heinz - whether we want to mark
>>>> Raid LVs partial in case they are actually 'only leg-pertial' and 
>>>> should
>>>> be actually activatable without partial activation  - which is ATM 
>>>> abused for this purpose.
>>
>> Degraded RAID layouts are always usable unless more than its parity 
>> devices or all its mirrors failed because of missing PVs.  Hence 
>> such activatable RaidLVs are not partial at the LV but at the SubLV 
>> Level.
>>
> agree.
>
>>>>
>>>> ATM I'm not sure we want to introduce new flags, which has only slight
>>>> deviation from current partial flag - which should deserve closer look
>>>> of its meaning.
>>>>
>>>> We'll try to find something with Heinz to agree with.
>>>>
>>> Ok, wait for feedback from Heinz.
>>
>> What are we missing if we define any SubLV partial state with 
>> PARTIAL_LV/not activatable and
>> leave it to the specific segment type handlers of the mappings on top 
>> of such SubLVs
>> to define their respective PARTIAL_LV state or reject activation. 
>> E.g. a fully usable RAID6 with a maximimum of 2 missing legs with 
>> those missing legs either being partial and RAID6 I/O addressing a 
>> missing segment -or- thise leg SubLVs not having been activated _not_ 
>> setting PATIAL_LV on the RAID6 LV ('lvs -oname,attr,devices' will 
>> show state details on the LV tree nodes).
>>
>> Let's discuss this first before adding MISSING_PV to the picture...
>>
> not to active PARTIAL_LV means the SubLV doesn't work? by suspend 
> dm-table?

PARTIAL_LV will allow the activation with segments mapped to 'error' 
targets.
IOW: the table will be resumed with segment mappings replaced.

E.g. (linear with 4 segements split across 4 PVs with PV#2 missing):

# dmsetup table t-l
0 2088960 linear 8:0 2048
2088960 2088960 linear 254:2 0
4177920 2088960 linear 65:176 2048
6266880 24576 linear 65:192 2048

# ll /dev/mapper/|grep dm-2
lrwxrwxrwx. 1 root root������ 7 Sep 18 13:25 t-l-missing_1_0 -> ../dm-2

# dmsetup table t-l-missing_1_0
0 2088960 error

FWIW: this works even all segments are gone presuming the VG is still 
accessible.


> It looks there is no big different between marking PARTIAL_LV flag and 
> suspend dm-table.
> For me, keep exist logic is more acceptable.
>
>> FWIW:
>> raid0 mappings with a subset of missing segments may not be of much 
>> use but will provide data still.
>>
> this situation will make upper layer software work abnormally. if a 
> upper layer software can directly manage
> subset of raid0LV, (in my opinion) there is no reason to set up raid0.
>
Right, you're seconding what I stated as '...not be of much use...'


So what do you and Zdenek think about the proposal to tag any LV tree 
nodes with PARTIAL_LV
on behalf of the involved segment type handler of the respective node 
(e.g. linear has to set it on any missing segment as oposed to RAID 
setting it if degradation prevents full access)?

>> Heinz
>>
>>
>>>
>>> I agree with you. the PARTIAL_LV is more closer to the new bit 
>>> NOT_USABLE_LV.
>>> There is another bit MISSING_PV, which is set when pv is missing or 
>>> the pv is not workable.
>>
>>> From my understanding, we could reuse the PARTIAL_LV to show 
>>> different meaning according to different context. For example, in 
>>> raid env, the top layer LV will be set PARTIAL_LV when the raid 
>>> array not usable (e.g. raid0 missing a disk). Other cases, within 
>>> raid limit, top layer raid LV won't be set. if following the rule, 
>>> there will no need to set the new bit NOT_USABLE_LV.
>>>
>>> Heming
>>>
>>>
>>> _______________________________________________
>>> linux-lvm mailing list
>>> linux-lvm@redhat.com
>>> https://www.redhat.com/mailman/listinfo/linux-lvm
>>> read the LVM HOW-TO at http://tldp.org/HOWTO/LVM-HOWTO/
>>
>

Re: [linux-lvm] [PATCH v2] lvs: add -o lv_usable

[heming.zhao]

On 9/18/20 7:38 PM, Heinz Mauelshagen wrote:
> 
> On 9/18/20 9:07 AM, heming.zhao wrote:
>> On 9/17/20 6:18 PM, Heinz Mauelshagen wrote:
>>> On 9/10/20 8:34 AM, heming.zhao wrote:
>>>> On 9/10/20 1:17 AM, Zdenek Kabelac wrote:
>>>>> Dne 09. 09. 20 v 18:47 Zhao Heming napsal(a):
>>>>>> report LV is usable for upper layer.
>>>>>>
>>>>>> leave issues
>>>>>> - this patch doesn't contain dm table comparison. So if the disk
>>>>>>    is removed then re-inserted, but the re-inserted disk
>>>>>>    major:minor is changed, the code doesn't have ability to detect.
>>>>>> - raid10: removing any 2 disks will think as array broken.
>>>>>>
>>>>>> Signed-off-by: Zhao Heming <heming.zhao@suse.com>
>>>>>> ---
>>>>>> v2:
>>>>>> - remove dm table parsing code in _lv_is_usable()
>>>>>> - add new status bit NOT_USABLE_LV.
>>>>>>    note, I chose the first available bit 0x0000000080000000
>>>>>> - _lvusable_disp() uses lv_is_usable() to return usable status
>>>>>>
>>>>>      dm_list_iterate_items(lvseg, &lv->segments) {
>>>>>>           for (s = 0; s < lvseg->area_count; ++s) {
>>>>>>               if (seg_type(lvseg, s) == AREA_PV) {
>>>>>> -                if (is_missing_pv(seg_pv(lvseg, s)))
>>>>>> +                pv = seg_pv(lvseg, s);
>>>>>> +                if (!(pv->dev) && is_missing_pv(pv)) {
>>>>>>                       lv->status |= PARTIAL_LV;
>>>>>> +                    lv->status |= NOT_USABLE_LV;
>>>>>> +                }
>>>>>>               }
>>>>>>           }
>>>>>>       }
>>>>>
>>>>> Hi
>>>>>
>>>>> As it can be seen here - there is big intersection with meaning of
>>>>> PARTIAL_LV.
>>>
>>>
>>> The semantics of a usable LV is fuzzy by definition, because for instance a multi-segment PARTIAL_LV
>>> linear LV with a subset if its segments missing is still accessible relative to the remaining segments
>>> thus doesn't make it unusable.   As a result, LVs failing t o activate would be the 'unusable ones'.
>>> The later is given for RAID when it's, e.g.  missing more than its maximum number of parity devices
>>> for striped RAID layouts.  So PARTIAL_LV is sufficient to tell that any LV is still partially usable.
>>>
>>>
>> the usable or unusable is up to from which side to see. I prefer to see from top to bottom, upper
>> software (e.g. FS, VM) see the virtual disk (e.g. which is made up of RaidLV) unusable when missing beyond
>> max limit parity devices.
> 
> We are on the same plate looking downstack.
> 
>> Or linear LV missing any one underlying devices.
>> The reason is that there are only few system/kernel level issue which can make lvm not to work. e.g.
>> device-mapper layer doesn't work, lvm internal bugs. The missing devices won't block lvm issue io, and this
>> time kernel (lower dm layer) will report io error to lvm.
>> So we could make the usable definition:
>> - whether lvm believes the uppser layer can successfully do io to the entire LV
> 
> 
> ...which is the semantics of PARTIAL_LV state flag when it is set (i.e. parts of the LV are accessible fine
> and other parts will cause I/O errors). So fully usable is the adequate of 'activated && !PARTIAL_LV'.
> 
> 
>>
>>>>>
>>>>> And the question is - what does it mean in the context of various segment
>>>>> types.
>>>>>
>>>>> I believe we need to discuss with Heinz - whether we want to mark
>>>>> Raid LVs partial in case they are actually 'only leg-pertial' and should
>>>>> be actually activatable without partial activation  - which is ATM abused for this purpose.
>>>
>>> Degraded RAID layouts are always usable unless more than its parity devices or all its mirrors failed because of missing PVs.  Hence such activatable RaidLVs are not partial at the LV but at the SubLV Level.
>>>
>> agree.
>>
>>>>>
>>>>> ATM I'm not sure we want to introduce new flags, which has only slight
>>>>> deviation from current partial flag - which should deserve closer look
>>>>> of its meaning.
>>>>>
>>>>> We'll try to find something with Heinz to agree with.
>>>>>
>>>> Ok, wait for feedback from Heinz.
>>>
>>> What are we missing if we define any SubLV partial state with PARTIAL_LV/not activatable and
>>> leave it to the specific segment type handlers of the mappings on top of such SubLVs
>>> to define their respective PARTIAL_LV state or reject activation. E.g. a fully usable RAID6 with a maximimum of 2 missing legs with those missing legs either being partial and RAID6 I/O addressing a missing segment -or- thise leg SubLVs not having been activated _not_ setting PATIAL_LV on the RAID6 LV ('lvs -oname,attr,devices' will show state details on the LV tree nodes).
>>>
>>> Let's discuss this first before adding MISSING_PV to the picture...
>>>
>> not to active PARTIAL_LV means the SubLV doesn't work? by suspend dm-table?
> 
> PARTIAL_LV will allow the activation with segments mapped to 'error' targets.
> IOW: the table will be resumed with segment mappings replaced.
> 
> E.g. (linear with 4 segements split across 4 PVs with PV#2 missing):
> 
> # dmsetup table t-l
> 0 2088960 linear 8:0 2048
> 2088960 2088960 linear 254:2 0
> 4177920 2088960 linear 65:176 2048
> 6266880 24576 linear 65:192 2048
> 
> # ll /dev/mapper/|grep dm-2
> lrwxrwxrwx. 1 root root       7 Sep 18 13:25 t-l-missing_1_0 -> ../dm-2
> 
> # dmsetup table t-l-missing_1_0
> 0 2088960 error
> 
> FWIW: this works even all segments are gone presuming the VG is still accessible.
> 
which level/layer does the error dm-table set? only the bottom linear level?

when missing is happening, within parity number, only set error dm-table on missed dev?
from upper layer software (e.g. fs, vm), this is no different behavior between the error is returned from
dm layer (by error type) and from scsi layer (lower dm layer). the result is upper layer know virtual disk
doesn't work.

when PARTIAL_LV number beyond max limited number, to set error dm-table on all the bottom level's linear devices? 
it will totally block upper layer to issue IO on exist devs, and protect exist data.
if your meaning is that this is new behavior of lvm. 
(i'm not sure) It looks the kernel raid code also blocking issue io in this condition.
So the setting error type dm-table is very/only useful for linear type. 

the purpose of I filed this patch is to give end user more info about raid env. not improve lvm error handling.
From my view point, this is a little repeated for exist md (raid) layer error handling.
for lvm or dm special virtual devices (like linear, thin?), I support to add new error handling.

> 
>> It looks there is no big different between marking PARTIAL_LV flag and suspend dm-table.
>> For me, keep exist logic is more acceptable.
>>
>>> FWIW:
>>> raid0 mappings with a subset of missing segments may not be of much use but will provide data still.
>>>
>> this situation will make upper layer software work abnormally. if a upper layer software can directly manage
>> subset of raid0LV, (in my opinion) there is no reason to set up raid0.
>>
> Right, you're seconding what I stated as '...not be of much use...'
> 
> 
> So what do you and Zdenek think about the proposal to tag any LV tree nodes with PARTIAL_LV
> on behalf of the involved segment type handler of the respective node (e.g. linear has to set it on any missing segment as oposed to RAID setting it if degradation prevents full access)?
> 
>>> Heinz
>>>
>>>
>>>>
>>>> I agree with you. the PARTIAL_LV is more closer to the new bit NOT_USABLE_LV.
>>>> There is another bit MISSING_PV, which is set when pv is missing or the pv is not workable.
>>>
>>>> From my understanding, we could reuse the PARTIAL_LV to show different meaning according to different context. For example, in raid env, the top layer LV will be set PARTIAL_LV when the raid array not usable (e.g. raid0 missing a disk). Other cases, within raid limit, top layer raid LV won't be set. if following the rule, there will no need to set the new bit NOT_USABLE_LV.
>>>>
>>>> Heming
>>>>
>>>>
>>>> _______________________________________________
>>>> linux-lvm mailing list
>>>> linux-lvm@redhat.com
>>>> https://www.redhat.com/mailman/listinfo/linux-lvm
>>>> read the LVM HOW-TO at http://tldp.org/HOWTO/LVM-HOWTO/
>>>
>>
>