[linux-lvm] [PATCH] lib/metadata: add new api lv_is_available()

[linux-lvm] [PATCH] lib/metadata: add new api lv_is_available()

[Zhao Heming]

It commit is enhancement for 1d0dc74f9147e3c1f3681efa4166cbe2edcb6571
1d0dc74f9147 only supports all raid type, this commit adds new api
and extends support scope with linear/mirror type.

This patch introduced a new flag NOT_AVAIL_LV, when a lv (includeing
sub-lv or child-lv) doesn't work, this flag will set.

With this patch, for linear type LV, one of two disks missing,
lvdisplay will show
from:
  LV Status              available (partial)
to:
  LV Status              NOT available (partial)

Signed-off-by: Zhao Heming <heming.zhao@suse.com>
---
 lib/display/display.c            |  7 +--
 lib/metadata/metadata-exported.h |  3 +
 lib/metadata/metadata.c          | 95 +++++++++++++++++++++++++++++++-
 lib/metadata/segtype.h           |  1 -
 lib/raid/raid.c                  | 54 ------------------
 5 files changed, 99 insertions(+), 61 deletions(-)

diff --git a/lib/display/display.c b/lib/display/display.c
index 3bb570f03..7c4ec9bb3 100644
--- a/lib/display/display.c
+++ b/lib/display/display.c
@@ -399,7 +399,7 @@ int lvdisplay_full(struct cmd_context *cmd,
 		   void *handle __attribute__((unused)))
 {
 	struct lvinfo info;
-	int inkernel, snap_active = 0, partial = 0, raid_is_avail = 1;
+	int inkernel, snap_active = 0, partial = 0, lv_is_avail = 1;
 	char uuid[64] __attribute__((aligned(8)));
 	const char *access_str;
 	struct lv_segment *snap_seg = NULL, *mirror_seg = NULL;
@@ -555,15 +555,14 @@ int lvdisplay_full(struct cmd_context *cmd,
 
 	if (lv_is_partial(lv)) {
 		partial = 1;
-		if (lv_is_raid(lv))
-			raid_is_avail = raid_is_available(lv) ? 1 : 0;
+		lv_is_avail = lv_is_available(lv) ? 1 : 0;
 	}
 
 	if (inkernel && info.suspended)
 		log_print("LV Status              suspended");
 	else if (activation())
 		log_print("LV Status              %savailable %s",
-			  (inkernel && raid_is_avail) ? "" : "NOT ",
+			  (inkernel && lv_is_avail) ? "" : "NOT ",
 			  partial ? "(partial)" : "");
 
 /********* FIXME lv_number
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 06ea757b8..a7fb88e6e 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_AVAIL_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_available(lv)	(((lv)->status & NOT_AVAIL_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 c0d42066d..828788566 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -2043,6 +2043,92 @@ 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
+ *
+ * TODO:
+ * thin, cache, integrity, vdo etc.
+ */
+static bool _lv_is_available(const struct logical_volume *lv)
+{
+	int s, missing_pv = 0, exist_pv = 0;
+	bool ret = true;
+	struct lv_segment *seg = 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) {
+				if (is_missing_pv(seg_pv(seg, s)))
+					missing_pv++;
+				else
+					exist_pv++;
+			}
+		}
+	}
+
+	log_print("missing pv: %d exist pv: %d", missing_pv, 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;
+	}
+	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;
+	}
+
+out:
+	return ret;
+}
+
 static int _lv_mark_if_partial_single(struct logical_volume *lv, void *data)
 {
 	unsigned s;
@@ -2052,8 +2138,10 @@ static int _lv_mark_if_partial_single(struct logical_volume *lv, void *data)
 	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)))
+				if (is_missing_pv(seg_pv(lvseg, s))) {
 					lv->status |= PARTIAL_LV;
+					lv->status |= NOT_AVAIL_LV;
+				}
 			}
 		}
 	}
@@ -2061,8 +2149,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_available(lv))
+			lv->status |= NOT_AVAIL_LV;
+	}
 
 	return 1;
 }
diff --git a/lib/metadata/segtype.h b/lib/metadata/segtype.h
index 1a840707d..08ddc3565 100644
--- a/lib/metadata/segtype.h
+++ b/lib/metadata/segtype.h
@@ -326,7 +326,6 @@ struct segment_type *init_unknown_segtype(struct cmd_context *cmd,
 
 #ifdef RAID_INTERNAL
 int init_raid_segtypes(struct cmd_context *cmd, struct segtype_library *seglib);
-bool raid_is_available(const struct logical_volume *lv);
 #endif
 
 #define THIN_FEATURE_DISCARDS			(1U << 0)
diff --git a/lib/raid/raid.c b/lib/raid/raid.c
index 4ed77e9a8..e88a15408 100644
--- a/lib/raid/raid.c
+++ b/lib/raid/raid.c
@@ -25,60 +25,6 @@
 #include "lib/metadata/metadata.h"
 #include "lib/metadata/lv_alloc.h"
 
-/*
- * below case think as available, return true:
- * - raid 1:@least 1 disk live
- * - raid 10: loose 1 disk
- * - raid 4/5: loose 1 disk
- * - raid 6: loose 2 disk
- *
- * raid 0: if there is any disk loose, return false
- * */
-bool raid_is_available(const struct logical_volume *lv)
-{
-	int s, missing_pv = 0, exist_pv = 0;
-	bool ret = true;
-	struct lv_segment *seg = NULL;
-
-	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++;
-			}
-		}
-	}
-	if (seg_is_any_raid0(first_seg(lv))){
-		ret = missing_pv ? false : true;
-		goto out;
-	}
-	if (seg_is_raid1(first_seg(lv))){
-		ret = exist_pv ? true : false;
-		goto out;
-	}
-	if (seg_is_any_raid10(first_seg(lv))) {
-		ret = (missing_pv > 1) ? false : true;
-		goto out;
-	}
-	if (seg_is_raid4(first_seg(lv))) {
-		ret = (missing_pv > 1) ? false : true;
-		goto out;
-	}
-	if (seg_is_any_raid5(first_seg(lv))) {
-		ret = (missing_pv > 1) ? false : true;
-		goto out;
-	}
-	if (seg_is_any_raid6(first_seg(lv))) {
-		ret = (missing_pv > 2) ? false : true;
-		goto out;
-	}
-
-out:
-	return ret;
-}
-
 static int _raid_target_present(struct cmd_context *cmd,
 				const struct lv_segment *seg __attribute__((unused)),
 				unsigned *attributes);
-- 
2.27.0

Re: [linux-lvm] [PATCH] lib/metadata: add new api lv_is_available()

[heming.zhao]

please ignore this patch. I forgot to add [patch v2] & history info.

On 8/28/20 12:05 AM, Zhao Heming wrote:
> It commit is enhancement for 1d0dc74f9147e3c1f3681efa4166cbe2edcb6571
> 1d0dc74f9147 only supports all raid type, this commit adds new api
> and extends support scope with linear/mirror type.
> 
> This patch introduced a new flag NOT_AVAIL_LV, when a lv (includeing
> sub-lv or child-lv) doesn't work, this flag will set.
> 
> With this patch, for linear type LV, one of two disks missing,
> lvdisplay will show
> from:
>    LV Status              available (partial)
> to:
>    LV Status              NOT available (partial)
> 
> Signed-off-by: Zhao Heming <heming.zhao@suse.com>

Re: [linux-lvm] [PATCH] lib/metadata: add new api lv_is_available()

[Zdenek Kabelac]

Dne 27. 08. 20 v 18:05 Zhao Heming napsal(a):
> It commit is enhancement for 1d0dc74f9147e3c1f3681efa4166cbe2edcb6571
> 1d0dc74f9147 only supports all raid type, this commit adds new api
> and extends support scope with linear/mirror type.
> 
> This patch introduced a new flag NOT_AVAIL_LV, when a lv (includeing
> sub-lv or child-lv) doesn't work, this flag will set.
> 
> With this patch, for linear type LV, one of two disks missing,
> lvdisplay will show
> from:
>    LV Status              available (partial)
> to:
>    LV Status              NOT available (partial)
> 

Hi



LV 'available' has one simple meaning - LV is present in DM table.
Status is either available or NOT available.

Partially is completely different flag.

Please do not mix existing logic and existing script with some new unknown args.


Also note - preferred usage is 'lvs'  - lvdisplay is mostly a backward 
compatible tool with sometimes 'easier' to get some info - but normally all
the info should be obtained with 'lvs'  tool which is highly configurable
and support much easier output for parsing in scripts.
(lvdisplay is missing TONS of attributes otherwise accessible with lvs)


Regards


Zdenek

Re: [linux-lvm] [PATCH] lib/metadata: add new api lv_is_available()

[David Teigland]

On Fri, Aug 28, 2020 at 06:04:44PM +0200, Zdenek Kabelac wrote:
> LV 'available' has one simple meaning - LV is present in DM table.
> Status is either available or NOT available.

LV Status can also be "suspended", so there's already some variation in
that field (which is why these free-form lvdisplay fields should be
avoided.)  But I agree, it's probably best to leave lvdisplay alone for
historical purposes and add anything new using just lvs fields.

Zhao, I think we should revert that commit, and come up with a new word to
describe this thing (which also needs a clear definition), and report it
in lvs.

Dave

Re: [linux-lvm] [PATCH] lib/metadata: add new api lv_is_available()

[heming.zhao]

Hello David, & Zdenek,

I got your meaning.
Thanks to take time to review my patch and explain the reason.

---
For Dave's question:
> Zhao, I think we should revert that commit, and come up with a new word to
> describe this thing (which also needs a clear definition), and report it
> in lvs.

For the lvs, I ever said in previous mail:
 is it necessary to add a new letter '(N)ot_available' in bit 9 (Volume Health) for top layer LV.

in my opinion, the 'not available' means the LV can't correctly do r/w io.

for raid, if the missing or breaking underlying devs number beyond raid level limit. the 
'not available' shoud be display.

for linear, any one of underlying dev is missing, upper layer module like fs may don't work
(e.g. missing first disk, fs will missing w/r first disk's super-block metadata). the
'not available' should be display.

for other type LV, I don't have too much experience to answer this question.

I give two example for my meaning
1> 
a raid1 lv missing one of underlying devs, the bit-9 won't change, keep 'p'
# lvs -a
  LV                 VG  Attr       LSize  Pool Origin Data%  Meta%  Move Log Cpy%Sync Convert
  raid1lv            vg1 rwi-a-r-p- 92.00m                                    100.00          
  [raid1lv_rimage_0] vg1 iwi-aor--- 92.00m                                                    
  [raid1lv_rimage_1] vg1 iwi-aor-p- 92.00m                                                    
  [raid1lv_rmeta_0]  vg1 ewi-aor---  4.00m                                                    
  [raid1lv_rmeta_1]  vg1 ewi-aor-p-  4.00m 

2>
if a raid0 is missing one of underlying devs, the bit-9 should change from below status:
# lvs -a
  WARNING: Couldn't find device with uuid iOTRmc-hdJx-p3Cd-DrBR-WRJo-Xweh-GTDJUg.
  WARNING: VG vg1 is missing PV iOTRmc-hdJx-p3Cd-DrBR-WRJo-Xweh-GTDJUg.
  LV                 VG  Attr       LSize   Pool Origin Data%  Meta%  Move Log Cpy%Sync Convert
  raid0lv            vg1 rwi-a-r-p- 192.00m                                                    
  [raid0lv_rimage_0] vg1 iwi-aor---  96.00m                                                    
  [raid0lv_rimage_1] vg1 iwi-aor-p-  96.00m

to below (a new 'n' letter):
# lvs -a
  WARNING: Couldn't find device with uuid iOTRmc-hdJx-p3Cd-DrBR-WRJo-Xweh-GTDJUg.
  WARNING: VG vg1 is missing PV iOTRmc-hdJx-p3Cd-DrBR-WRJo-Xweh-GTDJUg.
  LV                 VG  Attr       LSize   Pool Origin Data%  Meta%  Move Log Cpy%Sync Convert
  raid0lv            vg1 rwi-a-r-n- 192.00m                                                    
  [raid0lv_rimage_0] vg1 iwi-aor---  96.00m                                                    
  [raid0lv_rimage_1] vg1 iwi-aor-p-  96.00m


At last, the new letter attr needs a lot of jobs to do and need to take care of various LV type.
I can change my patch to modify lvs attr field for linear & raid type. 
But other LV types works need to do by other guys. 

Thanks,
heming

On 8/29/20 2:26 AM, David Teigland wrote:
> On Fri, Aug 28, 2020 at 06:04:44PM +0200, Zdenek Kabelac wrote:
>> LV 'available' has one simple meaning - LV is present in DM table.
>> Status is either available or NOT available.
> 
> LV Status can also be "suspended", so there's already some variation in
> that field (which is why these free-form lvdisplay fields should be
> avoided.)  But I agree, it's probably best to leave lvdisplay alone for
> historical purposes and add anything new using just lvs fields.
> 
> Zhao, I think we should revert that commit, and come up with a new word to
> describe this thing (which also needs a clear definition), and report it
> in lvs.
> 
> Dave
> 

Re: [linux-lvm] [PATCH] lib/metadata: add new api lv_is_available()

[Zdenek Kabelac]

Dne 30. 08. 20 v 17:49 heming.zhao@suse.com napsal(a):
> Hello David, & Zdenek,
> 
>> For the lvs, I ever said in previous mail:
>   is it necessary to add a new letter '(N)ot_available' in bit 9 (Volume Health) for top layer LV.

Hi

Bit-attr fields are somewhat 'overloaded' and more or less 'confusing' already 
and serve rather 'quick' check purpose for skilled lvm user.

So normally you would likely either want to experiment with field:

lvs -o+lv_health_status

or maybe there is needed some extra new field
(i.e. thin,cache,vdo... has lots of explicit fields)

Having and explicit 1-purpose thing is our current 'prefered' logic.

> in my opinion, the 'not available' means the LV can't correctly do r/w io.

The main trouble with adding 'more meanings' to the existing fields is - you
have big troubles if you have existing users/scripts and they are faced with
unknown output in fields.

That's why in general the 'availability' fields should not be mixed with 
meaning about healthiness or correctness or usability or whatever else comes.

> for raid, if the missing or breaking underlying devs number beyond raid level limit. the
> 'not available' shoud be display.

In practice there is ATM quite some fuzzy land at handling all states of mdadm 
raid logic within lvm2 -  it's not clear even in mdadm context and the lvm2 
layer with it's  PV & VG model makes it even way more complicated
(as 1 PV might be broken in many different ways - far more then 'mdadm' 
meaning of present/missing).

> for linear, any one of underlying dev is missing, upper layer module like fs may don't work
> (e.g. missing first disk, fs will missing w/r first disk's super-block metadata). the
> 'not available' should be display.

Here comes the issue you might not have yet realized.

Activation code currently has intentional 'disconnection' between metadata and 
real disk state. So there is big difference if the PV is missing and lvm2 
metadata KNOWS about the thing or the disk is missing and metadata 'thinks' 
device is there.

This has very big impact on the overall complexity of the whole 'activation' 
engine - where 'partial' activation has it's own recovery purpose - but 
unfortunately due to various API not-so-good designs got mixed-in with
'raid' logic of activation with missing devices for fully usable raid.

It's basically very complex thing with no so clear path forward and it will 
require some further brainstorming how to go forward.

Then there is the 'mdadm complexity' itself - when there is not quite clear 
even for our lvm2 team which policy for missing device is the best -
is there even is lot's of different type of being 'missing' - i.e. large 
arrays can have transiently missing devs - which can be handled by md raid
code without intervention of lvm2 - but it has lots of 'dark' edges

Anyway - all I want to say here - general 'a'  attr  5
(or lvs -o+lv_active)  had originally relatively simple meaning of having 
device present/suspended in DM table - but got overcomplexed to the level that
hardly anyone can see what's going on there (see 'man lvs' attr 5 doc)
In general this design is also not quite good when 'stacking' gets used -
i.e. what is thinLV from thin-pool on cache  raid dataLV in troubles....

How the device is usable and whether it's in partial mode or any other
sort of mode is basically a very complex topic.

Note - currently there is big topic even the initial activation
of raid during boot when you have VG composed from many PVs
and and few of them are needed to raid activation and even less
for 'usable raid activation'.    Current Dracut script is unfortunately
not doing right thing...

> 
> At last, the new letter attr needs a lot of jobs to do and need to take care of various LV type.
> I can change my patch to modify lvs attr field for linear & raid type.
> But other LV types works need to do by other guys.
> 

So you are welcome to try to come with ideas - but as mentioned above generic 
solution can be really hard - so an explicit '1-purpose' thing which
can be maintained is currently the easiest small-step forward option

Zdenek

Re: [linux-lvm] [PATCH] lib/metadata: add new api lv_is_available()

[David Teigland]

On Sun, Aug 30, 2020 at 11:49:38PM +0800, heming.zhao@suse.com wrote:
> in my opinion, the 'not available'

We already use the word available/unavailable in other ways, so let's use
"broken" for the moment, maybe we can find a better word.

'lvs -o broken' would report 0|1.  Choosing an attr letter to represent
that is not as important and can be decided on later.

> means the LV can't correctly do r/w io.
>
> for raid, if the missing or breaking underlying devs number beyond raid level limit. the 
> 'not available' shoud be display.
> 
> for linear, any one of underlying dev is missing, upper layer module like fs may don't work
> (e.g. missing first disk, fs will missing w/r first disk's super-block metadata). the
> 'not available' should be display.

That definition of "broken" could be a specific enough, if we can report
it accurately enough.  Do we need to say "io will succeed on the entire LV
(as far as lvm knows)"?  It would be nice to know in which cases lvm can
report it accurately, and in which cases lvm doesn't know enough to report
it accurately.  If it's not correct in many cases, then we should consider
a different definition (maybe raid-specific.)

> for other type LV, I don't have too much experience to answer this question.

Any LV not using raid (or mirror) will be equivalant to linear, and be
broken if a single disk is missing or broken.

Dave

Re: [linux-lvm] [PATCH] lib/metadata: add new api lv_is_available()

[heming.zhao]

On 9/1/20 6:37 AM, David Teigland wrote:
> On Sun, Aug 30, 2020 at 11:49:38PM +0800, heming.zhao@suse.com wrote:
>> in my opinion, the 'not available'
> 
> We already use the word available/unavailable in other ways, so let's use
> "broken" for the moment, maybe we can find a better word.
> 
> 'lvs -o broken' would report 0|1.  Choosing an attr letter to represent
> that is not as important and can be decided on later.
> 
'broken' is acceptable and good word.
I'm only afraid end user don't know there is a new string item for lvs.
like me, I just know the lv_health_status string item from Zdenek's mail. (sorry for my stupid)

>> means the LV can't correctly do r/w io.
>>
>> for raid, if the missing or breaking underlying devs number beyond raid level limit. the
>> 'not available' shoud be display.
>>
>> for linear, any one of underlying dev is missing, upper layer module like fs may don't work
>> (e.g. missing first disk, fs will missing w/r first disk's super-block metadata). the
>> 'not available' should be display.
> 
> That definition of "broken" could be a specific enough, if we can report
> it accurately enough.  Do we need to say "io will succeed on the entire LV
> (as far as lvm knows)"?  It would be nice to know in which cases lvm can
> report it accurately, and in which cases lvm doesn't know enough to report
> it accurately.  If it's not correct in many cases, then we should consider
> a different definition (maybe raid-specific.)
> 
I prefer lvm report status from the entire LV side.
lvm provides virtual blocks to upper layer softwares, most of softwares use whole virtual disk.
there must have some scenarios which lvm doesn't know LV status accurately.

there is example about lvm doesn't know LV status. (it's very like other modules bug not lvm problem)
if removing one of raidX sub-disks then do re-insert, the lvs will report LV work normally.
But from my env, I saw the re-inserted disk major:minor was changed. The device-mapper table still
keep old major:minor mapping, LV r/w io will send no-exist disk and trigger r/w error.
So if we want to report LV status accurately, the code should do a lot of works, like verifying DM table
mapping, verifying underlying devs, verifying raid level limit, ...

>> for other type LV, I don't have too much experience to answer this question.
> 
> Any LV not using raid (or mirror) will be equivalant to linear, and be
> broken if a single disk is missing or broken.
> 
> Dave
> 

Re: [linux-lvm] [PATCH] lib/metadata: add new api lv_is_available()

[Zdenek Kabelac]

Dne 01. 09. 20 v 0:37 David Teigland napsal(a):
> On Sun, Aug 30, 2020 at 11:49:38PM +0800, heming.zhao@suse.com wrote:
>> in my opinion, the 'not available'
> 
> We already use the word available/unavailable in other ways, so let's use
> "broken" for the moment, maybe we can find a better word.
> 
> 'lvs -o broken' would report 0|1.  Choosing an attr letter to represent
> that is not as important and can be decided on later.

I'd probably pick the 'opposite' word - i.e.  lvs -o lv_usable -

but ATM - I'm still no sure how the meaning would differ from  'partial'
(which seems like it"s still missing separate lvs attribute shown only through 
health attr column 9 letter).

But we probably need to look much deeper look at the meaning of partial -
which ATM is a simply transitivity enclosure over LV stack - while 
particularly in raid case we need to probably go deeper and device into
categories.

This would also greatly help the current activation logic as well - but we 
probably need to first acknowledge what is 'preferred' view from md raid side 
- as these do not have an exact match.

Zdenek

Re: [linux-lvm] [PATCH] lib/metadata: add new api lv_is_available()

[David Teigland]

On Tue, Sep 01, 2020 at 05:09:23PM +0800, heming.zhao wrote:
> 'broken' is acceptable and good word.
> I'm only afraid end user don't know there is a new string item for lvs.
> like me, I just know the lv_health_status string item from Zdenek's mail. (sorry for my stupid)

I also like Zdenek's "usable", which seems to be closer to what we mean.
In addition to the reporting field can could be a new letter in the 9th lv
attr field.  (We'd have to figure out the priority of this letter vs the
others.)

> I prefer lvm report status from the entire LV side.
> lvm provides virtual blocks to upper layer softwares, most of softwares use whole virtual disk.

Limit this to what lvm knows already.  The field will represent whether
lvm believes the uppser layer can successfully do io to the entire LV.

Dave

Re: [linux-lvm] [PATCH] lib/metadata: add new api lv_is_available()

[heming.zhao]

On 9/1/20 11:07 PM, David Teigland wrote:
> On Tue, Sep 01, 2020 at 05:09:23PM +0800, heming.zhao wrote:
>> 'broken' is acceptable and good word.
>> I'm only afraid end user don't know there is a new string item for lvs.
>> like me, I just know the lv_health_status string item from Zdenek's mail. (sorry for my stupid)
> 
> I also like Zdenek's "usable", which seems to be closer to what we mean.
> In addition to the reporting field can could be a new letter in the 9th lv
> attr field.  (We'd have to figure out the priority of this letter vs the
> others.)
> 
to add 9th (U)sable for the common case? I think it will overwrite (p)artial when top
layer LV non-usable. otherwise there may have no chance to show this new letter.
Zdenek ever said, He worried about compatibility issues. He preferred to add
'lv_usable' option, not new letter in 9th bit.

>> I prefer lvm report status from the entire LV side.
>> lvm provides virtual blocks to upper layer softwares, most of softwares use whole virtual disk.
> 
> Limit this to what lvm knows already.  The field will represent whether
> lvm believes the uppser layer can successfully do io to the entire LV.
> 
agree

> Dave
>