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

2015-05-26 Thread Lennart Poettering
On Mon, 25.05.15 21:06, Martin Pitt (martin.p...@ubuntu.com) wrote:

 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).

Not an API break. Only calls prefixed with sd_ are API, we never
export any others.

But yeah, path_is_mount_point() should be reworked to operate like
openat() and similar calls, i.e. taking an fd to a dir, plus a
filename below that dir, instead of directly and only an fd to the
file below that dir.

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] 220 regression: path_is_mount_point() for non-directories

2015-05-25 Thread Martin Pitt
Martin Pitt [2015-05-25 21:06 +0200]:
 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).

Ah no, it's just in src/shared, i. e. in libsystemd-shared.la, but not
exported to libsystemd.so. Thus no API break.

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