[ostree] deploy: Copy parent directories for modified config files
- From: Colin Walters <walters src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [ostree] deploy: Copy parent directories for modified config files
- Date: Tue, 16 Sep 2014 16:08:22 +0000 (UTC)
commit 8f4ffa6950ace466f9338d5e6960e198ae3813a0
Author: Colin Walters <walters verbum org>
Date: Mon Sep 15 09:29:26 2014 -0400
deploy: Copy parent directories for modified config files
Previously, in the case where a parent directory of a modified config
file was removed, we would throw an exception. This happens when
switching from a tree that has some software (e.g. firewalld), to one
that does not.
While it's nice to have this warning that your config file probably no
longer applies, there's no need to make it so...fatal.
It's particularly problematic that the only easy workaround is to
remove the config files from your current tree - which breaks
rollback.
The solution then is for for us to take ownership of the parent
directories too into the new /etc. Admins can clean up these files
afterwards at any time.
https://bugzilla.gnome.org/show_bug.cgi?id=734293
src/libostree/ostree-sysroot-deploy.c | 187 ++++++++++++++++-------
tests/test-admin-deploy-etcmerge-cornercases.sh | 32 +++-
2 files changed, 158 insertions(+), 61 deletions(-)
---
diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
index fbb3ea1..905854f 100644
--- a/src/libostree/ostree-sysroot-deploy.c
+++ b/src/libostree/ostree-sysroot-deploy.c
@@ -156,6 +156,50 @@ copy_one_file_fsync_at (int src_parent_dfd,
}
static gboolean
+dirfd_copy_attributes_and_xattrs (int src_parent_dfd,
+ const char *src_name,
+ int src_dfd,
+ int dest_dfd,
+ GCancellable *cancellable,
+ GError **error)
+{
+ gboolean ret = FALSE;
+ struct stat src_stbuf;
+ gs_unref_variant GVariant *xattrs = NULL;
+
+ /* Clone all xattrs first, so we get the SELinux security context
+ * right. This will allow other users access if they have ACLs, but
+ * oh well.
+ */
+ if (!dfd_and_name_get_all_xattrs (src_parent_dfd, src_name,
+ &xattrs, cancellable, error))
+ goto out;
+ if (!gs_fd_set_all_xattrs (dest_dfd, xattrs,
+ cancellable, error))
+ goto out;
+
+ if (fstat (src_dfd, &src_stbuf) != 0)
+ {
+ ot_util_set_error_from_errno (error, errno);
+ goto out;
+ }
+ if (fchown (dest_dfd, src_stbuf.st_uid, src_stbuf.st_gid) != 0)
+ {
+ ot_util_set_error_from_errno (error, errno);
+ goto out;
+ }
+ if (fchmod (dest_dfd, src_stbuf.st_mode) != 0)
+ {
+ ot_util_set_error_from_errno (error, errno);
+ goto out;
+ }
+
+ ret = TRUE;
+ out:
+ return ret;
+}
+
+static gboolean
copy_dir_recurse_fsync (int src_parent_dfd,
int dest_parent_dfd,
const char *name,
@@ -163,12 +207,10 @@ copy_dir_recurse_fsync (int src_parent_dfd,
GError **error)
{
gboolean ret = FALSE;
- struct stat src_stbuf;
int src_dfd = -1;
int dest_dfd = -1;
DIR *srcd = NULL;
struct dirent *dent;
- gs_unref_variant GVariant *xattrs = NULL;
if (!ot_gopendirat (src_parent_dfd, name, TRUE, &src_dfd, error))
goto out;
@@ -183,18 +225,10 @@ copy_dir_recurse_fsync (int src_parent_dfd,
if (!ot_gopendirat (dest_parent_dfd, name, TRUE, &dest_dfd, error))
goto out;
- /* Clone all xattrs first, so we get the SELinux security context
- * right. This will allow other users access if they have ACLs, but
- * oh well.
- */
- if (!dfd_and_name_get_all_xattrs (src_parent_dfd, name,
- &xattrs,
- cancellable, error))
- goto out;
- if (!gs_fd_set_all_xattrs (dest_dfd, xattrs,
- cancellable, error))
+ if (!dirfd_copy_attributes_and_xattrs (src_parent_dfd, name, src_dfd, dest_dfd,
+ cancellable, error))
goto out;
-
+
srcd = fdopendir (src_dfd);
if (!srcd)
{
@@ -233,22 +267,6 @@ copy_dir_recurse_fsync (int src_parent_dfd,
}
}
- if (fstat (src_dfd, &src_stbuf) != 0)
- {
- ot_util_set_error_from_errno (error, errno);
- goto out;
- }
- if (fchown (dest_dfd, src_stbuf.st_uid, src_stbuf.st_gid) != 0)
- {
- ot_util_set_error_from_errno (error, errno);
- goto out;
- }
- if (fchmod (dest_dfd, src_stbuf.st_mode) != 0)
- {
- ot_util_set_error_from_errno (error, errno);
- goto out;
- }
-
/* And finally, fsync the fd */
if (fsync (dest_dfd) != 0)
{
@@ -271,15 +289,88 @@ copy_dir_recurse_fsync (int src_parent_dfd,
return ret;
}
+static gboolean
+ensure_directory_from_template (int orig_etc_fd,
+ int modified_etc_fd,
+ int new_etc_fd,
+ const char *path,
+ int *out_dfd,
+ GCancellable *cancellable,
+ GError **error)
+{
+ gboolean ret = FALSE;
+ int src_dfd = -1;
+ int target_dfd = -1;
+
+ g_assert (path != NULL);
+ g_assert (*path != '/' && *path != '\0');
+
+ if (!ot_gopendirat (modified_etc_fd, path, TRUE, &src_dfd, error))
+ goto out;
+
+ /* Create with mode 0700, we'll fchmod/fchown later */
+ again:
+ if (mkdirat (new_etc_fd, path, 0700) != 0)
+ {
+ if (errno == EEXIST)
+ {
+ /* Fall through */
+ }
+ else if (errno == ENOENT)
+ {
+ gs_free char *parent_path = g_path_get_dirname (path);
+
+ if (strcmp (parent_path, ".") != 0)
+ {
+ if (!ensure_directory_from_template (orig_etc_fd, modified_etc_fd, new_etc_fd,
+ parent_path, NULL, cancellable, error))
+ goto out;
+
+ /* Loop */
+ goto again;
+ }
+ else
+ {
+ /* Fall through...shouldn't happen, but we'll propagate
+ * an error from open. */
+ }
+ }
+ else
+ {
+ ot_util_set_error_from_errno (error, errno);
+ g_prefix_error (error, "mkdirat: ");
+ goto out;
+ }
+ }
+
+ if (!ot_gopendirat (new_etc_fd, path, TRUE, &target_dfd, error))
+ goto out;
+
+ if (!dirfd_copy_attributes_and_xattrs (modified_etc_fd, path, src_dfd, target_dfd,
+ cancellable, error))
+ goto out;
+
+ ret = TRUE;
+ if (out_dfd)
+ {
+ g_assert (target_dfd != -1);
+ *out_dfd = target_dfd;
+ target_dfd = -1;
+ }
+ out:
+ if (src_dfd != -1)
+ (void) close (src_dfd);
+ if (target_dfd != -1)
+ (void) close (target_dfd);
+ return ret;
+}
+
/**
* copy_modified_config_file:
*
* Copy @file from @modified_etc to @new_etc, overwriting any existing
* file there. The @file may refer to a regular file, a symbolic
* link, or a directory. Directories will be copied recursively.
- *
- * Note this function does not (yet) handle the case where a directory
- * needed by a modified file is deleted in a newer tree.
*/
static gboolean
copy_modified_config_file (int orig_etc_fd,
@@ -292,8 +383,6 @@ copy_modified_config_file (int orig_etc_fd,
gboolean ret = FALSE;
struct stat modified_stbuf;
struct stat new_stbuf;
- const char *parent_slash;
- gs_free char *parent_path = NULL;
int dest_parent_dfd = -1;
if (fstatat (modified_etc_fd, path, &modified_stbuf, AT_SYMLINK_NOFOLLOW) < 0)
@@ -303,30 +392,16 @@ copy_modified_config_file (int orig_etc_fd,
goto out;
}
- parent_slash = strrchr (path, '/');
- if (parent_slash != NULL)
+ if (strchr (path, '/') != NULL)
{
- parent_path = g_strndup (path, parent_slash - path);
- dest_parent_dfd = ot_opendirat (new_etc_fd, parent_path, FALSE);
- if (dest_parent_dfd == -1)
- {
- if (errno == ENOENT)
- {
- g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
- "New tree removes parent directory '%s', cannot merge",
- parent_path);
- }
- else
- {
- g_prefix_error (error, "openat: ");
- ot_util_set_error_from_errno (error, errno);
- }
- goto out;
- }
+ gs_free char *parent = g_path_get_dirname (path);
+
+ if (!ensure_directory_from_template (orig_etc_fd, modified_etc_fd, new_etc_fd,
+ parent, &dest_parent_dfd, cancellable, error))
+ goto out;
}
else
{
- parent_path = NULL;
dest_parent_dfd = dup (new_etc_fd);
if (dest_parent_dfd == -1)
{
@@ -335,6 +410,8 @@ copy_modified_config_file (int orig_etc_fd,
}
}
+ g_assert (dest_parent_dfd != -1);
+
if (fstatat (new_etc_fd, path, &new_stbuf, AT_SYMLINK_NOFOLLOW) < 0)
{
if (errno == ENOENT)
@@ -351,7 +428,7 @@ copy_modified_config_file (int orig_etc_fd,
{
g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
"Modified config file newly defaults to directory '%s', cannot merge",
- parent_path);
+ path);
goto out;
}
else
diff --git a/tests/test-admin-deploy-etcmerge-cornercases.sh b/tests/test-admin-deploy-etcmerge-cornercases.sh
index 1790094..1464fb6 100644
--- a/tests/test-admin-deploy-etcmerge-cornercases.sh
+++ b/tests/test-admin-deploy-etcmerge-cornercases.sh
@@ -37,8 +37,12 @@ echo "rev=${rev}"
ostree admin --sysroot=sysroot deploy --karg=root=LABEL=MOO --karg=quiet --os=testos
testos:testos/buildmaster/x86_64-runtime
assert_has_dir sysroot/boot/ostree/testos-${bootcsum}
-# Ok, let's create a long directory chain with custom permissions
etc=sysroot/ostree/deploy/testos/deploy/${rev}.0/etc
+
+# modified config file
+echo "a modified config file" > ${etc}/NetworkManager/nm.conf
+
+# Ok, let's create a long directory chain with custom permissions
mkdir -p ${etc}/a/long/dir/chain
mkdir -p ${etc}/a/long/dir/forking
chmod 700 ${etc}/a
@@ -47,14 +51,24 @@ chmod 777 ${etc}/a/long/dir
chmod 707 ${etc}/a/long/dir/chain
chmod 700 ${etc}/a/long/dir/forking
+# Symlink to nonexistent path, to ensure we aren't walking symlinks
+ln -s no-such-file ${etc}/a/link-to-no-such-file
+
+# Remove a directory
+rm ${etc}/testdirectory -rf
+
# Now deploy a new commit
os_repository_new_commit
ostree --repo=sysroot/ostree/repo remote add --set=gpg-verify=false testos file://$(pwd)/testos-repo
testos/buildmaster/x86_64-runtime
ostree admin --sysroot=sysroot upgrade --os=testos
newrev=$(ostree --repo=sysroot/ostree/repo rev-parse testos/buildmaster/x86_64-runtime)
echo "newrev=${newrev}"
+newroot=sysroot/ostree/deploy/testos/deploy/${newrev}.0
+newetc=${newroot}/etc
+
+assert_file_has_content ${newroot}/usr/etc/NetworkManager/nm.conf "a default daemon file"
+assert_file_has_content ${newetc}/NetworkManager/nm.conf "a modified config file"
-newetc=sysroot/ostree/deploy/testos/deploy/${newrev}.0/etc
assert_file_has_mode() {
stat -c '%a' $1 > mode.txt
if ! grep -q -e "$2" mode.txt; then
@@ -63,6 +77,7 @@ assert_file_has_mode() {
fi
rm -f mode.txt
}
+
assert_file_has_mode ${newetc}/a 700
assert_file_has_mode ${newetc}/a/long 770
assert_file_has_mode ${newetc}/a/long/dir 777
@@ -70,6 +85,11 @@ assert_file_has_mode ${newetc}/a/long/dir/chain 707
assert_file_has_mode ${newetc}/a/long/dir/forking 700
assert_file_has_mode ${newetc}/a/long/dir 777
+test -L ${newetc}/a/link-to-no-such-file || assert_not_reached "should have symlink"
+
+assert_has_dir ${newroot}/usr/etc/testdirectory
+assert_not_has_dir ${newetc}/testdirectory
+
echo "ok"
# Add /etc/initially-empty
@@ -116,10 +136,10 @@ ostree --repo=${test_tmpdir}/testos-repo commit -b testos/buildmaster/x86_64-run
cd ${test_tmpdir}
newconfpath=sysroot/ostree/deploy/testos/deploy/${rev}.0/etc/initially-empty/mynewfile
touch ${newconfpath}
-if ostree admin --sysroot=sysroot upgrade --os=testos 2>err.txt; then
- assert_not_reached "upgrade should have failed"
-fi
-assert_file_has_content err.txt "New tree removes parent directory"
+ostree admin --sysroot=sysroot upgrade --os=testos
+rev=$(ostree --repo=sysroot/ostree/repo rev-parse testos/buildmaster/x86_64-runtime)
+assert_not_has_file sysroot/ostree/deploy/testos/deploy/${rev}.0/usr/etc/initially-empty
+assert_has_file sysroot/ostree/deploy/testos/deploy/${rev}.0/etc/initially-empty/mynewfile
rm ${newconfpath}
echo "ok"
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
