[systemd-devel] 220 regression: path_is_mount_point() for non-directories

[Martin Pitt]

Hello all,

in 220, path_is_mount_point() now always fails with "-20 ENOTDIR" when
calling it on files. This is problematic as it's perfectly valid to
have bind-mounted files; in fact, systemd's machine_id_setup() itself
creates a /run/machine-id → /etc/machine-id bind mount if /etc is
read-only at that time (and systemd-machine-id-commit will write it
once root fs becomes writable).

This was first introduced here:

  http://cgit.freedesktop.org/systemd/systemd/commit/?id=27cc6f166
      path-util: fix path_is_mount_point() for symlinks

which added O_DIRECTORY, so that openat() fails with the above error.
It also replaced path_get_parent() (which works fine for files) with
name_to_handle_at(fd, "..", ...) which only works for directories.

In fd_is_mount_point() we obviously don't have access to the file name
any more, and presumably we do want to keep it for efficiently
implementing the "rm-rf: never cross mount points" commit f25afeb.

Since all possible checks in fd_is_mount_point() rely on *at(, "..")
which doesn't work for files, one solution that I see
is to add a fallback path_is_mountpoint() if fd_is_mount_point() fails
with ENOTDIR; in that case, should we implement all fallbacks again
(lots of duplicated code), or just one (st_dev comparison, which seems
the most widely supported one)?

Alternatively we could pass a "parent_path" to fd_is_mount_point(),
compute that in path_is_mountpoint(), and don't specify one (or maybe
we even can specify it easily, I didn't check) in rm_rf_children() as
file bind mounts don't seem relevant there. This would be simpler, but
it would technically be an API break (unless we want to add
fd_is_mount_point_with_parent).

Does anyone have a better idea?

Thanks,

Martin
-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)