Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rofiles: Add --copyup option #1382

Closed
wants to merge 6 commits into from
Closed

rofiles: Add --copyup option #1382

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5becda7
rofiles: Add --copyup option
cgwalters Dec 14, 2017
bcae27b
fixup! rofiles: Add --copyup option
cgwalters Dec 14, 2017
3fdcb9e
fixup! rofiles: Add --copyup option
cgwalters Dec 15, 2017
086efd6
fixup! rofiles: Add --copyup option
cgwalters Dec 15, 2017
ab634a2
fixup! rofiles: Add --copyup option
cgwalters Dec 30, 2017
eec2ff2
fixup! rofiles: Add --copyup option
cgwalters Jan 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/rofiles-fuse/Makefile-inc.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,7 @@ bin_PROGRAMS += rofiles-fuse

rofiles_fuse_SOURCES = src/rofiles-fuse/main.c

rofiles_fuse_CFLAGS = $(AM_CFLAGS) -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 $(BUILDOPT_FUSE_CFLAGS) $(OT_INTERNAL_GIO_UNIX_CFLAGS) -I$(srcdir)/libglnx $(NULL)
rofiles_fuse_LDADD = libglnx.la $(BUILDOPT_FUSE_LIBS) $(OT_INTERNAL_GIO_UNIX_LIBS)
rofiles_fuse_CFLAGS = $(AM_CFLAGS) -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 $(BUILDOPT_FUSE_CFLAGS) \
$(OT_INTERNAL_GIO_UNIX_CFLAGS) -I $(srcdir)/src/libostree -I $(builddir)/src/libostree \
-I$(srcdir)/libglnx
rofiles_fuse_LDADD = libglnx.la $(BUILDOPT_FUSE_LIBS) $(OT_INTERNAL_GIO_UNIX_LIBS) libostree-1.la
127 changes: 98 additions & 29 deletions src/rofiles-fuse/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,14 @@
#include <glib.h>

#include "libglnx.h"
#include "ostree.h"

// Global to store our read-write path
static int basefd = -1;
/* Whether or not to automatically "copyup" (in overlayfs terms).
* What we're really doing is breaking hardlinks.
*/
static gboolean opt_copyup;

static inline const char *
ENSURE_RELPATH (const char *path)
Expand Down Expand Up @@ -200,52 +205,97 @@ callback_link (const char *from, const char *to)
/* Check whether @stbuf refers to a hardlinked regfile or symlink, and if so
* return -EROFS. Otherwise return 0.
*/
static int
can_write_stbuf (struct stat *stbuf)
static gboolean
can_write_stbuf (const struct stat *stbuf)
{
/* If it's not a regular file or symlink, ostree won't hardlink it, so allow
* writes - it might be a FIFO or device that somehow
* ended up underneath our mount.
*/
if (!(S_ISREG (stbuf->st_mode) || S_ISLNK (stbuf->st_mode)))
return 0;
return TRUE;
/* If the object isn't hardlinked, it's OK to write */
if (stbuf->st_nlink <= 1)
return 0;
return TRUE;
/* Otherwise, it's a hardlinked file or symlink; it must be
* immutable.
*/
return -EROFS;
return FALSE;
}

/* Check whether @path refers to a hardlinked regfile or symlink, and if so
* return -EROFS. Otherwise return 0.
*/
static int
can_write (const char *path)
gioerror_to_errno (GIOErrorEnum e)
{
struct stat stbuf;
if (fstatat (basefd, path, &stbuf, AT_SYMLINK_NOFOLLOW) == -1)
/* It's obviously crappy to have to do this but
* we also don't want to try to have "raw errno" versions
* of everything down in ostree_break_hardlink() so...
* let's just reverse map a few ones I think are going to be common.
*/
switch (e)
{
if (errno == ENOENT)
return 0;
case G_IO_ERROR_NOT_FOUND:
return ENOENT;
case G_IO_ERROR_IS_DIRECTORY:
return EISDIR;
case G_IO_ERROR_PERMISSION_DENIED:
return EPERM;
case G_IO_ERROR_NO_SPACE:
return ENOSPC;
default:
return EIO;
}
}

static int
verify_write_or_copyup (const char *path, const struct stat *stbuf)
{
struct stat stbuf_local;

/* If a stbuf wasn't provided, gather it now */
if (!stbuf)
{
if (fstatat (basefd, path, &stbuf_local, AT_SYMLINK_NOFOLLOW) == -1)
{
if (errno == ENOENT)
return 0;
else
return -errno;
}
stbuf = &stbuf_local;
}

/* Verify writability, if that fails, perform copy-up if enabled */
if (!can_write_stbuf (stbuf))
{
if (opt_copyup)
{
g_autoptr(GError) tmp_error = NULL;
if (!ostree_break_hardlink (basefd, path, FALSE, NULL, &tmp_error))
return -gioerror_to_errno ((GIOErrorEnum)tmp_error->code);
}
else
return -errno;
return -EROFS;
}
return can_write_stbuf (&stbuf);

return 0;
}

#define VERIFY_WRITE(path) do { \
int r = can_write (path); \
if (r != 0) \
return r; \
/* Given a path (which is absolute), convert it
* to a relative path (even for the caller) and
* perform either write verification or copy-up.
*/
#define PATH_WRITE_ENTRYPOINT(path) do { \
path = ENSURE_RELPATH (path); \
int r = verify_write_or_copyup (path, NULL); \
if (r != 0) \
return r; \
} while (0)

static int
callback_chmod (const char *path, mode_t mode)
{
path = ENSURE_RELPATH (path);
VERIFY_WRITE(path);
PATH_WRITE_ENTRYPOINT (path);

/* Note we can't use AT_SYMLINK_NOFOLLOW yet;
* https://marc.info/?l=linux-kernel&m=148830147803162&w=2
* https://marc.info/?l=linux-fsdevel&m=149193779929561&w=2
Expand All @@ -258,8 +308,8 @@ callback_chmod (const char *path, mode_t mode)
static int
callback_chown (const char *path, uid_t uid, gid_t gid)
{
path = ENSURE_RELPATH (path);
VERIFY_WRITE(path);
PATH_WRITE_ENTRYPOINT (path);

if (fchownat (basefd, path, uid, gid, AT_SYMLINK_NOFOLLOW) != 0)
return -errno;
return 0;
Expand All @@ -268,12 +318,9 @@ callback_chown (const char *path, uid_t uid, gid_t gid)
static int
callback_truncate (const char *path, off_t size)
{
glnx_autofd int fd = -1;
PATH_WRITE_ENTRYPOINT (path);

path = ENSURE_RELPATH (path);
VERIFY_WRITE(path);

fd = openat (basefd, path, O_NOFOLLOW|O_WRONLY);
glnx_autofd int fd = openat (basefd, path, O_NOFOLLOW|O_WRONLY);
if (fd == -1)
return -errno;

Expand All @@ -286,6 +333,9 @@ callback_truncate (const char *path, off_t size)
static int
callback_utimens (const char *path, const struct timespec tv[2])
{
/* This one isn't write-verified, we support changing times
* even for hardlinked files.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you expand on this? Don't we need to keep bare objects at OSTREE_TIMESTAMP to be consistent?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an interesting point...gets a bit into a philosophical question of whether rofiles-fuse is for "anything using hardlinks" or whether it's really specific to ostree. The "anything using hardlinks" case might be a backup system or non-os-data object store was my thought originally.

For example I could probably have "object semantics" for my ~/Documents which has a bunch of random PDFs and .txt and LibreOffice files etc. that should really never be overwritten in place. OTOH I've been thinking about storing all of that in git.

So...hmm. It's not a new issue here and we should think about changing this, but I worry a bit about the backcompat issues. In practice nothing we use rofiles-fuse for should be touching timestamps for files it didn't create at the time I would guess.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example I could probably have "object semantics" for my ~/Documents which has a bunch of random PDFs and .txt and LibreOffice files etc. that should really never be overwritten in place. OTOH I've been thinking about storing all of that in git.

I've been using git-annex for this for some time now. Good news, it's in fedora.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, git-annex is likely an excellent example of a possible user of rofiles-fuse semantics, specifically its direct mode. Though I didn't look closely.

*/
path = ENSURE_RELPATH (path);

if (utimensat (basefd, path, tv, AT_SYMLINK_NOFOLLOW) == -1)
Expand Down Expand Up @@ -324,13 +374,27 @@ do_open (const char *path, mode_t mode, struct fuse_file_info *finfo)
return -errno;
}

int r = can_write_stbuf (&stbuf);
int r = verify_write_or_copyup (path, &stbuf);
if (r != 0)
{
(void) close (fd);
return r;
}

/* In the copyup case, we need to re-open */
if (opt_copyup)
{
(void) close (fd);
/* Note that unlike the initial open, we will pass through
* O_TRUNC. More ideally in this copyup case we'd avoid copying
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so then we don't need to ftruncate down below again, right? Shouldn't the if condition at 399 be changed to

if (!opt_copyup && (finfo->flags & O_TRUNC))

?

* the whole file in the first place, but eh. It's not like we're
* high performance anyways.
*/
fd = openat (basefd, path, finfo->flags & ~(O_EXCL|O_CREAT), mode);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we filtering out O_EXCL and O_CREAT here? This is something that should've been caught by the first openat, right? Or is this just to not have a TOCTOU race on the file existing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're internally reopening the file we just created. So if the user provided O_EXCL we need to filter that out. We might as well drop O_CREAT too.

if (fd == -1)
return -errno;
}

/* Handle O_TRUNC here only after verifying hardlink state */
if (finfo->flags & O_TRUNC)
{
Expand Down Expand Up @@ -521,6 +585,7 @@ struct fuse_operations callback_oper = {
enum {
KEY_HELP,
KEY_VERSION,
KEY_COPYUP,
};

static void
Expand Down Expand Up @@ -565,6 +630,9 @@ rofs_parse_opt (void *data, const char *arg, int key,
case KEY_HELP:
usage (outargs->argv[0]);
exit (EXIT_SUCCESS);
case KEY_COPYUP:
opt_copyup = TRUE;
return 0;
default:
fprintf (stderr, "see `%s -h' for usage\n", outargs->argv[0]);
exit (EXIT_FAILURE);
Expand All @@ -577,6 +645,7 @@ static struct fuse_opt rofs_opts[] = {
FUSE_OPT_KEY ("--help", KEY_HELP),
FUSE_OPT_KEY ("-V", KEY_VERSION),
FUSE_OPT_KEY ("--version", KEY_VERSION),
FUSE_OPT_KEY ("--copyup", KEY_COPYUP),
FUSE_OPT_END
};

Expand Down
22 changes: 21 additions & 1 deletion tests/test-rofiles-fuse.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ skip_without_user_xattrs

setup_test_repository "bare"

echo "1..8"
echo "1..11"

cd ${test_tmpdir}
mkdir mnt
Expand Down Expand Up @@ -117,3 +117,23 @@ echo "ok checkout copy fallback"

# check that O_RDONLY|O_CREAT is handled correctly; used by flock(1) at least
flock mnt/nonexistent-file echo "ok create file in ro mode"
echo "ok flock"

# And now with --copyup enabled

fusermount -u ${test_tmpdir}/mnt
assert_not_has_file mnt/firstfile
rofiles-fuse --copyup checkout-test2 mnt
assert_file_has_content mnt/firstfile first
echo "ok copyup mount"

firstfile_orig_inode=$(stat -c %i checkout-test2/firstfile)
for path in firstfile{,-link}; do
echo truncating > mnt/${path}
assert_file_has_content mnt/${path} truncating
assert_not_file_has_content mnt/${path} first
done
firstfile_new_inode=$(stat -c %i checkout-test2/firstfile)
assert_not_streq "${firstfile_orig_inode}" "${firstfile_new_inode}"

echo "ok copyup"