This request was evaluated by Red Hat Product Management for inclusion in a Red
Hat Enterprise Linux major release.  Product Management has requested further
review of this request by Red Hat Engineering, for potential inclusion in a Red
Hat Enterprise Linux Major release.  This request is not yet committed for
inclusion.

Ping about this bug.

We don't have a solution for this yet.  Discussions ongoing.

A new rule IMPORT{db} was added in udev version 152. This makes it possible to reuse existing information from udev database for any CHANGE event and regenerate databse content (which would be cleared otherwise).

That could help the information we set in libdevmapper for particular ioctls (and returned back within DM_COOKIE env. variable that is set for udev events) to survive even while processing the artificial events (the ones originated in the "watch" rule or "udevadm trigger" call).

However, we still don't have a solution for proper synchronization with such spurious events which is another side of the problem that should be tackled properly as well.

Adding this rules, OPTIONS="watch" in /lib/udev/rules.d/10-dm.rules, causes
a lot of unexpected I/Os to devices. This changes a behavior of systems.
In RHEL6, please consider to keep the same behavior as RHEL5. See bug 591606
for details.

OK, let's be straight - the "watch" rule just can't be used for dm devices unless there's a way to synchronize with the events generated as an outcome of its use. But that would mean total redesign of how the WATCH rule works.

We can't support this rule. We will probably, contrariwise, add more OPTIONS:="nowatch" rules in the future for more dm devices if that will be required for certain problematic situations (for now, we set nowatch rule for all devices that have DM_UDEV_DISABLE_OTHER_RULES uevent env. variable set - this one is already honoured by udisks udev rules and so the watch is not set for totally inappropriate devices, but we've added the explicit "nowatch" as well just to make sure generaly).

The use of the watch rule causes lots of problems with synchronisation - we haven't found a reasonable way to make any synchronisation with such events. The example of problems the use of this rule brings is bug #570359, #577798, #591606...

I've already discussed this issue with Kay Sievers, the udev maintainer - he recognized that this is an issue and that the use of the WATCH rule is not optimal in all situations and that we should try to find an alternative solution. Unfortunately, we don't have any satisfying one yet.

Anyway, the requirement in this bug can't be satisfied without imposing any problems and risks while managing device-mapper devices. This really needs an alternative solution so the use of the watch rule can be replaced by something less invasive.

Since the WATCH rule will never get into device-mapper rules because of the risks it imposes and because we can't satisfy the requirement stated in this bug report then, I'm setting state to CLOSED with WONTFIX.

(we're looking for alternative solutions instead)

Can't really do any meaningful UI management tool without this - so reopening so it can block any bugs asking for such tools. That, and it's an *extremely reasonable request* to have this - it's what you need in order to have stable device names (e.g. /dev/disk/... symlinks) working when devices are being managed.

Also, as you are aware, anything in the kernel- or user-space can (and will!) generate uevents so if your software doesn't work with this - then it won't work without it either (might take longer for it to fail though).

Even if you don't have plans to fix this immediately (as you point out, sorting you sync mechanism is a prerequisite to fixing this), please don't close it.

(In reply to comment #9)
> Can't really do any meaningful UI management tool without this - so reopening
> so it can block any bugs asking for such tools. That, and it's an *extremely

OK, if you need that...

I just wanted to point out that the WATCH rule as it exists today is unsupportable because of the synchronisation problem (and other problems like generating too much useless IO).

I can't imagine a sane solution with existing form of the WATCH rule and its current use. And I'm not the only one - that's why I'm calling for a way to rethink this somehow. Can we provide the functionality that is expected from the WATCH rule the other way? The one that is much more optimal and doesn't touch devices when it's actually not needed. The area that the WATCH rule covers is a superset of what is actually needed, we really need to narrow it down.

As for the synchronisation with such events - this is a tough task, I have to admit.

> reasonable request* to have this - it's what you need in order to have stable
> device names (e.g. /dev/disk/... symlinks) working when devices are being
> managed.
> 
> Also, as you are aware, anything in the kernel- or user-space can (and will!)
> generate uevents so if your software doesn't work with this - then it won't
> work without it either (might take longer for it to fail though).
> 

We don't have a problem with these spurious uevents as before - we have all the information we need now. We can reuse the udev database using the new IMPORT{db} udev rule. That's kind of sorted out. The only primary problem remaining is the synchronisation.

> Even if you don't have plans to fix this immediately (as you point out, sorting
> you sync mechanism is a prerequisite to fixing this), please don't close it.

Well, that's not about sorting out device-mapper udev sync mechanism - I can't synchronize with something that is *totally* asynchronous in its nature. I'm afraid there's nothing else to "fix" on our side. With events that I can expect, it's fine. But it is a real pain with events coming from unknown sources at unknown times. And this is not about device-mapper vs. udev anymore. It's about udev in general and its asynchronous nature that causes problems (and the rules that run various scans in response).

This issue has been proposed when we are only considering blocker
issues in the current Red Hat Enterprise Linux release. It has
been denied for the current Red Hat Enterprise Linux release.

** If you would still like this issue considered for the current
release, ask your support representative to file as a blocker on
your behalf. Otherwise ask that it be considered for the next
Red Hat Enterprise Linux release. **

Also worth noting that even the current usage of 'watch' doesn't work in a cluster where the notification is only issued on the machine where the change was made instead of every machine using the device.  Clustered udev?

Can we explore the alternative, where software that makes a change these tools need to know about is responsible for issuing notification events?  In a cluster, the clustering software would be responsible for propagating this to other machines.

Say we pick one tool first and work through exactly how/if it could work.

And can we restate this sync problem on here more precisely as it exists today i.e. taking account of all the workarounds we already made?

First, downstream RH bugzilla is really the *wrong* place for this conversation to be happening. It should happen on the udev mailing list.

(In reply to comment #15)
> Also worth noting that even the current usage of 'watch' doesn't work in a
> cluster where the notification is only issued on the machine where the change
> was made instead of every machine using the device.  Clustered udev?

Really? So just because your LVM software is "clustered" (for some value of clustered) it means that now udev has to be "clustered"? Or if I have some database software that is "clustered" (maybe another kind of "clustered" than LVM's notion - such software do exist) then automatically the lower layers should be "clustered" too? I don't think so.

I mean, that's not how software really works. Here's what I'm saying: things LVM clustering is a *high-level feature* - conflating it with low-level stuff like udev is exactly as useful as suggesting the kernel should be distributed onto multiple machine or that exporting /sys via NFS is a useful thing to do (e.g. not). So, sorry to say, but suggesting "clustered udev" is, at best, totally missing the point of how the OS works...

Note: nothing prevents anyone from writing a distributed system that just synthesizes the 'change' uevent on other machines (the machine on the receiving end would just 'echo change > /sys/path/to/uevent') if they know that the backing device has been invalidated. That would be a useful thing for clustered LVM to do, BTW. Or any distributed system, really.

> Can we explore the alternative, where software that makes a change these tools
> need to know about is responsible for issuing notification events?  In a
> cluster, the clustering software would be responsible for propagating this to
> other machines.

Again, this is an additional and unnecessary step - the user process has _already_ told the OS kernel that it is writing data by, you know, writing the data - it is completely unnecessary to have to make the user process do 'echo change > /sys/path/to/uevent'. The current mechanism works *just fine*

You seem to be underestimating how useful the current 'watch' machinery is - you must never use /dev/disk symlinks. Or never used the UI.

Sorry but there's been so much discussion about all this that I fail to remember what the problem really is.... I mean, from a high level it's very simple. We simply want the udev database to stay current when people change the contents of a block device. That is, we want to run blkid(8) to probe the block device. Whenever it changes (that is, when the last closer of a writable fd closes the fd). That's a *very* basic thing. We need this because

 - we want to keep the /dev/disk/ hierarchy current
 - we want to keep various User Interface elements current

even when people zero the device via dd(1) or whatever. Before we can make forward progress, or even continue to have a discussion, we probably need to agree that these are the requirements and they cannot change. Because, my experience shows that it is near impossible to build a supportable general purpose OS without these requirements.

Btw, if performance is a concern (and it easily could be, no argument there), there should of course be a way to turn "watch" off for users and deployments that know what they are doing. Or for embedded products.

But the "watch" feature *must* be on by default in the OS - otherwise it gets to be a nightmare to build any kind of User Interface software (and a bigger nightmare to support it); basically, it would mean going back to the stone-age where each piece of different UI software (both graphical and textual) is its own sile and probes all the devices on start-up - not only is this *wasteful*, it is annoying for the user to have to wait for this to happen. And it's hard to debug as you can't ask for e.g. the output of 'udevadm info --export-db'.

A major point of Linux+udev is that everyone can share e.g. drive and blockdev identification via blkid(8) - and as a result, we save a lot of CPU+IO that way. Additionally, we can make debugging a lot easier. And the UI behaves as you *expect* when you e.g. do a 'dd if=/dev/zero of=/dev/sdz bs=512 count=100' or 'mkfs -t ext3 /dev/sdz' on the command-line (this will make the desktop icon disappear or appear, depending). Similarly, the /dev/disk hierarchy is updated.

LVM and device-mapper is absolutely not special in this regard - replace /dev/sdz with /dev/mapper/my-luks-volume in the above paragraph and it will work the same way.

If things *didn't* work this way, people would consider it a bug. So it really needs to work this way.

(In reply to comment #18)
> where each piece of different UI software (both graphical and textual) is its
> own sile and probes all the devices on start-up - not only is this *wasteful*,
      ^^^^
I meant _silo_ here. E.g. to emphasize the lack of sharing code and infrastructure.

(In reply to comment #17)
> So, sorry to say, but suggesting "clustered udev" is, at best,
> totally missing the point of how the OS works...

The phrase "clustered udev" implies nothing about how it is implemented - a cluster layer above the existing udev layer.  But something is necessary if watch rules are to be used in a cluster - and these types of clusters are in use and supported today but anything using the "watch" rule fails.

> Again, this is an additional and unnecessary step - the user process has
> _already_ told the OS kernel that it is writing data by, you know, writing the
> data - it is completely unnecessary to have to make the user process do 'echo
> change > /sys/path/to/uevent'. The current mechanism works *just fine*

The user process may or may not have written something after opening it with write access.  It has not necessarily have written something that processes like udisks need to know about: they only need to be notified of a subset (potentially a tiny subset) of writes, not all writes.  Sledgehammer to crack a nut.

> That is, we want to run blkid(8) to probe the block
> device. Whenever it changes (that is, when the last closer of a writable fd
> closes the fd). 

You only need the blkid database to be updated when something it has cached in it changed.  Rerunning blkid every time the last closer of a writable fd closed the fd causes unnecessary work when nothing that matters to blkid actually changed.  I'm arguing that the program that makes these changes should be responsible for triggering the notifications.

>  - we want to keep the /dev/disk/ hierarchy current
>  - we want to keep various User Interface elements current

I'm not disputing your requirement - I'm arguing that the implementation using "watch" rules is inefficient and troublesome, and that we should be working on finding a better way to achieve what's needed.

(In reply to comment #18)
> A major point of Linux+udev is that everyone can share e.g. drive and blockdev
> identification via blkid(8) - and as a result, we save a lot of CPU+IO that
> way.

So all programs on the system that update things that blkid records should notify the blkid database.  Programs should not maintain their own records.  One silo instead of many.  We are in agreement on the aim, but disagreeing on the means to the end...

(In reply to comment #17)
> First, downstream RH bugzilla is really the *wrong* place for this conversation
> to be happening. It should happen on the udev mailing list.

When I last spoke to Kay he implied he had implemented the 'watch' rule facility with great reluctance and did not think people should really need to use it.

(I don't subscribe to the udev list as I presume most of its contents would not be interesting to me.  Cross-post to lvm-devel if you want to switch to email.)

Alasdair, please be accurate: the blkid database is something entirely different (it is best left undescribed, for the purpose of this bug report anyway) - and it is _never_ ever used when populating the  udev database (tmpfs-based, e.g. non-persistent). I take it you mean the udev database in your above messages?

I based those comments on this sentence "we want to run blkid(8) to probe the block device".  We only want the information cached in one place - the location of that - libblkid, udev or anywhere else - is not relevant to my arguments.

(And I'd probably argue that blkid should be broken up and built into udev anyway...)

(The bit of blkid that knows about lvm should be provided by a callout to lvm code;  the bit that knows about md should be provided by a callout to md code etc.)

(In reply to comment #20)
> The user process may or may not have written something after opening it with
> write access.  It has not necessarily have written something that processes
> like udisks need to know about: they only need to be notified of a subset
> (potentially a tiny subset) of writes, not all writes.  Sledgehammer to crack a
> nut.

No, it's completely fine doing it this way. In fact, because people were noticing blkid instances (with negligible impact) we've fixed a bunch of user programs that opened the device node read-write when only read-only was needed. One example is e.g. hdparm(8). That's good. The same way e.g. SELinux can notice if you leak FDs - e.g. a nice-to-have.

> I'm not disputing your requirement - 

I'm glad we agree this is needed.

> I'm arguing that the implementation using
> "watch" rules is inefficient and troublesome, and that we should be working on
> finding a better way to achieve what's needed.

Please provide data for interesting real-world work-loads before making outrageous claims like "Sledgehammer to crack a nut". As far as I can tell, we have had no problems whatsoever with this setup and, FYI, we've been doing this for many years now.

(On the other hand, we have had *nothing but problems* with software e.g. opening all device nodes just because they are their own silo - for example, if you have 64 PVs you end up doing 64*64=4096 device probes when processing uevents when the devices are added. Which is very costly.)

Please also realize that we're talking implementation details here - heck, if
performance is problem we can easily limit/serialize/queue 'change' uevents so
the amount of blkid(8) jobs to reprobe block devices is kept below a threshold.

If you don't want to measure the impact then at least think about it: If a block-device is

 - mounted; or
 - claimed by another device (e.g. PV in a LVM VG)

then the 'change' uevents are of course not generated. So in reality, this is not really a big deal. And for the odd workloads, the user or deployment should be able to turn things of cf. what I said in comment 18.

Please also realize how *simple* the current solution is compared to having to
patch everything on the planet to do what the kernel already knows. Simplicity
is a key to making things work. We don't want to over-engineer things here -
Linux + user-land is complex enough as is.

Sorry if this sounds like I'm complaining but we've been asking for LVM/device-mapper to catch up to the rest of Linux+udev storage for 3-4 years now. We even now agree that 'change' uevents are needed. Can we at least implement this in the community distros (such as Fedora) and then deal with any "performance concerns", real or otherwise, later? I mean, we don't get anywhere if we're trying to solve everything at once...

(In reply to comment #24)
> I based those comments on this sentence "we want to run blkid(8) to probe the
> block device".  We only want the information cached in one place - the location
> of that - libblkid, udev or anywhere else - is not relevant to my arguments.

It is extremely relevant to be precise and not use incorrect words and terms. In this case, it's very confusing what you are saying because there really is such a thing as the blkid database (as I said, the blkid database is a legacy thing that I and many others believe can never work... but that's irrelevant to this discussion).

(In reply to comment #25)
> (And I'd probably argue that blkid should be broken up and built into udev
> anyway...)

Actually, it used to be like that - libblkid's current probing code stems from udev's (now defunct) libvolume_id that Kay and I started. Kay and Karel found that it was better if the code lived in util-linux-ng.

(In reply to comment #26)
> (The bit of blkid that knows about lvm should be provided by a callout to lvm
> code;  the bit that knows about md should be provided by a callout to md code
> etc.)

Uhm, sorry, but, please familarize yourself with how the OS works. The way it works now is that blkid(8) is used for the identification on what is on the block device

 # udevadm info -q all -n /dev/sdb | grep ID_FS
 E: ID_FS_UUID=19fd657b-1610-e54d-7714-72ae01c3bec1
 E: ID_FS_UUID_ENC=19fd657b-1610-e54d-7714-72ae01c3bec1
 E: ID_FS_UUID_SUB=862274cb-a56b-d82b-1898-4da72e145c95
 E: ID_FS_UUID_SUB_ENC=862274cb-a56b-d82b-1898-4da72e145c95
 E: ID_FS_LABEL=rhea:BigBeta
 E: ID_FS_LABEL_ENC=rhea:BigBeta
 E: ID_FS_VERSION=1.2
 E: ID_FS_TYPE=linux_raid_member
 E: ID_FS_USAGE=raid

 (see /lib/udev/rules.d/60-persistent-storage.rules)

and mdadm(8) provides more detail

 # udevadm info -q all -n /dev/sdb | grep MD_
 E: MD_LEVEL=raid10
 E: MD_DEVICES=12
 E: MD_NAME=rhea:BigBeta
 E: MD_UUID=19fd657b:1610e54d:771472ae:01c3bec1
 E: MD_UPDATE_TIME=1288866561
 E: MD_DEV_UUID=862274cb:a56bd82b:18984da7:2e145c95
 E: MD_EVENTS=186

 (see /lib/udev/rules.d/64-md-raid.rules)

It works this way because we don't want to spawn 10 different tools (one for lvm, one for md-raid, one for btrfs and so on) - we launch the generic one to do the id and then the specialized one to provide more information. It works this way because of performance concerns (Kay, myself and others take performance quite seriously).

FWIW, and sorry to keep harping about this, but we've been asking for dm/lvm to provide the same kind of information for *years* (I could find the RH bug for the original "dm-setup --export" patch but...) but there is still nothing. So I ended up doing the latter part for LVM as a part of udisks, for example:

 # udevadm info -q all -n /dev/sdd2 |grep UDISKS_LVM2
 E: UDISKS_LVM2_PV_VG_UUID=V1R1Zd-Egxv-er5x-ybm8-DcUC-JeaJ-Drz0Zs
 E: UDISKS_LVM2_PV_VG_NAME=vg_phobos
 E: UDISKS_LVM2_PV_VG_SIZE=999989182464
 E: UDISKS_LVM2_PV_VG_FREE_SIZE=0
 E: UDISKS_LVM2_PV_VG_EXTENT_SIZE=4194304
 E: UDISKS_LVM2_PV_VG_EXTENT_COUNT=238416
 E: UDISKS_LVM2_PV_VG_SEQNUM=11
 E: UDISKS_LVM2_PV_VG_PV_LIST=uuid=wqpGot-fTcH-ro4n-z3zj-jnS2-ZwuS- dn3Q70;size=999989182464;allocated_size=999989182464
 E: UDISKS_LVM2_PV_VG_LV_LIST=name=lv_root;uuid=8UZKjQ-i8IZ-hfIr-zves-KTr8-W2or-ngOgCY;size=991684460544;;active=1 name=lv_swap;uuid=U84QpZ-YYCa-nAJz-v2a5-W4g3-EGO1-Pg1Cc3;size=8304721920;;active=1
 E: UDISKS_LVM2_PV_UUID=wqpGot-fTcH-ro4n-z3zj-jnS2-ZwuS-dn3Q70
 E: UDISKS_LVM2_PV_NUM_MDA=1

 (see /lib/udev/rules.d/80-udisks.rules)

The UDISKS_LVM2_* properties and MD_* properties is what enables someone to write User Interface applications like

 http://people.freedesktop.org/~david/palimpsest-lvm2-pvs-on-mp.png
 http://people.freedesktop.org/~david/gdu-lvm2-w-pv-allocated-size.png
 http://people.freedesktop.org/~david/lvm2-default-fedora-install.png
 http://people.freedesktop.org/~david/palimpsest-md-raid-components-on-mp.png

without having to probe any disk at all whatsoever.

Of course, this should be part of the LVM tools just like the MD_* properties are part of the mdadm(8) binary...

(In reply to comment #27)
> Please provide data for interesting real-world work-loads before making
> outrageous claims like "Sledgehammer to crack a nut". As far as I can tell, we
> have had no problems whatsoever with this setup and, FYI, we've been doing this
> for many years now.

Not in RHEL and causing all these problems so have to keep telling people to disable it.  We want a solution that doesn't mean some people have to turn it off.

> Please also realize how *simple* the current solution is compared to having to
> patch everything on the planet to do what the kernel already knows.

The kernel doesn't normally already know.  Only the application performing the write actually knows what it has written and whether that means something else should be notified (a la BLKRRPART if it changed a partition table).


But as ever, we have multiple independent problems to solve.

1) Mechanism to wait for all watch rule processing to finish, and to block new processing from running during critical sections of application code that expect exclusive device access (partly dealt with the last bit of that already);

2) Ability only to perform processing that updates the "silo" when something stored needs to change, avoiding unnecessary processing when it doesn't;

3) Cluster support for notifications (needed regardless of whether watch rule mechanism is used or some alternative is found).

(In reply to comment #28)
> It works this way because we don't want to spawn 10 different tools (one for
> lvm, one for md-raid, one for btrfs and so on) - we launch the generic one to
> do the id and then the specialized one to provide more information. It works
> this way because of performance concerns (Kay, myself and others take
> performance quite seriously).

The problem is that this makes for two independent bits of code knowing how to handle LVM metadata.  There should only be one - even source code lib functions provided to some blkid plugin API by some lvm-devel package if linking against some lvm lib is too hard, I don't care, but the blkid package has no business containing code that knows details of lvm internals.

> FWIW, and sorry to keep harping about this, but we've been asking for dm/lvm to
> provide the same kind of information for *years* (I could find the RH bug for
> the original "dm-setup --export" patch but...) but there is still nothing. So 

We've had this available for some time.  (Or was the bz still blocked on one field that we felt it inappropriate to provide?  I forget.)

(In reply to comment #30)
> The problem is that this makes for two independent bits of code knowing how to
> handle LVM metadata.  There should only be one - even source code lib functions
> provided to some blkid plugin API by some lvm-devel package if linking against
> some lvm lib is too hard, I don't care, but the blkid package has no business
> containing code that knows details of lvm internals.

Maybe. But no such library or plug-in infrastructure existed when we needed this and attempts at involving you guys have not exactly been fruitful. It's also, IMHO, *wrong* to start with huge plans involving plug-in APIs and so forth - you will die of complexity and never get anywhere.

Sure, maybe some day libblkid can offer that kind of stuff.. and then FS vendors can provide probing plug-ins the same way the can provide a .ko file for the kernel FS driver.... but until then it's so much easier the way it works now.

Again, please get involved and please stop dreaming about way-too-complex architectures. Especially when you haven't contributed much towards solving the general problem (not trying to belittle LVM/device-mapper here - just that it feels like you are second-guessing everything that has been done (which is a lot) without actually having made any of the experiences that we have (which really shows in this conversation).

> > FWIW, and sorry to keep harping about this, but we've been asking for dm/lvm to
> > provide the same kind of information for *years* (I could find the RH bug for
> > the original "dm-setup --export" patch but...) but there is still nothing. So 
> 
> We've had this available for some time.  (Or was the bz still blocked on one
> field that we felt it inappropriate to provide?  I forget.)

Uhm, no. You added some very highly configurable stuff to dm-setup to print fields that almost can be used in the udev database (but not quite, there are still some issues with quoting). Which was totally not what we asked for (a patch for dm-setup --export was provided but you rejected it), and, as a result didn't work. And it turned out we needed more than just that (needed some of the logic in liblvm2app). Do you really think I put all the stuff in udisks just for fun?

(In reply to comment #29)
> (In reply to comment #27)
> > Please provide data for interesting real-world work-loads before making
> > outrageous claims like "Sledgehammer to crack a nut". As far as I can tell, we
> > have had no problems whatsoever with this setup and, FYI, we've been doing this
> > for many years now.
> 
> Not in RHEL and causing all these problems so have to keep telling people to
> disable it.  We want a solution that doesn't mean some people have to turn it
> off.

Citation needed. No, I'm serious. And I highly suspect that "all these problems" have been self-inflicted due to how e.g. mkinitrd and nash works (these are no longer in Fedora). Every other distro doesn't seem to have "all these problems".

> > Please also realize how *simple* the current solution is compared to having to
> > patch everything on the planet to do what the kernel already knows.
> 
> The kernel doesn't normally already know.  Only the application performing the
> write actually knows what it has written and whether that means something else
> should be notified (a la BLKRRPART if it changed a partition table).

Wrong. The kernel *of course* knows when the last closer of a writable fd closes the fd. It tells user space which then probes for signatures again (for part tables, filesystems, LVM etc.). This is the way it works today. And it works just fine.

(In reply to comment #32)

> Wrong. The kernel *of course* knows when the last closer of a writable fd
> closes the fd. It tells user space which then probes for signatures again (for
> part tables, filesystems, LVM etc.). This is the way it works today. And it
> works just fine.

This is *not* the same thing.  Probing is *not* required every time the last closer of a writable fd closes it.  The "silo" only needs updating when the "signatures" were changed.  Devices are written to for other reasons too, not just to change a "signature".

The probing runs every time it thinks it *might* have been changed - even if almost all the time on some systems it didn't.

(In reply to comment #32)
> Citation needed. No, I'm serious.

I've lost count of the number of occasions where "temporarily disable udev" makes the reported problem go away.  First example a search finds: bug 577798  (falls into my category 1 above)

Comment #7 was an example of 'lots of unnecessary I/O'

(In reply to comment #31)

> Uhm, no. You added some very highly configurable stuff to dm-setup to print
> fields that almost can be used in the udev database (but not quite, there are
> still some issues with quoting). 

Remind me: What still needs fixing?

> Which was totally not what we asked for (a
> patch for dm-setup --export was provided but you rejected it)

(Written in a way that hard-coded your particular requirements but we wanted it to be something of more general use, which we did.)

(In reply to comment #36)
> Comment #7 was an example of 'lots of unnecessary I/O'

I take it you mean bug 591606. This is NOT an example of a real-world use-case. And *exactly* the same thing happens for non-dm devices and we don't hear people complaining about that. Think of it this way: It's just the way Linux works now - if you have a block device that is neither mounted nor claimed then it will be reprobed whenever the last writer closes the device. It's just the way things work now and it's fine. App authors can modify their apps to keep a non-read-only fd around if this happens too often. Or they can use bigger hammers like a udev rule.

(In reply to comment #35)
> (In reply to comment #32)
> > Citation needed. No, I'm serious.
> 
> I've lost count of the number of occasions where "temporarily disable udev"
> makes the reported problem go away.  First example a search finds: bug 577798 
> (falls into my category 1 above)

Eh, Alasdair, I asked for examples of where "performance" was a concern and you answer with something else? I don't get it - please don't conflate matters. Clearly bug 577798 is not about performance - it is just a bug that reveals wrong assumptions made in your lvm/dm and udev integration code (origin of 'change' uevents'). Which has nothing to do with performance.

(In reply to comment #33)
> (In reply to comment #32)
> 
> > Wrong. The kernel *of course* knows when the last closer of a writable fd
> > closes the fd. It tells user space which then probes for signatures again (for
> > part tables, filesystems, LVM etc.). This is the way it works today. And it
> > works just fine.
> 
> This is *not* the same thing.  Probing is *not* required every time the last
> closer of a writable fd closes it.  The "silo" only needs updating when the
> "signatures" were changed.  Devices are written to for other reasons too, not
> just to change a "signature".

Of course. But the former is a subset of the latter so it just means we probe a bit too often. Which is fine - just means that the database is updated anyway, sometimes more than it needs to be. You are concerned this is killing performance. I have said that it doesn't appear to be that way for real-world work-loads and you have STILL NOT shown me any real-world work-loads where this is a problem. You have, however, tried hand-waved a ton about it.

(In reply to comment #38)
> if you have a block device that is neither mounted nor claimed then
> it will be reprobed whenever the last writer closes the device. It's just the
> way things work now and it's fine.

Why is it fine to do repeated probing when nothing meaningful changed and the machine knows that but fails to inform the prober of that?  A waste of resource and the side-effects of the probing make the other problems worse (when something genuinely wants to claim the unowned device and finds it can't because the prober is using it.)

> it is just a bug that reveals
> wrong assumptions made in your lvm/dm and udev integration code (origin of
> 'change' uevents')

But it is impossible to handle these situations without changing the udev framework.  As currently implemented the use of watch rules makes a system unsupportable as far as I'm concerned.

(In reply to comment #37)
> (In reply to comment #31)
> 
> > Uhm, no. You added some very highly configurable stuff to dm-setup to print
> > fields that almost can be used in the udev database (but not quite, there are
> > still some issues with quoting). 
> 
> Remind me: What still needs fixing?
> 
> > Which was totally not what we asked for (a
> > patch for dm-setup --export was provided but you rejected it)
> 
> (Written in a way that hard-coded your particular requirements but we wanted it
> to be something of more general use, which we did.)

What we need is _not_ a toolbox so we can write our own 200-character dm-setup commands to print things out in a format so it can (maybe) be imported into the udev database. While that is cute and very generic it is COMPLETELY missing the point - and it is a cheap way for you to not participate at all!

Look at the mdadm RPM package for guidance. That piece of software includes an --export option and a udev rule such that the udev database contains information about md block devices.

 # rpm -qf /lib/udev/rules.d/64-md-raid.rules 
 mdadm-3.1.3-0.git20100804.2.fc15.x86_64

This setup guarantees that MD_* properties are in the udev database. This means that applications that care about md devices can get all this information WITHOUT DOING ANY IO by simply looking in the udev database. That's exactly what applications want.

Basically, I need for the LVM tools to do something similar for e.g. LVM PVs. As I said, I've already done this in udisks so I would appreciate if it's the same format, see

 http://cgit.freedesktop.org/udisks/tree/src/probers/udisks-lvm-pv-export.c

Can you see the difference? What is needed is _concrete_ participation from you in form of you guaranteeing the availability and currentness [1] of properties in the udev database for dm- devices. What you have offered so far is very very far from that.

[1] : If 'currentness' is even a real word. Don't know. Anyway, what I'm asking for, is that you ensure that the udev properties are kept current. For example, if I change the name of a LV, then the udev property with said name needs to be updated as well.

(In reply to comment #40)
> (In reply to comment #38)
> > if you have a block device that is neither mounted nor claimed then
> > it will be reprobed whenever the last writer closes the device. It's just the
> > way things work now and it's fine.
> 
> Why is it fine to do repeated probing when nothing meaningful changed and the
> machine knows that but fails to inform the prober of that?  A waste of resource
> and the side-effects of the probing make the other problems worse (when
> something genuinely wants to claim the unowned device and finds it can't
> because the prober is using it.)

Simply because it never hurts on current meaningful workloads so the benefits (up-to-date udev database, simpler OS architecture etc) outweighs the cost many many times. And if it's hurting one application there are ways to avoid the probing. It's just the way Linux works now. It's been that way for a couple of years, btw. I'm not sure why you keep fighting this. Why?

> > it is just a bug that reveals
> > wrong assumptions made in your lvm/dm and udev integration code (origin of
> > 'change' uevents')
> 
> But it is impossible to handle these situations without changing the udev
> framework.  As currently implemented the use of watch rules makes a system
> unsupportable as far as I'm concerned.

That is completely incorrect. The problem is the fiasco with you assuming that only 'change' uevents can originate from your own tools. Listen, watch rules works just fine - it's just that it causes problems with the way you have made lvm/dm work.

(In reply to comment #42)
> That is completely incorrect. The problem is the fiasco with you assuming that
> only 'change' uevents can originate from your own tools.

Wasn't this fixed recently btw? I remember Kay recently added a way in udev so you can inherit properties from the database. Which means that you only need to run the prober the *first* time you see the device (on e.g. 'add' or first 'change'). Subsequent events can then import that from the existing device. For example, for a hypothetical FIRST_SEEN property you'd do

 IMPORT{db}="FIRST_SEEN"
 ENV{FIRST_SEEN}=="", PROGRAM="/bin/date +%%s", ENV{FIRST_SEEN}="%c"

This ensure we only run e.g. /bin/date the _first_ time we see the device.

(In reply to comment #43)
> (In reply to comment #42)
> > That is completely incorrect. The problem is the fiasco with you assuming that
> > only 'change' uevents can originate from your own tools.
> 
> Wasn't this fixed recently btw? I remember Kay recently added a way in udev so
> you can inherit properties from the database. Which means that you only need to
> run the prober the *first* time you see the device (on e.g. 'add' or first
> 'change'). Subsequent events can then import that from the existing device. For
> example, for a hypothetical FIRST_SEEN property you'd do
> 
>  IMPORT{db}="FIRST_SEEN"
>  ENV{FIRST_SEEN}=="", PROGRAM="/bin/date +%%s", ENV{FIRST_SEEN}="%c"
> 
> This ensure we only run e.g. /bin/date the _first_ time we see the device.

What I'm getting at is this is that you can use this (relatively) new IMPORT{db} udev facility to import the cookie from the database. That way you can trivially deal with 'change' uevents originating from outside your own tools.

(In reply to comment #42)
> I'm not sure why you keep fighting this. Why?

Because it a manifestation of the design that cannot work in principle.
 
> That is completely incorrect. The problem is the fiasco with you assuming that
> only 'change' uevents can originate from your own tools. Listen, watch rules
> works just fine - it's just that it causes problems with the way you have made
> lvm/dm work.

The problem is that watch rule processing is asynchronous *by design* and watch rules are not passive, but run things that interfere with other processes running on the system stopping them working properly.  Watch rules are being used as a 'hack' to avoid fixing a system design flaw.

(In reply to comment #44)
> What I'm getting at is this is that you can use this (relatively) new
> IMPORT{db} udev facility to import the cookie from the database. That way you
> can trivially deal with 'change' uevents originating from outside your own
> tools.

Yes, we do that now and that old problem is gone, but it doesn't deal with the asynchronous watch rules.

(In reply to comment #46)
> (In reply to comment #44)
> > What I'm getting at is this is that you can use this (relatively) new
> > IMPORT{db} udev facility to import the cookie from the database. That way you
> > can trivially deal with 'change' uevents originating from outside your own
> > tools.
> 
> Yes, we do that now and that old problem is gone, 

Cool.

> but it doesn't deal with the
> asynchronous watch rules.

OK, that's news to me. What specifically is the problem?

(In reply to comment #41)
> What we need is _not_ a toolbox so we can write our own 200-character dm-setup
> commands to print things out in a format so it can (maybe) be imported into the
> udev database.

The request I saw was for an export facility.  It was not for a udev state maintained by LVM.  We can certainly look into that again, but I recall when we originally discussed storing lvm information directly in udev we concluded the database structure was too limited for it to work properly and we needed something richer.  We can look at what you're actually storing now and see if it's possible for the lvm package to maintain that directly, but I think it's more likely we will need a richer database structure.

(Or rather, we'll struggle to maintain database integrity if we use the udev database - queries may see inconsistencies because information has to be duplicated.)

> That is completely incorrect. The problem is the fiasco with you assuming that
> only 'change' uevents can originate from your own tools. Listen, watch rules
> works just fine - it's just that it causes problems with the way you have made
> lvm/dm work.

This is not true.

David, with all respect to you - please try to stop writing such comments and try to understand that this is not attack to you but just description of real problems we are solving for months.

At least me and Peter are directly discussing problems with Kay and I think we are in agreement where the problems are and what the remaining problems are.

The watch for all devices simple doesn't not work. We saw this several times and spent hours trying to fix or workaround problems related to this. Not all problems are fixable though.

If you want example, it is cryptsetup and temporary-cryptsetup device. NO APPLICATION except cryptsetup must access this device, it is security problem.
I spent a lot of time to fix or workaround this upstream.

Another problem was with temporary pvmove device - bug #520913.

Also bug #591606 and bug #628762.

Just examples.

I can describe what are the proposed solutions and that they are not imagination of lvm developers but are continuously discussed and agreed with udev upstream.
The udev attributes for DM devices are updated and stored in udev database already acording to events and some tools already rely on that (including your udisks). Peter probably can describe it better but it is not the point.

But seems you are trying just to creating another flame.
I would like to kindly ask you to avoid this, thank you!

Some people were working VERY HARD to fix udev integration, mainly Peter did huge work here. Only one problem remains, the watch rule. And there is reason for that.

(In reply to comment #48)
> (In reply to comment #41)
> > What we need is _not_ a toolbox so we can write our own 200-character dm-setup
> > commands to print things out in a format so it can (maybe) be imported into the
> > udev database.
> 
> The request I saw was for an export facility.  It was not for a udev state
> maintained by LVM.  We can certainly look into that again, but I recall when we
> originally discussed storing lvm information directly in udev we concluded the
> database structure was too limited for it to work properly and we needed
> something richer.  We can look at what you're actually storing now and see if
> it's possible for the lvm package to maintain that directly, but I think it's
> more likely we will need a richer database structure.

That would be great. Current udisks-lvm-pv-export output is pretty much what I need since it allows a user app to convey the entirety of VGs, LVs and PVs without any kernel state (e.g. mapped devices) around. This is needed to have UI that allows you to start a LV/VG.

I get around not having a "rich database" structure by simply duplicating the information on each PV. Which takes up a bit more space but only on the order of a couple of KB per PV. It works just fine.

The _main_ requirement is that I can access this information at any time without doing any IO at all. Specifically, in udisks/palimpsest, I synthesize VG and LV objects from this information. The current source

 http://cgit.freedesktop.org/udisks/tree/src/probers/udisks-lvm-pv-export.c

should explain the format. Let me know what you think about it. The easiest way to try it out is to try a recent Fedora and just do

 udevadm info -q all -n /dev/sdz

where /dev/sdz is a LVM2 PV. Oh, and also try and run Palimpest (Applications->System Tools->Disk Utility) and see it update in real time as you change your PVs, LVs and VGs.

(FWIW, I do exactly the same for MD-RAID - in this case, the MD_* properties on each RAID component convey information about the array the component is part of. This means we can show this even when there is no kernel state. Again, this is needed to have UI to allow starting arrays.)

(In reply to comment #50)
> But seems you are trying just to creating another flame.
> I would like to kindly ask you to avoid this, thank you!
> 
> Some people were working VERY HARD to fix udev integration, mainly Peter did
> huge work here. Only one problem remains, the watch rule. And there is reason
> for that.

FWIW, I don't think that's fair... I'm genuinely not trying to flame anyone. And I know a lot of people have spent a lot of time getting things to work. And I appreciate it. But I do get very frustrated when I feel like I have to keep explaining why we need some features and I keep having to rebut the same things over and over. And, sorry, sometimes it's hard to do that without sounding like a jerk. I don't know what else to say.

(In reply to comment #49)
> (Or rather, we'll struggle to maintain database integrity if we use the udev
> database - queries may see inconsistencies because information has to be
> duplicated.)

(Not even worth trying to do that, I think. In particular, punt it to apps to do that however they want - and sometimes such a "raw view" is even needed so it is possible to convey in the UI exactly what PV is out of date (by e.g. inspecting UDISKS_LVM2_PV_VG_SEQNUM). I guess the difference is this: I want raw access to the metadata so I can piece together whatever complex picture I desire - you want to give me an abstract view of it.)

(In reply to comment #52)
> and I keep having to rebut the same things over and over.

See, we the "+watch" is a problem. We just today discussed that there should be some way how an application can say "I own the device now, please do not touch it, I'll let you know when I am done" (was on #systemd  - yes, it should be probably on #udev but that's not such problem though:)
If this is implemented it will help things a lot IMHO...

For attributes - I think DM attributes are already reliable stored in udev database, for LVM ones we can check how to handle it.

Udisks probe is one way (btw your current algorithm in udisks-lvm-pv-export is not optimal and will cause scalable problems for big VGs - but that's for another bug report).
Alasdair already mentioned that this is solvable (actualy I think udisks probe is ok here - just it should be written in some optimal way to be scalable).
If other susbsystem need this, maybe it shold be moved down the stack (core udev rules?).

I have several reports where "please try to remove udisks - yes, it helps" was the solution - so we need to find way how to handle it without such workarounds.
(Desktop is usually ok but we see problems on server with many LVs.)

udev and DM are two worlds - cooperation is full of compromises (we cannot rewrite one or other subsyem now - both are widely used).
(I would say if DM is designed today it takes udev into account from the beginning and many problems simple do not exists - but DM is there since 2.4 kernel days! Probably Alasdair disagrees here but that's just my opinion;-)

It's bad that problem was not solved earlier but that's history. It is changing now - that's the message I am trying to say here.

I think I could add some comments myself - as I try to optimize lvm processing time and the largest time consumption for some operations is presented by latencies we have from udev processing. Some issue could be solved internally in our lvm code (and there is quite a few problems) - but some of them needs wider thinking and agreement. 

Anyway here are my comments:

Keeping udev database always actual with all the metadata information is quite CPU intensive task.

I can perfectly understand David's desktop point of view - where you have just few devices and you want to keep UI always up-to-date - but we are currently resolving issues for thousands devs in the system.

My question is - if I run palimsest like once a year to change my partition table - why should I pay this high price to update all the udev metadata with every device open/close operation.

I'm not sure how many tools really need all the data actual all the time?
(obviously udev's internal symlinking stuff is the killer here....)

Isn't cheaper to have 'lazy evaluation' mechanism - and update data before they are needed and keep the flag marking when data could be still trusted?

I think with some deeper thinking we could probably resolve few things with some systemd rule but this brings another point - see bug 655857 - take sample code from attachment and watch what would happen with your system when you add just 500 devs. Not only it will kill systemd at this moment (rawhide, and it is fixable by increasing some hardcoded limits there) - but many  udisk & gdu & gfs daemons will occupy CPU for quite some time - and that's just 500 devices.

Ok - we are thinking about improving our processing time inside lvm - I think we will need to accelerate project which has been partially described in mornfall's lvmetad project - though it should be scaled a bit further.
For scaling to 1000's devs in the system it's impossible to do such massive scanning as we do for now in lvm - so we are pretty aware we need to resolve few things on our side - but essentially random device opens without any basic knowledge when we can do some 'real' work with these device does not really help to move forward  (and that's exactly what asynchronous watch rule does)

I'd have maybe crazy idea - we could resolve this problem with some new dm target/flag - if it's not solvable on udev level -(side note - current philosophy of lvm tools is - to allow root user to do anything - as the root is not supposed to crimp himself - but watch is killing this logic)- so we could try to think adding another level of security here.

So whenever we need to ensure no one is supposed to open dm device - add either special flag or special target replaced with table switch - which would prohibit any user-space open trials. 

And now - we can try to think about this solution for dm - and there is probably a large chance to have usable solution with this strategy for resolving udev problems - but than we resolve the problem only for dm subsystem - and every other 'bigger' subsystem would need to go through the same circle.

Is that right ? - to resolve clearly userspace device access problem with kernel sledgehammer technology (as that's exactly what would be the open denial)

(In reply to comment #55)
> So whenever we need to ensure no one is supposed to open dm device - add either
> special flag or special target replaced with table switch - which would
> prohibit any user-space open trials. 

Not going to happen on my watch - that's EVMS vs device-mapper again.

(In reply to comment #56)
> (In reply to comment #55)
> > So whenever we need to ensure no one is supposed to open dm device - add either
> > special flag or special target replaced with table switch - which would
> > prohibit any user-space open trials. 
> 
> Not going to happen on my watch - that's EVMS vs device-mapper again.

Yes - known position - I rather wanted to demonstrate what is the problem - because I think David is not aware of all aspects we would need to resolve.

I think it's worth to mention that being able to keep things running in some deterministic way is very important - and that's what need to be better designed and supported by udev -  I do believe it's not so hard to came up with some solution which would make udev more predictable element in the system.

i.e. Comment 9  is valid - but the process of discovering new information about this needs to be very well designed and needs to be monitorable - not a wild random execution of unpredictable set of rules.

I think this request is obsolete now (bug #738479). If not, please reopen.