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

[heming.zhao]

On 9/7/20 10:32 PM, Zdenek Kabelac wrote:
> Dne 05. 09. 20 v 11:08 heming.zhao at suse.com 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 at suse.com>
>>> ---
>>>    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