Bug 1723745 - vgextend and some other cmds don't build dev alias name cache before real job
Summary: vgextend and some other cmds don't build dev alias name cache before real job
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: LVM and device-mapper
Classification: Community
Component: lvm2
Version: 2.02.180
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
: ---
Assignee: David Teigland
QA Contact: cluster-qe@redhat.com
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-06-25 09:30 UTC by heming.zhao
Modified: 2020-12-02 22:55 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-12-02 22:55:01 UTC
Embargoed:
pm-rhel: lvm-technical-solution?
pm-rhel: lvm-test-coverage?


Attachments (Terms of Use)

Description heming.zhao 2019-06-25 09:30:52 UTC
Description of problem:

with filter:
filter = [ "a|^/dev/sd.*|", "r|.*|" ]

below copy from ML, wrote by Martin Wilck:
```
IMO both pvcreate and vgextend
should accept the device because of the first "a" rule. In any case,
it's obviously wrong that the two tools behave differently.

Note also that this difference occurs only if lvmetad is used. Without
lvmetad, both commands accept the device. The reason is that in this
case, lvmcache_label_scan(), which also builds the alias tree, is
called before applying the filter. With lvmetad OTOH,
lvmcache_label_scan() is basically a noop.

IMO this should be fixed by adding a call to
lvmcache_seed_infos_from_lvmetad() before applying the device filter to
vgextend. vgcreate() calls it early on, right after
lvmcache_label_scan(); the same might work for vgextend as well.
Alternatively, it might be possible to add a call to 
lvmcache_seed_infos_from_lvmetad() to pvcreate_each_device(); in that
case it might even be possible to remove the early calls in vgcreate().
I don't understand the initialization sequence of LVM2 commands well
enough to create a patch myself. Besides vgextend, other LVM2 commands
that need to apply the filter may be affected, too, as 
lvmcache_seed_infos_from_lvmetad() seems to be used only in a few hand-
selected code paths.
```

Version-Release number of selected component (if applicable):
2.02.180

How reproducible:


Steps to Reproduce:
1. setup filter = [ "a|^/dev/sd.*|", "r|.*|" ]  in /etc/lvm/lvm.conf
2. pvcreate /dev/disk/by-id/xxx-disk1
3. vgcreate vgtst /dev/disk/by-id/xxx-disk1
4. vgextend vgtst /dev/disk/by-id/xxx-disk2

Actual results:

pvcreate & vgcreate work normally.
vgextend is failed by filter

Expected results:

vgextend should execute successfully.

Additional info:

Comment 1 David Teigland 2019-06-25 20:20:40 UTC
Thanks for the detailed report and analysis.  Failing to recognize device aliases with lvmetad seems to be a problem in vgextend and pvremove (please let me know if you find any other cases.)  I believe it's the result of this commit:
https://sourceware.org/git/?p=lvm2.git;a=commit;h=c0973e70a58e7e14e9cca29a0f8ad12719ea554f

The following diff fixes the problem for me.  If you are already recompiling lvm source to test changes, I'd be happy to hear if this works for you.

diff --git a/tools/toollib.c b/tools/toollib.c
index 3221e5f1ca22..2e3fef66ffdc 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -5542,6 +5542,7 @@ int pvcreate_each_device(struct cmd_context *cmd,
        }
 
        lvmcache_label_scan(cmd);
+       dev_cache_scan();
 
        /*
         * Translate arg names into struct device's.

Comment 2 heming.zhao 2019-06-26 03:43:50 UTC
1> I only find vgextend & pvremove have this issue.

2>
It looks double call dev_cache_scan() if lvmetad disabled.
(I'm not familiar with lvm2 code), below looks better: 
```c
        lvmcache_label_scan(cmd);
+        if (!dev_cache_has_scanned())
+                dev_cache_scan();
```

I compiled & tried above codes, it work fine.

Comment 3 David Teigland 2019-06-26 14:08:58 UTC
Good suggestion, thanks for the confirmation, I'll get a fix pushed to the stable branch.

Comment 4 David Teigland 2019-06-26 21:08:27 UTC
pushed fix to stable-2.02
https://sourceware.org/git/?p=lvm2.git;a=commit;h=b13ebfa4c289a5bc6eb4f8ba26126db8e6d78296


# pvs --config 'devices/preferred_names=["/dev/disk/by-id/wwn"]'
  PV                                                     VG           Fmt  Attr PSize    PFree   
  /dev/disk/by-id/wwn-0x690b11c0000438ad0000056550910404 test5        lvm2 a--  <931.01g <931.01g
  /dev/disk/by-id/wwn-0x690b11c0000438ad0000056b50910415 test5        lvm2 a--  <931.01g <931.01g

# pvs
  PV         VG           Fmt  Attr PSize    PFree   
  /dev/sdb   test5        lvm2 a--  <931.01g <931.01g
  /dev/sdd   test5        lvm2 a--  <931.01g <931.01g

# vgreduce test5 /dev/disk/by-id/wwn-0x690b11c0000438ad0000056b50910415 
  Removed "/dev/sdd" from volume group "test5"

# pvremove /dev/disk/by-id/wwn-0x690b11c0000438ad0000056b50910415
  Labels on physical volume "/dev/disk/by-id/wwn-0x690b11c0000438ad0000056b50910415" successfully wiped.

# vgextend test5 /dev/disk/by-id/wwn-0x690b11c0000438ad0000056b50910415
  Physical volume "/dev/disk/by-id/wwn-0x690b11c0000438ad0000056b50910415" successfully created.
  Volume group "test5" successfully extended

# vgreduce test5 /dev/disk/by-id/wwn-0x690b11c0000438ad0000056b50910415 
  Removed "/dev/sdd" from volume group "test5"

# vgextend test5 /dev/disk/by-id/wwn-0x690b11c0000438ad0000056b50910415
  Volume group "test5" successfully extended

# vgreduce test5 /dev/sdd
  Removed "/dev/sdd" from volume group "test5"

# vgextend test5 /dev/sdd
  Volume group "test5" successfully extended

Comment 5 heming.zhao 2019-06-27 03:21:25 UTC
Hello David,

Thank you for your quickly reply & fix.


Note You need to log in before you can comment on or make changes to this bug.