Re: [PATCH] lvs: add -o lv_usable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/7/20 10:32 PM, Zdenek Kabelac wrote:
Dne 05. 09. 20 v 11:08 heming.zhao@xxxxxxxx napsal(a):


On 9/5/20 5:06 PM, Zhao Heming wrote:
report LV is usable for upper layer.

Signed-off-by: Zhao Heming <heming.zhao@xxxxxxxx>
---
   lib/activate/activate.h          |   2 +
   lib/activate/dev_manager.c       |  67 ++++++++++++++++
   lib/metadata/metadata-exported.h |   1 +
   lib/metadata/metadata.c          | 130 +++++++++++++++++++++++++++++++
   lib/report/columns.h             |   1 +
   lib/report/properties.c          |   2 +
   lib/report/report.c              |  13 ++++
   lib/report/values.h              |   1 +
   8 files changed, 217 insertions(+)


my test result:

## linear


Hi

We will need to take closer look  - the patchset itself is not going in right direction - since all the info about missing or present devices is already
available within lvm engine and we want to minimize 'repeated' query
(ideally all the information should be checked only once - otherwise
the decision state-machine is producing random-end result garbage - which
is the very hard to trace and debug.

So all the code which is doing runtime query again can't be accepted.

Next main rule is - we cache status values whenever possible - so there
should be at most  1 'status' per device per LV - but since lvm2 already
knows from scanning point and from metadata parsing which device is missing
the logic of evaluation of LV's usability needs to be based on these values.

But I think we also need some per-segment methods evaluating usability.

Regards

Zdenek


Hello Zdenek,

Thank you for your review comments, I got your meaning.

Does it acceptable to add new status bit in lv->status?
I ever sent it in previoud patch "[PATCH v2] lib/metadata: add new api lv_is_available()"
The define as below (the 'NOT_AVAIL_LV' will change to 'NOT_USABLE_LV'):
```
+#define NOT_AVAIL_LV	UINT64_C(0x0000000080000000)	/* LV - derived flag, not
+							   written out in metadata*/

+#define lv_is_available(lv)	(((lv)->status & NOT_AVAIL_LV) ? 0 : 1)
```

some description about the new patch:
- it will combine with two patches:
  - [PATCH v2] lib/metadata: add new api lv_is_available()
  - [PATCH] lvs: add -o lv_usable
- the new patch will add new status bit NOT_USABLE_LV, and this bit will be
  set in _lv_mark_if_partial_single().
- The only output which related with new status bit: lvs -o lv_usable

if above is acceptable, I will send the v2 patch. if not, I will give up this 'lv_usable' patch.

Thanks


_______________________________________________
linux-lvm mailing list
linux-lvm@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/linux-lvm
read the LVM HOW-TO at http://tldp.org/HOWTO/LVM-HOWTO/




[Index of Archives]     [Gluster Users]     [Kernel Development]     [Linux Clusters]     [Device Mapper]     [Security]     [Bugtraq]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]

  Powered by Linux