[ostree] deploy: Use syncfs() in addition to sync()
- From: Colin Walters <walters src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [ostree] deploy: Use syncfs() in addition to sync()
- Date: Tue, 21 Apr 2015 01:06:24 +0000 (UTC)
commit c58a5c0cb3fe441d61670b613a3513dde6851503
Author: Colin Walters <walters verbum org>
Date: Tue Apr 14 13:47:08 2015 -0400
deploy: Use syncfs() in addition to sync()
For some sort of crazy reason, the `sync()` system call doesn't
actually return an error code, even though from what I can tell in the
kernel it wouldn't be terribly hard to add.
Regardless though, it is better for userspace apps to use `syncfs()`
to avoid flushing filesystems unrelated to what they want to sync. In
the case of OSTree, this does matter - for example you might have a
network mount point backing your database, and we don't want to block
upgrades on syncing it.
This change is safe because we're doing syncfs in *addition* to the
previous global `sync()` (a revision from an earlier patch).
Now because OSTree only touches the `/` mount point which covers the
repository, the deployment roots (including their copy of `/etc`), as
well as `/boot`, we should at some point later be able to drop the
`sync()` call. Note that on initial system installs we do relabel
`/var` but that shouldn't happen at ostree time - any new directories
are taken care of via `systemd-tmpfiles` on boot.
src/libostree/ostree-sysroot-deploy.c | 57 +++++++++++++++++++++++++++++----
1 files changed, 50 insertions(+), 7 deletions(-)
---
diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
index 142a9fc..5501fb5 100644
--- a/src/libostree/ostree-sysroot-deploy.c
+++ b/src/libostree/ostree-sysroot-deploy.c
@@ -1005,16 +1005,59 @@ checksum_from_kernel_src (GFile *src,
return TRUE;
}
-/* FIXME: We should really do individual fdatasync() on files/dirs,
- * since this causes us to block on unrelated I/O. However, it's just
- * safer for now.
+static gboolean
+syncfs_dir_at (int dfd,
+ const char *path,
+ GCancellable *cancellable,
+ GError **error)
+{
+ gboolean ret = FALSE;
+ glnx_fd_close int child_dfd = -1;
+
+ if (!glnx_opendirat (dfd, path, TRUE, &child_dfd, error))
+ goto out;
+ if (syncfs (child_dfd) != 0)
+ {
+ glnx_set_error_from_errno (error);
+ goto out;
+ }
+
+ ret = TRUE;
+ out:
+ return ret;
+}
+
+/* First, sync the root directory as well as /var and /boot which may
+ * be separate mount points. Then *in addition*, do a global
+ * `sync()`.
*/
static gboolean
-full_system_sync (GCancellable *cancellable,
+full_system_sync (OstreeSysroot *self,
+ GCancellable *cancellable,
GError **error)
{
+ gboolean ret = FALSE;
+
+ if (syncfs (self->sysroot_fd) != 0)
+ {
+ glnx_set_error_from_errno (error);
+ goto out;
+ }
+
+ if (!syncfs_dir_at (self->sysroot_fd, "boot", cancellable, error))
+ goto out;
+
+ /* And now out of an excess of conservativism, we still invoke
+ * sync(). The advantage of still using `syncfs()` above is that we
+ * actually get error codes out of that API, and we more clearly
+ * delineate what we actually want to sync in the future when this
+ * global sync call is removed.
+ */
sync ();
- return TRUE;
+
+ ret = TRUE;
+ out:
+ return ret;
}
static gboolean
@@ -1537,7 +1580,7 @@ ostree_sysroot_write_deployments (OstreeSysroot *self,
if (!requires_new_bootversion)
{
- if (!full_system_sync (cancellable, error))
+ if (!full_system_sync (self, cancellable, error))
{
g_prefix_error (error, "Full sync: ");
goto out;
@@ -1629,7 +1672,7 @@ ostree_sysroot_write_deployments (OstreeSysroot *self,
}
}
- if (!full_system_sync (cancellable, error))
+ if (!full_system_sync (self, cancellable, error))
{
g_prefix_error (error, "Full sync: ");
goto out;
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]