Colin Walters [Tue, 18 Apr 2017 16:41:40 +0000 (12:41 -0400)]
checkout: Merge union/add logic for copies during checkout
We really have an astonishing variety of similar functions which write files and
symlinks. I was working on a different PR and the duplication between the
union-mode and add-mode/none-mode checkout functions bothered me.
I realized that the "handle EEXIST" tri-state maps directly to the
`GLnxLinkTmpfileReplaceMode`, so deduping things makes even more sense.
Colin Walters [Fri, 14 Apr 2017 18:00:55 +0000 (14:00 -0400)]
repo/checkout: Cache lookups of dirmeta objects
I was reading a strace the other day and noticed we were loading the same
`.dirmeta` object many times. Unlike the other object types, `.dirmeta` objects
don't accumulate much over time; there are only so many directory metadata types.
(Without SELinux involved it'd probably be 5-6 I'd guess offhand).
For `fedora-atomic/25/x86_64/docker-host` there are currently 34 `.dirmeta` in
the tree.
But how many times during a checkout did we load those 34 dirmeta objects?
With a quick strace:
After, as you'd expect, we just loaded `34` from disk. We do
6 system calls (`openat+fstat+fstat+read+read+close`) per dirmeta,
so we dropped a total of 42780 system calls - which is about 20% of the total
system calls made.
`perf record` tells me that we're spending ~40 of our time in the kernel during
a checkout, so reducing syscall traffic helps. Though most of that appears to be
in the VFS and XFS layers for `linkat` (which isn't surprising).
So how much did perf improve? Well, on my workstation, I get a lot of
fluctuation in timing, sometimes by 30%, so this was well within the noise. But
it's well worth speeding up checkout, and I think this optimization will shine
more as we improve performance elsewhere.
Colin Walters [Fri, 14 Apr 2017 14:34:38 +0000 (10:34 -0400)]
Rename "osname" → "stateroot"
I never really liked the term "osname". I feel "stateroot" is a *lot* clearer,
since the osname/stateroot mostly just holds `/var`. Further it avoids the `os`
prefix which is already overloaded.
Some of the existing docs already talked about "operating system state", which
further reinforces this.
There's *lot* more things than this which reference the term "osname", but I
don't want to change *everything* yet in this patch in case we decide to do
something different - this just gets the highlights.
Colin Walters [Fri, 21 Apr 2017 01:26:17 +0000 (21:26 -0400)]
lib/cleanup: Port some of the cleanup code to fd-relative and new style
There aren't many users of `g_file_enumerator_iterate()` left - those
remaining are usually good candidates for porting. There's some more
porting to do in this file; a mix of trivial and harder. This
one is a good candidate for an individual commit.
Colin Walters [Fri, 14 Apr 2017 19:15:23 +0000 (15:15 -0400)]
repo: Optimize bare-user content object reads a bit
`perf record ostree checkout ...` for a bare-user repo was telling
me we were spending a good 13% of our time in the depchain of `ot_lgexattrat()`.
The problem here is that traversing the `/proc` path turns out to be
somewhat expensive - there are LSM (SELinux) checks, etc.
For regular files, opening and just getting the xattr, then closing is still
quite cheap. For symlinks, we'll always need to open anyways.
This appears to shave about ~0.1 seconds off of a checkout of
`fedora-atomic/25/x86_64/docker-host` on my workstation.
Oh, and this was the last user of `ot_lgexattrat()` so we can kill it, which is
nice - the xattr code should really live in libglnx.
Colin Walters [Tue, 18 Apr 2017 14:13:45 +0000 (10:13 -0400)]
checkout: Fix bare-user symlink checkouts
Logic error introduced after refactoring; we hoisted the
`is_bare_user_symlink` variable to the top, but its computation
below. But the `is_bare` symlink depended on it.
Colin Walters [Tue, 11 Apr 2017 21:22:54 +0000 (17:22 -0400)]
pull: Support deltas for explicit commits
I think the majority of OSTree usage calls pull with refs, not
explicit commits. We even added special "override syntax" with
`@` (e.g. `ostree pull foo@ab12c34`) as a hybrid.
However, some users may want to still pull explicit commits
for whatever reason. The old static delta logic looked at
the previous commit of the ref. However, in https://github.com/ostreedev/ostree/pull/710
we enhanced the logic to look at all local commits.
It's now a lot more natural to teach the delta logic
to support revisions, e.g. `ostree pull someorigin ab12c34`.
This also fixes the problem that before, `--require-static-deltas`
was completely ignored when processing revisions.
This is a nontrivial refactoring of the logic, but the end
result feels a lot more readable to me.
Colin Walters [Mon, 3 Apr 2017 16:46:37 +0000 (12:46 -0400)]
soup: Hold a ref to the pending URI during completion processing
It was reported that in the range request handling, we called `remove_pending()`
twice (once in processing it, and once potentially in the local_error cleanup),
and this could be viewed as a use-after-free. However, right now the range
cleanup and `local_error` being set are mututally exclusive.
Further, the task object already holds a strong reference, so I observed the
refcount was 2. For both of these reasons, there is no use-after-free in
practice.
Colin Walters [Wed, 29 Mar 2017 20:51:39 +0000 (16:51 -0400)]
Add flag to make SELinux label failure fatal, add hack for /proc
I was working on `rpm-ostree livefs` which does some ostree-based
filesystem diffs, and noticed that we were ending up with `/proc`
not being labeled in our base trees.
Reading the selinux-policy source, indeed we have:
```
/proc -d <<none>>
/proc/.* <<none>>
```
This dates pretty far back. We really don't want unlabeled
content in ostree. In this case it's mostly OK since the kernel
will assign a label, but again *everything* should be labeled via
OSTree so that it's all consistent, which will fix `ostree diff`.
Notably, `/proc` is the *only* file path that isn't covered when composing a
Fedora Atomic Host. So I added a hack here to hardcode it (although I'm a bit
uncertain about whether it should really be `proc_t` on disk before systemd
mounts or not).
Out of conservatism, I made this a flag, so if we hit issues down the line, we
could easily change rpm-ostree to stumble on as it did before.
Colin Walters [Thu, 30 Mar 2017 17:38:08 +0000 (13:38 -0400)]
sepolicy: Fix regressions from introduction of sepolicy_new_at()
Being bitten by lack of PR testing here. There are two bugs:
- First and foremost, I forgot that GObject will call the property setters with
the defaults. This meant we were getting both path="/var/tmp/blah" and fd=-1,
and we were accepting -1 as a fd, which then got converted into AT_FDCWD
which was wrong.
- Since these properties are construct only and mutually exclusive, don't
try to handle one resetting the other. Assert that exactly one of them is set.
Colin Walters [Thu, 23 Mar 2017 17:06:07 +0000 (13:06 -0400)]
Add Coccinelle usage: one for blacklisting, one for patch collection
This is inspired by the [Coccinelle](http://coccinelle.lip6.fr/) usage
in systemd. I also took it a bit further and added infrastructure
to have spatches which should never apply. This acts as a blacklist.
The reason to do the latter is that coccinelle is *way* more powerful than the
regular expresssions we have in `make syntax-check`.
I started with blacklisting `g_error_free()` directly. The reason that's bad is
it leaves a dangling pointer.
Colin Walters [Wed, 29 Mar 2017 02:31:03 +0000 (22:31 -0400)]
lib: Delete old GFile path helpers, and migrate single last user
I happened to read this file and realized there's a lot of cruft left over from
the time when I liked `GFile` and `malloc()`ing like 50 times just to make a
pathname string. Delete it.
Colin Walters [Tue, 28 Mar 2017 19:23:14 +0000 (15:23 -0400)]
lib: Fix OSTREE_CHECK_VERSION()
Actually trying to use this in rpm-ostree, it kept returning successfully when I
didn't expect it to... The first conditional was always succeeding even when I
was asking for a newer minor.
Colin Walters [Thu, 23 Mar 2017 20:49:58 +0000 (16:49 -0400)]
build: Expose autocleanups unconditionally, start using them
I'd like to do this in rpm-ostree at least. Originally I was looking at porting
to `G_DECLARE_FINAL_TYPE` but eh, this is easier for now and won't bump our GLib
dependency which might matter for our embedded users.
For now I just did a few replacements in the `remote` command line. A full port
can come as we do other code cleanups.
This will actually break the flatpak build right now, but
that's easy to fix. And we concluded in e.g.
https://bugs.freedesktop.org/show_bug.cgi?id=95065#c5
it's a bug for downstream projects to do that.
Colin Walters [Tue, 28 Mar 2017 15:16:09 +0000 (11:16 -0400)]
sysroot: Don't cache sepolicy
In [this commit](https://github.com/ostreedev/ostree/commit/6ce80f9685e6273dbcb4731d6650a10976ea220a)
for some reason I added a `sepolicy` member to the sysroot. I
have no idea why I did that, and it's conceptually wrong
since the policy is specific to a *deployment*.
This bit me when I was working on [a pull request](https://github.com/ostreedev/ostree/pull/763)
elsewhere, since at that point it was `NULL`.
We already pass around the sepolicy in the deployment code, so just stop caching
it.
This is because this host version doesn't yet have
https://github.com/ostreedev/ostree/pull/710
which incidentally fixed this for the case where the OS vendor is using
summary files.
Some organizations may not be using summary files - at least we still try to
support that case. So let's copy the logic very recently added in that commit to
handle the legacy case too.
No new tests since this is a nice-to-have - we really do
expect people to be using summary files now.
This is somewhat complicated by such repos only properly supporting
some subset of file metadata (uid/gid 0, etc). We fix this by
always commiting with filters that make it work.
This adds to file permission masks the same bitmask that will
be applied to file objects in bare-user* repos. This will be
needed in the testsuite to ensure that the things we commit
will be expressable in bare-user-only repos.
This mode is similar to bare-user, but does not store the permission,
ownership (uid/gid) and xattrs in an xattr on the file objects in the
repo. Additionally it stores symlinks as symlinks rather than as
regular files+xattrs, like the bare mode. The later is needed because
we can't store the is-symlink in the xattr.
This means that some metadata is lost, such as the uid. When reading a
repo like this we always report uid, gid as 0, and no xattrs, so
unless this is true in the commit the resulting repository will
not fsck correctly.
However, it the main usecase of the repository is to check out with
--user-mode, then no information is lost, and the repository can
work on filesystems without xattrs (such as tmpfs).
Colin Walters [Fri, 24 Mar 2017 14:35:59 +0000 (10:35 -0400)]
repo+tests: Add [core]disable-xattrs=true, use it on overlayfs
There are a lot of things suboptimal about this approach, but
on the other hand we need to get our CI back up and running.
The basic approach is to - in the test suite, detect if we're on overlayfs. If
so, set a flag in the repo, which gets picked up by a few strategic places in
the core to turn on "ignore xattrs".
I also had to add a variant of this for the sysroot work.
The core problem here is while overlayfs will let us read and
see the SELinux labels, it won't let us write them.
Down the line, we should improve this so that we can selectively ignore e.g.
`security.*` attributes but not `user.*` say.
More sophisticated users of libostree like rpm-ostree need control over things
like the system repository. Previously we introduced a "no cleanup" flag to
`ostree_sysroot_simple_write_deployment()`, but that's a high level API that
does filtering on its own.
Since rpm-ostree needs more control, let's expose the bare essentials of the
"sysroot commit" operation with an extensible options structure, where one of
the options is whether or not to do post-transaction repository operations.
Colin Walters [Tue, 21 Mar 2017 00:51:22 +0000 (20:51 -0400)]
sepolicy: Add better private API for setfscreatecon
Use `g_auto()` more sanely with a struct implmenting the "is initialized"
pattern. This is way less ugly for callers, and fixes bugs like
us calling `setfscreatecon()` even if an error occurred beforehand.
Also fold in the logic for "NULL or not loaded" sepolicy into the setup rather
than requiring callers to inline it.
Colin Walters [Sat, 18 Mar 2017 13:49:59 +0000 (09:49 -0400)]
sepolicy: Add ostree_sepolicy_new_at()
I'm porting other code away from `GFile`, and while we don't use this
internally, it will let us do so at a later date. I'm averse to changing the
code right now as we don't have good CI coverage of this.
Colin Walters [Tue, 21 Mar 2017 19:07:41 +0000 (15:07 -0400)]
build: Various fixes for openssl build
This actually worked before because `pkg-config --cflags openssl`
is empty, and the linker was satisfying `-lssl -lcrypto` from our
indirect dependencies.
Also, in fact we *currently* just want `pkg-config libcrypto` i.e.
`-lcrypto`, since we aren't actually using openssl to speak TLS.
This doesn't actually matter on Fedora at least since they're both in the
`openssl-libs` package, but they are separate for a reason.
Colin Walters [Mon, 20 Mar 2017 14:35:40 +0000 (10:35 -0400)]
sysroot: Prep refactoring of cleanup logic
For future work I'm going to tweak how we handle cleanup, and
the private cleanup flags didn't really end up being used - we
only specify "prune repo or not". So fold that into a boolean for now.
The sysroot deploy logic then has a single "do_postclean" boolean, which is all
I want to expose as public API.
Colin Walters [Mon, 13 Mar 2017 18:41:14 +0000 (14:41 -0400)]
core: Support building with OpenSSL for checksums
Add an OpenSSL backend to the checksum input stream, which is where we do a lot
of checksumming (object commit, static deltas).
The raw OpenSSL performance is
[approximately double](https://gist.github.com/cgwalters/169349fd1c06fd4fb4d3a7ce33303222) on
my laptop; not only does OpenSSL have e.g. hand-tuned x86_64 assembly, the
current implementation uses the
[Intel SHA extensions](https://en.wikipedia.org/wiki/Intel_SHA_extensions).
Another reason to do this is I was idly thinking about adding
[Curve25519](https://en.wikipedia.org/wiki/Curve25519) signatures (like e.g.
Alpine does) instead of/in addition to GPG. The rationale for that is
that GPG is pretty heavyweight, both in code footprint and the simple
fact that EC keys are way smaller.
I didn't benchmark ostree with this; we have bigger performance problems
really like the fact we just malloc way too much. But, it's a step
in the right direction I think in combination with the libcurl work
where we're linking to openssl anyways.
Colin Walters [Mon, 13 Mar 2017 19:40:24 +0000 (15:40 -0400)]
lib: Add a private copy of checksum-instream
The current `OstreeChecksumInputStream` is public due to a historical
mistake. I'd like to add an OpenSSL checksum backend, but that's
harder without breaking this API.
Let's ignore it and create a new private version, so it's easier to do the
GLib/OpenSSL abstraction in one place.
Colin Walters [Mon, 13 Mar 2017 01:04:51 +0000 (21:04 -0400)]
lib: Exclude soup header from introspection
We should fix this more reliably by defining a separate variable for
introspection (and gtk-doc) sources at some point, but this does the trick for
now.
Avoids a spam of warnings, and it's definitely not public API.
Colin Walters [Mon, 13 Mar 2017 00:53:28 +0000 (20:53 -0400)]
builtin/show: Convert to direct return/decl-after-stmt style
I just happened to be reading this code; still testing
the waters for the new code style. If we decide to do more
of this it's likely worth trying out a [Coccinelle](http://coccinelle.lip6.fr/)
patch or something.
Colin Walters [Mon, 13 Mar 2017 13:45:51 +0000 (09:45 -0400)]
core: Add runtime ostree_check_version()
[Previously](https://github.com/ostreedev/ostree/pull/728) we added compile-time
checking for versions, but there are use cases for runtime checking as well,
because in a number of API calls we use `GVariant` as an API extension
mechanism.
Colin Walters [Tue, 7 Mar 2017 22:32:50 +0000 (17:32 -0500)]
Disable "ostree trivial-httpd" by default now
This goes farther along the path of deprecating it; consumers should
have been ported at this point. Though probably a lot of people
may still use `rpm-ostree-toolbox`.
OSTree currently provides no way to inspect the versioning
information at run time, being only available at compile
time through pkg-config.
This is a problem for e.g. Flatpak, that needs to check
whether the 'update-frequency' option is available. Checking
at compile time isn't great since it's not looking for new
symbols, but only if an optional feature is present.
This commit, then, adds a new header that is generated
at compile time, exposing OSTree's versioning information.
Colin Walters [Thu, 9 Mar 2017 02:47:00 +0000 (21:47 -0500)]
Release 2017.3
It's been almost a month, I think the current git is working well and
not too risky. We have some new API additions which I think the
various consumers of them are going to want.
repo-pull: add option to set the async update frequency
When using Flatpak with GNOME Software, it is important to
show the progress of the download and install as close as
possible to the real progress.
However, OSTree forces the frequency to call the async
progress callback to 1 second, which causes an unpleasant
effect on the UI, specially when the download size is so
small that everything happens in less than 1 second.
Fix that by adding making OSTree read a custom 'update-frequency'
option and set the timeout source timeout to that. If
no custom frequency is passed, we assume the default 1
second timeout, maintaining the current behavior.
Colin Walters [Mon, 6 Mar 2017 19:51:14 +0000 (14:51 -0500)]
repo/checkout: Verify early if src/destination are on same device
At least in all Linux kernels up to today, one can never `link()` across
devices, so we might as well verify that up front. This will help for a future
patch to add a new type of union-add checkout, since Linux checks for `EEXIST`
before `EXDEV`.
Colin Walters [Fri, 3 Mar 2017 17:10:46 +0000 (12:10 -0500)]
Allow and start using C99 declaration-after-statement
I've seen code in a few places that I think on balance is definitely better this
way. Some of our functions have huge variable declaration sections.
This change includes one small example where we could start using declarations
after statements.
A concern I had was - how does this interact with `__attribute__((cleanup))` and
early returns? I tested it, and AFAICS the behavior is what you'd expect - the
cleanup function isn't called if its variable isn't reachable.
Philip Withnall [Mon, 6 Mar 2017 17:48:36 +0000 (17:48 +0000)]
libostree: Allow compression level to be set for archive-z2 stream
Add a ostree_raw_file_to_archive_z2_stream_with_options() variant of
ostree_raw_file_to_archive_z2_stream(), to allow a compression-level
option to be passed in and passed through to zlib.
This is useful when building archive-z2 files on the fly for
transmission over a non-bandwidth-limited channel, such as a local
network. In this case, CPU time is more valuable than bandwidth, so we
want a low compression level.
Signed-off-by: Philip Withnall <withnall@endlessm.com> Closes: #721
Approved by: cgwalters
Colin Walters [Sat, 4 Mar 2017 15:11:15 +0000 (10:11 -0500)]
fetcher/curl: Fix leaks caught by ASAN
I had to rebuild `glib` with `-fsanitize=address` in order to get a stack trace
to finally get this one. However, *installing* that glib "system wide"
in my container breaks everything (including `rpm-ostree`, `dnf`, `pkg-config` etc.)
that wasn't built with ASAN.
So my test scenario right now is to extract the libs and do e.g.:
```
make && env LD_LIBRARY_PATH=$HOME/src/distgit/fedora/glib2/asan-libs make check TESTS=tests/test-basic.sh
```
Philip Withnall [Mon, 6 Mar 2017 12:19:28 +0000 (12:19 +0000)]
build: Fix disabling --enable-man if xsltproc is not available
If --enable-man=maybe (the default), and xsltproc is not available, the
configuration code would check for it, correctly set enable_man=no, then
incorrectly overwrite that with enable_man=yes, which would result in
later trying to execute $(XSLTPROC) when it’s empty.
Signed-off-by: Philip Withnall <withnall@endlessm.com> Closes: #720
Approved by: cgwalters
Colin Walters [Fri, 24 Feb 2017 02:49:21 +0000 (21:49 -0500)]
pull: Use all available commits for delta sources
The previous logic for static deltas was to use as a FROM
revision the current branch tip. However, we want
to support deltas between branches in an automatic
fashion.
If a summary file is available, we already have an
enumerated list of deltas - so the logic introduced
here is to search it, and find the newest commit
we have locally that matches the TO revision target.
This builds on some thoughts from
https://github.com/ostreedev/ostree/pull/151#issuecomment-232390232
Closes: https://github.com/ostreedev/ostree/pull/151 Closes: #710
Approved by: giuseppe
Jonathan Lebon [Thu, 2 Mar 2017 17:11:17 +0000 (12:11 -0500)]
pull: don't use static deltas if archive repo
In https://github.com/ostreedev/ostree/pull/408, we disabled the use of
static deltas when mirroring. Later,
https://github.com/ostreedev/ostree/pull/506 loosened this up again so
that we could use static deltas when mirroring into bare{-user} repos.
However, the issue which originally spurrred #408 is even more generic
than that: we want to avoid static deltas for any archive repo, not just
when doing a mirror pull. This patch tightens this up, and also
relocates the decision code to make it easier to read.
Colin Walters [Thu, 23 Feb 2017 14:24:58 +0000 (09:24 -0500)]
fetcher: Log failures into journal
Particularly when HTTP requests fail, I really want a lot more information.
We could theoretically stuff it into the `GError` message field, but
that gets ugly *fast*.
Using the systemd journal allows us to log things in a structured fashion.
Right now e.g. rpm-ostree won't be aware of this additional information,
but I think we could teach it to be down the line.
In the short term, users can learn to find it from `systemctl status rpm-ostreed`
or `journalctl -b -r -u rpm-ostreed`, etc.
One thing I'd like to do next is log successful fetches of e.g. commit objects
as well with more information about the originating server (things like the
final URL if we were redirected, did we use TLS pinning, what was the negotiated
TLS version+cipher, etc).
Colin Walters [Fri, 24 Feb 2017 15:18:27 +0000 (10:18 -0500)]
deploy/libmount: Fix build with old util-linux 2.23 (CentOS7)
https://github.com/ostreedev/ostree/pull/705 broke the build
on CentOS 7 which only has util-linux 2.23.
When I was thinking about this, I realized that there must really be a way to
make this safe even for older versions. Looking at that version of util-linux,
all we need to do is invert the order of frees so we `mnt_free_table()` *before*
`mnt_free_cache()`, like util-linux does:
Colin Walters [Fri, 24 Feb 2017 14:33:20 +0000 (09:33 -0500)]
tree-wide: Squash noncritical compiler warnings
Should fix everything from
<https://kojipkgs.fedoraproject.org//packages/ostree/2017.2/3.fc25/data/logs/x86_64/build.log>
Anything that uses autocleanups should *always* be initialized directly I think,
even if a few lines down we directly assign, since this way it's more robust
against refactoring.
And the `freopen()` warnings are right - IMO we should *always* check return
values.
Colin Walters [Thu, 23 Feb 2017 14:40:17 +0000 (09:40 -0500)]
deploy: Correctly use libmount unref() calls rather than free()
We saw a random ostree SEGV start popping up in our CI environment:
https://github.com/projectatomic/rpm-ostree/pull/641#issuecomment-281870424
Looking at this code more and comparing it to what util-linux does, I noticed we
had a write-after-free, since `mnt_unref_table()` will invoke
`mnt_unref_cache()` on its cache, and that function does:
Colin Walters [Fri, 17 Feb 2017 15:22:43 +0000 (10:22 -0500)]
main: Make ostree --version output YAML (and add gitrev)
I learned today that `docker version` does this and I really like
the idea. While we have the patient open, also add the gitrev
with code taken from https://github.com/projectatomic/rpm-ostree/pull/584
Colin Walters [Tue, 21 Feb 2017 18:58:40 +0000 (13:58 -0500)]
build: Add --with-smack, use it to reset contexts for writing objects
At some point we'll want to follow what systemd is doing and add
better support for smack, along the lines of `OstreeSePolicy`. However,
short term this patch fixes AGL which uses Smack.
Colin Walters [Fri, 17 Feb 2017 21:59:21 +0000 (16:59 -0500)]
ci: Hard error on all -fsanitize=undefined warnings
I saw in a recent test log a ton of spam
```
libglnx/glnx-dirfd.c:253:3: runtime error: null pointer passed as argument 1, which is declared to never be null
```
which actually turned out to be libglnx getting reverted. But
let's be sure now we actually bomb out quickly on UBSAN warnings
in general.
projects / ostree.git / log