All of lore.kernel.org
 help / color / mirror / Atom feed
* [linux-lvm] Missing error handling in lv_snapshot_remove
@ 2013-08-06 17:37 Bastian Blank
  2013-08-07  9:13 ` Zdenek Kabelac
  2013-08-07  9:22 ` Andreas Pflug
  0 siblings, 2 replies; 13+ messages in thread
From: Bastian Blank @ 2013-08-06 17:37 UTC (permalink / raw)
  To: linux-lvm

[-- Attachment #1: Type: text/plain, Size: 3832 bytes --]

Hi

I tried to tackle a particular bug that shows up in Debian for some time
now. Some blamed the udev rules and I still can't completely rule them
out. But this triggers a much worse bug in the error cleanup of the
snapshot remove. I reproduced this with Debian/Linux 3.2.46/LVM 2.02.99
without udevd running and Fedora 19/LVM 2.02.98-10.fc19.

On snapshot removal, LVM first converts the device into a regular LV
(lv_remove_snapshot) and in a second step removes this LV
(lv_remove_single). Is there a reason for this two step removal? An
error during removal leaves a non-snapshot LV behind.

I hold the cow device open so it will run into the error condition:
| $ sleep 100 < /dev/mapper/vg-test_snap-cow&

Then try to remove the LV:
| $ lvremove vg/test_snap

lv_remove_snapshot first suspends all devices:

| #metadata/lv_manip.c:4429     Removing snapshot test_snap
| #libdm-deptree.c:1314     Suspending vg-test_base (253:8) with device flush
| #ioctl/libdm-iface.c:1724         dm suspend   (253:8) NFS    [16384] (*1)
| #libdm-common.c:210         Suspended device counter increased to 1
| #ioctl/libdm-iface.c:1724         dm info   (253:9) NF   [16384] (*1)
| #libdm-deptree.c:1314     Suspending vg-test_snap (253:9) with device flush
| #ioctl/libdm-iface.c:1724         dm suspend   (253:9) NFS    [16384] (*1)
| #libdm-common.c:210         Suspended device counter increased to 2
| #ioctl/libdm-iface.c:1724         dm info   (253:10) NF   [16384] (*1)
| #libdm-deptree.c:1314     Suspending vg-test_base-real (253:10) with device flush
| #ioctl/libdm-iface.c:1724         dm suspend   (253:10) NFS    [16384] (*1)
| #libdm-common.c:210         Suspended device counter increased to 3
| #ioctl/libdm-iface.c:1724         dm info   (253:11) NF   [16384] (*1)
| #libdm-deptree.c:1314     Suspending vg-test_snap-cow (253:11) with device flush
| #ioctl/libdm-iface.c:1724         dm suspend   (253:11) NFS    [16384] (*1)
| #libdm-common.c:210         Suspended device counter increased to 4

Commits the VG:

| #format_text/format-text.c:735         Committing vg metadata (1276) to /dev/xvdb header@4096

Resumes three of the devices, but not vg-test_base:

| #libdm-deptree.c:1263     Resuming vg-test_snap-cow (253:11)
| #ioctl/libdm-iface.c:1724         dm resume   (253:11) NF   [16384] (*1)
| #libdm-common.c:1338         vg-test_snap-cow: Stacking NODE_ADD (253,11) 0:6 0660 [trust_udev]
| #libdm-common.c:1348         vg-test_snap-cow: Stacking NODE_READ_AHEAD 0 (flags=0)
| #libdm-common.c:221         Suspended device counter reduced to 3
| #libdm-deptree.c:1263     Resuming vg-test_base-real (253:10)
| #ioctl/libdm-iface.c:1724         dm resume   (253:10) NF   [16384] (*1)
| #libdm-common.c:1338         vg-test_base-real: Stacking NODE_ADD (253,10) 0:6 0660 [trust_udev]
| #libdm-common.c:1348         vg-test_base-real: Stacking NODE_READ_AHEAD 0 (flags=0)
| #libdm-common.c:221         Suspended device counter reduced to 2
| #libdm-deptree.c:1263     Resuming vg-test_snap (253:9)
| #ioctl/libdm-iface.c:1724         dm resume   (253:9) NF   [16384] (*1)
| #libdm-common.c:1338         vg-test_snap: Stacking NODE_ADD (253,9) 0:6 0660 [trust_udev]
| #libdm-common.c:1348         vg-test_snap: Stacking NODE_READ_AHEAD 256 (flags=1)
| #libdm-common.c:221         Suspended device counter reduced to 1

Now it fails to do lv_activate on the cow device, because it is still
open:

| #libdm-deptree.c:1562   Unable to deactivate open vg-test_snap-cow (253:11)
| #metadata/snapshot_manip.c:291   Failed to activate test_snap.

And exits without further error handling and with one suspended device:

|  libdevmapper exiting with 1 device(s) still suspended.

Bastian

-- 
Beam me up, Scotty, there's no intelligent life down here!

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [linux-lvm] Missing error handling in lv_snapshot_remove
  2013-08-06 17:37 [linux-lvm] Missing error handling in lv_snapshot_remove Bastian Blank
@ 2013-08-07  9:13 ` Zdenek Kabelac
  2013-08-07 12:36   ` Bastian Blank
  2013-08-08 13:33   ` Ritesh Raj Sarraf
  2013-08-07  9:22 ` Andreas Pflug
  1 sibling, 2 replies; 13+ messages in thread
From: Zdenek Kabelac @ 2013-08-07  9:13 UTC (permalink / raw)
  To: linux-lvm, waldi

Dne 6.8.2013 19:37, Bastian Blank napsal(a):
> Hi
>
> I tried to tackle a particular bug that shows up in Debian for some time
> now. Some blamed the udev rules and I still can't completely rule them
> out. But this triggers a much worse bug in the error cleanup of the
> snapshot remove. I reproduced this with Debian/Linux 3.2.46/LVM 2.02.99
> without udevd running and Fedora 19/LVM 2.02.98-10.fc19.
>
> On snapshot removal, LVM first converts the device into a regular LV
> (lv_remove_snapshot) and in a second step removes this LV
> (lv_remove_single). Is there a reason for this two step removal? An
> error during removal leaves a non-snapshot LV behind.
>
> I hold the cow device open so it will run into the error condition:
> | $ sleep 100 < /dev/mapper/vg-test_snap-cow&


You are breaking the lvm2 logic thus pushing the code to go through unexpected 
error code path - user is never supposed to open so called 'private' 
/dev/mapper/ devices.

>
> Then try to remove the LV:
> | $ lvremove vg/test_snap

With upstream lvm2 code - there is embedded 'retry' loop - so the removal
should be retried for couple times (controllable by lvm.conf).

That's because udev WATCH rule might be fired basically anytime after close of 
device opened in write mode - so it may happen lvm2 checks device is not 
opened and could be removed, but the udev WATCH rules opens temporarily device 
and lvm2 then fails to remove device, which has been previously detected as 
unused.

So to fight with this issue - for unmounted device lvm2 retries remove 
operation couple times, with the hope WATCH rule scan finishes quickly.

In the future kernel target driver may support something like device 
auto-remove, but it's not yet there in upstream kernel....


>
> lv_remove_snapshot first suspends all devices:
>
> | #metadata/lv_manip.c:4429     Removing snapshot test_snap
> | #libdm-deptree.c:1314     Suspending vg-test_base (253:8) with device flush
> | #ioctl/libdm-iface.c:1724         dm suspend   (253:8) NFS    [16384] (*1)
> | #libdm-common.c:210         Suspended device counter increased to 1
> | #ioctl/libdm-iface.c:1724         dm info   (253:9) NF   [16384] (*1)
> | #libdm-deptree.c:1314     Suspending vg-test_snap (253:9) with device flush
> | #ioctl/libdm-iface.c:1724         dm suspend   (253:9) NFS    [16384] (*1)
> | #libdm-common.c:210         Suspended device counter increased to 2
> | #ioctl/libdm-iface.c:1724         dm info   (253:10) NF   [16384] (*1)
> | #libdm-deptree.c:1314     Suspending vg-test_base-real (253:10) with device flush
> | #ioctl/libdm-iface.c:1724         dm suspend   (253:10) NFS    [16384] (*1)
> | #libdm-common.c:210         Suspended device counter increased to 3
> | #ioctl/libdm-iface.c:1724         dm info   (253:11) NF   [16384] (*1)
> | #libdm-deptree.c:1314     Suspending vg-test_snap-cow (253:11) with device flush
> | #ioctl/libdm-iface.c:1724         dm suspend   (253:11) NFS    [16384] (*1)
> | #libdm-common.c:210         Suspended device counter increased to 4
>
> Commits the VG:
>
> | #format_text/format-text.c:735         Committing vg metadata (1276) to /dev/xvdb header at 4096
>
> Resumes three of the devices, but not vg-test_base:
>
> | #libdm-deptree.c:1263     Resuming vg-test_snap-cow (253:11)
> | #ioctl/libdm-iface.c:1724         dm resume   (253:11) NF   [16384] (*1)
> | #libdm-common.c:1338         vg-test_snap-cow: Stacking NODE_ADD (253,11) 0:6 0660 [trust_udev]
> | #libdm-common.c:1348         vg-test_snap-cow: Stacking NODE_READ_AHEAD 0 (flags=0)
> | #libdm-common.c:221         Suspended device counter reduced to 3
> | #libdm-deptree.c:1263     Resuming vg-test_base-real (253:10)
> | #ioctl/libdm-iface.c:1724         dm resume   (253:10) NF   [16384] (*1)
> | #libdm-common.c:1338         vg-test_base-real: Stacking NODE_ADD (253,10) 0:6 0660 [trust_udev]
> | #libdm-common.c:1348         vg-test_base-real: Stacking NODE_READ_AHEAD 0 (flags=0)
> | #libdm-common.c:221         Suspended device counter reduced to 2
> | #libdm-deptree.c:1263     Resuming vg-test_snap (253:9)
> | #ioctl/libdm-iface.c:1724         dm resume   (253:9) NF   [16384] (*1)
> | #libdm-common.c:1338         vg-test_snap: Stacking NODE_ADD (253,9) 0:6 0660 [trust_udev]
> | #libdm-common.c:1348         vg-test_snap: Stacking NODE_READ_AHEAD 256 (flags=1)
> | #libdm-common.c:221         Suspended device counter reduced to 1
>
> Now it fails to do lv_activate on the cow device, because it is still
> open:
>
> | #libdm-deptree.c:1562   Unable to deactivate open vg-test_snap-cow (253:11)
> | #metadata/snapshot_manip.c:291   Failed to activate test_snap.
>
> And exits without further error handling and with one suspended device:
>
> |  libdevmapper exiting with 1 device(s) still suspended.
>

There has been bug affecting cluster usage of exclusive snapshots in pre .99 
version - the order of taking locks for devices was not correct, and if there
has  been clvmd restart during snapshot - it has caused some problems.

But for current (.99) code - in normal case the operation should work 
properly. For any unpredictable errors -  lvm2 command should print error 
message and it's up-to admin to fix dangling device and table entries.

So in the case you are trying - lvm2 is expected to fail - since lvm2 doesn't 
support the use of  /dev/mapper/ entries  (which is where the -cow is only 
visible).

User of lvm2 is expected to open only 'public' /dev/vgname/lvname  entries. 
If anyone tries to 'lock' on /dev/mapper/names, lvm2 cannot be blamed for 
reporting errors - simply because this is unsupported. So lvm2 shows message, 
exits, and expects admin to fix problem in the system.

Of course some error paths are really tricky and there is too many of them and 
there is BIG room for improvement, so patches are welcomed -  the rule here 
is, that no devices should be left in suspend state - since this is causing 
real problems.

You could probably join #irc channel to discuss possible improvements here.

Zdenek

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [linux-lvm] Missing error handling in lv_snapshot_remove
  2013-08-06 17:37 [linux-lvm] Missing error handling in lv_snapshot_remove Bastian Blank
  2013-08-07  9:13 ` Zdenek Kabelac
@ 2013-08-07  9:22 ` Andreas Pflug
  2013-08-07  9:41   ` Zdenek Kabelac
  1 sibling, 1 reply; 13+ messages in thread
From: Andreas Pflug @ 2013-08-07  9:22 UTC (permalink / raw)
  To: linux-lvm

Am 06.08.13 19:37, schrieb Bastian Blank:
> Hi
>
> I tried to tackle a particular bug that shows up in Debian for some time
> now. Some blamed the udev rules and I still can't completely rule them
> out. But this triggers a much worse bug in the error cleanup of the
> snapshot remove. I reproduced this with Debian/Linux 3.2.46/LVM 2.02.99
> without udevd running and Fedora 19/LVM 2.02.98-10.fc19.
>
> On snapshot removal, LVM first converts the device into a regular LV
> (lv_remove_snapshot) and in a second step removes this LV
> (lv_remove_single). Is there a reason for this two step removal? An
> error during removal leaves a non-snapshot LV behind.
Ah, this explains why sometimes my backup stops: I take a snapshot,
rsync the stuff and remove the snapshot with a daily cron job, but I
observed twice that a non-snapshot volume named like a backup snapshot
was lingering around, preventing the script to work. So this is no
exotic corner case, but happens in real life.

I observe this since I dist-upgraded to wheezy.

Regards,
Andreas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [linux-lvm] Missing error handling in lv_snapshot_remove
  2013-08-07  9:22 ` Andreas Pflug
@ 2013-08-07  9:41   ` Zdenek Kabelac
  2013-08-07 17:18     ` Andreas Pflug
  0 siblings, 1 reply; 13+ messages in thread
From: Zdenek Kabelac @ 2013-08-07  9:41 UTC (permalink / raw)
  To: LVM general discussion and development; +Cc: Andreas Pflug

Dne 7.8.2013 11:22, Andreas Pflug napsal(a):
> Am 06.08.13 19:37, schrieb Bastian Blank:
>> Hi
>>
>> I tried to tackle a particular bug that shows up in Debian for some time
>> now. Some blamed the udev rules and I still can't completely rule them
>> out. But this triggers a much worse bug in the error cleanup of the
>> snapshot remove. I reproduced this with Debian/Linux 3.2.46/LVM 2.02.99
>> without udevd running and Fedora 19/LVM 2.02.98-10.fc19.
>>
>> On snapshot removal, LVM first converts the device into a regular LV
>> (lv_remove_snapshot) and in a second step removes this LV
>> (lv_remove_single). Is there a reason for this two step removal? An
>> error during removal leaves a non-snapshot LV behind.
> Ah, this explains why sometimes my backup stops: I take a snapshot,
> rsync the stuff and remove the snapshot with a daily cron job, but I
> observed twice that a non-snapshot volume named like a backup snapshot
> was lingering around, preventing the script to work. So this is no
> exotic corner case, but happens in real life.
>
> I observe this since I dist-upgraded to wheezy.
>

Because Debian is using non-upstream udev rules.

With upstream udev rules with standard real-life use, this situation
cannot happen - since these rules are constructed to play better with
udev WATCH rule.

Zdenek

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [linux-lvm] Missing error handling in lv_snapshot_remove
  2013-08-07  9:13 ` Zdenek Kabelac
@ 2013-08-07 12:36   ` Bastian Blank
  2013-08-07 13:32     ` Alasdair G Kergon
  2013-08-07 15:13     ` Zdenek Kabelac
  2013-08-08 13:33   ` Ritesh Raj Sarraf
  1 sibling, 2 replies; 13+ messages in thread
From: Bastian Blank @ 2013-08-07 12:36 UTC (permalink / raw)
  To: linux-lvm

On Wed, Aug 07, 2013 at 11:13:53AM +0200, Zdenek Kabelac wrote:
> Dne 6.8.2013 19:37, Bastian Blank napsal(a):
> >I hold the cow device open so it will run into the error condition:
> >| $ sleep 100 < /dev/mapper/vg-test_snap-cow&
> You are breaking the lvm2 logic thus pushing the code to go through
> unexpected error code path - user is never supposed to open so
> called 'private' /dev/mapper/ devices.

I'm a developer and use it to trigger an error condition. Please don't
start with that crap about what a user should or should not do.

> >Then try to remove the LV:
> >| $ lvremove vg/test_snap
> With upstream lvm2 code - there is embedded 'retry' loop - so the removal
> should be retried for couple times (controllable by lvm.conf).

Please show that it actually does anything in this case. This is no
condition that goes away, but a logic bug.

> That's because udev WATCH rule might be fired basically anytime
> after close of device opened in write mode - so it may happen lvm2
> checks device is not opened and could be removed, but the udev WATCH
> rules opens temporarily device and lvm2 then fails to remove device,
> which has been previously detected as unused.

There is not udevd running! Please explain how udev can be a problem in
this case.

> There has been bug affecting cluster usage of exclusive snapshots in
> pre .99 version - the order of taking locks for devices was not
> correct, and if there
> has  been clvmd restart during snapshot - it has caused some problems.

Did you actually read the code? At least I can clearly see that the
error logic is broken.

> But for current (.99) code - in normal case the operation should
> work properly. For any unpredictable errors -  lvm2 command should
> print error message and it's up-to admin to fix dangling device and
> table entries.

It is up to LVM to not break the system with suspended devices.

Bastian

-- 
Insufficient facts always invite danger.
		-- Spock, "Space Seed", stardate 3141.9

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [linux-lvm] Missing error handling in lv_snapshot_remove
  2013-08-07 12:36   ` Bastian Blank
@ 2013-08-07 13:32     ` Alasdair G Kergon
  2013-08-07 15:13     ` Zdenek Kabelac
  1 sibling, 0 replies; 13+ messages in thread
From: Alasdair G Kergon @ 2013-08-07 13:32 UTC (permalink / raw)
  To: linux-lvm

On Wed, Aug 07, 2013 at 02:36:57PM +0200, Bastian Blank wrote:
> I'm a developer and use it to trigger an error condition. Please don't
> start with that crap about what a user should or should not do.
 
Use the software however you want, but upstream LVM only supports the
direct use of devices exposed under /dev/$VG.  Any other devices it
creates are private and direct access is unsupported and can cause
problems as you have demonstrated.

These situations you are investigating are caused by external
interference with LVM operations and should not occur if LVM is used
as intended, so it is better for the code to 'fail safe' and stop so the
cause can be investigated and then the system cleaned up safely.  An
attempt to clean up automatically would risk making the situation worse.

Alasdair

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [linux-lvm] Missing error handling in lv_snapshot_remove
  2013-08-07 12:36   ` Bastian Blank
  2013-08-07 13:32     ` Alasdair G Kergon
@ 2013-08-07 15:13     ` Zdenek Kabelac
  1 sibling, 0 replies; 13+ messages in thread
From: Zdenek Kabelac @ 2013-08-07 15:13 UTC (permalink / raw)
  To: LVM general discussion and development, waldi

Dne 7.8.2013 14:36, Bastian Blank napsal(a):
> On Wed, Aug 07, 2013 at 11:13:53AM +0200, Zdenek Kabelac wrote:
>> Dne 6.8.2013 19:37, Bastian Blank napsal(a):
>>> I hold the cow device open so it will run into the error condition:
>>> | $ sleep 100 < /dev/mapper/vg-test_snap-cow&
>> You are breaking the lvm2 logic thus pushing the code to go through
>> unexpected error code path - user is never supposed to open so
>> called 'private' /dev/mapper/ devices.
>
> I'm a developer and use it to trigger an error condition. Please don't
> start with that crap about what a user should or should not do.

I'm just telling you where the development is focused on what are the rules 
here. Of course anyone open  lvm2 private device - but then you must not
be surprised it will fail.

>
>>> Then try to remove the LV:
>>> | $ lvremove vg/test_snap
>> With upstream lvm2 code - there is embedded 'retry' loop - so the removal
>> should be retried for couple times (controllable by lvm.conf).
>
> Please show that it actually does anything in this case. This is no
> condition that goes away, but a logic bug.

The point here is - that only known tool which breaks our rule and opens
lvm2 private devices even though we want them to be private (and we cannot
do that on kernel level since there is nothing like private device) is the 
udev. There are number of flags we try to avoid udev scanning - but
it's not perfect.

Otherwise if the root wants to shoot his feet - there are surely much more 
easier ways for system destruction than playing with lvm2 private device.

>> That's because udev WATCH rule might be fired basically anytime
>> after close of device opened in write mode - so it may happen lvm2
>> checks device is not opened and could be removed, but the udev WATCH
>> rules opens temporarily device and lvm2 then fails to remove device,
>> which has been previously detected as unused.
>
> There is not udevd running! Please explain how udev can be a problem in
> this case.

As said above - we expect only udev to open devices with our reserved 
suffixes.  Other usage is unexpected and unsupported.

>> There has been bug affecting cluster usage of exclusive snapshots in
>> pre .99 version - the order of taking locks for devices was not
>> correct, and if there
>> has  been clvmd restart during snapshot - it has caused some problems.
>
> Did you actually read the code? At least I can clearly see that the
> error logic is broken.

You are breaking to open doors - send a patch to improve error logic.

>> But for current (.99) code - in normal case the operation should
>> work properly. For any unpredictable errors -  lvm2 command should
>> print error message and it's up-to admin to fix dangling device and
>> table entries.
>
> It is up to LVM to not break the system with suspended devices.
>

Again - as said previously lvm2 should not be leaving suspended device,
but since there are more important bugs then fixing these in not likely to 
happen code path.

Zdenek

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [linux-lvm] Missing error handling in lv_snapshot_remove
  2013-08-07  9:41   ` Zdenek Kabelac
@ 2013-08-07 17:18     ` Andreas Pflug
  2013-08-08 10:01       ` Zdenek Kabelac
  0 siblings, 1 reply; 13+ messages in thread
From: Andreas Pflug @ 2013-08-07 17:18 UTC (permalink / raw)
  To: Zdenek Kabelac; +Cc: LVM general discussion and development

On 08/07/13 11:41, Zdenek Kabelac wrote:
> Dne 7.8.2013 11:22, Andreas Pflug napsal(a):
>> Am 06.08.13 19:37, schrieb Bastian Blank:
>>> Hi
>>>
>>> I tried to tackle a particular bug that shows up in Debian for some 
>>> time
>>> now. Some blamed the udev rules and I still can't completely rule them
>>> out. But this triggers a much worse bug in the error cleanup of the
>>> snapshot remove. I reproduced this with Debian/Linux 3.2.46/LVM 2.02.99
>>> without udevd running and Fedora 19/LVM 2.02.98-10.fc19.
>>>
>>> On snapshot removal, LVM first converts the device into a regular LV
>>> (lv_remove_snapshot) and in a second step removes this LV
>>> (lv_remove_single). Is there a reason for this two step removal? An
>>> error during removal leaves a non-snapshot LV behind.
>> Ah, this explains why sometimes my backup stops: I take a snapshot,
>> rsync the stuff and remove the snapshot with a daily cron job, but I
>> observed twice that a non-snapshot volume named like a backup snapshot
>> was lingering around, preventing the script to work. So this is no
>> exotic corner case, but happens in real life.
>>
>> I observe this since I dist-upgraded to wheezy.
>>
>
> Because Debian is using non-upstream udev rules.
>
> With upstream udev rules with standard real-life use, this situation
> cannot happen - since these rules are constructed to play better with
> udev WATCH rule.

Hm, does udev play a role on this at all? Without having dived the code, 
I'd assume udev has only to do with creation and deletion of 
/dev/mapper/... and/or /dev/vgname/... devices (upon lvchange -aX), but 
not with lvm metadata manipulation.

Regards
Andreas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [linux-lvm] Missing error handling in lv_snapshot_remove
  2013-08-07 17:18     ` Andreas Pflug
@ 2013-08-08 10:01       ` Zdenek Kabelac
  2013-08-09  7:57         ` Andreas Pflug
  0 siblings, 1 reply; 13+ messages in thread
From: Zdenek Kabelac @ 2013-08-08 10:01 UTC (permalink / raw)
  To: linux-lvm

Dne 7.8.2013 19:18, Andreas Pflug napsal(a):
> On 08/07/13 11:41, Zdenek Kabelac wrote:
>> Dne 7.8.2013 11:22, Andreas Pflug napsal(a):
>>> Am 06.08.13 19:37, schrieb Bastian Blank:
>>>> Hi
>>>>
>>>> I tried to tackle a particular bug that shows up in Debian for some time
>>>> now. Some blamed the udev rules and I still can't completely rule them
>>>> out. But this triggers a much worse bug in the error cleanup of the
>>>> snapshot remove. I reproduced this with Debian/Linux 3.2.46/LVM 2.02.99
>>>> without udevd running and Fedora 19/LVM 2.02.98-10.fc19.
>>>>
>>>> On snapshot removal, LVM first converts the device into a regular LV
>>>> (lv_remove_snapshot) and in a second step removes this LV
>>>> (lv_remove_single). Is there a reason for this two step removal? An
>>>> error during removal leaves a non-snapshot LV behind.
>>> Ah, this explains why sometimes my backup stops: I take a snapshot,
>>> rsync the stuff and remove the snapshot with a daily cron job, but I
>>> observed twice that a non-snapshot volume named like a backup snapshot
>>> was lingering around, preventing the script to work. So this is no
>>> exotic corner case, but happens in real life.
>>>
>>> I observe this since I dist-upgraded to wheezy.
>>>
>>
>> Because Debian is using non-upstream udev rules.
>>
>> With upstream udev rules with standard real-life use, this situation
>> cannot happen - since these rules are constructed to play better with
>> udev WATCH rule.
>
> Hm, does udev play a role on this at all? Without having dived the code, I'd
> assume udev has only to do with creation and deletion of /dev/mapper/...
> and/or /dev/vgname/... devices (upon lvchange -aX), but not with lvm metadata
> manipulation.


Udev attempts to update it device database after any change event
(you could observe its work with udevadm monitor)

So in your case -  you unmount filesystem -> close device -> fires WATCH event 
with some randomly delayed (systemd)udevd scan machism - so in unpredictable 
moment blkid opens device and scans its sectors (keeping device open and 
interfering with deactivate operation). For this short-time opens there is now 
built-in retry which tries to deactivate device several times when it's known 
device is not mounted.

Zdenek

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [linux-lvm] Missing error handling in lv_snapshot_remove
  2013-08-07  9:13 ` Zdenek Kabelac
  2013-08-07 12:36   ` Bastian Blank
@ 2013-08-08 13:33   ` Ritesh Raj Sarraf
  2013-08-09  9:50     ` Zdenek Kabelac
  1 sibling, 1 reply; 13+ messages in thread
From: Ritesh Raj Sarraf @ 2013-08-08 13:33 UTC (permalink / raw)
  To: linux-lvm; +Cc: Sarraf, Ritesh

Hello Zdenek,

On Wednesday 07 August 2013 02:43 PM, Zdenek Kabelac wrote:
> 
> You are breaking the lvm2 logic thus pushing the code to go
> through unexpected error code path - user is never supposed to open
> so called 'private' /dev/mapper/ devices.

Just checking if this applies to other device types of DM, or just LVM?

We recommend our users to rely on /dev/mapper/* Multipath Devices, as
_persistent_ ones.

We also recommend to pvcreate on top of /dev/mapper/* Multipath Devices.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [linux-lvm] Missing error handling in lv_snapshot_remove
  2013-08-08 10:01       ` Zdenek Kabelac
@ 2013-08-09  7:57         ` Andreas Pflug
  2013-08-09  9:40           ` Zdenek Kabelac
  0 siblings, 1 reply; 13+ messages in thread
From: Andreas Pflug @ 2013-08-09  7:57 UTC (permalink / raw)
  To: linux-lvm

Am 08.08.13 12:01, schrieb Zdenek Kabelac:
> Dne 7.8.2013 19:18, Andreas Pflug napsal(a):
>> On 08/07/13 11:41, Zdenek Kabelac wrote:
>>> Dne 7.8.2013 11:22, Andreas Pflug napsal(a):
>>>> Am 06.08.13 19:37, schrieb Bastian Blank:
>>>>> Hi
>>>>>
>>>>> I tried to tackle a particular bug that shows up in Debian for
>>>>> some time
>>>>> now. Some blamed the udev rules and I still can't completely rule
>>>>> them
>>>>> out. But this triggers a much worse bug in the error cleanup of the
>>>>> snapshot remove. I reproduced this with Debian/Linux 3.2.46/LVM
>>>>> 2.02.99
>>>>> without udevd running and Fedora 19/LVM 2.02.98-10.fc19.
>>>>>
>>>>> On snapshot removal, LVM first converts the device into a regular LV
>>>>> (lv_remove_snapshot) and in a second step removes this LV
>>>>> (lv_remove_single). Is there a reason for this two step removal? An
>>>>> error during removal leaves a non-snapshot LV behind.
>>>> Ah, this explains why sometimes my backup stops: I take a snapshot,
>>>> rsync the stuff and remove the snapshot with a daily cron job, but I
>>>> observed twice that a non-snapshot volume named like a backup snapshot
>>>> was lingering around, preventing the script to work. So this is no
>>>> exotic corner case, but happens in real life.
>>>>
>>>> I observe this since I dist-upgraded to wheezy.
>>>>
>>>
>>> Because Debian is using non-upstream udev rules.
>>>
>>> With upstream udev rules with standard real-life use, this situation
>>> cannot happen - since these rules are constructed to play better with
>>> udev WATCH rule.
>>
>> Hm, does udev play a role on this at all? Without having dived the
>> code, I'd
>> assume udev has only to do with creation and deletion of /dev/mapper/...
>> and/or /dev/vgname/... devices (upon lvchange -aX), but not with lvm
>> metadata
>> manipulation.
>
>
> Udev attempts to update it device database after any change event
> (you could observe its work with udevadm monitor)
>
> So in your case -  you unmount filesystem -> close device -> fires
> WATCH event with some randomly delayed (systemd)udevd scan machism -
> so in unpredictable moment blkid opens device and scans its sectors
> (keeping device open and interfering with deactivate operation). For
> this short-time opens there is now built-in retry which tries to
> deactivate device several times when it's known device is not mounted.

So in order to harden my script against this problem, I should
deactivate the volume explicitely, wait a while and then remove it?

Regards,
Andreas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [linux-lvm] Missing error handling in lv_snapshot_remove
  2013-08-09  7:57         ` Andreas Pflug
@ 2013-08-09  9:40           ` Zdenek Kabelac
  0 siblings, 0 replies; 13+ messages in thread
From: Zdenek Kabelac @ 2013-08-09  9:40 UTC (permalink / raw)
  To: LVM general discussion and development; +Cc: Andreas Pflug

Dne 9.8.2013 09:57, Andreas Pflug napsal(a):
> Am 08.08.13 12:01, schrieb Zdenek Kabelac:
>> Dne 7.8.2013 19:18, Andreas Pflug napsal(a):
>>> On 08/07/13 11:41, Zdenek Kabelac wrote:
>>>> Dne 7.8.2013 11:22, Andreas Pflug napsal(a):
>>>>> Am 06.08.13 19:37, schrieb Bastian Blank:
>>>>>> Hi
>>>>>>
>>>>>> I tried to tackle a particular bug that shows up in Debian for
>>>>>> some time
>>>>>> now. Some blamed the udev rules and I still can't completely rule
>>>>>> them
>>>>>> out. But this triggers a much worse bug in the error cleanup of the
>>>>>> snapshot remove. I reproduced this with Debian/Linux 3.2.46/LVM
>>>>>> 2.02.99
>>>>>> without udevd running and Fedora 19/LVM 2.02.98-10.fc19.
>>>>>>
>>>>>> On snapshot removal, LVM first converts the device into a regular LV
>>>>>> (lv_remove_snapshot) and in a second step removes this LV
>>>>>> (lv_remove_single). Is there a reason for this two step removal? An
>>>>>> error during removal leaves a non-snapshot LV behind.
>>>>> Ah, this explains why sometimes my backup stops: I take a snapshot,
>>>>> rsync the stuff and remove the snapshot with a daily cron job, but I
>>>>> observed twice that a non-snapshot volume named like a backup snapshot
>>>>> was lingering around, preventing the script to work. So this is no
>>>>> exotic corner case, but happens in real life.
>>>>>
>>>>> I observe this since I dist-upgraded to wheezy.
>>>>>
>>>>
>>>> Because Debian is using non-upstream udev rules.
>>>>
>>>> With upstream udev rules with standard real-life use, this situation
>>>> cannot happen - since these rules are constructed to play better with
>>>> udev WATCH rule.
>>>
>>> Hm, does udev play a role on this at all? Without having dived the
>>> code, I'd
>>> assume udev has only to do with creation and deletion of /dev/mapper/...
>>> and/or /dev/vgname/... devices (upon lvchange -aX), but not with lvm
>>> metadata
>>> manipulation.
>>
>>
>> Udev attempts to update it device database after any change event
>> (you could observe its work with udevadm monitor)
>>
>> So in your case -  you unmount filesystem -> close device -> fires
>> WATCH event with some randomly delayed (systemd)udevd scan machism -
>> so in unpredictable moment blkid opens device and scans its sectors
>> (keeping device open and interfering with deactivate operation). For
>> this short-time opens there is now built-in retry which tries to
>> deactivate device several times when it's known device is not mounted.
>
> So in order to harden my script against this problem, I should
> deactivate the volume explicitely, wait a while and then remove it?

If you call  'udevadm settle'  after umount --  it will wait till udev
finishes its work.

However recent lvm2 has the 'retry' loop built-in - so it should not be needed 
if the proper udev rules are in place.

Zdenek

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [linux-lvm] Missing error handling in lv_snapshot_remove
  2013-08-08 13:33   ` Ritesh Raj Sarraf
@ 2013-08-09  9:50     ` Zdenek Kabelac
  0 siblings, 0 replies; 13+ messages in thread
From: Zdenek Kabelac @ 2013-08-09  9:50 UTC (permalink / raw)
  To: LVM general discussion and development; +Cc: Sarraf, Ritesh, Ritesh Raj Sarraf

Dne 8.8.2013 15:33, Ritesh Raj Sarraf napsal(a):
> Hello Zdenek,
>
> On Wednesday 07 August 2013 02:43 PM, Zdenek Kabelac wrote:
>>
>> You are breaking the lvm2 logic thus pushing the code to go
>> through unexpected error code path - user is never supposed to open
>> so called 'private' /dev/mapper/ devices.
>
> Just checking if this applies to other device types of DM, or just LVM?
>
> We recommend our users to rely on /dev/mapper/* Multipath Devices, as
> _persistent_ ones.
>
> We also recommend to pvcreate on top of /dev/mapper/* Multipath Devices.


multipath  !=  lvm2

Multipath has its own rules how to use devices and make the available for use.

In lvm2 you could use lvm.conf preferred_names in this form:

preferred_names = [ "^/dev/mpath/", "^/dev/mapper/mpath", "^/dev/[hs]d" ]

But it's rather distro-specific how the multipath devices are made available 
for the user - so the names above are mostly usable for RedHat distors
(I think Suse uses slightly different logic)

Note - using   pvcreate on /dev/mapper is nothing against the lvm2 rule.

lvm2 rule is about supported device path for LVs -  whenever you try to use 
LV,  you should always use    '/dev/vgname/lvname'  path.

So   /dev/mapper/vgname-lvname  is not supported way though it will work in 
most cases - but i.e. one of the problems you may have is, that your tool 
would need to properly handle '-' symbol here,  other issue could be, that in 
/dev/mapper you see a lot more devices i.e. all mirror legs and other so 
called private devices which you could  misuse for 'mount' and do a lot of 
damage to internal metadata.

So the simple rule for LVs is to use   /dev/vgname/lvname.

Zdenek

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2013-08-09  9:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-06 17:37 [linux-lvm] Missing error handling in lv_snapshot_remove Bastian Blank
2013-08-07  9:13 ` Zdenek Kabelac
2013-08-07 12:36   ` Bastian Blank
2013-08-07 13:32     ` Alasdair G Kergon
2013-08-07 15:13     ` Zdenek Kabelac
2013-08-08 13:33   ` Ritesh Raj Sarraf
2013-08-09  9:50     ` Zdenek Kabelac
2013-08-07  9:22 ` Andreas Pflug
2013-08-07  9:41   ` Zdenek Kabelac
2013-08-07 17:18     ` Andreas Pflug
2013-08-08 10:01       ` Zdenek Kabelac
2013-08-09  7:57         ` Andreas Pflug
2013-08-09  9:40           ` Zdenek Kabelac

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.