From: Eric Curtin Date: Tue, 28 Jul 2026 13:27:15 +0000 (+0100) Subject: shared/switch-root: sync only file systems becoming unreachable, not everything X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;p=thirdparty%2Fsystemd.git shared/switch-root: sync only file systems becoming unreachable, not everything switch_root() calls a blanket sync() before detaching the old root file system, in order to make sure it is in a good state before it becomes unreachable via MNT_DETACH/pivot_root(). A global sync() however flushes out *every* mounted file system on the system, not just the ones we are actually about to detach. On real-world systems that commonly have several additional mounted file systems (separate /home, /var, additional data partitions, network shares, removable media, ...) this needlessly delays switch_root() with completely unrelated I/O. This matters in particular for initrd-switch-root.service, which runs this code on the critical path of pretty much every single boot with an initrd, and for soft-reboot. Replace the global sync() with a new sync_departing_file_systems() helper that walks /proc/self/mountinfo and calls syncfs() on every file system except: - 'new_root' and anything mounted below it: these remain mounted and reachable after the transition and keep being synced normally as part of their regular life cycle, so they don't need to be force-flushed here. - API/pseudo file systems (proc, sysfs, cgroupfs, autofs, ...), network file systems, and overlayfs (which has no backing store of its own), as determined by the new fstype_is_worth_syncing() predicate. There is nothing meaningful to flush on any of these, and more importantly, opening an untriggered autofs mount point would needlessly trigger it, and opening a stale network mount could block for a long time - exactly what we are trying to avoid on this code path. - Any flavour of FUSE (plain 'fuse', 'fuseblk', or a 'fuse.', e.g. sshfs, rclone, gvfs, ntfs-3g, exfat-fuse, ...), classified via the new fstype_is_fuse() predicate in src/basic/mountpoint-util.c, plus a few other, non-FUSE guest/host file sharing file systems with the same "backed by a companion daemon/hypervisor that could be wedged" risk profile (virtiofs, vboxsf, vmhgfs). All I/O against any of these, including the syncfs() we'd otherwise issue, is routed through an arbitrary userspace daemon (or, for virtiofs/vboxsf/vmhgfs, the host/ hypervisor side), which could hang indefinitely if wedged, dead, or otherwise unresponsive - there's no timeout on this code path. 'fuseblk' might sound exempt given the name, and does wrap an actual block device, but that doesn't bound its syncfs() latency by the kernel block layer alone the way a native block device file system's is: the request is still serviced by the same FUSE daemon as any other FUSE variant, and can hang exactly the same way, so it is excluded here too, trading its comparatively minor data-safety benefit for avoiding that unbounded hang risk. '9p' (which can be used with a writeback cache and hence carry real dirty data, e.g. common in QEMU/KVM guests) and the shared-storage cluster file systems 'gfs', 'gfs2' and 'ocfs2' (which fstype_is_network() also happens to classify as "network" file systems, since they additionally rely on a networked distributed lock manager for coordination) are deliberately *not* excluded: unlike FUSE/virtiofs/etc., these are serviced by a mature, in-kernel client (talking directly to the hypervisor over a bounded virtio transport, or to real - if shared - block storage), not an arbitrary, potentially wedged userspace daemon, so they carry the same bounded, local sync latency any other block device backed file system already does here. Skipping them would needlessly sacrifice the data-safety guarantee the original blanket sync() gave them, without meaningfully improving safety. - Mount table entries that we can positively confirm are currently shadowed by another mount stacked on top of them at the same path: since we can only reach a file system by (re-)opening its target path, and that always resolves to whatever is currently on top, syncing by path alone could end up flushing the wrong superblock. Detect this via the new shared libmount_fs_id_matches_path() helper (factored out of, and now also used by, the pre-existing get_sub_mounts(), which needed the exact same check for the same reason). This same check is also applied to a mountinfo entry whose target is 'new_root' itself (not just anything strictly below it): comparing its mount ID against new_root's own, freshly determined mount ID tells apart the file system that is actually still reachable there (which we continue to skip) from a stale entry that merely shares the exact same path (e.g. if new_root wasn't already its own mount point and got bind-mounted onto itself earlier in switch_root()), which is departing just the same and must not be skipped just because of that coincidence. Every failure mode that means we can no longer be sure we've covered every departing file system correctly - libmount being unavailable, /proc/self/mountinfo (or a specific entry in it) failing to parse, being unable to tell whether a specific entry is currently shadowed, or syncfs_path() itself failing for an otherwise-eligible entry - is handled the exact same way: propagate the error up and let the sole caller, switch_root(), fall back to one plain, global sync() to cover everything, rather than deciding on and performing that fallback (or, worse, silently skipping the affected file system without any fallback at all) at each of these different spots individually. This should be rare in practice, so it doesn't meaningfully undercut the benefit of the targeted sync in the common case. Everything else that's actually about to become unreachable (the old root itself, but also any other, unrelated real file system that happens to be mounted underneath it and gets detached along with it) is still synced, so this keeps the same safety guarantee the original blanket sync() gave for file systems that actually do go away here. Uses the existing syncfs_path() helper for the actual open+syncfs. sync_departing_file_systems() itself returns -EOPNOTSUPP if libmount support isn't compiled in, handled the same way by switch_root() as any of its other error returns. Note we intentionally don't use O_PATH file descriptors here: syncfs() requires a 'real' file descriptor and fails with EBADF on O_PATH ones. Also note there remains an inherent, narrow TOCTOU race between the mount-ID check described above and the open() syncfs_path() performs right after it: if something else mounts something new on top of a given 'path' in between, that open() could still end up triggering an automount, or hanging on a stale mount, since there is no open()/ openat() equivalent of statx()'s AT_NO_AUTOMOUNT to prevent this for a "real" (non-O_PATH) file descriptor. Unlike the other failure modes handled here, a hanging open() can't be recovered from by falling back to sync() afterwards, since control never returns to do so. Closing this fully would require disproportionate effort (e.g. performing the open() in a separate, killable/timeout-bounded process) for a window that is already narrow, since this code only runs with most other activity on the system already quiesced during the switch_root() transition itself, so it is accepted as-is (see the comment at the call site for details). This mirrors the same reasoning already applied to the shutdown path in src/shutdown/shutdown.c, which deliberately avoids a 'dumb' sync() there for identical reasons. --- diff --git a/src/basic/mountpoint-util.c b/src/basic/mountpoint-util.c index 4db4d8162fd..f84fb04a674 100644 --- a/src/basic/mountpoint-util.c +++ b/src/basic/mountpoint-util.c @@ -424,6 +424,14 @@ bool fstype_is_blockdev_backed(const char *fstype) { return !STR_IN_SET(fstype, "9p", "overlay") && !fstype_is_network(fstype) && !fstype_is_api_vfs(fstype); } +bool fstype_is_fuse(const char *fstype) { + assert(fstype); + + /* Plain 'fuse', 'fuseblk' (e.g. ntfs-3g, exfat-fuse — used for file systems backed by an actual + * block device), or any 'fuse.' (e.g. sshfs, rclone, gvfs, ...). */ + return STR_IN_SET(fstype, "fuse", "fuseblk") || startswith(fstype, "fuse."); +} + bool fstype_is_ro(const char *fstype) { /* All Linux file systems that are necessarily read-only */ return STR_IN_SET(fstype, diff --git a/src/basic/mountpoint-util.h b/src/basic/mountpoint-util.h index 7f29ffe1778..563685e7153 100644 --- a/src/basic/mountpoint-util.h +++ b/src/basic/mountpoint-util.h @@ -64,6 +64,7 @@ bool fstype_needs_quota(const char *fstype); bool fstype_has_internal_quota(const char *fstype); bool fstype_is_api_vfs(const char *fstype); bool fstype_is_blockdev_backed(const char *fstype); +bool fstype_is_fuse(const char *fstype); bool fstype_is_ro(const char *fsype); bool fstype_can_discard(const char *fstype); bool fstype_can_ownership(const char *fstype); diff --git a/src/shared/libmount-util.c b/src/shared/libmount-util.c index ad81957df7c..ad1afb651ee 100644 --- a/src/shared/libmount-util.c +++ b/src/shared/libmount-util.c @@ -8,6 +8,7 @@ #include #include "fstab-util.h" +#include "mountpoint-util.h" DLSYM_PROTOTYPE(mnt_free_iter) = NULL; DLSYM_PROTOTYPE(mnt_free_table) = NULL; @@ -112,6 +113,33 @@ int libmount_is_leaf( return r == 1; } +/* Checks whether 'fs' (an entry from a parsed mountinfo table) still refers to the file system currently + * reachable at its own target path. This can be false if something else has since been mounted on top of + * it, shadowing it: opening 'path' would then resolve to whatever is currently on top, not to 'fs' itself. + * + * Returns 1 if the mount IDs match (i.e. 'fs' is still the topmost, reachable mount at its path), 0 if + * they don't (with a debug log message explaining why), and a negative errno otherwise. */ +int libmount_fs_id_matches_path(struct libmnt_fs *fs, const char *path) { + int id1, id2, r; + + assert(fs); + assert(path); + + id1 = sym_mnt_fs_get_id(fs); + + r = path_get_mnt_id(path, &id2); + if (r < 0) + return log_debug_errno(r, "Failed to get mount ID of '%s': %m", path); + + if (id1 != id2) { + log_debug("The mount IDs of '%s' obtained by libmount and path_get_mnt_id() are different (%i vs %i).", + path, id1, id2); + return 0; + } + + return 1; +} + #endif int dlopen_libmount(int log_level) { diff --git a/src/shared/libmount-util.h b/src/shared/libmount-util.h index 056a67da218..c2ca56b7e99 100644 --- a/src/shared/libmount-util.h +++ b/src/shared/libmount-util.h @@ -77,6 +77,8 @@ int libmount_is_leaf( struct libmnt_table *table, struct libmnt_fs *fs); +int libmount_fs_id_matches_path(struct libmnt_fs *fs, const char *path); + #else struct libmnt_monitor; diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c index 543682f1117..17c43420e6a 100644 --- a/src/shared/mount-util.c +++ b/src/shared/mount-util.c @@ -1810,7 +1810,6 @@ int get_sub_mounts(const char *prefix, SubMount **ret_mounts, size_t *ret_n_moun _cleanup_free_ char *p = NULL; struct libmnt_fs *fs; const char *path; - int id1, id2; r = sym_mnt_table_next_fs(table, iter, &fs); if (r == 1) @@ -1825,18 +1824,11 @@ int get_sub_mounts(const char *prefix, SubMount **ret_mounts, size_t *ret_n_moun if (isempty(path_startswith(path, prefix))) continue; - id1 = sym_mnt_fs_get_id(fs); - r = path_get_mnt_id(path, &id2); - if (r < 0) { - log_debug_errno(r, "Failed to get mount ID of '%s', ignoring: %m", path); - continue; - } - if (id1 != id2) { - /* The path may be hidden by another over-mount or already remounted. */ - log_debug("The mount IDs of '%s' obtained by libmount and path_get_mnt_id() are different (%i vs %i), ignoring.", - path, id1, id2); + /* The path may be hidden by another over-mount or already remounted; skip it in that case + * (libmount_fs_id_matches_path() already logs the details at debug level). */ + r = libmount_fs_id_matches_path(fs, path); + if (r <= 0) continue; - } /* If possible on a newer kernel, use MS_PRIVATE to decouple it from the original mount. * Otherwise MNT_DETACH of the source path could propagate through and unmount the diff --git a/src/shared/switch-root.c b/src/shared/switch-root.c index 3bb26115235..bcb19f59516 100644 --- a/src/shared/switch-root.c +++ b/src/shared/switch-root.c @@ -10,13 +10,231 @@ #include "chase.h" #include "errno-util.h" #include "fd-util.h" +#include "libmount-util.h" #include "log.h" #include "mkdir.h" #include "mount-util.h" #include "mountpoint-util.h" +#include "path-util.h" #include "rm-rf.h" #include "stat-util.h" +#include "string-util.h" +#include "strv.h" #include "switch-root.h" +#include "sync-util.h" + +#if HAVE_LIBMOUNT +/* Returns true if it is both meaningful and safe to synchronize a file system of the given type directly + * via its own mount point here. + * + * False for API/pseudo file systems (proc, sysfs, cgroupfs, autofs, ...) and network file systems (nfs, + * cifs, ...): the former have nothing worth flushing, and opening either could actively backfire — trigger + * an untouched autofs mount point, or block for a long time on a stale/unreachable network mount, which is + * exactly what we must not risk on this code path. Also false for overlayfs, which has no persistent + * backing store of its own: any real data lives in the underlying directories, which (if they are real, + * separately mounted file systems) are covered on their own via their own mount table entry. + * + * Also false for *any* flavour of FUSE, i.e. plain 'fuse', 'fuseblk', or a 'fuse.' (e.g. sshfs, + * rclone, gvfs, ntfs-3g, exfat-fuse, ...), as classified by fstype_is_fuse(): all I/O against any of these, + * including the syncfs() we'd otherwise issue, is routed through an arbitrary userspace daemon, which could + * hang indefinitely if wedged, dead, or otherwise unresponsive - there's no timeout on this code path. + * 'fuseblk' might sound like it is plain block device backed given the name, and does wrap an actual block + * device, but that doesn't make its syncfs() latency bounded by the kernel block layer alone the way a + * native block device file system's is: the request is still serviced by the same FUSE daemon as any other + * FUSE variant, and can hang exactly the same way. We accept losing the (comparatively minor) data-safety + * benefit of syncing a healthy fuseblk file system in order to avoid this unbounded hang risk. + * + * The same "backed by a companion daemon/hypervisor that might be wedged" risk applies to a few other, + * non-FUSE guest/host file sharing file systems, namely 'virtiofs', 'vboxsf', and 'vmhgfs': these are + * excluded for the exact same reason FUSE is. + * + * '9p' on the other hand is *not* excluded, even though fstype_is_blockdev_backed() (which exists for a + * different purpose, namely quota/attribute support elsewhere) does exclude it: 9p file systems (as + * commonly used for host/guest sharing in QEMU/KVM VMs) can be mounted with a writeback cache and hence + * may carry real dirty data of their own that needs flushing here, just like any other departing file + * system. Unlike FUSE/virtiofs/etc., 9p is a mature, in-kernel client talking directly to the hypervisor + * over a bounded virtio transport, not an arbitrary, potentially wedged userspace daemon, so this is a + * judgement call weighing that against the above, less predictable, third-party guest tools/daemons. + * + * Similarly, the shared-storage cluster file systems 'gfs', 'gfs2', and 'ocfs2' are *not* excluded either, + * even though fstype_is_network() (again, designed for a different, unrelated purpose) treats them as + * "network" file systems: unlike genuine network file systems (nfs, cifs, ...), they carry real dirty data + * of their own and are flushed with the same bounded, local I/O as any other block device backed file + * system, since they're backed by real (if shared) block storage. They only get lumped in with "network" + * file systems because they additionally rely on a networked distributed lock manager for cluster + * coordination, which has no bearing on the risk profile of syncing them here. */ +static bool fstype_is_worth_syncing(const char *fstype) { + if (!fstype) + return true; /* don't know, better be safe than sorry and try to sync it anyway */ + + if (streq(fstype, "overlay")) + return false; + + if (STR_IN_SET(fstype, "gfs", "gfs2", "ocfs2")) + return true; /* cluster fs: not really a "network" fs despite fstype_is_network(), see above */ + + if (fstype_is_api_vfs(fstype) || fstype_is_network(fstype)) + return false; + + if (fstype_is_fuse(fstype)) + return false; + + /* Other guest/host file sharing file systems backed by a companion daemon or hypervisor service + * that could likewise be wedged, dead, or otherwise unresponsive. */ + if (STR_IN_SET(fstype, "virtiofs", "vboxsf", "vmhgfs")) + return false; + + return true; +} +#endif + +/* Flushes out the file systems that are about to become unreachable/"departing" as we switch to + * 'new_root', so that they are in a good state before they possibly are detached with MNT_DETACH. + * Explicitly excludes 'new_root' and any file systems mounted below it, since those remain mounted and + * reachable after the transition, and will continue to be written to/synced normally as part of their + * regular life cycle afterwards. This deliberately avoids a global sync() (which would also flush out any + * other, completely unrelated file systems that happen to be mounted on the system, e.g. any additional + * data partitions, network shares, removable media, …), since this code path is very much on the critical + * path during boot (as part of initrd-switch-root.service) and soft-reboot. + * + * Also skips file system types for which fstype_is_worth_syncing() returns false, see there for the details + * and reasoning. + * + * On any failure that means we can't be sure we've covered everything (libmount unavailable, or + * /proc/self/mountinfo can't be parsed, in full or in part), returns a negative error and leaves it up to + * the caller to fall back to a plain, global sync() instead, rather than doing that here itself: that way + * the fallback logic lives in exactly one place. */ +static int sync_departing_file_systems(const char *new_root) { +#if HAVE_LIBMOUNT + _cleanup_(mnt_free_tablep) struct libmnt_table *table = NULL; + _cleanup_(mnt_free_iterp) struct libmnt_iter *iter = NULL; + int r, new_root_mnt_id; + + r = libmount_parse_mountinfo(/* source= */ NULL, &table, &iter); + if (r < 0) + return log_debug_errno(r, "Failed to parse /proc/self/mountinfo: %m"); + + /* Determine new_root's own, definitely-current mount ID once upfront, so that below we can tell + * apart the file system that is actually still going to be reachable at 'new_root' after the + * switch (which we want to skip, see below) from any other, stale/shadowed mountinfo entry that + * merely happens to share the exact same target path (which we do not want to skip, see below). */ + r = path_get_mnt_id(new_root, &new_root_mnt_id); + if (r < 0) + return log_debug_errno(r, "Failed to determine current mount ID of '%s': %m", new_root); + + for (;;) { + struct libmnt_fs *fs; + const char *path, *fstype, *rest; + + r = sym_mnt_table_next_fs(table, iter, &fs); + if (r == 1) /* EOF */ + break; + if (r < 0) + /* Something went wrong walking the remainder of the table. We can't tell which + * (if any) of the remaining file systems still need to be synced, so let the + * caller fall back to a global sync() to cover them (and everything we might have + * already processed again, that's harmless), rather than risk silently skipping + * something that matters. */ + return log_debug_errno(r, "Failed to get next entry from /proc/self/mountinfo: %m"); + + path = sym_mnt_fs_get_target(fs); + if (!path) + /* Same reasoning as above: we can't identify (let alone sync) this entry at all, + * so let the caller fall back to a global sync() rather than silently drop it. In + * practice this shouldn't happen for a real mountinfo entry. */ + return log_debug_errno(SYNTHETIC_ERRNO(ENODATA), "Mount entry without a target path found, giving up."); + + rest = path_startswith(path, new_root); + if (rest) { + /* Anything strictly below new_root remains mounted and reachable after the switch + * as part of its subtree, hence doesn't need to be synced out defensively here. */ + if (!isempty(rest)) + continue; + + /* 'path' is new_root itself. If this entry's mount ID matches the one we + * determined new_root's current mount actually has, this is that same, still + * reachable file system: skip it exactly as with anything below it, to avoid + * needlessly syncing something that isn't going away. Otherwise, this entry must + * be a stale one that's now shadowed by the (different) file system actually + * mounted at new_root (e.g. if new_root wasn't already its own mount point and got + * bind-mounted onto itself earlier in switch_root()) and is about to become + * unreachable just the same: let it fall through to the general shadow/sync + * handling below instead of silently skipping it here too. */ + if (sym_mnt_fs_get_id(fs) == new_root_mnt_id) + continue; + } + + fstype = sym_mnt_fs_get_fstype(fs); + if (!fstype_is_worth_syncing(fstype)) { + log_debug("Not synchronizing '%s': file system type '%s' is not worth (or not safe) to synchronize here.", + path, strna(fstype)); + continue; + } + + /* mountinfo may list the same target path more than once, if one mount shadows another + * (i.e. something else has since been mounted on top of it). Opening 'path' always + * resolves to whatever is currently visible there, i.e. the top-most mount, which might + * not be the (possibly departing) one this specific entry refers to. This uses the same + * check get_sub_mounts() already does for the same reason. + * + * If this entry is confirmed to be currently shadowed (r == 0), its own superblock isn't + * reachable by path at all any more, and if it's the one carrying dirty data that's about + * to become unreachable, that data would be silently dropped if we just skipped it here + * (unlike the blanket sync() we're replacing, which covers shadowed superblocks too, since + * it isn't path based). + * + * If we fail to determine this either way (r < 0), we also don't know what's currently at + * 'path' any more: unlike the (automount/network safe) check we just did, plain + * syncfs_path() doesn't suppress automounts, so blindly opening 'path' here could trigger + * an untouched autofs mount point, or block on a stale network mount that has since been + * stacked on top - exactly what fstype_is_worth_syncing() above is trying to prevent us from + * doing. + * + * Rather than risk either of these, let the caller fall back to one global sync(): a + * global sync() trivially covers this (and every other) file system correctly, so there's + * nothing left to do here afterwards. This should be rare in practice (shadowed departing + * mounts, or failures determining this, are both unusual), so this doesn't meaningfully + * undercut the benefit of the targeted sync in the common case. */ + r = libmount_fs_id_matches_path(fs, path); + if (r < 0) + return log_debug_errno(r, "Failed to determine whether '%s' is currently shadowed by another mount: %m", path); + if (r == 0) + return log_debug_errno(SYNTHETIC_ERRNO(ESTALE), "'%s' is currently shadowed by another mount, giving up.", path); + + /* Note there's an inherent, unavoidable TOCTOU race between the check above and the + * open() syncfs_path() is about to do below: if something else mounts something new on + * top of 'path' in between (e.g. an autofs mount point gets triggered by an unrelated + * process, or a network mount appears), that open() could still end up triggering an + * automount, or blocking on that new, possibly stale, mount — the very thing the check + * above exists to prevent. Unlike a failed lookup (handled above) or a failed syncfs() + * (handled below), a *hanging* open() can't be recovered from by falling back to sync() + * afterwards, since we'd never get back here to do so. There is no way to open a real, + * syncfs()-capable file descriptor while also reliably suppressing automounts the way + * statx()'s AT_NO_AUTOMOUNT does for the check above (that flag has no open()/openat() + * equivalent), so this narrow window can't be fully closed without disproportionate + * effort (e.g. performing the open() in a separate, killable/timeout-bounded process). + * We accept it here: this code only runs with most other activity on the system already + * quiesced (during the switch_root() transition itself), so the window for something else + * to concurrently and adversarially remount 'path' in the first place is already narrow. */ + r = syncfs_path(AT_FDCWD, path); + if (r < 0) + /* We can't tell here whether this failed because we couldn't even open 'path' + * (e.g. a transient error, or its state changed between the mount ID check above + * and this open()), in which case a global sync() would still cover it just fine, + * or because syncfs() itself hit a genuine, lower-level I/O error, in which case a + * global sync() would likely run into the very same error and not actually recover + * anything. Since we can't distinguish the two, and dropping this file system's + * writeback silently would violate our "never regress the safety of the sync() we + * replace" guarantee for the (plausibly more common) former case, let the caller + * fall back to a global sync() here too, same as for the other cases above. */ + return log_debug_errno(r, "Failed to synchronize file system '%s': %m", path); + } + + return 0; +#else + return -EOPNOTSUPP; +#endif +} int switch_root(const char *new_root, const char *old_root_after, /* path below the new root, where to place the old root after the transition; may be NULL to unmount it */ @@ -100,10 +318,18 @@ int switch_root(const char *new_root, /* We are about to unmount various file systems with MNT_DETACH (either explicitly via umount() or * indirectly via pivot_root()), and thus do not synchronously wait for them to be fully sync'ed — * all while making them invisible/inaccessible in the file system tree for later code. That makes - * sync'ing them then difficult. Let's hence issue a manual sync() here, so that we at least can - * guarantee all file systems are an a good state before entering this state. */ - if (!FLAGS_SET(flags, SWITCH_ROOT_DONT_SYNC)) - sync(); + * sync'ing them then difficult. Let's hence issue a manual sync here, so that we at least can + * guarantee the file systems that are about to become unreachable are in a good state before + * entering this state. See sync_departing_file_systems() above for why we don't just call the + * global sync() here unconditionally: only fall back to it if the smarter, targeted sync couldn't + * be performed (e.g. libmount is unavailable, or /proc/self/mountinfo couldn't be parsed). */ + if (!FLAGS_SET(flags, SWITCH_ROOT_DONT_SYNC)) { + r = sync_departing_file_systems(new_root); + if (r < 0) { + log_debug_errno(r, "Failed to selectively synchronize departing file systems, falling back to global sync(): %m"); + sync(); + } + } /* Work-around for kernel design: the kernel refuses MS_MOVE if any file systems are mounted * MS_SHARED. Hence remount them MS_PRIVATE here as a work-around.