Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
On Wed, 28.01.15 14:09, Martin Pitt (martin.p...@ubuntu.com) wrote:
From 0cc891bcd8d3fa9967dd733292caf86a43dd3503 Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Wed, 28 Jan 2015 13:57:47 +0100
Subject: [PATCH 2/2] rules: clean up stale CD drive mounts after ejection
Ejecting a CD with the hardware drive button only causes a change uevent, but
the device node stays around (just without a medium). Pick up these uevents
and
mark the device as SYSTEMD_READY=0 on ejection, so that systemd stops the
device unit and consequently all mount units on it.
On media insertion, mark the device as SYSTEMD_READY=1 again.
https://bugs.freedesktop.org/show_bug.cgi?id=72206
https://bugzilla.opensuse.org/show_bug.cgi?id=909418
https://bugs.archlinux.org/task/42071
https://bugs.launchpad.net/bugs/1168742
---
rules/60-cdrom_id.rules | 6 ++
1 file changed, 6 insertions(+)
diff --git a/rules/60-cdrom_id.rules b/rules/60-cdrom_id.rules
index 6eaf76a..7bfb12e 100644
--- a/rules/60-cdrom_id.rules
+++ b/rules/60-cdrom_id.rules
@@ -15,6 +15,12 @@ ENV{DISK_EJECT_REQUEST}==?*, RUN+=cdrom_id
--eject-media $devnode, GOTO=cdr
# enable the receiving of media eject button events
IMPORT{program}=cdrom_id --lock-media $devnode
+# ejecting a CD does not remove the device node, so mark the systemd device
+# unit as inactive while there is no medium; this automatically cleans up of
+# stale mounts after ejecting
+ENV{DISK_MEDIA_CHANGE}==?*, ENV{ID_CDROM_MEDIA}!=?*,
ENV{SYSTEMD_READY}=0
+ENV{DISK_MEDIA_CHANGE}==?*, ENV{ID_CDROM_MEDIA}==?*,
ENV{SYSTEMD_READY}=1
Hmm, we probably should do the same for USB memory card drives. No
idea how media sensing works there. Any chance you can look into that
too?
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
Lennart Poettering [2015-01-28 13:33 +0100]:
Hmm, yeah, we apparently only add that for file systems listed in
/etc/fstab...
If you change the get_mount_parameters_fragment() invocation at the
beginning of mount_add_device_links() in src/core/mount.c to
get_mount_parameters(), does this make things work for you?
This change might have more effects than just making this work, but I
think it's the right thing to do. Could you test please?
BAZINGA! Thanks for this, now it works perfectly! Now the mount unit
looks like this:
| Where=/media/martin/Ubuntu 15.04 amd64
| What=/dev/sr0
| [...]
| Id=media-martin-Ubuntu\x5cx2015.04\x5cx20amd64.mount
| Names=media-martin-Ubuntu\x5cx2015.04\x5cx20amd64.mount
| Requires=-.mount
| Wants=system.slice
| BindsTo=dev-sr0.device
| WantedBy=dev-sr0.device
| Conflicts=umount.target
| Before=local-fs.target umount.target
| After=system.slice -.mount local-fs-pre.target systemd-journald.socket
dev-sr0.device
| [...]
and requesting the CD eject causes cdrom_id --eject, which triggers
the new DISK_MEDIA_CHANGE rule, which stops the .device unit, which in
turn stops the .mount unit.
Patches attached for both parts of the issue. The first one is really
your patch, so please do commit yourself if you prefer (feel free to
steal any or all of my patch description, of course).
I'll also adjust the fd.o bug report accordingly.
Thanks,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 0e7493429c017d8a74a299a3e60278c4f68af430 Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Wed, 28 Jan 2015 13:53:25 +0100
Subject: [PATCH 1/2] core/mount: add dependencies to dynamically mounted
mounts too
Add unit dependencies for dynamic (i. e. not from fstab) mounts. With that,
mount units properly bind to their underlying device, and thus get
automatically stopped/unmounted when the underlying device goes away.
This cleans up stale mounts from unplugged devices.
Thanks to Lennart Poettering for pointing out the fix!
---
src/core/mount.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/core/mount.c b/src/core/mount.c
index f944c02..b13712f 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -300,7 +300,7 @@ static int mount_add_device_links(Mount *m) {
assert(m);
-p = get_mount_parameters_fragment(m);
+p = get_mount_parameters(m);
if (!p)
return 0;
--
2.1.4
From 0cc891bcd8d3fa9967dd733292caf86a43dd3503 Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Wed, 28 Jan 2015 13:57:47 +0100
Subject: [PATCH 2/2] rules: clean up stale CD drive mounts after ejection
Ejecting a CD with the hardware drive button only causes a change uevent, but
the device node stays around (just without a medium). Pick up these uevents and
mark the device as SYSTEMD_READY=0 on ejection, so that systemd stops the
device unit and consequently all mount units on it.
On media insertion, mark the device as SYSTEMD_READY=1 again.
https://bugs.freedesktop.org/show_bug.cgi?id=72206
https://bugzilla.opensuse.org/show_bug.cgi?id=909418
https://bugs.archlinux.org/task/42071
https://bugs.launchpad.net/bugs/1168742
---
rules/60-cdrom_id.rules | 6 ++
1 file changed, 6 insertions(+)
diff --git a/rules/60-cdrom_id.rules b/rules/60-cdrom_id.rules
index 6eaf76a..7bfb12e 100644
--- a/rules/60-cdrom_id.rules
+++ b/rules/60-cdrom_id.rules
@@ -15,6 +15,12 @@ ENV{DISK_EJECT_REQUEST}==?*, RUN+=cdrom_id --eject-media $devnode, GOTO=cdr
# enable the receiving of media eject button events
IMPORT{program}=cdrom_id --lock-media $devnode
+# ejecting a CD does not remove the device node, so mark the systemd device
+# unit as inactive while there is no medium; this automatically cleans up of
+# stale mounts after ejecting
+ENV{DISK_MEDIA_CHANGE}==?*, ENV{ID_CDROM_MEDIA}!=?*, ENV{SYSTEMD_READY}=0
+ENV{DISK_MEDIA_CHANGE}==?*, ENV{ID_CDROM_MEDIA}==?*, ENV{SYSTEMD_READY}=1
+
KERNEL==sr0, SYMLINK+=cdrom, OPTIONS+=link_priority=-100
LABEL=cdrom_end
--
2.1.4
signature.asc
Description: Digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
Lennart Poettering [2015-01-27 23:33 +0100]:
That sounds good indeed! I can sit down at qemu tomorrow and simulate
some CD insertions/removals, and come up with an udev rule for this.
That would be great. It would really just be a matter of setting
SYSTEMD_READY=0 for block devices where media sense suggests that no
media is in the drive. Probably a one line fix...
Turns out that there's no proper way to simulate that under QEMU --
that has no eject button, and the eject monitor command blocks
as long as the OS is still accessing it (i. e. it's mounted).
I borrowed the laptop from a friend which has a CD drive (I can do
this until Saturday while I'm in Belgium, FTR), and confirm the stale
mounts. I tried adding these rules to 60-cdrom_id.rules after the
IMPORT line:
# remove a medium/eject CD: disable corresponding .mount units
ENV{DISK_MEDIA_CHANGE}==?*, ENV{ID_CDROM_MEDIA}!=?*,
ENV{SYSTEMD_READY}=0
# insert a medium; undo the previous rule
ENV{DISK_MEDIA_CHANGE}==?*, ENV{ID_CDROM_MEDIA}==?*,
ENV{SYSTEMD_READY}=1
These essentially make SYSTEMD_READY the negation of ID_CDROM_MEDIA,
but as the latter isn't actually 0 but disappears, and we can't
negate in udev without resorting to (much slower) shell hacks, we
need two rules.
In udevadm I see that this has the intended effect -- as soon as I
eject the CD, /dev/sr0 gets ENV{SYSTEMD_READY}=0. But there's still
something missing, as merely adding this property doesn't yet tell
systemd to stop the unit -- media-ubuntu-5ML.mount is still active
after that. systemd.device(5) also only documents the other transition
from 0 to 1, but not from 1 to 0. So do we need to teach systemd to listen
to 1 → 0 transitions too for stopping?
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
Martin Pitt [2015-01-28 10:35 +0100]: Turns out that there's no proper way to simulate that under QEMU -- that has no eject button, and the eject monitor command blocks as long as the OS is still accessing it (i. e. it's mounted). Lies! In fact eject ide1-cd0 works rather well, it causes the DISK_EJECT_REQUEST=1 uevent, and then the cdrom_id --eject magic takes place. So in case anyone wonders how to reproduce this: $ qemu-system-x86_64 -enable-kvm -m 2048 -snapshot -drive file=my_os.img,if=virtio -drive if=ide,index=1,media=cdrom -monitor stdio QEMU 2.1.2 monitor - type 'help' for more information # take any ISO you like (qemu) change ide1-cd0 /home/martin/Downloads/ubuntu/vivid-desktop-amd64.iso (qemu) eject ide1-cd0 Device 'ide1-cd0' is locked Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
On Wed, 28.01.15 12:06, Martin Pitt (martin.p...@ubuntu.com) wrote: I. e. the .mount does not seem to be bound on the .device, and isn't taken down automatically (stopping the mount manually works fine). In show I don't see a BindsTo, and the Requires also doesn't mention the /dev/sr1 device: Hmm, yeah, we apparently only add that for file systems listed in /etc/fstab... If you change the get_mount_parameters_fragment() invocation at the beginning of mount_add_device_links() in src/core/mount.c to get_mount_parameters(), does this make things work for you? This change might have more effects than just making this work, but I think it's the right thing to do. Could you test please? Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
Zbigniew Jędrzejewski-Szmek [2015-01-28 16:29 +0100]: | Where=/media/martin/Ubuntu 15.04 amd64 | What=/dev/sr0 | [...] | Id=media-martin-Ubuntu\x5cx2015.04\x5cx20amd64.mount | Names=media-martin-Ubuntu\x5cx2015.04\x5cx20amd64.mount | Requires=-.mount | Wants=system.slice | BindsTo=dev-sr0.device | WantedBy=dev-sr0.device | Conflicts=umount.target | Before=local-fs.target umount.target This looks wrong. It should have no relation to local-fs.target, which is only for things from /etc/fstab (and manually created .mount units). Ah, indeed it is. Just note that this isn't due to this patch, but already the status in current master. The only difference with this patch is the addition of BindsTo/Wants=dev-sr0.device. Thanks for pointing this out! Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
On Wed, Jan 28, 2015 at 02:09:37PM +0100, Martin Pitt wrote:
Lennart Poettering [2015-01-28 13:33 +0100]:
Hmm, yeah, we apparently only add that for file systems listed in
/etc/fstab...
If you change the get_mount_parameters_fragment() invocation at the
beginning of mount_add_device_links() in src/core/mount.c to
get_mount_parameters(), does this make things work for you?
This change might have more effects than just making this work, but I
think it's the right thing to do. Could you test please?
BAZINGA! Thanks for this, now it works perfectly! Now the mount unit
looks like this:
| Where=/media/martin/Ubuntu 15.04 amd64
| What=/dev/sr0
| [...]
| Id=media-martin-Ubuntu\x5cx2015.04\x5cx20amd64.mount
| Names=media-martin-Ubuntu\x5cx2015.04\x5cx20amd64.mount
| Requires=-.mount
| Wants=system.slice
| BindsTo=dev-sr0.device
| WantedBy=dev-sr0.device
| Conflicts=umount.target
| Before=local-fs.target umount.target
This looks wrong. It should have no relation to local-fs.target, which
is only for things from /etc/fstab (and manually created .mount units).
Zbyszek
| After=system.slice -.mount local-fs-pre.target systemd-journald.socket
dev-sr0.device
| [...]
and requesting the CD eject causes cdrom_id --eject, which triggers
the new DISK_MEDIA_CHANGE rule, which stops the .device unit, which in
turn stops the .mount unit.
Patches attached for both parts of the issue. The first one is really
your patch, so please do commit yourself if you prefer (feel free to
steal any or all of my patch description, of course).
I'll also adjust the fd.o bug report accordingly.
Thanks,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 0e7493429c017d8a74a299a3e60278c4f68af430 Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Wed, 28 Jan 2015 13:53:25 +0100
Subject: [PATCH 1/2] core/mount: add dependencies to dynamically mounted
mounts too
Add unit dependencies for dynamic (i. e. not from fstab) mounts. With that,
mount units properly bind to their underlying device, and thus get
automatically stopped/unmounted when the underlying device goes away.
This cleans up stale mounts from unplugged devices.
Thanks to Lennart Poettering for pointing out the fix!
---
src/core/mount.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/core/mount.c b/src/core/mount.c
index f944c02..b13712f 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -300,7 +300,7 @@ static int mount_add_device_links(Mount *m) {
assert(m);
-p = get_mount_parameters_fragment(m);
+p = get_mount_parameters(m);
if (!p)
return 0;
--
2.1.4
From 0cc891bcd8d3fa9967dd733292caf86a43dd3503 Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Wed, 28 Jan 2015 13:57:47 +0100
Subject: [PATCH 2/2] rules: clean up stale CD drive mounts after ejection
Ejecting a CD with the hardware drive button only causes a change uevent, but
the device node stays around (just without a medium). Pick up these uevents
and
mark the device as SYSTEMD_READY=0 on ejection, so that systemd stops the
device unit and consequently all mount units on it.
On media insertion, mark the device as SYSTEMD_READY=1 again.
https://bugs.freedesktop.org/show_bug.cgi?id=72206
https://bugzilla.opensuse.org/show_bug.cgi?id=909418
https://bugs.archlinux.org/task/42071
https://bugs.launchpad.net/bugs/1168742
---
rules/60-cdrom_id.rules | 6 ++
1 file changed, 6 insertions(+)
diff --git a/rules/60-cdrom_id.rules b/rules/60-cdrom_id.rules
index 6eaf76a..7bfb12e 100644
--- a/rules/60-cdrom_id.rules
+++ b/rules/60-cdrom_id.rules
@@ -15,6 +15,12 @@ ENV{DISK_EJECT_REQUEST}==?*, RUN+=cdrom_id
--eject-media $devnode, GOTO=cdr
# enable the receiving of media eject button events
IMPORT{program}=cdrom_id --lock-media $devnode
+# ejecting a CD does not remove the device node, so mark the systemd device
+# unit as inactive while there is no medium; this automatically cleans up of
+# stale mounts after ejecting
+ENV{DISK_MEDIA_CHANGE}==?*, ENV{ID_CDROM_MEDIA}!=?*,
ENV{SYSTEMD_READY}=0
+ENV{DISK_MEDIA_CHANGE}==?*, ENV{ID_CDROM_MEDIA}==?*,
ENV{SYSTEMD_READY}=1
+
KERNEL==sr0, SYMLINK+=cdrom, OPTIONS+=link_priority=-100
LABEL=cdrom_end
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
On Wed, 28.01.15 16:29, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote: On Wed, Jan 28, 2015 at 02:09:37PM +0100, Martin Pitt wrote: Lennart Poettering [2015-01-28 13:33 +0100]: Hmm, yeah, we apparently only add that for file systems listed in /etc/fstab... If you change the get_mount_parameters_fragment() invocation at the beginning of mount_add_device_links() in src/core/mount.c to get_mount_parameters(), does this make things work for you? This change might have more effects than just making this work, but I think it's the right thing to do. Could you test please? BAZINGA! Thanks for this, now it works perfectly! Now the mount unit looks like this: | Where=/media/martin/Ubuntu 15.04 amd64 | What=/dev/sr0 | [...] | Id=media-martin-Ubuntu\x5cx2015.04\x5cx20amd64.mount | Names=media-martin-Ubuntu\x5cx2015.04\x5cx20amd64.mount | Requires=-.mount | Wants=system.slice | BindsTo=dev-sr0.device | WantedBy=dev-sr0.device | Conflicts=umount.target | Before=local-fs.target umount.target This looks wrong. It should have no relation to local-fs.target, which is only for things from /etc/fstab (and manually created .mount units). I think ordering before local-fs.target is the right thing in this case. Wants= or Reuqires= would be inappropriate, but After= is OK and needed AFAICS. It is useful during shutdown, so that the mount units stay around until all services with After=local-fs.target are shut down, and not any earlier than that. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
On Wed, 28.01.15 14:09, Martin Pitt (martin.p...@ubuntu.com) wrote:
Lennart Poettering [2015-01-28 13:33 +0100]:
Hmm, yeah, we apparently only add that for file systems listed in
/etc/fstab...
If you change the get_mount_parameters_fragment() invocation at the
beginning of mount_add_device_links() in src/core/mount.c to
get_mount_parameters(), does this make things work for you?
This change might have more effects than just making this work, but I
think it's the right thing to do. Could you test please?
BAZINGA! Thanks for this, now it works perfectly! Now the mount unit
looks like this:
Applied both patches, but dropped this line:
+ENV{DISK_MEDIA_CHANGE}==?*, ENV{ID_CDROM_MEDIA}==?*,
ENV{SYSTEMD_READY}=1
SYSTEMD_READY=1 is the implied default, hence there's no reason to set
it.
Note that this patch only covers CD driver media sensing. We really
should hanlde SD/CF card readers the same way. Unfortunately however
there's currently no nice property available that we could check for
media with. This would have have to be added to the blkid builtin or
so.
I have now added this to the TODO list. Would be happy to merge a
clean patch!
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
On Wed, Jan 28, 2015 at 05:24:19PM +0100, Lennart Poettering wrote: On Wed, 28.01.15 16:29, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote: On Wed, Jan 28, 2015 at 02:09:37PM +0100, Martin Pitt wrote: Lennart Poettering [2015-01-28 13:33 +0100]: Hmm, yeah, we apparently only add that for file systems listed in /etc/fstab... If you change the get_mount_parameters_fragment() invocation at the beginning of mount_add_device_links() in src/core/mount.c to get_mount_parameters(), does this make things work for you? This change might have more effects than just making this work, but I think it's the right thing to do. Could you test please? BAZINGA! Thanks for this, now it works perfectly! Now the mount unit looks like this: | Where=/media/martin/Ubuntu 15.04 amd64 | What=/dev/sr0 | [...] | Id=media-martin-Ubuntu\x5cx2015.04\x5cx20amd64.mount | Names=media-martin-Ubuntu\x5cx2015.04\x5cx20amd64.mount | Requires=-.mount | Wants=system.slice | BindsTo=dev-sr0.device | WantedBy=dev-sr0.device | Conflicts=umount.target | Before=local-fs.target umount.target This looks wrong. It should have no relation to local-fs.target, which is only for things from /etc/fstab (and manually created .mount units). I think ordering before local-fs.target is the right thing in this case. Wants= or Reuqires= would be inappropriate, but After= is OK and needed AFAICS. It is useful during shutdown, so that the mount units stay around until all services with After=local-fs.target are shut down, and not any earlier than that. OK. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
Hey Lennart, Lennart Poettering [2015-01-27 0:55 +0100]: Hmm? I don't see how mount propagation would break 60-cdrom_id... The eject ioctl operates on the device node, and does not care for mounts. This problem sounds made-up to me. Right now cdrom_id indeed wouldn't be affected as it doesn't unmount a CD which is about to ejected. That's the very problem that was recently discussed here: http://lists.freedesktop.org/archives/systemd-devel/2015-January/026948.html The two proposed solutions were to either teach cdrom_id --eject to umount the device or just call the actual eject program which gets this pretty much right. But neither would work because of the unshared mount ns in udev. Moreover, if you want to do mounts or umounts on plug or play, then use a proper daemon, like udisks. udisks actually used to have both the CD-ROM polling (which since then moved into the kernel) and the post-eject cleanup. If we want to keep the udev mount unsharing, we could put it back into udisks; but that wouldn't be that robust as udisks isn't guaranteed to actually run, both because it's a D-BUS activated service and a lot of server-ish machines or lightweight desktops don't even have it. Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
On Tue, 27.01.15 17:22, Lennart Poettering (lenn...@poettering.net) wrote: On Tue, 27.01.15 16:24, Martin Pitt (martin.p...@ubuntu.com) wrote: Well, again, the right answer then is to handle it with .mount units, How would that look like, on a very high level? Create .mount units on the fly with udev rules when devices appear, and asking systemd to unmount them via a remove uevent, instead of having cdrom_id do the umount directly? The .mount units of device nodes already have a BindsTo= dependency on their respective backing .device units. This should have the effect that systemd will take the .mount units down if the .device units are removed. Are you saying that doesn't work? So I figure the bit that is missing here is the fact that the .device units for CD drives and USB card readers don't care for media sense right now. The .device units for CD drives and USB card readers are available as long as the CD drive or USB card reader is plugged in, it doesn't care for any media being in it. That means that automatic unmounting by systemd due to the device going away will only happen if you actually unplug the CD driver or the USB card, but not already when the media is ejected. However, I think it would make a ton of sense to change that, and set SYSTEMD_READY=0 on all block devices where the media sensing suggests that no medium is in it. This would mean that these devices don't show up as systemd units until you actually put a medium in it. That would be a change of semantics, but I think a useful one. What do you think? Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
On Tue, 27.01.15 23:20, Martin Pitt (martin.p...@ubuntu.com) wrote: Hey Lennart, Lennart Poettering [2015-01-27 22:46 +0100]: So I figure the bit that is missing here is the fact that the .device units for CD drives and USB card readers don't care for media sense right now. Indeed, I had a similar thought on my evening walk: This works well for USB sticks and the like, but the /dev/sr0 node for CDs always stays around after eject. IOW, you get a change event, not a remove one. This is also why the cleanup in udisks 1.x (probably) didn't actually work. However, I think it would make a ton of sense to change that, and set SYSTEMD_READY=0 on all block devices where the media sensing suggests that no medium is in it. This would mean that these devices don't show up as systemd units until you actually put a medium in it. That would be a change of semantics, but I think a useful one. That sounds good indeed! I can sit down at qemu tomorrow and simulate some CD insertions/removals, and come up with an udev rule for this. That would be great. It would really just be a matter of setting SYSTEMD_READY=0 for block devices where media sense suggests that no media is in the drive. Probably a one line fix... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
On Tue, 27.01.15 23:33, Martin Pitt (martin.p...@ubuntu.com) wrote:
Lennart Poettering [2015-01-27 22:46 +0100]:
However, I think it would make a ton of sense to change that, and set
SYSTEMD_READY=0 on all block devices where the media sensing suggests
that no medium is in it.
Thinking about it again, we already know that this rule in
60-cdrom_id.rules still works:
ENV{DISK_EJECT_REQUEST}==?*, RUN+=cdrom_id --eject-media $devnode,
GOTO=cdrom_end
Could we not simply add the ENV{SYSTEMD_READY}=0 there, and then
reset it (to 1) in the following rule?
Well, in that case it would only be set when somebody actually
presses the eject button. I am pretty sure though we should also set
the flag when a cdrom drive appears first and has no medium in it...
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
Lennart Poettering [2015-01-27 22:46 +0100]:
However, I think it would make a ton of sense to change that, and set
SYSTEMD_READY=0 on all block devices where the media sensing suggests
that no medium is in it.
Thinking about it again, we already know that this rule in
60-cdrom_id.rules still works:
ENV{DISK_EJECT_REQUEST}==?*, RUN+=cdrom_id --eject-media $devnode,
GOTO=cdrom_end
Could we not simply add the ENV{SYSTEMD_READY}=0 there, and then
reset it (to 1) in the following rule?
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
Hey Lennart, Lennart Poettering [2015-01-27 22:46 +0100]: So I figure the bit that is missing here is the fact that the .device units for CD drives and USB card readers don't care for media sense right now. Indeed, I had a similar thought on my evening walk: This works well for USB sticks and the like, but the /dev/sr0 node for CDs always stays around after eject. IOW, you get a change event, not a remove one. This is also why the cleanup in udisks 1.x (probably) didn't actually work. However, I think it would make a ton of sense to change that, and set SYSTEMD_READY=0 on all block devices where the media sensing suggests that no medium is in it. This would mean that these devices don't show up as systemd units until you actually put a medium in it. That would be a change of semantics, but I think a useful one. That sounds good indeed! I can sit down at qemu tomorrow and simulate some CD insertions/removals, and come up with an udev rule for this. Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
On Tue, Jan 27, 2015 at 05:33:06PM +0100, Martin Pitt wrote: Lennart Poettering [2015-01-27 17:22 +0100]: The .mount units of device nodes already have a BindsTo= dependency on their respective backing .device units. This should have the effect that systemd will take the .mount units down if the .device units are removed. Are you saying that doesn't work? It's been ages since I've had a CD drive with an eject button, so I can't say myself. But given the recent reports on the list from Robert Milasan and Dave Reiser it seems that *something* here is still broken? Well, my test case was flawed and the mount namespacing in udevd isn't actually relevant to the problem. I've since found myself uninterested in solving this particular problem. dR ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
On Tue, 27.01.15 09:40, Martin Pitt (martin.p...@ubuntu.com) wrote: Hey Lennart, Lennart Poettering [2015-01-27 0:55 +0100]: Hmm? I don't see how mount propagation would break 60-cdrom_id... The eject ioctl operates on the device node, and does not care for mounts. This problem sounds made-up to me. Right now cdrom_id indeed wouldn't be affected as it doesn't unmount a CD which is about to ejected. That's the very problem that was recently discussed here: http://lists.freedesktop.org/archives/systemd-devel/2015-January/026948.html The two proposed solutions were to either teach cdrom_id --eject to umount the device or just call the actual eject program which gets this pretty much right. But neither would work because of the unshared mount ns in udev. So, why is this a new problem, and why do you say that MountFlags=slave broke anything? I mean, cdrom_id cannot do unmounts (and it really shouldn't), And an eject invocation was never part of the udev rules, so there was really nothing that broke. So, these things never worked, and MountFlags=slave didn't change anything about it. There's some part of the story that I am missing, or something makes no sense at all... Moreover, if you want to do mounts or umounts on plug or play, then use a proper daemon, like udisks. udisks actually used to have both the CD-ROM polling (which since then moved into the kernel) and the post-eject cleanup. Why was the removed, and with what was it replaced when it was? If we want to keep the udev mount unsharing, we could put it back into udisks; but that wouldn't be that robust as udisks isn't guaranteed to actually run, both because it's a D-BUS activated service and a lot of server-ish machines or lightweight desktops don't even have it. Well, again, the right answer then is to handle it with .mount units, which can correctly deal with partitioned block devices and other file systems mounted on top of these pluggable block devices. In general though I don't think people should expect that pluggable devices are handled nicely if you uninstall the daemon that deals with pluggable devices... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
Lennart Poettering [2015-01-27 13:52 +0100]: So, why is this a new problem, and why do you say that MountFlags=slave broke anything? I didn't say that. :-) I just said that due to this the two proposed solutions of cleaning up the mounts after CD ejection won't work. I mean, cdrom_id cannot do unmounts (and it really shouldn't), And an eject invocation was never part of the udev rules, so there was really nothing that broke. So, these things never worked, and MountFlags=slave didn't change anything about it. Correct. udisks actually used to have both the CD-ROM polling (which since then moved into the kernel) and the post-eject cleanup. Why was the removed, and with what was it replaced when it was? So udisks 1.x had a force_removal() function which was called on a remove uevent and cleaned up stale mounts. Apparenlty that never made it into the udisks 2.x rewrite unless I'm missing something. But as it obviously doesn't seem to work right now (i. e. CD mounts are kept stale), I guess it's due to that. Well, again, the right answer then is to handle it with .mount units, How would that look like, on a very high level? Create .mount units on the fly with udev rules when devices appear, and asking systemd to unmount them via a remove uevent, instead of having cdrom_id do the umount directly? Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
On Tue, 27.01.15 16:24, Martin Pitt (martin.p...@ubuntu.com) wrote: Well, again, the right answer then is to handle it with .mount units, How would that look like, on a very high level? Create .mount units on the fly with udev rules when devices appear, and asking systemd to unmount them via a remove uevent, instead of having cdrom_id do the umount directly? The .mount units of device nodes already have a BindsTo= dependency on their respective backing .device units. This should have the effect that systemd will take the .mount units down if the .device units are removed. Are you saying that doesn't work? Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
Lennart Poettering [2015-01-27 17:22 +0100]: The .mount units of device nodes already have a BindsTo= dependency on their respective backing .device units. This should have the effect that systemd will take the .mount units down if the .device units are removed. Are you saying that doesn't work? It's been ages since I've had a CD drive with an eject button, so I can't say myself. But given the recent reports on the list from Robert Milasan and Dave Reiser it seems that *something* here is still broken? Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
On Mon, 26.01.15 08:59, Dave Reisner (dreis...@archlinux.org) wrote: This reverts part of c2c13f2df42e0, which introduced this with no explanation as to *why*. Enslaving the mount namespace breaks default behavior included in rules/60-cdrom_id.rules. Specifically, filesystems on optical media will not be properly unmounted when the physical eject button is used in the absence of a helper tool like udisks2. Hmm? I don't see how mount propagation would break 60-cdrom_id... The eject ioctl operates on the device node, and does not care for mounts. This problem sounds made-up to me. Moreover, if you want to do mounts or umounts on plug or play, then use a proper daemon, like udisks. And if you don#t want to do that, then make systemd handle this: by setting up .mount and .device units as needed. This has the great advantage that it can deal with full mount heirarchies, and properly does this things in an asynchronous way. udev rules are simply the wrong place to do mounts, they don't belong there. We sandbox all daemons we ship, as far as we can. Unfortunately since udev is pluggable and rules may invoke external tools we cannot sandbox too much. But we can certainly sandbox the mount propagation, since we know that it's not the right place to do mounts. And so we do. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-udevd.service: restore mount propagation
On Mon, 26.01.15 15:44, Michael Biebl (mbi...@gmail.com) wrote: 2015-01-26 14:59 GMT+01:00 Dave Reisner dreis...@archlinux.org: This reverts part of c2c13f2df42e0, which introduced this with no explanation as to *why*. Enslaving the mount namespace breaks default behavior included in rules/60-cdrom_id.rules. Specifically, filesystems on optical media will not be properly unmounted when the physical eject button is used in the absence of a helper tool like udisks2. This was discussed here: http://lists.freedesktop.org/archives/systemd-devel/2015-January/026948.html And has been reported to several bug trackers: https://bugs.archlinux.org/task/42071 http://bugzilla.opensuse.org/show_bug.cgi?id=909418 https://bugs.freedesktop.org/bugzilla/show_bug.cgi?id=72206 We had to drop this from the Debian package as well: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=762018 This appears to be bug in laptop-mode-utils, not in udev. I think it would be a good idea to fix bugs where they are, instead of taping over them in other packages. Also, I the bug report is pretty incomplete, it does not explain in any way why disabled mount propagation should result in a read-only root? This is all fishy. Please try to figure out what *really* is going on before fixing things at the wrong places. Thanks, Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
