I am not sure I like this much I must say. Solving this only for cdroms sounds so backwards, as cdroms don't really exist anymore, do they? I personally do not own any such drive anymore...

I think it would be best if we'd just leave this all to udisks to figure out, instead of second guessing that.

A major problem with all of this is that a mounts can be complex, and be stacked: consider a LUKS file system on a cdrom: when the cdrom is ejected, then we should also shut that down. And people can stack things arbitrarily of course. This means that any such unmounting should really go through systemd's dep-tree which can deal with these things, and never do the unmount directly.

I'd be open to adding an optional x-systemd.unmount-on-unplug mount option or so, that can be used when establishing a mount point from the command line via the "-o" switch. If specified we'd upgrade the "requires" back to a "BindsTo" as it was before that commit. Then, if users want systemd to do the unmounting they can opt-in for it. In addition we could also define a new udev property similar to this: when set on a block device, and we are about to create a device dep from a mount unit on it, then we upgrade the requires back to a bindsto, too. Then, we could mark cdroms like this and you get the behaviour you want: automatically for all cdroms and whatever else is marked like this in the udev db, and explicitly if the user asks so on the mount cmdline.

Does that make sense?

Agreed that this sounds backwards however cdroms are also special: udev is handling eject events in order to catch change/eject events and update its db accordingly. Because of that it has to deal with the press of the eject button. And since it doesn't really know how it's currently used, it blindly ejects the medium unconditionally (as far as I understood it) leaving some inconsistent states at least...

Actually I'm wondering how is this going to work if an application is burning a media. Does the application have a chance to handle the eject event in order to notify the user a nice notification that the burning process is going to be interrupted ?

Your idea to enable "auto-unmount" feature on any device sounds reasonable since it deals with complex/stacked file systems.

Maybe a similar idea would be to introduce transient mounts (similar to transient services). We could use "systemd-mount" for example which would be exactly the same as a plain mount except that a mount unit would be created on the fly and started. And similarly create a udev property to enable transient mounts automatically for a device.

WDYT ?

well, my intention was always that udisks or whatever manages local removable media would ultimately take care of all of this, and do this by creating transient units in systemd as necessary. i.e. instead of turning systemd-mount into a fully fledged daemon for remvoable media, just beef up udisks to do some of the things systemd-mount can do.

cd burning is very different from read access. There's usually no mounting involved when burn an ISO: cdrecord talks directly to the SCSI device...

cd burning is very different from read access. There's usually no mounting involved when burn an ISO: cdrecord talks directly to the SCSI device...

But I was more wondering if ejecting the media (mounted or not, i.e. unconditionally) is a good idea specially since a burning process may be in progress...

Anyway I'll send a new PR implementing the "x-systemd.unmount-on-unplug" mount option.

I'd be open to adding an optional x-systemd.unmount-on-unplug mount option or so, that can be used when establishing a mount point from the command line via the "-o" switch.

@poettering the "x-*" options passed via the "-o" switch are interpreted as comments by mount(8) and are not passed to the kernel. So I don't see how systemd can retrieve them... am I missing something ?

OTOH systemd-mount has the --bind-device option which seems to work with both automount and mount units (haven't tested it yet)...

So it seems that only the new udev property is missing, no ?

@fbuihuu /bin/mount used libmount, and we generally do that too. It will store the userspace mount options in /run when creating mounts, and merge them in again when you enumerate the mounts using the library.

@poettering hmm are you sure about that ?

mount(8) man page claims that the "x-*" options are interpreted as comments.

And that seems to be the case here:

$ mount -o x-systemd.foo /dev/sdb1 /mnt/
$ cat /run/mount/utab
<empty>
$ rpm -q util-linux
util-linux-2.28.2-1.4.x86_64

And again since systemd-mount has the '--bind-device' option, why not simply relying on this ?

@poettering also I looked at the udev property and how it could be done and it's going to get ugly with the current implementation (unless I'm missing something)...

The main issue is that mount units from /proc/self/mountinfo can be created before the device has been detected by udev. Therefore the udev tag is not available at the time we link the mount unit to the device one.

One possible way to fix this at a first glance would be to defer the creation of mount units until the device is fully initialized (and maybe that would allow to remote that "TENTATIVE" device state that was introduced to deal with this weird temporary transition) but that would require some changes.

Or maybe you have something different in mind ?

mount(8) man page claims that the "x-*" options are interpreted as comments.

Hmm, but I am pretty sure mount should still save them to utab.

@karelzak any chance you can change this in libmount, and leave the x-* options around for retrieval in utab?

The main issue is that mount units from /proc/self/mountinfo can be created before the device has been detected by udev. Therefore the udev tag is not available at the time we link the mount unit to the device one.

well, but it's not a prob if both Requires= and BindsTo= are created for a device. And it's fine to add in deps later on. Hence, I think it's fine if we first just do the REquires= (if we see it in mountinfo) and later on add a BindsTo= on top (as soon as we have the info from udev).

@poettering I updated the branch according to your feedback. I still added the "x-systemd.device-bound" mount option even if currently it has no effect since libmount doesn't leave the x-* options in utab.

Please have a look, thanks.

@poettering could you give some feedback ? Thanks !

@fbuihuu so @karelzak was on vacation, he promised me to have another look on the x- situation...

Looks great already. @karelzak any chance you can make the "x-" options visible through libmount?

@poettering we should be good now and thanks for the feedback.

The basic utab idea is to minimize the number of the situations when we use it (because maintain anything about mount points in userspace sucks). So there is very small and explicitly defined set of the mount options that we maintain in userspace. The ideal solution is not to store anything in userspace. Really :-)

IMHO save all x-* to utab sounds like bad idea (it reminds old ugly mtab).

I see two ways:

a) introduce another namespace (e.g. X-*) that will be maintained in userspace too and carefully use it.

b) use libmount "attributes". It's string maintained by libmount and always stored in userspace. It's used for example by mount.nfs for their special userspace stuff. It's ignored my mount(2) and mount(8), and there is no command line interface for the "attributes". You have to use libmount to access the string.

I vote for a) if you really need it.

Note that for udisks we have introduced helper= and uhelper= mount options (see man umount, section EXTERNAL HELPERS). These options are stored in userspace and redirects mount(8) calls to /sbin/umount. (e.g. /sbin/umount.udisks ... probably dead thing now).

@karelzak is there any plan to store the user options in the kernel ? (since saving them in userspace sucks).

Also what is the advantage to use a new namespace for the X-* options ?

Thanks.

@karelzak well, if you store info in userspace at all, then the x- options should be placed there too. I mean, which other projects except systemd are actually making use of x- options? I mean, it's still very much the exception that a mount point carries an "x-" option at all, isn't it?

Also, if you have utab, why not actually use it?

I always assumed that the x- options would end up in utab, I am really surprised that they aren't. It's really strange that systemd currently care about x- only in fstab, but not otherwise I think...

I really don#t like the X- idea I must say. This would mean we'd stop using "x-" entirely in systemd, and just use X- everywhere, and I'd really like to avoid such a deprecation...

I'd really like to ask you to reconsider this, and just store the entire option string in utab, with only the bits that can be reconstructed from the kernel removed. This would not only fix things for systemd, but also be closer to what people probably expect from the output of "mount": that it closely reflects the exact line originally configured for the mount point...

@karelzak coincidentally we just #4817 was just reported which boils down to the same issue...

I am not entirely sure what to do about this, in particular as we so far told people to use x-systemd.after= and similar options, and that's what is deployed. If userspace options actually are lost after mount then I wonder what adopting libmount actually brought us, it was the major reasons why we started using it...

We can add a secondary utab managed by systemd of course, which at least caches the options that we set ourselves, but that would suck hard, and still not solve the problem for folks who want to use /bin/mount from the command line...

Or if you do add the "X-" thing we could implicitly uppercase things whenever we read /etc/fstab, but it would suck hard if you'd have to use a different set of options in /etc/fstab than on the "mount -o" command line...

Well, x-* has been always designed for fstab only (and in the man page we have "These options are not stored in the mtab file, nor sent to the mount.type helpers nor to the mount(2) system call.").

Also, if you have utab, why not actually use it?

because it will be always poor solution, if anyone bypass mount(8)/libmount and for example call mount("/foo", "/bar", "none", MS_MOVE, NULL) and "/foo" is stored in utab then we won't be able to merge the utab entry with appropriate mountinfo entry, and so on. Another story are unshared namespaces, etc.

So I can enable this feature for systemd, but you have to understand the limitations and disadvantages...

The ideal solution would be store x-* in kernel, but I have doubts kernel guys will listen to us. I have tried this years ago without any result. It's still the same; mount(2) sucks, but nobody works on something else.

@fbuihuu "Also what is the advantage to use a new namespace for the X-* options ?"

Well, don't store all x-* in userspace, but force people think about it. For example we have x-mount.mkdir, it does not make sense to have it in utab; it's really fstab only stuff.

I thought the idea is to use it for new options (like systemd.unmount-on-unplug), in this case new prefix is probably no problem, but it seems @poettering wants it also for already existing options like x-systemd.after ... for this purpose X-* is bad idea and we need it for x-* :-(

What about to add to the libmount exception for "x-systemd.*" to store these options to utab?

We already have #ifdefs for systemd in util-linux and this solution is without any impact to another x-* users and it should be good enough for systemd.

Fixed by util-linux/util-linux@5c493bd (the commit message contains wrong reference (URL) for another bug ignore it. Sorry.).

Now X-* as well as x-systemd are maintained by utab.

@karelzak , thanks!
I'm reading Testing for x-initrd.mount in should_umount() broken
Should people rename x-initrd.* to X-initrd.*?

@evverx ...oh, systemd uses also something else that x-systemd ?

Yes, the patch in the util-linux implements X-* is persistent (so available for umount too) and x-* as fstab comment only.

Is it big problem rename to X-initrd? I have added exception for x-systemd because many people already use it for many options, add another exception for x-initrd seems like ugly solution.

Maybe the ideal solution would be to do it in opposite way: x-* as persistent and X-* as fstab comment only. This way will not be 100% backwardly compatible, but maybe it's smaller problem than rename... ah...

oh, systemd uses also something else that x-systemd ?

I found x-initrd.mount only
https://www.freedesktop.org/software/systemd/man/systemd.mount.html#x-initrd.mount

x-initrd.mount
An additional filesystem to be mounted in the initramfs

Is it big problem rename to X-initrd?

Well, x-initrd.mount is broken and nobody cares (except #4817). Seems like nobody uses it.
I guess It's not a big deal to rename x to X

Sorry, I mean: it's not a big problem

Sorry, I mean: it's not a big problem

I was wrong: https://github.com/dracutdevs/dracut/blob/fea9be7ac109b45bd53e309c6e56e342d05e5ef2/modules.d/99fs-lib/fs-lib.sh#L225

@fbuihuu , sorry, I didn't read the code. But I noticed BindsTo=. How does this PR interact with #4733?

Sounds like make the libmount change in reverse way (x-* persistent and X-* fstab only) would be less invasive and without any exception (x-systemd.).

@karelzak , well, maybe

add another exception for x-initrd seems like ugly solution

But this doesn't break

Well, x-* has been always designed for fstab only (and in the man page we have "These options are not stored in the mtab file, nor sent to the mount.type helpers nor to the mount(2) system call.")

personally, I don't have a strong opinion on that.

I have thought about it and modified the libmount patch in way originally suggested by @poettering. The current util-linux master branch:

• all x-* stored in utab and available for umount and another operations
• all X-* works as fstab comments only
• no exceptions for systemd in libmount code

From my point of view the problem is solved now and it would be nice if you can test systemd against util-linux git tree.

@karelzak , thanks!
I ran a simple test:

bash-4.3# echo 'none           /test       tmpfs    rw,x-initrd.mount' >>/etc/fstab
bash-4.3# # note: don't run systemctl daemon-reload !!!
bash-4.3# mkdir -p /test
bash-4.3# mount /test
bash-4.3# systemctl show test.mount | grep -i self
bash-4.3# systemctl show test.mount | grep -i umount

2.28.2:

bash-4.3# systemctl show test.mount | grep -i self
SourcePath=/proc/self/mountinfo

bash-4.3# systemctl show test.mount | grep -i umount
Conflicts=umount.target
Before=local-fs.target umount.target

2.29.101-8048

# systemctl show test.mount | grep -i self
SourcePath=/proc/self/mountinfo

bash-4.3# systemctl show test.mount | grep -i umount
Conflicts=umount.target

Seems like should_umount returns false there and returns true there. Hm...

@karelzak , this is a systemd's bug:

• mount_setup_unit calls should_umount
• should_umount checks from_proc_self_mountinfo (false)
• mount_setup_unit sets

        MOUNT(u)->from_proc_self_mountinfo = true;

        free_and_replace(p->what, w);
        free_and_replace(p->options, o);
        free_and_replace(p->fstype, f);

• should_umount checks from_proc_self_mountinfo (true)

$ sudo gdb -p 1
...
(gdb) b should_umount
Breakpoint 1 at 0x7fc44a408334: file src/core/mount.c, line 390.
(gdb) c
Continuing.
...
Breakpoint 1, should_umount (m=0x7fc44a85a950) at src/core/mount.c:390
390     static bool should_umount(Mount *m) {
(gdb) n
393             if (PATH_IN_SET(m->where, "/", "/usr") ||
(gdb) n
394                 path_startswith(m->where, "/run/initramfs"))
(gdb) n
393             if (PATH_IN_SET(m->where, "/", "/usr") ||
(gdb) n
397             p = get_mount_parameters(m);
(gdb) s
get_mount_parameters (m=0x7fc44a85a950) at src/core/mount.c:254
254             assert(m);
(gdb) n
256             if (m->from_proc_self_mountinfo)
(gdb) n
(gdb) p m->parameters_proc_self_mountinfo
$6 = {what = 0x0, options = 0x0, fstype = 0x0}
(gdb) p m->from_proc_self_mountinfo
$7 = false
259             return get_mount_parameters_fragment(m);
(gdb) n
260     }
(gdb) n
should_umount (m=0x7fc44a85a950) at src/core/mount.c:398
398             if (p && fstab_test_option(p->options, "x-initrd.mount\0") &&
(gdb) p p
$1 = (MountParameters *) 0x0
(gdb) c
Continuing.

Breakpoint 1, should_umount (m=0x7fc44a85a950) at src/core/mount.c:390
390     static bool should_umount(Mount *m) {
(gdb) n
393             if (PATH_IN_SET(m->where, "/", "/usr") ||
(gdb) n
394                 path_startswith(m->where, "/run/initramfs"))
(gdb) n
393             if (PATH_IN_SET(m->where, "/", "/usr") ||
(gdb) n
397             p = get_mount_parameters(m);
(gdb) s
get_mount_parameters (m=0x7fc44a85a950) at src/core/mount.c:254
254             assert(m);
(gdb) n
256             if (m->from_proc_self_mountinfo)
(gdb) n
257                     return &m->parameters_proc_self_mountinfo;
(gdb) p m->parameters_proc_self_mountinfo
$2 = {what = 0x7fc44a846400 "none", options = 0x7fc44a79d500 "rw,relatime,x-initrd.mount", fstype = 0x7fc44a867fe0 "tmpfs"}
(gdb) n
260     }
(gdb) n
should_umount (m=0x7fc44a85a950) at src/core/mount.c:398
398             if (p && fstab_test_option(p->options, "x-initrd.mount\0") &&
(gdb) p p
$3 = (MountParameters *) 0x7fc44a85acc0
(gdb) n
399                 !in_initrd())
(gdb) n
398             if (p && fstab_test_option(p->options, "x-initrd.mount\0") &&
(gdb) n
400                     return false;
(gdb) n
403     }

src/core/mount.c uses mnt_table_parse_mtab(), so the userspace mount options should be available for systemd, and from line

gdb)  p m->parameters_proc_self_mountinfo
$2 = {what = 0x7fc44a846400 "none", options = 0x7fc44a79d500 "rw,relatime,x-initrd.mount", fstype = 0x7fc44a867fe0 "tmpfs"}

it really seems the x-initrd is there. Not sure there is the problem. I can try to debug it tomorrow,

@karelzak

it really seems the x-initrd is there

Yes, x-initrd is there

# mount -t tmpfs -o x-initrd.mount tmpfs /test
...
Breakpoint 1, mount_setup_unit (m=0x7fe7638d4080, what=0x7fe763965040 "tmpfs", where=0x7fe76399c680 "/test", options=0x7fe76399bb00 "rw,relatime,x-initrd.mount", fstype=0x7fe76399b9b0 "tmpfs",
    set_flags=true) at src/core/mount.c:1372
1372                    bool set_flags) {
(gdb) p options
$1 = 0x7fe76399c130 "rw,relatime,x-initrd.mount"
...
1428                            if (r < 0)
(gdb)
1431                            if (should_umount(MOUNT(u))) {
(gdb)
1432                                    r = unit_add_dependency_by_name(u, UNIT_CONFLICTS, SPECIAL_UMOUNT_TARGET, NULL, true);

Actually, we should free_and_replace(p->options, o) before should_umount.
So, libmount works fine. Thanks!

@karelzak thanks a ton for changing the semantics there, this is fantastic!

I merged this PR now, the rearranging of the checking of x- mount option that @evverx suggested we can fix in a later commit