linux-lvm.redhat.com archive mirror
 help / color / mirror / Atom feed
From: "heming.zhao@suse.com" <heming.zhao@suse.com>
To: David Teigland <teigland@redhat.com>,
	Zdenek Kabelac <zkabelac@redhat.com>
Cc: linux-lvm@redhat.com
Subject: Re: [linux-lvm] [PATCH] lib/metadata: add new api lv_is_available()
Date: Sun, 30 Aug 2020 23:49:38 +0800	[thread overview]
Message-ID: <4cce4f81-24b6-3730-a331-8b31cf57cd51@suse.com> (raw)
In-Reply-To: <20200828182608.GA17233@redhat.com>

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
> 

  reply	other threads:[~2020-08-30 15:49 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-27 16:05 Zhao Heming
2020-08-27 16:07 ` heming.zhao
2020-08-28 16:04 ` Zdenek Kabelac
2020-08-28 18:26   ` David Teigland
2020-08-30 15:49     ` heming.zhao [this message]
2020-08-30 17:06       ` Zdenek Kabelac
2020-08-31 22:37       ` David Teigland
2020-09-01  9:09         ` heming.zhao
2020-09-01 15:07           ` David Teigland
2020-09-01 16:15             ` heming.zhao
2020-09-01 11:43         ` Zdenek Kabelac

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=4cce4f81-24b6-3730-a331-8b31cf57cd51@suse.com \
    --to=heming.zhao@suse.com \
    --cc=linux-lvm@redhat.com \
    --cc=teigland@redhat.com \
    --cc=zkabelac@redhat.com \
    --subject='Re: [linux-lvm] [PATCH] lib/metadata: add new api lv_is_available()' \
    /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

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).