When falling back to copying, we previously would only chmod checked out
files in the non-user-checkout mode. Fix this by always doing chmod.
The file_mode was being prepared but never actually applied.
Add a basic test in the archive-z2 --> usermode checkout case in which
we're guaranteed to always fall back to copy mode.
Closes: #633Closes: #903
Approved by: cgwalters
This was making it impossible to pull or mirror a large ostree repo, and
according to Colin is no longer necessary. It works fine with a test
against a repo with 2741 commit and 451468 objects in it.
Closes: #899Closes: #904
Approved by: jlebon
This reverts commit 1eff3e8343. There
are a few issues with it. It's not a critical thing for now, so
let's ugly up the git history and revisit when we have time to
debug it and add more tests.
Besides the below issue, I noticed that the simple `ostree remote add`
now writes to `/ostree/repo/config` because we *aren't* using the
`--sysroot` argument.
Closes: https://github.com/ostreedev/ostree/issues/901Closes: #902
Approved by: mike-nguyen
There was a lot of conditionals inside `write_object()` differentating
between metadata/content, and then for content, on the different repo
types. Further, in the metadata path since the logic is simpler, can
present a non-streaming API, and further use `OtTmpfile`, etc.
Splitting them up helps drop a lot of conditionals. We introduce a small
`CleanupUnlinkat` that allows us to fully convert to the new code style in both
functions.
This itself is still prep for fully switching to `GLnxTmpfile`.
Closes: #881
Approved by: jlebon
If we have an expected checksum, call `fstatat(repo_dfd, checksum)`
early on before we do much else. This actually duplicates code,
but future work here is going to split up the metadata/content
commit paths, so they'll need to diverge anyways.
Closes: #881
Approved by: jlebon
First, the streaming metadata API is pretty dumb, since metadata
should be small. Really we should have supported a `GBytes`
version. Currently, this API *is* used when we do local pulls,
so this commit has test coverage. However, I plan to change
the object import to avoid using this. But that's fine, since
I can't think of why someone would use this API.
Next, the only difference between `ostree_repo_write_metadata()` and
`ostree_repo_write_metadata_trusted()` is whether or not we pass
an output checksum; so just dedup the implementations.
Also while I'm here break out the input length validation and do
it early in the streaming case.
Closes: #881
Approved by: jlebon
Using `${sysroot}` to mean the physical storage root: We don't want to write to
`${sysroot}/etc/ostree/remotes.d`, since nothing will read it, and really
`${sysroot}` should just have `/ostree` (ideally). Today the Anaconda rpmostree
code ends up writing there. Fix this by adding a notion of "physical" sysroot.
We determine whether the path is physical by checking for `/sysroot`, which
exists in deployment roots (and there shouldn't be a `${sysroot}/sysroot`).
In order to unit test this, I added a `--sysroot` argument to `remote add`.
However, doing this better would require reworking the command line parsing for
the `remote` argument to support specifying `--repo` or `--sysroot`, and I
didn't quite want to do that yet in this patch.
Closes: https://github.com/ostreedev/ostree/issues/892Closes: #896
Approved by: jlebon
I saw a few instances of `glnx_set_error_from_errno() + return FALSE`,
and fixed them and did a bit of style conversion.
Closes: #895
Approved by: jlebon
Having a failable accessor is annoying, since it's really common
to reference both. Instead, open the repo once when we load
the sysroot, and provide a non-failable accessor.
This is also prep for `ostree_repo_open_at()`, which collapses the separation
between `ostree_repo_new()` and `ostree_repo_open()`.
Closes: #886
Approved by: jlebon
This is prep for introducing a fd-relative `ostree_repo_new_at()`.
Previously, `ostree_repo_is_system()` compared `GFile` paths, but
there's a much simpler check we can do first - if this repository
was created via `OstreeSysroot`, it must be a system repo.
Closes: #886
Approved by: jlebon
This is a de-scoping of work I did in preparation for
rpm-ostree [live updates](https://github.com/projectatomic/rpm-ostree/pull/652).
Originally I was going to expose this as a public API.
However, I decided to do things differently, but the cleanup here for new code
style and fd-relative is nice to have anyways.
We rework things to use `OstreeDeployment*`, which the caller is expected to
already have, rather than `GFile*`s pointing to the config directories.
Closes: #741
Approved by: jlebon
The summary file can get large, but it compresses well (something
which is not true of other files in the ostree repo which are
already compressed). By sending Accept-Encoding: gzip (and
handling the compressed results) we send a lot less data.
I set up the flathub repo (http://flathub.org/repo) to enable
gzip for the summary file (only), and the result is that the
331514 byte large summary was transferred in 122889 bytes.
On my (fast) network this decreased the time i took to do
"flatpak remote-ls flathub" by about 100msec.
This fixes https://github.com/ostreedev/ostree/issues/802Closes: #882
Approved by: cgwalters
It's hard right now to do a full port to the new libglnx tmpfile
API since there are complex cases in the commit path which deal
with symlinks as well.
Let's make things more gradual by introducing the important part (struct with
autocleanup) here in libotutil, port what we can. This will make a future
complete port easier.
Closes: #871
Approved by: jlebon
A commit can now include a "ostree.endoflife-rebase" metadata key
pointing to a new ref.
When updating, the sysroot upgrader will see this and proceed to
pull and deploy the new ref instead. The origin file in the new
deployment will point to the new ref.
This functionality is planned to be used in Endless OS. We will create
a lesser tested branch for brand new, cutting edge hardware support,
and ship that on hardware platforms that require the latest drivers.
However, once our slower-moving official release is later updated to
support the new hardware, we will use this functionality to migrate
those bleeding-edge users over to the official release.
Closes: #874
Approved by: cgwalters
The whole ostree-remote.h file is only included in the public ostree.h
header if OSTREE_ENABLE_EXPERIMENTAL_API is defined, so there’s no need
to change the set of methods defined in it according to whether we’re
compiling with experimental API.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #875
Approved by: cgwalters
Make it an internal, not static, API; like _ostree_repo_add_remote(). It
will be used in many the same situations.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #875
Approved by: cgwalters
Return whether the remote already existed. This is an internal API, so
it’s not an API break. The return value will be useful in upcoming
commits for working out whether to later remove a remote again.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #875
Approved by: cgwalters
Add a name argument to the internal OstreeRemote constructor,
since this member (and several derived from it) is non-nullable,
and hence must always be set at construction time.
This changes the only call sites of the constructor to use the new API,
which is internal.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #875
Approved by: cgwalters
Just for internal documentation; g-ir-scanner doesn’t read or understand
them.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #875
Approved by: cgwalters
Now that we’ve got a public, sealed OstreeRemote structure, we can start
carefully exposing members of it as API.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #875
Approved by: cgwalters
If somehow a repo has gpg verification on but doesn't have signatures
present for the existing commit, ostree would error out if it needs to
scan the commit object (e.g. if there are no updates available).
An instance of this is currently happening in Fedora AH, in which
signatures are not shipped in the ISO due to filesystem restrictions.
Another possible scenario is if a content provider switches from not
signing commits to signing them; even if older commits are retroactively
signed, clients' local commit objects would error out if they needed
scanning.
This patch adds a check to ensure that we always attempt to fetch the
detached metadata and wait for its result (whether it exists or not)
before moving on to scan their corresponding commit objects.
See also: https://github.com/projectatomic/rpm-ostree/issues/630Closes: #873
Approved by: cgwalters
Porting a lot of this file would be hard since in many cases we do processing in
the `out:` section, so let's do what we can.
Closes: #870
Approved by: jlebon
Unbreaks mounting in CentOS. Newer systemd in Fedora pulls didn't need this, I
think due to `RequiresMountsFor=`. Anyways, this is what the fstab generator
does, and it's clearly right ✓.
Closes: https://github.com/ostreedev/ostree/issues/867Closes: #869
Approved by: jlebon
Follow up to a previous patch that addressed a double-close; I
realized we already had a helper for doing "open dfd iter, do nothing
if we get ENOENT". Raise it to libotuil, and port all consumers.
Closes: #863
Approved by: jlebon
I noticed an instance of this while working on https://github.com/ostreedev/ostree/pull/861
Which apparently I cargo-culted into the new system generator bits.
Let's break this out as a small concise change.
Closes: #866
Approved by: jlebon
If one wants to set up a mount for `/var` in `/etc/fstab`, it
won't be mounted since `ostree-prepare-root` set up a bind mount for
`/var` to `/sysroot/ostree/$stateroot/var`, and systemd will take
the already extant mount over what's in `/etc/fstab`.
There are a few options to fix this, but what I settled on is parsing
`/etc/fstab` in a generator (exactly like `systemd-fstab-generator` does),
except here we look for an explicit mount for `/var`, and if one *isn't* found,
synthesize the default ostree mount to the stateroot. Another nice property is
that if an admin creates a `var.mount` unit in `/etc` for example, that will
also override our mount.
Note that today ostree doesn't hard depend on systemd, so this behavior only
kicks in if we're built with systemd *and* libmount support (for parsing
`/etc/fstab`). I didn't really test that case though.
Initially I started writing this as a "pure libc" program, but at one point
decided to use `libostree.so` to find the booted deployment. That didn't work
out because `/boot` wasn't necessarily mounted and hence we couldn't find the
bootloader config. A leftover artifact from this is that the generator code
calls into libostree via the "cmd private" infrastructure. But it's an easy way
to share code, and doesn't hurt.
Closes: #859
Approved by: jlebon
By checking the mount status, we avoid remounting things if we don't
need to. And printing a single line per mount helps debugging when
things go wrong.
Closes: #859
Approved by: jlebon
I really have no idea what I was thinking with that list of mount points. It
seems arbitrary. Sadly `git log` doesn't help, and there's no comments.
Basically, the only mounts we should care about are those that libostree
creates. Which are just `/sysroot` and `/var`. Systemd will handle the other
things like `/tmp`, it's not our job, and we shouldn't touch them.
Closes: #859
Approved by: jlebon
In current systemd, there is:
[systemd-volatile-root](https://www.freedesktop.org/software/systemd/man/systemd-volatile-root.service.html)
which was introduced by [this commit](91214a37ef).
I'd like to make further changes to how we handle `/var`, and I don't
want to reason about the interaction of our "tmpfs var" with too many
other things.
The comment about having "all /var handling in one place" was always inaccurate
given that we rely on systemd for mounting. And in general, I don't want to
duplicate too many things systemd does - it does them well, documents them, etc.
As far as I know, it was basically just Owen who was using this for the GNOME
hardware testing effort, and I'm sure he could easily switch over to
`systemd.volatile=state`.
Closes: #856
Approved by: owtaylor
This is what caused the merge of
https://github.com/projectatomic/rpm-ostree/pull/652
to blow up, since https://github.com/ostreedev/ostree/pull/848
landed right before we tried to merge it.
When I was writing that PR I remember having an uncertain feeling
since we were doing a `mkdirat` above, but at the time I thought
we'd have test suite coverage...turns out we didn't.
For backwards compatibility, we need to continue to do a `mkdirat` here of the
parent. However...I can't think of a reason anyone would *want* that behavior.
Hence, let's add a special trick - if the destination name is `.`, we skip
`mkdirat()`. That way rpm-ostree for example can open a dfd for `/etc` and avoid
the `mkdir`.
Fold the subpath tests into `test-basic.sh` since it's not worth a separate
file. Add a test case for checking out a file.
Closes: #854
Approved by: jlebon
As $DEITY intended.
I was reading the `prepare-root.c` code and the indentation damage was
distracting. Squash tabs that have leaked into various places in the code. I
didn't yet touch the `src/libostree` bits as that has higher potential for
conflict.
Closes: #852
Approved by: jlebon
Looking at `perf record ostree checkout`, some things stand out; e.g.:
```
+ 27.63% 0.07% ostree libgio-2.0.so.0.5000.3 [.] g_file_enumerator_iterate
+ 22.74% 0.28% ostree libostree-1.so.1.0.0 [.] ostree_repo_file_tree_query_child
+ 13.74% 0.08% ostree libostree-1.so.1.0.0 [.] ot_variant_bsearch_str
```
The GIO abstractions are already fairly heavyweight, and `OstreeRepoFile` mallocs
a lot too.
Make things more efficient here by dropping the GIO bits for reading ostree data -
we just read from the variants directly and iterate over them. The end result
here is that according to perf we go from ~40% of our time in the kernel to
~70%, and things like `g_file_enumerator_iterate()` drop entirely out of the
hot set.
Closes: #848
Approved by: jlebon
Since we now have a cleaner separation of "toplevel checkout prep"
versus "recursive checkout", handle the special case of checking out
a single file at first rather than later.
Prep for future work in optimizing this function more.
Closes: #848
Approved by: jlebon
Only non-mechanical bit here was creating a local autoptr for a bit
where we'd previously done an unref for a struct member.
Closes: #847
Approved by: jlebon
Rather than `g_output_stream_splice()`, where the input is a regular
file.
See https://github.com/GNOME/libglnx/pull/44 for some more information.
I didn't try to measure the performance difference, but seeing the
read()/write() to/from userspace mixed in with the pointless `poll()` annoyed me
when reading strace.
As a bonus, we will again start using reflinks (if available) for `/etc`,
which is a regression from the https://github.com/ostreedev/ostree/pull/797
changes (which before used `glnx_file_copy_at()`).
Also, for the first time we'll use reflinks when doing commits from file-backed
content. This happens in `rpm-ostree compose tree` today for example.
Update submodule: libglnx
Closes: #817
Approved by: jlebon
Continuing to chip away at this. Using `g_file_enumerator_iterate()`
here helps notably.
I started on the much bigger `ostree_diff_dirs_with_options()` but
it's a lot messier - for later.
Closes: #844
Approved by: jlebon
Previously it was static to ostree-repo.c. Make it usable throughout
libostree so it can be used by an upcoming commit, but also expose the
typedef and reference counting functions so that opaque OstreeRemote
pointers can be used by user code, in anticipation of exposing more of
its API publicly in future.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #832
Approved by: cgwalters
This allows consumers of libostree to check at configure time whether it
supports the feature they want.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #832
Approved by: cgwalters
There are currently no unstable APIs, but some will be added in
following commits. They will be built and exposed in the libostree
global symbol list iff configured with --enable-experimental-api.
Distributions should not package OSTree with --enable-experimental-api.
This is designed for previewing new APIs on controlled platforms; any of
the APIs hidden behind this option may be changed or removed at any
point.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #832
Approved by: cgwalters
Read the http-headers (a(ss)) option in
ostree_repo_remote_fetch_summary_with_options like
ostree_repo_pull_with_options and add the headers to the fetcher. This
allows things like providing additional authorization headers to the
HTTP requests.
Closes: #839
Approved by: cgwalters
Allow GI bindings to delete refs through ostree_repo_transaction_set_ref
and ostree_repo_transaction_set_refspec by setting the checksum to NULL.
Closes: #834
Approved by: cgwalters
Rather than hard-coding the names as strings. This makes the code a
little more maintainable.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #826
Approved by: cgwalters
If a summary metadata key is well-known, like ostree.static-deltas, or
ostree.summary.last-modified, format it a little more nicely.
This is especially important for timestamps like last-modified, since
otherwise they’re formatted as a big-endian uint64, which is basically
unusable for the user.
Non-formatted output can still be retrieved using the OSTREE_DUMP_RAW
flag, and the non-formatted key name is always printed for clarity.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #826
Approved by: cgwalters
This allows a locally generated summary file to be viewed. It accepts
the same arguments as `ostree remote summary` (i.e. --raw).
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #826
Approved by: cgwalters
• Commit timestamps, so it’s easy to work out whether a given commit is
newer than the one we have locally
• Summary file timestamp, so it’s easy to work out whether the summary
file is more up to date than another summary file
• Summary file expiry time, so clients can work out when they should
expect the summary file to next be updated, and hence can query for
it at roughly the right time
The expiry time requires input from the user, so is currently never set
automatically. Programs using libostree can set it if they wish.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #826
Approved by: cgwalters
While running the testsuite under valgrind a small memory leak showed up:
==16487== 65 bytes in 1 blocks are definitely lost in loss record 773 of 1,123
==16487== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==16487== by 0x6048E08: g_malloc (gmem.c:94)
==16487== by 0x6062EAE: g_strdup (gstrfuncs.c:363)
==16487== by 0x54CE3E6: write_object (ostree-repo-commit.c:776)
==16487== by 0x54CF2D4: ostree_repo_write_metadata (ostree-repo-commit.c:1528)
==16487== by 0x54CF505: _ostree_repo_write_directory_meta (ostree-repo-commit.c:1712)
==16487== by 0x54D0AB4: write_dfd_iter_to_mtree_internal (ostree-repo-commit.c:2650)
==16487== by 0x54D0E2D: ostree_repo_write_dfd_to_mtree (ostree-repo-commit.c:2793)
==16487== by 0x1190C4: ostree_builtin_commit (ot-builtin-commit.c:474)
==16487== by 0x11F2EE: ostree_run (ot-main.c:200)
==16487== by 0x116F32: main (main.c:78)
The reason for this is that ot_checksum_instream_get_string returns a chunk of newly allocated memory which never got freed.
Make actual_checksum something that gets autocleanend and own the memory
assigned to it in all cases.
Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
Closes: #827
Approved by: pwithnall
If we're freeing the segment, it's basically always better to use
`autoptr()`. Fewer lines, more reliable, etc.
Noticed an instance of this in the pull code while reviewing a different PR,
decided to do a grep for it and fix it tree wide.
Closes: #836
Approved by: pwithnall
If one of the progress keys is set in a pull operation, a ::changed
signal is emitted on the progress object, and the callback for that
could query any of the progress keys — so they all need to be set,
otherwise we get an assertion failure in ostree_async_progress_get() due
to a named key not existing.
Spotted by Dan Nicholson in PR #819.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #835
Approved by: cgwalters
The summary URL override is looked up with "&s", which directly
exchanges the data to a pointer without allocation. This was causing a
segfault calling ostree_repo_remote_fetch_summary_with_options from
pygobject.
Closes: #829
Approved by: jlebon
g_date_time_new_from_unix_utc() will not always return a valid GDateTime
— if the input timestamp is too big, GDateTime cannot represent it, and
the constructor returns NULL.
Add some missing checks for these situations. We don’t ever expect
timestamps to be this big, but they could be as a result of corruption
or a malicious repository.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #825
Approved by: cgwalters
The keyring isn't large, so let's just fall back to copying it
rather than requiring `renameat()`.
Prep for `ostree_repo_open_at()`.
Closes: #821
Approved by: jlebon
Use the new well-known `status` key for OstreeAsyncProgress to get and
set the status atomically with other keys in an OstreeAsyncProgress
instance.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #819
Approved by: cgwalters
Rework how the status is handled in OstreeAsyncProgress so that it’s now
a well-known key in the hash table. This means that it can be retrieved
and set atomically with other keys using
ostree_async_progress_[get|set]().
The behaviour of ostree_async_progress_[get|set]_status() is preserved,
with the caveat that `status` can now also be accessed using the other
API on OstreeAsyncProgress, and has to be accessed with the right
GVariant type.
Internally, a NULL status is represented by an empty status string
(since ostree_async_progress_[get|set]_variant() deliberately don’t
allow NULL variants to be set against keys, since that would break the
ostree_async_progress_get() API).
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #819
Approved by: cgwalters
This will eliminate most of the potential races in progress reporting.
ostree_repo_pull_default_console_progress_changed() still calls three
getters, so there may still be races there, however.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #819
Approved by: cgwalters
OstreeAsyncProgress is thread-safe: it can have keys changed by one
thread while another is getting the same keys (modulo some locking
contention). However, the thread safety is done at the function call
level: if some code calls an OstreeAsyncProgress getter several times,
the key fetches are not atomic with respect to each other.
In the case of contention on the lock, this can result in consumers of
OstreeAsyncProgress data seeing an inconsistent state between the
properties they query, which could result in progress reporting
inaccuracies.
In the uncontested case, this results in the OstreeAsyncProgress lock
being locked and unlocked many times more than necessary.
Try to improve this by adding new API, which supports getting and
setting multiple keys atomically:
• ostree_async_progress_get()
• ostree_async_progress_set()
The new API uses GVariants and varargs: keys are passed as a
GVariantType string followed by arguments as for g_variant_new() or
g_variant_get(), followed by the next key, etc.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #819
Approved by: cgwalters
OstreeAsyncProgress currently does some contortions to try and avoid
allocating space for guints and guint64s (on 64-bit platforms), but this
means it uses two GHashTables. A GHashTable allocates 8 buckets even
when empty. Given that the largest usage of OstreeAsyncProgress in
libostree puts 13 uints and 5 uint64s in it, this optimisation does not
save significant (if any) memory.
Instead, change OstreeAsyncProgress to store values internally as
GVariants, and expose this with some new API:
• ostree_async_progress_get_variant()
• ostree_async_progress_set_variant()
Each GVariant is allocated on the heap. As they are immutable, they are
thread-safe once returned by a getter.
The existing API continues to work as before, except in the case where a
key is set/got as both a uint and a uint64 — there will now be a
collision (and a GVariant type checking failure) whereas previously
there was no collision. Nothing in OSTree uses OstreeAsyncProgress this
way though.
The new API can be used to share more complex data via the progress API.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #819
Approved by: cgwalters
Instead of using G_OPTION_ARG_STRING, use G_OPTION_ARG_FILENAME, which
handles filename encoding conversion differently from the locale
conversion which G_OPTION_ARG_STRING. This will fix argument handling on
systems where the filename encoding is not the same as the locale
encoding (which is fairly unlikely since most systems use UTF-8).
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #810
Approved by: cgwalters
I happened to glance at the top of my most recent patch and
noticed that I used an `throw_errno()` function in a non-errno place.
I scanned the patch for other instances of this but didn't find one.
Closes: #811
Approved by: jlebon
I was planning to change some of the object loading code in the
future, so here's some porting.
Note that I rewrote `_ostree_repo_has_loose_object()` since it
used an error return across multiple functions.
Honestly I'm not sure about this `TEMP_FAILURE_RETRY()` business...
in reality we're going to end up with a ton of code linked in
process that doesn't do it. Unix sucks =( But I'm keeping
what was there out of consistency.
Closes: #809
Approved by: jlebon
This did a `closedir` in the `goto out` section before, but it
turns out more nicely if we follow the usual pattern of doing
the `open(O_DIRECTORY)` in the callee function and handle `ENOENT`
there.
Closes: #809
Approved by: jlebon
This is a variant of the efforts in https://github.com/ostreedev/ostree/pull/741
Working on `rpm-ostree livefs`, I realized though I needed to just
check out *new* files directly into the live `/etc` (and possibly
delete obsolete files).
The way the current `/etc` merge works is fundamentally different from
that. So my plan currently is to probably do something like:
- Compute diff
- Check out each *new* file individually (as a copy)
- Optionally delete obsolete files
Also, a few other things become more important - in the current deploy code, we
copy all of the files, then relabel them. But we shouldn't expose to *live*
systems the race conditions of doing that, plus we should only relabel files we
checked out.
By converting the deploy's /etc code to use this, we fix the same TODO item
there around atomically having the label set up as we create files. And further,
if we kill the `/var` relabeling which I think is unnecessary since Anaconda
does it, we could delete large chunks of code there.
In the implementation, there are two types of things: regular files, and
symlinks. For regular files, in the `O_TMPFILE` case, we have the ability to
do *everything* atomically (including SELinux labeling) before linking it into
place. So let's just use that. For symlinks, we use `setfscreatecon()`.
Closes: #797
Approved by: jlebon
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.
Closes: #801
Approved by: jlebon
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:
```
$ strace -s 2048 -f -o strace.log ostree --repo=repo-build checkout -U fedora-atomic/25/x86_64/docker-host host-test-checkout
$ grep dirmeta strace.log | wc -l
7165
```
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.
Closes: #795
Approved by: jlebon
These are leftovers from the packfile code and should have been
deleted in commit: 2a0601efc7
I noticed this now since I wanted to add a new type of caching.
Closes: #795
Approved by: jlebon
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.
Closes: #794
Approved by: jlebon
This is intended to be used for copying `/usr/etc` → `/etc` for
deployments.
A TODO here is to use `glnx_file_copy_at()` if the repo mode allows
it - then we'd use reflinks if available.
Closes: #804
Approved by: jlebon
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.
Closes: #803
Approved by: jlebon
I plan to make some future changes here, and wanted to do this
first.
Random side note; how about converting the do/while loops for `EINTR` to
`TEMP_FAILURE_RETRY()`? We're very inconsistent about that...
Closes: #792
Approved by: jlebon
This could help others who want to integrate with other init
systems/initramfs.
Commit-message-by: Colin Walters <walters@verbum.org>
Closes: #784
Approved by: cgwalters
`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.
Closes: #796
Approved by: jlebon
This is only about 40%, and mostly simpler functions. It's
nice to switch to `g_autoptr(GMatchInfo)` instead of our inline version.
I decided to add more usage of `ot_transfer_out_value()`, though it'd
be nice to try to have a copy of that in libglnx (or possibly glib).
Closes: #791
Approved by: jlebon
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.
Closes: https://github.com/ostreedev/ostree/issues/798Closes: #799
Approved by: jlebon
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.
Closes: https://github.com/ostreedev/ostree/issues/783Closes: #787
Approved by: cgwalters
Previously we'd assert and dump core if one used `checkout -H` without
`-U` on a bare-user repo, because we'd hit the bare-user symlink case.
Rework the code to handle this, and add tests. I hit this when I was going to
suggest to someone to use `-H` to ensure they were getting hardlinks.
Closes: #779
Approved by: jlebon
Testing a fetch of `fedora-atomic/.../docker-host` from
an nginx instance over `https://127.0.0.1` using Fedora 25
versions. Average over 3 runs:
Before: ~24.6 seconds
After: ~19 seconds
Speedup: ~30%
Closes: https://github.com/ostreedev/ostree/issues/778Closes: #780
Approved by: jlebon
In particular the 26-variable monster 👹 in `install_deployment_kernel()` is
slain🗡. I didn't touch every function here, trying to keep things gradual.
Closes: #781
Approved by: jlebon
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.
Reported-By: "Siddharth Sharma" <siddharth@redhat.com>
Closes: #774
Approved by: jlebon
I was planning to change one here, decided to do a conversion
of some of the simpler functions in this file to keep up momentum.
Closes: #776
Approved by: jlebon
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.
Closes: #768
Approved by: jlebon
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.
Closes: #769
Approved by: jlebon
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.
Closes: #754
Approved by: jlebon
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.
Closes: #767
Approved by: jlebon
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.
Closes: #766
Approved by: jlebon
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.
Closes: #756
Approved by: jlebon
I didn't touch everything since at least `commit_loose_object_trusted`
does this:
```
out:
if (G_UNLIKELY (error && *error))
g_prefix_error (error, "Writing object %s.%s: ", checksum, ostree_object_type_to_string (objtype));
```
Which...it'd be interesting to make into an autocleanup. But for now just
keeping up with converting things bit by bit.
Closes: #761
Approved by: jlebon
In [this commit](6ce80f9685)
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.
Closes: #764
Approved by: jlebon
I was playing around in a FAH vagrant box, and hit:
```
Receiving delta parts: 3/4 453.2 kB/s 1.8 MB/145.8 MB
error: opcode set-read-source: No such file object b6e54ba3471b9c116ce6b9bfbf9e55fec60d35cfdb9ae5ae1ee219af02a591b7
```
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.
Closes: #739
Approved by: jlebon
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.
Closes: #750
Approved by: cgwalters
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).
Closes: #750
Approved by: cgwalters
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.
Closes: https://github.com/ostreedev/ostree/issues/758Closes: #759
Approved by: jlebon
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.
Closes: #745
Approved by: jlebon
I happened to be reading this one recently for a reason I forget,
and it's a relatively easy conversion.
Also one not conflicting with any outstanding patches.
Closes: #752
Approved by: jlebon
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.
Prep for more users of this function.
Closes: #746
Approved by: jlebon
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.
Closes: #746
Approved by: jlebon
The first options are owner_uid/owner_gid, which makes it possible to use diff
on local files where --owner-uid/gid have been passed to commit.
Closes: #740
Approved by: cgwalters
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.
Closes: #744
Approved by: jlebon
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.
Closes: #738
Approved by: jlebon
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.
Closes: #738
Approved by: jlebon
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.
Closes: #733
Approved by: jlebon
[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.
Closes: #735
Approved by: jlebon
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`.
Closes: #723
Approved by: jlebon
Jonathan Lebon
Brian C. Lane
Colin Walters
Krzesimir Nowak
Alexander Larsson
Daniel Drake
Philip Withnall
Dan Nicholson
Sjoerd Simons
Francesco Giannelli
Daniel J Walsh
Anton Gerasimov
Erik Larsson