linux-lvm.redhat.com archive mirror
 help / color / mirror / Atom feed
From: "heming.zhao" <heming.zhao@suse.com>
To: Heinz Mauelshagen <heinzm@redhat.com>,
	LVM general discussion and development <linux-lvm@redhat.com>,
	Zdenek Kabelac <zkabelac@redhat.com>
Cc: teigland@sourceware.org
Subject: Re: [linux-lvm] [PATCH v2] lvs: add -o lv_usable
Date: Fri, 18 Sep 2020 15:07:09 +0800	[thread overview]
Message-ID: <b9524b39-3171-887f-efa8-e13a9a9aad51@suse.com> (raw)
In-Reply-To: <58d180ab-489d-465f-35de-8442dde067b8@redhat.com>

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/
> 

  reply	other threads:[~2020-09-18  7:07 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-09 16:47 [linux-lvm] [PATCH v2] lvs: add -o lv_usable Zhao Heming
2020-09-09 17:17 ` Zdenek Kabelac
2020-09-10  6:34   ` heming.zhao
2020-09-17 10:18     ` Heinz Mauelshagen
2020-09-18  7:07       ` heming.zhao [this message]
2020-09-18 11:38         ` Heinz Mauelshagen
2020-09-19  3:58           ` heming.zhao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b9524b39-3171-887f-efa8-e13a9a9aad51@suse.com \
    --to=heming.zhao@suse.com \
    --cc=heinzm@redhat.com \
    --cc=linux-lvm@redhat.com \
    --cc=teigland@sourceware.org \
    --cc=zkabelac@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).