From 149ddca5a1e031efbc66b3d6c513425b1ef16f19 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Dec 2017 11:10:35 -0500 Subject: [PATCH 1/3] lib/core: Add a "break hardlink" API This imports the code from rpm-ostree: https://github.com/projectatomic/rpm-ostree/blob/9ff9f6c997d914cb7d97d6b59d8045ba64a1882c/src/libpriv/rpmostree-util.c#L742 I plan to use this for rofiles-fuse to implement copyup: https://github.com/ostreedev/ostree/issues/1377 But it's just obviously generally useful for projects using libostree I think. --- src/libostree/libostree-devel.sym | 1 + src/libostree/ostree-core.c | 80 +++++++++++++++++++++++++++++++ src/libostree/ostree-core.h | 7 +++ tests/test-basic-c.c | 68 ++++++++++++++++++++++++++ 4 files changed, 156 insertions(+) diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index ca3afa872a..582e8c92f2 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -22,6 +22,7 @@ LIBOSTREE_2017.15 { ostree_repo_fsck_object; ostree_repo_mark_commit_partial; + ostree_break_hardlink; } LIBOSTREE_2017.14; /* Stub section for the stable release *after* this development one; don't diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index c029aa4793..7c1a3f99bc 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -749,6 +749,86 @@ ostree_content_file_parse (gboolean compressed, cancellable, error); } +/** + * ostree_break_hardlink: + * @dfd: Directory fd + * @path: Path relative to @dfd + * @skip_xattrs: Do not copy extended attributes + * @error: error + * + * In many cases using libostree, a program may need to "break" + * hardlinks by performing a copy. For example, in order to + * logically append to a file. + * + * This function performs full copying, including e.g. extended + * attributes and permissions of both regular files and symbolic links. + * + * If the file is not hardlinked, this function does nothing and + * returns successfully. + * + * This function does not perform synchronization via `fsync()` or + * `fdatasync()`; the idea is this will commonly be done as part + * of an `ostree_repo_commit_transaction()`, which itself takes + * care of synchronization. + * + * Since: 2017.15 + */ +gboolean ostree_break_hardlink (int dfd, + const char *path, + gboolean skip_xattrs, + GCancellable *cancellable, + GError **error) +{ + struct stat stbuf; + + if (!glnx_fstatat (dfd, path, &stbuf, AT_SYMLINK_NOFOLLOW, error)) + return FALSE; + + if (!S_ISLNK (stbuf.st_mode) && !S_ISREG (stbuf.st_mode)) + return glnx_throw (error, "Unsupported type for entry '%s'", path); + + const GLnxFileCopyFlags copyflags = + skip_xattrs ? GLNX_FILE_COPY_NOXATTRS : 0; + + if (stbuf.st_nlink > 1) + { + guint count; + gboolean copy_success = FALSE; + char *path_tmp = glnx_strjoina (path, ".XXXXXX"); + + for (count = 0; count < 100; count++) + { + g_autoptr(GError) tmp_error = NULL; + + glnx_gen_temp_name (path_tmp); + + if (!glnx_file_copy_at (dfd, path, &stbuf, dfd, path_tmp, copyflags, + cancellable, &tmp_error)) + { + if (g_error_matches (tmp_error, G_IO_ERROR, G_IO_ERROR_EXISTS)) + continue; + g_propagate_error (error, g_steal_pointer (&tmp_error)); + return FALSE; + } + + copy_success = TRUE; + break; + } + + if (!copy_success) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_EXISTS, + "Exceeded limit of %u file creation attempts", count); + return FALSE; + } + + if (!glnx_renameat (dfd, path_tmp, dfd, path, error)) + return FALSE; + } + + return TRUE; +} + /** * ostree_checksum_file_from_input: * @file_info: File information diff --git a/src/libostree/ostree-core.h b/src/libostree/ostree-core.h index 3e3631fb0b..01dded94a0 100644 --- a/src/libostree/ostree-core.h +++ b/src/libostree/ostree-core.h @@ -438,6 +438,13 @@ gboolean ostree_checksum_file (GFile *f, GCancellable *cancellable, GError **error); +_OSTREE_PUBLIC +gboolean ostree_break_hardlink (int dfd, + const char *path, + gboolean skip_xattrs, + GCancellable *cancellable, + GError **error); + /** * OstreeChecksumFlags: * diff --git a/tests/test-basic-c.c b/tests/test-basic-c.c index f7e8543846..4ca379e8a1 100644 --- a/tests/test-basic-c.c +++ b/tests/test-basic-c.c @@ -22,6 +22,7 @@ #include #include #include +#include #include "libglnx.h" #include "libostreetest.h" @@ -236,6 +237,72 @@ test_object_writes (gconstpointer data) } } +static gboolean +impl_test_break_hardlink (int tmp_dfd, + const char *path, + GError **error) +{ + const char *linkedpath = glnx_strjoina (path, ".link"); + struct stat orig_stbuf; + if (!glnx_fstatat (tmp_dfd, path, &orig_stbuf, AT_SYMLINK_NOFOLLOW, error)) + return FALSE; + + /* Calling ostree_break_hardlink() should be a noop */ + struct stat stbuf; + if (!ostree_break_hardlink (tmp_dfd, path, TRUE, NULL, error)) + return FALSE; + if (!glnx_fstatat (tmp_dfd, path, &stbuf, AT_SYMLINK_NOFOLLOW, error)) + return FALSE; + + g_assert_cmpint (orig_stbuf.st_dev, ==, stbuf.st_dev); + g_assert_cmpint (orig_stbuf.st_ino, ==, stbuf.st_ino); + + if (linkat (tmp_dfd, path, tmp_dfd, linkedpath, 0) < 0) + return glnx_throw_errno_prefix (error, "linkat"); + + if (!ostree_break_hardlink (tmp_dfd, path, TRUE, NULL, error)) + return FALSE; + if (!glnx_fstatat (tmp_dfd, path, &stbuf, AT_SYMLINK_NOFOLLOW, error)) + return FALSE; + /* This file should be different */ + g_assert_cmpint (orig_stbuf.st_dev, ==, stbuf.st_dev); + g_assert_cmpint (orig_stbuf.st_ino, !=, stbuf.st_ino); + /* But this one is still the same */ + if (!glnx_fstatat (tmp_dfd, linkedpath, &stbuf, AT_SYMLINK_NOFOLLOW, error)) + return FALSE; + g_assert_cmpint (orig_stbuf.st_dev, ==, stbuf.st_dev); + g_assert_cmpint (orig_stbuf.st_ino, ==, stbuf.st_ino); + + (void) unlinkat (tmp_dfd, path, 0); + (void) unlinkat (tmp_dfd, linkedpath, 0); + + return TRUE; +} + +static void +test_break_hardlink (void) +{ + int tmp_dfd = AT_FDCWD; + g_autoptr(GError) error = NULL; + + /* Regular file */ + const char hello_hardlinked_content[] = "hello hardlinked content"; + glnx_file_replace_contents_at (tmp_dfd, "test-hardlink", + (guint8*)hello_hardlinked_content, + strlen (hello_hardlinked_content), + GLNX_FILE_REPLACE_NODATASYNC, + NULL, &error); + g_assert_no_error (error); + (void)impl_test_break_hardlink (tmp_dfd, "test-hardlink", &error); + g_assert_no_error (error); + + /* Symlink */ + if (symlinkat ("some-path", tmp_dfd, "test-symhardlink") < 0) + err (1, "symlinkat"); + (void)impl_test_break_hardlink (tmp_dfd, "test-symhardlink", &error); + g_assert_no_error (error); +} + static GVariant* xattr_cb (OstreeRepo *repo, const char *path, @@ -376,6 +443,7 @@ int main (int argc, char **argv) g_test_add_data_func ("/raw-file-to-archive-stream", repo, test_raw_file_to_archive_stream); g_test_add_data_func ("/objectwrites", repo, test_object_writes); g_test_add_func ("/xattrs-devino-cache", test_devino_cache_xattrs); + g_test_add_func ("/break-hardlink", test_break_hardlink); g_test_add_func ("/remotename", test_validate_remotename); return g_test_run(); From 7dc56759f25c5082d8876bf1ffd6ffda14482e4b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Dec 2017 12:09:28 -0500 Subject: [PATCH 2/3] lib/core: Optimize breaking hardlinks for regfiles It'd all be really nice if there was some sort of `O_TMPFILE` for symlinks, but anyways the way we were doing a generic "make temp file than rename" actually defeats some of the point of `O_TMPFILE`. It's now fully safe to do "copy to self", so let's do that for regfiles. --- src/libostree/ostree-core.c | 101 ++++++++++++++++++++++-------------- 1 file changed, 61 insertions(+), 40 deletions(-) diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index 7c1a3f99bc..679c952914 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -749,6 +749,50 @@ ostree_content_file_parse (gboolean compressed, cancellable, error); } +static gboolean +break_symhardlink (int dfd, + const char *path, + struct stat *stbuf, + GLnxFileCopyFlags copyflags, + GCancellable *cancellable, + GError **error) +{ + guint count; + gboolean copy_success = FALSE; + char *path_tmp = glnx_strjoina (path, ".XXXXXX"); + + for (count = 0; count < 100; count++) + { + g_autoptr(GError) tmp_error = NULL; + + glnx_gen_temp_name (path_tmp); + + if (!glnx_file_copy_at (dfd, path, stbuf, dfd, path_tmp, copyflags, + cancellable, &tmp_error)) + { + if (g_error_matches (tmp_error, G_IO_ERROR, G_IO_ERROR_EXISTS)) + continue; + g_propagate_error (error, g_steal_pointer (&tmp_error)); + return FALSE; + } + + copy_success = TRUE; + break; + } + + if (!copy_success) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_EXISTS, + "Exceeded limit of %u file creation attempts", count); + return FALSE; + } + + if (!glnx_renameat (dfd, path_tmp, dfd, path, error)) + return FALSE; + + return TRUE; +} + /** * ostree_break_hardlink: * @dfd: Directory fd @@ -784,48 +828,25 @@ gboolean ostree_break_hardlink (int dfd, if (!glnx_fstatat (dfd, path, &stbuf, AT_SYMLINK_NOFOLLOW, error)) return FALSE; - if (!S_ISLNK (stbuf.st_mode) && !S_ISREG (stbuf.st_mode)) + if (stbuf.st_nlink <= 1) + return TRUE; /* Note early return */ + + const GLnxFileCopyFlags copyflags = skip_xattrs ? GLNX_FILE_COPY_NOXATTRS : 0; + + if (S_ISREG (stbuf.st_mode)) + /* Note it's now completely safe to copy a file to itself, + * as glnx_file_copy_at() is documented to do an O_TMPFILE + rename() + * with GLNX_FILE_COPY_OVERWRITE. + */ + return glnx_file_copy_at (dfd, path, &stbuf, dfd, path, + copyflags | GLNX_FILE_COPY_OVERWRITE, + cancellable, error); + else if (S_ISLNK (stbuf.st_mode)) + return break_symhardlink (dfd, path, &stbuf, copyflags, + cancellable, error); + else return glnx_throw (error, "Unsupported type for entry '%s'", path); - const GLnxFileCopyFlags copyflags = - skip_xattrs ? GLNX_FILE_COPY_NOXATTRS : 0; - - if (stbuf.st_nlink > 1) - { - guint count; - gboolean copy_success = FALSE; - char *path_tmp = glnx_strjoina (path, ".XXXXXX"); - - for (count = 0; count < 100; count++) - { - g_autoptr(GError) tmp_error = NULL; - - glnx_gen_temp_name (path_tmp); - - if (!glnx_file_copy_at (dfd, path, &stbuf, dfd, path_tmp, copyflags, - cancellable, &tmp_error)) - { - if (g_error_matches (tmp_error, G_IO_ERROR, G_IO_ERROR_EXISTS)) - continue; - g_propagate_error (error, g_steal_pointer (&tmp_error)); - return FALSE; - } - - copy_success = TRUE; - break; - } - - if (!copy_success) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_EXISTS, - "Exceeded limit of %u file creation attempts", count); - return FALSE; - } - - if (!glnx_renameat (dfd, path_tmp, dfd, path, error)) - return FALSE; - } - return TRUE; } From e4c32efa295fbb7f2b71c58df280e73efdc6e4b3 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Dec 2017 13:07:27 -0500 Subject: [PATCH 3/3] fixup! lib/core: Add a "break hardlink" API --- apidoc/ostree-sections.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index d60e8f2a10..d3cf1c68f7 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -133,6 +133,7 @@ ostree_content_file_parse_at ostree_raw_file_to_archive_z2_stream ostree_raw_file_to_archive_z2_stream_with_options ostree_raw_file_to_content_stream +ostree_break_hardlink ostree_checksum_file_from_input ostree_checksum_file ostree_checksum_file_at