Skip to content

Commit

Permalink
lib/deploy: Use a FIFREEZE/FITHAW cycle for /boot
Browse files Browse the repository at this point in the history
See: http://marc.info/?l=linux-fsdevel&m=149520244919284&w=2

XFS doesn't flush the journal on `syncfs()`. GRUB doesn't know how to follow the
XFS journal, so if the filesystem is in a dirty state (possible with xfs
`/boot`, extremely likely with `/`, if the journaled data includes content for
`/boot`, the system may be unbootable if a system crash occurs.

Fix this by doing a `FIFREEZE`+`FITHAW` cycle.  Now, most people
probably would have replaced the `syncfs()` invocation with those two
ioctls.  But this would have become (I believe) the *only* place in
libostree where we weren't safe against interruption.  The failure
mode would be ugly; nothing else would be able to write to the filesystem
until manual intervention.

The real fix here I think is to land an atomic `FIFREEZETHAW` ioctl
in the kernel.  I might try a patch.

In the meantime though, let's jump through some hoops and set up
a "watchdog" child process that acts as a fallback unfreezer.

Closes: #876
  • Loading branch information
cgwalters committed Aug 3, 2017
1 parent 7f33d94 commit 2256829
Showing 1 changed file with 73 additions and 9 deletions.
82 changes: 73 additions & 9 deletions src/libostree/ostree-sysroot-deploy.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,13 @@

#include <gio/gunixinputstream.h>
#include <gio/gunixoutputstream.h>
#include <glib-unix.h>
#include <sys/mount.h>
#include <sys/statvfs.h>
#include <sys/ioctl.h>
#include <sys/poll.h>
#include <linux/fs.h>
#include <err.h>

#ifdef HAVE_LIBMOUNT
#include <libmount.h>
Expand Down Expand Up @@ -973,17 +978,73 @@ checksum_from_kernel_src (const char *name,
}

static gboolean
syncfs_dir_at (int dfd,
const char *path,
GCancellable *cancellable,
GError **error)
fsfreeze_thaw_cycle (int rootfs_dfd,
GCancellable *cancellable,
GError **error)
{
glnx_fd_close int child_dfd = -1;
if (!glnx_opendirat (dfd, path, TRUE, &child_dfd, error))
GLNX_AUTO_PREFIX_ERROR ("During fsfreeze-thaw", error);

int pipefds[2];
if (!g_unix_open_pipe (pipefds, FD_CLOEXEC, error))
return FALSE;
if (syncfs (child_dfd) != 0)
return glnx_throw_errno_prefix (error, "syncfs(%s)", path);
glnx_fd_close int pipe_read = pipefds[0];
glnx_fd_close int pipe_write = pipefds[1];

pid_t pid = fork ();
if (pid < 0)
return glnx_throw_errno_prefix (error, "fork");

char c = '!';
if (pid == 0) /* Child watchdog/unfreezer process. */
{
/* Wait for the parent to say it's going to freeze. */
(void) TEMP_FAILURE_RETRY (read (pipefds[0], &c, sizeof (c)));
/* Now we wait for the second message from the parent saying the freeze is
* complete. We have a 30 second timeout; if somehow the parent hasn't
* signaled completion, go ahead and unfreeze.
*/
const int timeout_ms = 30000;
struct pollfd pfds[1];
pfds[0].fd = pipe_read;
pfds[0].events = POLLIN | POLLHUP;
int r = TEMP_FAILURE_RETRY (poll (pfds, 1, timeout_ms));
/* Do a thaw if we hit an error, or if the poll timed out */
if (r <= 0)
{
if (ioctl (rootfs_dfd, FITHAW, 0) != 0)
err (1, "FITHAW");
/* But if we got an error from poll, let's log it */
if (r < 0)
err (1, "poll");
}
exit (0);
}
else /* Parent process. */
{
/* Signal the child to start the watchdog */
if (TEMP_FAILURE_RETRY (write (pipe_write, &c, sizeof (c))) < 0)
return glnx_throw_errno_prefix (error, "write");
/* Do a freeze/thaw cycle; TODO add a FIFREEZETHAW ioctl */
if (ioctl (rootfs_dfd, FIFREEZE, 0) != 0)
{
if (errno == EOPNOTSUPP)
{
/* Not supported? OK, let's just do a syncfs */
if (syncfs (rootfs_dfd) != 0)
return glnx_throw_errno_prefix (error, "syncfs");
/* Write the completion, and return */
if (TEMP_FAILURE_RETRY (write (pipe_write, &c, sizeof (c))) < 0)
return glnx_throw_errno_prefix (error, "write");
return TRUE;
}
else
return glnx_throw_errno_prefix (error, "ioctl(FIFREEZE)");
}
if (ioctl (rootfs_dfd, FITHAW, 0) != 0)
return glnx_throw_errno_prefix (error, "ioctl(FITHAW)");
if (TEMP_FAILURE_RETRY (write (pipe_write, &c, sizeof (c))) < 0)
return glnx_throw_errno_prefix (error, "write");
}
return TRUE;
}

Expand Down Expand Up @@ -1011,7 +1072,10 @@ full_system_sync (OstreeSysroot *self,
out_stats->root_syncfs_msec = (end_msec - start_msec);

start_msec = g_get_monotonic_time () / 1000;
if (!syncfs_dir_at (self->sysroot_fd, "boot", cancellable, error))
glnx_fd_close int boot_dfd = -1;
if (!glnx_opendirat (self->sysroot_fd, "boot", TRUE, &boot_dfd, error))
return FALSE;
if (!fsfreeze_thaw_cycle (boot_dfd, cancellable, error))
return FALSE;
end_msec = g_get_monotonic_time () / 1000;
out_stats->boot_syncfs_msec = (end_msec - start_msec);
Expand Down

0 comments on commit 2256829

Please sign in to comment.