* [linux-lvm] [PATCH] lvdisplay: dispaly correct status on linear type
@ 2020-08-26 15:50 Zhao Heming
2020-08-26 15:58 ` heming.zhao
2020-08-26 17:03 ` David Teigland
0 siblings, 2 replies; 4+ messages in thread
From: Zhao Heming @ 2020-08-26 15:50 UTC (permalink / raw)
To: linux-lvm; +Cc: teigland, Zhao Heming
It commit is enhancement for 1d0dc74f9147e3c1f3681efa4166cbe2edcb6571
1d0dc74f9147 only supports all raid type, this commit adds linear type.
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/lv_manip.c | 76 ++++++++++++++++++++++++++++++++
lib/metadata/metadata-exported.h | 1 +
lib/metadata/segtype.h | 1 -
lib/raid/raid.c | 54 -----------------------
5 files changed, 80 insertions(+), 59 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/lv_manip.c b/lib/metadata/lv_manip.c
index bb2ad5beb..968ffbf53 100644
--- a/lib/metadata/lv_manip.c
+++ b/lib/metadata/lv_manip.c
@@ -700,6 +700,82 @@ bad:
return 0;
}
+
+/*
+ * 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
+ *
+ * MIRROR type:
+ * the failing rule
+ * - 2-way mirror to linear device
+ * - 3-way mirror convert to 2-way mirror
+ * mirror type won't be in 'not available' status, this func
+ * won't need to verify mirror type.
+ *
+ * TODO:
+ * thin, cache, integrity, vdo etc.
+ */
+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;
+
+ 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 (seg_pv(seg, s)->status & MISSING_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;
+ }
+ 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;
+}
+
struct dm_list_and_mempool {
struct dm_list *list;
struct dm_pool *mem;
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 06ea757b8..2c1e23195 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -291,6 +291,7 @@
int lv_layout_and_role(struct dm_pool *mem, const struct logical_volume *lv,
struct dm_list **layout, struct dm_list **role);
+bool lv_is_available(const struct logical_volume *lv);
/* Ordered list - see lv_manip.c */
typedef enum {
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [linux-lvm] [PATCH] lvdisplay: dispaly correct status on linear type
2020-08-26 15:50 [linux-lvm] [PATCH] lvdisplay: dispaly correct status on linear type Zhao Heming
@ 2020-08-26 15:58 ` heming.zhao
2020-08-26 17:03 ` David Teigland
1 sibling, 0 replies; 4+ messages in thread
From: heming.zhao @ 2020-08-26 15:58 UTC (permalink / raw)
To: linux-lvm; +Cc: teigland
Same as the commit writing, this commit is code enhancement not bugfix.
Last commit 1d0dc74f9147e3c1f3681efa4166cbe2edcb6571 had been verified by one of suse customer.
They did test for types: mirror, raid0, raid1, raid5, linear.
For this commit, I only verified linear & raid cases. I have little experience with type: snapshot, thin, vdo, cache etc. So I am not sure whether this commit contains any bug when LV type is not linear & raid.
On 8/26/20 11:50 PM, Zhao Heming wrote:
> It commit is enhancement for 1d0dc74f9147e3c1f3681efa4166cbe2edcb6571
> 1d0dc74f9147 only supports all raid type, this commit adds linear type.
>
> 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/lv_manip.c | 76 ++++++++++++++++++++++++++++++++
> lib/metadata/metadata-exported.h | 1 +
> lib/metadata/segtype.h | 1 -
> lib/raid/raid.c | 54 -----------------------
> 5 files changed, 80 insertions(+), 59 deletions(-)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [linux-lvm] [PATCH] lvdisplay: dispaly correct status on linear type
2020-08-26 15:50 [linux-lvm] [PATCH] lvdisplay: dispaly correct status on linear type Zhao Heming
2020-08-26 15:58 ` heming.zhao
@ 2020-08-26 17:03 ` David Teigland
2020-08-27 15:52 ` heming.zhao
1 sibling, 1 reply; 4+ messages in thread
From: David Teigland @ 2020-08-26 17:03 UTC (permalink / raw)
To: Zhao Heming; +Cc: linux-lvm
On Wed, Aug 26, 2020 at 11:50:44PM +0800, Zhao Heming wrote:
> It commit is enhancement for 1d0dc74f9147e3c1f3681efa4166cbe2edcb6571
> 1d0dc74f9147 only supports all raid type, this commit adds linear type.
>
> 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)
It would be nice if you could come up with a more formal definition for
what you're calling "lv_is_available", and also in the same terms define
the existing "lv_is_partial", explaining how they differ. Both properties
may be useful to show in lvs reporting fields (and possibly lv_attr).
Reporting this state only via the lvdisplay line above seems strange.
> + } else if (seg_type(seg, s) == AREA_PV) {
> + if (seg_pv(seg, s)->status & MISSING_PV)
Using only MISSING_PV is probably insufficient, because that flag is used
only when the metadata is written while the device is missing. i.e.
pv->dev can be NULL when the flag is not set.
Dave
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [linux-lvm] [PATCH] lvdisplay: dispaly correct status on linear type
2020-08-26 17:03 ` David Teigland
@ 2020-08-27 15:52 ` heming.zhao
0 siblings, 0 replies; 4+ messages in thread
From: heming.zhao @ 2020-08-27 15:52 UTC (permalink / raw)
To: David Teigland; +Cc: linux-lvm
thank you for your kindly review.
On 8/27/20 1:03 AM, David Teigland wrote:
> On Wed, Aug 26, 2020 at 11:50:44PM +0800, Zhao Heming wrote:
>> It commit is enhancement for 1d0dc74f9147e3c1f3681efa4166cbe2edcb6571
>> 1d0dc74f9147 only supports all raid type, this commit adds linear type.
>>
>> 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)
>
> It would be nice if you could come up with a more formal definition for
> what you're calling "lv_is_available", and also in the same terms define
> the existing "lv_is_partial", explaining how they differ. Both properties
> may be useful to show in lvs reporting fields (and possibly lv_attr).
> Reporting this state only via the lvdisplay line above seems strange.
I will send the v2 patch very soon.
the v2 patch only for lvdisplay cmd. if my code acceptable, I plan update lvs
cmd in v3, add a new letter (N)ot_available in bit 9 (Volume Health).
>
>> + } else if (seg_type(seg, s) == AREA_PV) {
>> + if (seg_pv(seg, s)->status & MISSING_PV)
>
> Using only MISSING_PV is probably insufficient, because that flag is used
> only when the metadata is written while the device is missing. i.e.
> pv->dev can be NULL when the flag is not set.
>
I have a little confused with your comments.
These 2 lines are same as _lv_mark_if_partial_single() code:
```
if (seg_type(lvseg, s) == AREA_PV) {
if (is_missing_pv(seg_pv(lvseg, s))) {
```
why _lv_mark_if_partial_single can use these 2 lines?
please wait for the v2 patch, I changed some code logic, set the NOT_AVAIL_LV in
_lv_mark_if_partial_single().
> Dave
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-27 15:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-26 15:50 [linux-lvm] [PATCH] lvdisplay: dispaly correct status on linear type Zhao Heming
2020-08-26 15:58 ` heming.zhao
2020-08-26 17:03 ` David Teigland
2020-08-27 15:52 ` heming.zhao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).