Rework devino caching to support invalidation if metadata changes #1170

jlebon · 2017-09-13T16:51:22Z

cgwalters · 2017-09-13T17:00:12Z

src/libostree/ostree-repo-commit.c

@@ -2858,16 +2858,6 @@ write_dfd_iter_to_mtree_internal (OstreeRepo                  *self,
      if (!glnx_fstatat (src_dfd_iter->fd, dent->d_name, &stbuf, AT_SYMLINK_NOFOLLOW, error))
        return FALSE;

-      const char *loose_checksum = devino_cache_lookup (self, modifier, stbuf.st_dev, stbuf.st_ino);


I think this patch should be included with the last commit since they're not
really independent in the sense that I think we were doing this early lookup
intentionally before. It was intending to be a fast path.

cgwalters · 2017-09-13T17:09:08Z

src/libostree/ostree-repo-commit.c

-      loose_checksum = devino_cache_lookup (self, modifier,
-                                            g_file_info_get_attribute_uint32 (child_info, "unix::device"),
-                                            g_file_info_get_attribute_uint64 (child_info, "unix::inode"));
+      /* only check the devino cache if the file info was not modified */


What about xattrs? For the rpm-ostree use case it'd make commit notably more expensive as suddenly we'd have to load and compare the xattrs.

If we're explicitly not comparing xattrs then let's at least add a comment.

Ahh of course, we need to check xattrs too. Will add that.

Hmm, do you mean in the rpm-ostree local assembly path? Though we don't provide an xattr modifier when checking out pkgs there, so we can probably do it in a smart way so we don't even bother comparing in such cases.

Hmm...but actually in the layering case I think we should be setting the sepolicy to ensure we relabel any files created by scripts. (In practice this probably doesn't matter much and really everything should mostly be usr_t but still).

jlebon · 2017-09-13T22:43:51Z

OK, this now also checks for modifications to extended attributes. One thing I'm not sure of is:

This patch does slightly change the semantics of the xattr callback.
Previously, returning NULL from the cb meant no xattrs at all. Now, it
means to default to the on-disk state. We might want to consider putting
that behind a flag instead. It seems like a more useful behaviour so
that callers can only override the files they want to without losing
original on-disk state (and if they don't want that, just return an
empty GVariant).

Without that change, really there's not much point in using a devino cache if also using an xattr cb.

cgwalters · 2017-09-25T15:28:53Z

Pull out the first commit to a separate PR?

cgwalters · 2017-09-25T15:29:36Z

I'd like to defer this until we've had a chance to discuss in #1165

rh-atomic-bot · 2017-09-26T17:31:10Z

☔ The latest upstream changes (presumably e44631e) made this pull request unmergeable. Please resolve the merge conflicts.

cgwalters · 2017-09-27T16:01:19Z

src/libostree/ostree-repo-commit.c

@@ -2557,6 +2571,7 @@ write_directory_content_to_mtree_internal (OstreeRepo                  *self,
  g_autoptr(GFileInfo) modified_info = NULL;
  OstreeRepoCommitFilterResult filter_result =
    _ostree_repo_commit_modifier_apply (self, modifier, child_relpath, child_info, &modified_info);
+  const gboolean child_info_was_modified = (child_info != modified_info);


I think my main problem is this - if we have say a commit filter active and that filter just wants to change some paths like gnome-continuous does with suid binaries, this will still forcibly re-checksum everything since the info will be different.

I think we need to compare it structurally the same way we do xattrs.

This allows users to easily re-initialize the test repo.

jlebon · 2017-09-28T19:18:04Z

Rebased and updated to also inspect the GFileInfo. ⬆️

cgwalters · 2017-09-28T19:56:20Z

tests/libtest.sh

@@ -141,6 +141,7 @@ setup_test_repository () {
    fi

    cd ${test_tmpdir}
+    rm -rf repo


This makes sense, though FWIW I tend to do:

$ make check TESTS=tests/test-commit-sign.sh TEST_SKIP_CLEANUP=err $ make && sudo make install $ cd /var/tmp/tap-test.XXX... $ ostree ...

cgwalters · 2017-09-28T20:00:25Z

src/libostree/ostree-core.c

+
+  /* those aren't stored by ostree, but used by the devino cache */
+  g_file_info_set_attribute_uint32 (ret, "unix::device", stbuf->st_dev);
+  g_file_info_set_attribute_uint64 (ret, "unix::inode", stbuf->st_ino);


Ooh, tricky.

cgwalters · 2017-09-28T20:02:53Z

src/libostree/ostree-repo-commit.c

+  /* fetch on-disk xattrs if needed & not disabled */
+  g_autoptr(GVariant) original_xattrs = NULL;
+  if (!(modifier && (modifier->flags & (OSTREE_REPO_COMMIT_MODIFIER_FLAGS_SKIP_XATTRS |
+                                        OSTREE_REPO_COMMIT_MODIFIER_FLAGS_CANONICAL_PERMISSIONS)) > 0)


Not new, but maybe while we're here, host out the flags checking like:

const gboolean flags_skip_xattrs = (modifier->flags & (OSTREE_REPO_COMMIT_MODIFIER_FLAGS_SKIP_XATTRS | OREE_REPO_COMMIT_MODIFIER_FLAGS_CANONICAL_PERMISSIONS)) > 0; if (!(modifier && flags_skip_xattrs && !self->disable_xattrs)) { ...

cgwalters · 2017-09-28T20:14:26Z

tests/basic-test.sh

+assert_not_file_has_content diff-test2 'M */baz/cowro$'
+assert_not_file_has_content diff-test2 'baz/saucer'
+# only /baz/cow is a cache miss
+assert_file_has_content stats.txt '^Content Written: 1$'


Ah, right I forgot we already had a way to unit test this. Cool.

cgwalters · 2017-09-28T20:15:57Z

tests/test-basic-c.c

+
+  if (on_overlay)
+    {
+      g_test_skip ("overlayfs detected");


This is fine for now, though a more accurate fix would be porting the "can relabel" logic. Though OTOH I agree it is perhaps easier for dev testing to set the directory. Maybe we want TMPDIR_PREFIX though?

cgwalters · 2017-09-28T20:30:39Z

Eh, might as well zap 🗲 this one; we can do my one minor style tweak later.

@rh-atomic-bot r+ 211e748

rh-atomic-bot · 2017-09-28T20:49:53Z

⌛ Testing commit 211e748 with merge d6e76b2...

This allows users to easily re-initialize the test repo. Closes: #1170 Approved by: cgwalters

This was originally part of #1199, but never got in. The work in #1218 is awesome for CI, though for the developer workflow, I find it much easier to just set $TEST_TMPDIR to a non-overlayfs mount point. Teach the test libraries to just look at $PWD rather than /. Also export the function that does the overlayfs checking. Prep for a future patch. Closes: #1170 Approved by: cgwalters

We can't use the cache if the file we want to commit has been modified by the client through the file info or xattr modifiers. We would prematurely look into the cache in `write_dfd_iter_to_mtree_internal`, regardless of whether any filtering applied. We remove that path there, and make sure that we only use the cache if there were no modifications. We rename the `get_modified_xattrs` to `get_final_xattrs` to reflect the fact that the xattrs may not be modified. One tricky bit that took me some time was that we now need to store the st_dev & st_ino values in the GFileInfo because the cache lookup relies on it. I'm guessing we regressed on this at some point. This patch does slightly change the semantics of the xattr callback. Previously, returning NULL from the cb meant no xattrs at all. Now, it means to default to the on-disk state. We might want to consider putting that behind a flag instead. Though it seems like a more useful behaviour so that callers can only override the files they want to without losing original on-disk state (and if they don't want that, just return an empty GVariant). Closes: #1165 Closes: #1170 Approved by: cgwalters

rh-atomic-bot · 2017-09-28T20:59:17Z

💔 Test failed - status-atomicjenkins

smcv · 2017-09-29T14:54:37Z

tests/libostreetest.c

+{
+  /* Keep this in sync with the overlayfs bits in libtest.sh */
+  struct statfs stbuf;
+  const char *pwd = g_getenv ("PWD");


Surely g_get_current_dir() (needs to be assigned to a g_autofree gchar *), or simply replace pwd with "." below?

I'm not completely sure of the startup sequence of this stuff, but if the test isn't guaranteed to have been started by a shell designed for interactive use, then I don't think PWD is guaranteed to be up-to-date.

According to its documentation, g_get_current_dir() returns PWD, but only after verifying that PWD is in fact the same directory as ".".

Sure. I had started with getenv ("TEST_TMPDIR") at first, though changed it afterwards since it doesn't actually appear when running with gdtr.

smcv · 2017-09-29T14:56:12Z

tests/libtest.sh

@@ -73,7 +73,7 @@ export OSTREE_GPG_HOME=${test_tmpdir}/gpghome/trusted
 # See comment in ot-builtin-commit.c and https://github.com/ostreedev/ostree/issues/758
 # Also keep this in sync with the bits in libostreetest.c
 echo evaluating for overlayfs...
-case $(stat -f --printf '%T' /) in
+case $(stat -f --printf '%T' $PWD) in


Same here: this could be . (although at least this is a bash script so PWD is guaranteed(?) to be set and up-to-date)

smcv · 2017-09-29T14:57:43Z

tests/test-basic-c.c

+  g_assert (ret);
+
+  gboolean on_overlay;
+  ret = ot_check_for_overlay (&on_overlay, &error);


If this is a proxy for "check for a filesystem where we can set xattrs", please probe for that instead - tmpfs can't have xattrs either, and for optimal speed (and lack of fsync slowdown), some Debian buildds use a chroot where the entire visible filesystem is on tmpfs.

Yeah, @cgwalters echoed the same sentiments higher up. I'll rework it.

We can't use the cache if the file we want to commit has been modified by the client through the file info or xattr modifiers. We would prematurely look into the cache in `write_dfd_iter_to_mtree_internal`, regardless of whether any filtering applied. We remove that path there, and make sure that we only use the cache if there were no modifications. We rename the `get_modified_xattrs` to `get_final_xattrs` to reflect the fact that the xattrs may not be modified. One tricky bit that took me some time was that we now need to store the st_dev & st_ino values in the GFileInfo because the cache lookup relies on it. I'm guessing we regressed on this at some point. This patch does slightly change the semantics of the xattr callback. Previously, returning NULL from the cb meant no xattrs at all. Now, it means to default to the on-disk state. We might want to consider putting that behind a flag instead. Though it seems like a more useful behaviour so that callers can only override the files they want to without losing original on-disk state (and if they don't want that, just return an empty GVariant). Closes: ostreedev#1165

Instead of checking for overlayfs, let's explicitly check for our ability to relabel files since we now have a `libtest` function to do this. Also port that logic to `libostreetest`. Note that overlayfs *does* allow manipulating user xattrs. So ideally, we should break down `OSTREE_NO_XATTRS` further to distinguish between tests that use bare repos from other modes. We check the current directory instead of `/` so that developers can just point `TEST_TMPDIR` to a non-overlayfs mount point when hacking from a container.

jlebon · 2017-09-29T20:44:18Z

OK, I reworked the tests so that they explicitly check for relabeling support rather than overlayfs. It's a bit confusing because OSTREE_NO_XATTRS should be broken down further depending on what repo/checkout/commit modes we're trying to test. Anyway, I think I did the right thing for now in ⬆️.

Since we now have a subtest there that needs full xattr support.

jlebon · 2017-09-29T21:59:48Z

And now we also run as an installed test to make sure the new xattr test actually runs! ⬆️

cgwalters · 2017-09-29T23:55:00Z

tests/libostreetest.c

+  g_autoptr(GBytes) bytes = glnx_fgetxattr_bytes (tmpf.fd, "security.selinux", &local_error);
+  if (!bytes)
+    {
+      /* libglnx preserves errno */


Hmm...not explicitly. I think it likely mostly works today, but I'm certain that e.g. glnx_null_throw_errno_prefix() might invoke malloc() which can definitely make system calls... if we want to make that guarantee I think we'll need something more like systemd's _PROTECT_ERRNO.

Nevermind, I forgot we do save it.

cgwalters · 2017-09-29T23:58:25Z

tests/libtest.sh

+
+_have_user_xattrs=''
+have_user_xattrs() {
+    assert_has_setfattr


Minor, but maybe move the assertion below the test so we don't call it every time?

cgwalters · 2017-09-30T00:05:01Z

OK, this is looking pretty good to me! Let's get it in, we can do any followups after. Maybe at some point we can stub out a context or case that uses tmpfs so @smvc doesn't have to come in and fix things up after every release.

@rh-atomic-bot r+ 9410a8f

rh-atomic-bot · 2017-09-30T00:05:10Z

⚡ Test exempted: merge already tested.

We can't use the cache if the file we want to commit has been modified by the client through the file info or xattr modifiers. We would prematurely look into the cache in `write_dfd_iter_to_mtree_internal`, regardless of whether any filtering applied. We remove that path there, and make sure that we only use the cache if there were no modifications. We rename the `get_modified_xattrs` to `get_final_xattrs` to reflect the fact that the xattrs may not be modified. One tricky bit that took me some time was that we now need to store the st_dev & st_ino values in the GFileInfo because the cache lookup relies on it. I'm guessing we regressed on this at some point. This patch does slightly change the semantics of the xattr callback. Previously, returning NULL from the cb meant no xattrs at all. Now, it means to default to the on-disk state. We might want to consider putting that behind a flag instead. Though it seems like a more useful behaviour so that callers can only override the files they want to without losing original on-disk state (and if they don't want that, just return an empty GVariant). Closes: #1165 Closes: #1170 Approved by: cgwalters

Instead of checking for overlayfs, let's explicitly check for our ability to relabel files since we now have a `libtest` function to do this. Also port that logic to `libostreetest`. Note that overlayfs *does* allow manipulating user xattrs. So ideally, we should break down `OSTREE_NO_XATTRS` further to distinguish between tests that use bare repos from other modes. We check the current directory instead of `/` so that developers can just point `TEST_TMPDIR` to a non-overlayfs mount point when hacking from a container. Closes: #1170 Approved by: cgwalters

Since we now have a subtest there that needs full xattr support. Closes: #1170 Approved by: cgwalters

…xattrs This is more subtle fallout from: ostreedev#1170 AKA commit: 8fe4536 Before, if we found a devino cache hit, we'd use it unconditionally. Recall that `bare-user` repositories are very special in that they're the only mode where the on disk state ("physical state") is not the "real" state. The latter is stored in the `user.ostreemeta` xattr. (`bare-user` repos are also highly special in that symlinks are regular files physically, but that's not immediately relevant here). Since we now have `bare-user-only` for the "pure unprivileged container" case, `bare-user` should just be used for "OS builds" which have nonzero uids (and possibly SELinux labels etc.) In an experimental tool I'm writing "skopeo2ostree" which imports OCI images into refs, then squashes them together into a single final commit, we lost the the `81` group ID for `/usr/libexec/dbus-1/dbus-daemon-launch-helper`. This happened because the commit code was loading the "physical" disk state, where the uid/gid are zero because that's the uid I happened to be using. We didn't just directly do the link speedup because I was using `--selinux-policy` which caused the xattrs to change, which caused us to re-commit objects from the physical state. The unit test I added actually doesn't quite trigger this, but I left it because "why not". Really testing this requires the installed test which uses SELinux policy from `/`. The behavior without this fix looks like: ``` -00755 0 0 12 { [(b'user.ostreemeta', [byte 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x51, 0x00, 0x00, 0x81, 0xed]), (b'security.selinux', b'system_u:object_r:lib_t:s0')] } /usr/lib/dbus-daemon-helper ``` which was obviously totally broken - we shouldn't be picking up the `user.ostreemeta` xattr and actually committing it of course.

This is more subtle fallout from: ostreedev#1170 AKA commit: 8fe4536 Before, if we found a devino cache hit, we'd use it unconditionally. Recall that `bare-user` repositories are very special in that they're the only mode where the on disk state ("physical state") is not the "real" state. The latter is stored in the `user.ostreemeta` xattr. (`bare-user` repos are also highly special in that symlinks are regular files physically, but that's not immediately relevant here). Since we now have `bare-user-only` for the "pure unprivileged container" case, `bare-user` should just be used for "OS builds" which have nonzero uids (and possibly SELinux labels etc.) In an experimental tool I'm writing "skopeo2ostree" which imports OCI images into refs, then squashes them together into a single final commit, we lost the the `81` group ID for `/usr/libexec/dbus-1/dbus-daemon-launch-helper`. This happened because the commit code was loading the "physical" disk state, where the uid/gid are zero because that's the uid I happened to be using. We didn't just directly do the link speedup because I was using `--selinux-policy` which caused the xattrs to change, which caused us to re-commit objects from the physical state. The unit test I added actually doesn't quite trigger this, but I left it because "why not". Really testing this requires the installed test which uses SELinux policy from `/`. The behavior without this fix looks like: ``` -00755 0 0 12 { [(b'user.ostreemeta', [byte 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x51, 0x00, 0x00, 0x81, 0xed]), (b'security.selinux', b'system_u:object_r:lib_t:s0')] } /usr/lib/dbus-daemon-helper ``` which was obviously totally broken - we shouldn't be picking up the `user.ostreemeta` xattr and actually committing it of course.

This is more subtle fallout from: #1170 AKA commit: 8fe4536 Before, if we found a devino cache hit, we'd use it unconditionally. Recall that `bare-user` repositories are very special in that they're the only mode where the on disk state ("physical state") is not the "real" state. The latter is stored in the `user.ostreemeta` xattr. (`bare-user` repos are also highly special in that symlinks are regular files physically, but that's not immediately relevant here). Since we now have `bare-user-only` for the "pure unprivileged container" case, `bare-user` should just be used for "OS builds" which have nonzero uids (and possibly SELinux labels etc.) In an experimental tool I'm writing "skopeo2ostree" which imports OCI images into refs, then squashes them together into a single final commit, we lost the the `81` group ID for `/usr/libexec/dbus-1/dbus-daemon-launch-helper`. This happened because the commit code was loading the "physical" disk state, where the uid/gid are zero because that's the uid I happened to be using. We didn't just directly do the link speedup because I was using `--selinux-policy` which caused the xattrs to change, which caused us to re-commit objects from the physical state. The unit test I added actually doesn't quite trigger this, but I left it because "why not". Really testing this requires the installed test which uses SELinux policy from `/`. The behavior without this fix looks like: ``` -00755 0 0 12 { [(b'user.ostreemeta', [byte 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x51, 0x00, 0x00, 0x81, 0xed]), (b'security.selinux', b'system_u:object_r:lib_t:s0')] } /usr/lib/dbus-daemon-helper ``` which was obviously totally broken - we shouldn't be picking up the `user.ostreemeta` xattr and actually committing it of course. Closes: #1297 Approved by: jlebon

I was seeing the `Writing OSTree commit...` phase of rpm-ostree being very slow lately. This turns out to be more fallout from ostreedev#1170 AKA commit: 8fe4536 Loading the xattrs is slow on my system (F27AW, XFS+LVM, NVMe). I haven't fully traced through why, but AIUI at least on XFS the xattrs are often stored outside of the inode so it's a little bit like doing an `open()+read()`. Plus there's the LSM overhead, etc. The thing is that for rpm-ostree's package layering use case, we basically always want to treat the on-disk state as canonical. (There's a subtle case here if one does overrides for something that contains policy but we'll fix that). Anyways, so we're in a state now where we do the slow but correct thing by default, which seems sane. But let's allow the app to opt-in to telling us "really trust devino". The difference between a `stat()` + hash table lookup versus the full xattr load on my test case of `rpm-ostree install ./tree-1.7.0-10.fc27.x86_64.rpm` is absolutely dramatic; consistently on the order of 10s without this support, and <1s with (800ms). No tests yet.

I was seeing the `Writing OSTree commit...` phase of rpm-ostree being very slow lately. This turns out to be more fallout from ostreedev#1170 AKA commit: 8fe4536 Loading the xattrs is slow on my system (F27AW, XFS+LVM, NVMe). I haven't fully traced through why, but AIUI at least on XFS the xattrs are often stored outside of the inode so it's a little bit like doing an `open()+read()`. Plus there's the LSM overhead, etc. The thing is that for rpm-ostree's package layering use case, we basically always want to treat the on-disk state as canonical. (There's a subtle case here if one does overrides for something that contains policy but we'll fix that). Anyways, so we're in a state now where we do the slow but correct thing by default, which seems sane. But let's allow the app to opt-in to telling us "really trust devino". The difference between a `stat()` + hash table lookup versus the full xattr load on my test case of `rpm-ostree install ./tree-1.7.0-10.fc27.x86_64.rpm` is absolutely dramatic; consistently on the order of 10s without this support, and <1s with (800ms).

I was seeing the `Writing OSTree commit...` phase of rpm-ostree being very slow lately. This turns out to be more fallout from #1170 AKA commit: 8fe4536 Loading the xattrs is slow on my system (F27AW, XFS+LVM, NVMe). I haven't fully traced through why, but AIUI at least on XFS the xattrs are often stored outside of the inode so it's a little bit like doing an `open()+read()`. Plus there's the LSM overhead, etc. The thing is that for rpm-ostree's package layering use case, we basically always want to treat the on-disk state as canonical. (There's a subtle case here if one does overrides for something that contains policy but we'll fix that). Anyways, so we're in a state now where we do the slow but correct thing by default, which seems sane. But let's allow the app to opt-in to telling us "really trust devino". The difference between a `stat()` + hash table lookup versus the full xattr load on my test case of `rpm-ostree install ./tree-1.7.0-10.fc27.x86_64.rpm` is absolutely dramatic; consistently on the order of 10s without this support, and <1s with (800ms). Closes: #1357 Approved by: jlebon

For our build-system we have a [patched version of `fakeroot`][1] which understands the `user.ostreemeta` xattr. It's intended for use with `ostree checkout --user-mode --require-hardlinks`. When programs are run with `LD_PRELOAD=libfakeroot...` they see the ownership and permissions that ostree understands, rather than the real ones. Our build system mostly consists of calls to: ostree checkout --user-mode --require-hardlinks $COMMIT root LD_PRELOAD=fakeroot <make some changes> ostree commit --tree=dir=root --link-checkout-speedup --devino-canonical in the past this seemed to work fine but at some point I started losing the setuid bit on `sudo`. I suspect the previous behaviour was a bug, possibly fixed in ostreedev#1170 (8fe4536). This commit makes the `ostreemeta` preserving behaviour explicit. If the user.ostreemeta xattr doesn't exist the file is given the sensible default uid/gid of 0:0. `--use-bare-user-xattrs` will currently be ignored when loading a commit from a tarball. This can be fixed in the future if there's any demand for it. [1]: https://github.com/stb-tester/fakeroot/tree/ostree

cgwalters reviewed Sep 13, 2017

View reviewed changes

jlebon force-pushed the pr/devino-cache-bug branch from 21776b6 to 7d9059c Compare September 13, 2017 22:42

cgwalters added the for-after-release label Sep 14, 2017

jlebon mentioned this pull request Sep 26, 2017

lib/commit: fix using uninitialized var #1216

Closed

alexlarsson mentioned this pull request Sep 26, 2017

DevIno cache doesn't work when you're using a modifier on the commit #1165

Closed

cgwalters reviewed Sep 27, 2017

View reviewed changes

cgwalters added reviewed and removed for-after-release labels Sep 27, 2017

tests/libtest.sh: always nuke repo and files

7a76821

This allows users to easily re-initialize the test repo.

jlebon force-pushed the pr/devino-cache-bug branch from 7d9059c to 211e748 Compare September 28, 2017 19:16

jlebon removed the reviewed label Sep 28, 2017

cgwalters reviewed Sep 28, 2017

View reviewed changes

cgwalters added the homu/approved label Sep 28, 2017

rh-atomic-bot pushed a commit that referenced this pull request Sep 28, 2017

tests/libtest.sh: always nuke repo and files

a8e8c5a

This allows users to easily re-initialize the test repo. Closes: #1170 Approved by: cgwalters

smcv requested changes Sep 29, 2017

View reviewed changes

jlebon removed the homu/approved label Sep 29, 2017

jlebon added 3 commits September 29, 2017 20:25

fixup! lib/commit: don't query devino cache for modified files

87a2947

jlebon force-pushed the pr/devino-cache-bug branch from 211e748 to 37a2fad Compare September 29, 2017 20:40

jlebon added 3 commits September 29, 2017 21:04

fixup! tests: check for relabeling rather than overlay

3433694

fixup! tests: check for relabeling rather than overlay

5cc1dc7

tests/installed: also run test-basic-c

d1fa17c

Since we now have a subtest there that needs full xattr support.

fixup! tests/installed: also run test-basic-c

9410a8f

cgwalters reviewed Sep 29, 2017

View reviewed changes

cgwalters added the homu/approved label Sep 30, 2017

rh-atomic-bot closed this in e4a90ca Sep 30, 2017

rh-atomic-bot pushed a commit that referenced this pull request Sep 30, 2017

tests/installed: also run test-basic-c

4262a4b

Since we now have a subtest there that needs full xattr support. Closes: #1170 Approved by: cgwalters

cgwalters mentioned this pull request Oct 9, 2017

System unbootable after upgrading ostree advancedtelematic/meta-updater#144

Closed

cgwalters mentioned this pull request Oct 20, 2017

lib/commit: Fix hardlink checkout commit with bare-user + mod xattrs #1297

Closed

cgwalters mentioned this pull request Dec 1, 2017

lib/repo: Add a DEVINO_CANONICAL commit modifier flag #1357

Closed

cgwalters changed the title ~~Pr/devino cache bug~~ Rework devino caching to support invalidation if metadata changes Dec 8, 2017

jlebon deleted the pr/devino-cache-bug branch June 14, 2018 01:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework devino caching to support invalidation if metadata changes #1170

Rework devino caching to support invalidation if metadata changes #1170

jlebon commented Sep 13, 2017

cgwalters Sep 13, 2017

cgwalters Sep 13, 2017

jlebon Sep 13, 2017 •

edited

Loading

cgwalters Sep 13, 2017

jlebon commented Sep 13, 2017

cgwalters commented Sep 25, 2017

cgwalters commented Sep 25, 2017

rh-atomic-bot commented Sep 26, 2017

cgwalters Sep 27, 2017

jlebon commented Sep 28, 2017

cgwalters Sep 28, 2017

cgwalters Sep 28, 2017

cgwalters Sep 28, 2017

cgwalters Sep 28, 2017

cgwalters Sep 28, 2017

cgwalters commented Sep 28, 2017

rh-atomic-bot commented Sep 28, 2017

rh-atomic-bot commented Sep 28, 2017

smcv Sep 29, 2017

jlebon Sep 29, 2017

smcv Sep 29, 2017

smcv Sep 29, 2017

jlebon Sep 29, 2017

jlebon commented Sep 29, 2017

jlebon commented Sep 29, 2017

cgwalters Sep 29, 2017

cgwalters Sep 30, 2017

cgwalters Sep 29, 2017

cgwalters commented Sep 30, 2017

rh-atomic-bot commented Sep 30, 2017

Rework devino caching to support invalidation if metadata changes #1170

Rework devino caching to support invalidation if metadata changes #1170

Conversation

jlebon commented Sep 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlebon Sep 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlebon commented Sep 13, 2017

cgwalters commented Sep 25, 2017

cgwalters commented Sep 25, 2017

rh-atomic-bot commented Sep 26, 2017

Choose a reason for hiding this comment

jlebon commented Sep 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgwalters commented Sep 28, 2017

rh-atomic-bot commented Sep 28, 2017

rh-atomic-bot commented Sep 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlebon commented Sep 29, 2017

jlebon commented Sep 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgwalters commented Sep 30, 2017

rh-atomic-bot commented Sep 30, 2017

jlebon Sep 13, 2017 •

edited

Loading