In the production case since we used `daemon()` our stderr is `/dev/null`¹
there's not much use in logging errors from `FITHAW` or `exit(1)`, and doing so
breaks the test suite which checks the return from `waitpid()`. There's nothing
we can really do if `FITHAW` fails, and in most of those cases `EINVAL`,
`EOPNOTSUPP`, we *shouldn't* do anything anyways.
¹ Though perhaps we should set up the systemd journal, but let's not
go there right now.
Closes: #1084
Approved by: jlebon
I added `waitpid()`, but that didn't actually help because we were
`daemon()`izing. Don't daemonize if we're testing so that we can `waitpid()`.
Note I still haven't reproduced this race locally, but I'm pretty sure this will
fix it.
While here, actually check the return value from `waitpid()` just in case
something goes wrong there.
Closes: #1084
Approved by: jlebon
This is in line with the "/etc is for sysadmins", "/usr is OS" model;
e.g. systemd's bash completions go there.
Making this change since I was looking at the required spec file changes.
Closes: #1083
Approved by: mbarnes
This is mostly the `copy_file_range` changes plus the Coverity files.
```
Colin Walters (4):
localalloc: Abort on EBADF from close() by default
local-alloc: Remove almost all macros like glnx_free, glnx_unref_variant
console: Fix Coverity NULL deref warning
fdio: Merge systemd code to use copy_file_range(), use FICLONE
Jonathan Lebon (1):
console: trim useless check
Matthew Leeds (1):
dirfd: Fix typo in comment
Philip Withnall (1):
glnx-console: Add missing NULL check before writing out text
```
Update submodule: libglnx
Closes: #1081
Approved by: jlebon
This essentially completes our fd-relative conversion.
While here, I cleaned up the semantics of `ostree_repo_create()` and
`ostree_repo_create_at()` to be more atomic - basically various scripts were
testing for the `objects` subdirectory, so let's formalize that.
Closes: #820
Approved by: jlebon
Saw this in a PR result; we need to wait for the child to have written its
result to stderr before we exit, otherwise the test suite may not read it in
time.
Closes: #1070
Approved by: jlebon
This will allow us to drop the awful hack in rpm-ostree where we watch our own
stdout. In general, libraries shouldn't write to stdout.
Also we can kill the systemd journal wrapper code. There's some duplication at
each call site now...but it's easier than trying to write a `sd_journal_send()`
wrapper.
I was originally going to have this emit all of the structured data too as a
`GVariant` but decided it wasn't worth it right now.
Closes: #1052
Approved by: jlebon
I'd like to move the new canonical kernel directory to `/usr/lib/modules/$kver`,
as Fedora has done. The `get_kernel_from_tree()` function now abstracts over
parsing the data (src vs destination filenames, as well as checksum) in
preparation for adding the new case.
In preparation for this, let's change the current test suite to use the
*current* directory of `/usr/lib/ostree-boot`, and also add coverage of `/boot`.
Closes: #1053
Approved by: jlebon
See: http://marc.info/?l=linux-fsdevel&m=149520244919284&w=2
XFS doesn't flush the journal on `syncfs()`. GRUB doesn't know how to follow the
XFS journal, so if the filesystem is in a dirty state (possible with xfs
`/boot`, extremely likely with `/`, if the journaled data includes content for
`/boot`, the system may be unbootable if a system crash occurs.
Fix this by doing a `FIFREEZE`+`FITHAW` cycle. Now, most people
probably would have replaced the `syncfs()` invocation with those two
ioctls. But this would have become (I believe) the *only* place in
libostree where we weren't safe against interruption. The failure
mode would be ugly; nothing else would be able to write to the filesystem
until manual intervention.
The real fix here I think is to land an atomic `FIFREEZETHAW` ioctl
in the kernel. I might try a patch.
In the meantime though, let's jump through some hoops and set up
a "watchdog" child process that acts as a fallback unfreezer.
Closes: https://github.com/ostreedev/ostree/issues/876Closes: #1049
Approved by: jlebon
Coverity spotted an infloop here since we were incrementing `i++`
instead of `j++`. But adding a test revealed other bugs - we need
to keep the arrays in sync.
Coverity CID: 1452204
Closes: #1041
Approved by: pwithnall
If a delta happens to have zero objects, we could end up doing
a divide-by-zero when inferring endianness. In practice,
a zero-object delta isn't possible to generate I think, but
let's make sure the code is defensive all the same.
Spotted by Coverity.
Coverity CID: 1452208
Closes: #1041
Approved by: pwithnall
Part of cleaning up our usage of libglnx; we want to use what's in GLib where we
can.
Had to change a few .c files to `#include ostree.h` early on to pick up
autoptrs for the core types.
Closes: #1040
Approved by: jlebon
There are multiple use cases where we'd like to alias refs.
First, having a "stable" alias which gets swapped across major
versions: https://pagure.io/atomic-wg/issue/228
Another case is when a ref is obsoleted;
<https://pagure.io/atomic-wg/issue/303>
This second one could be done with endoflife rebase, but I think
this case is better on the server side, as we might later change
our minds and do actual releases there.
I initially just added some test cases for symlinks in the `refs/heads` dir to
ensure this actually works (and it did), but I think it's worth having APIs.
Closes: #1033
Approved by: jlebon
I plan to at some point change rpm-ostree to read the journal messages from
libostree and render things like the time we spent in syncfs().
Closes: #1044
Approved by: jlebon
Define typedefs for read/write archives, and use the GLib
autocleanups for them. Prep for updating libglnx to drop its
custom autocleanup macros.
Closes: #1042
Approved by: jlebon
Prep for dropping `GLNX_DEFINE_CLEANUP_FUNCTION` from libglnx
in favor of using GLib's `G_DEFINE_AUTO_CLEANUP_FREE_FUNC()`.
Closes: #1042
Approved by: jlebon
It looks like `curl_multi_socket_action()` will return an error
if *one* of the requests has an error, but we already check
for that explicitly by iterating over each handle.
In libcurl, the "easy" layer doesn't really make use of this
return value. I did a bit of looking elsewhere; systemd
does check it as a runtime error, not an assertion. librepo
doesn't use the multi interface.
Closes: https://github.com/ostreedev/ostree/issues/1035Closes: #1038
Approved by: jlebon
Coverity complained that the `else if (bytes_read == 0)` was dead
code if we happened to find it was already false when testing
`else if (G_UNLIKELY (bytes_read == 0 ...`.
There was nothing wrong with the logic, but let's rework it to
only test the value once; I think it does end up nicer anyways.
Coverity CID: 1452186
Closes: #1037
Approved by: jlebon
Coverity spotted that we had an off-by-one here since we were using
`i+1`. Fix this by adding a `-1` to the bounds check. Also use
`sizeof()` to ensure the data and size are coupled.
Coverity CID: 1452207
Closes: #1037
Approved by: jlebon
It's designed for test suites and non-critical random uses like this. This
silences a Coverity warning about weak randomness.
Closes: #1037
Approved by: jlebon
No real problems here, but Coverity likes to see consistent checking of return
values, and I agree with it.
Coverity CID: 1452213
Coverity CID: 1452211
Closes: #1037
Approved by: jlebon
This is a continuation of addition of journaling to libostree; see
e.g. <https://github.com/ostreedev/ostree/pull/708>.
I wanted more information at the end of fetches; in particular
some details about the delta execution (what opcodes etc.), but
this is a first step: we log things like the transferred data
as well as whether or not GPG was enabled, etc.
One awkward thing about this is how we map the fetcher options like
`tls-ca-path` back out into an enum for the code to log. But eh, hard to fix
without a bigger refactoring.
Closes: #1032
Approved by: jlebon
These were previously private, but since we expect people to use them, let's add
`#define`s like we did for some of the other commit metadata.
Closes: #1028
Approved by: jlebon
Mostly for the latest `-Wmaybe-uninitialized` fix, but while here also port some
places to newer APIs.
Update submodule: libglnx
Closes: #1027
Approved by: jlebon
Our CI runs use `-Werror`; there's no point to our default warning set kicking
in, it just bloats the command line output.
Closes: #1023
Approved by: jlebon
I hit an unused-variable warning with `GLNX_AUTO_PREFIX_ERROR` for
rpm-ostree and led me to wonder why ostree didn't fail, then I noticed
we had lost the special `-Werror=unused-variable` bit. Let's go
ahead and use `-Werror` for clang too.
Closes: #1023
Approved by: jlebon
Previous to this commit, the gjs tests were installed-only; and our
logic for handling the "--enable-installed-tests=exclusive" logic
actually also meant they weren't installed.
It did work for me locally with `--enable-installed-tests`.
However, to make things fully symmetric, let's enable the js tests to also be
run under `make check`.
Also remove `corrupt-repo-ref.js` from the PAPR invocation since it's not
actually a unit test, it's a utility helper.
Closes: #1022
Approved by: jlebon
Unfortunately we can't do gobject-introspection based tests
while compiling with `-fsanitize=address`, since it needs to hook
`malloc` early on.
Add a new suite which just runs the introspection-based tests without ASAN.
Closes: #1016
Approved by: jlebon
Regression from previous tmpfile refactoring; unfortunately
the `OSTREE_REPO_COMMIT_MODIFIER_FLAGS_GENERATE_SIZES` option
only has coverage via gjs currently.
Might expose it via the cmdline in a later option, but in the big picture the
idea was that this data is better kept in static deltas.
Closes: https://github.com/ostreedev/ostree/issues/1014Closes: #1016
Approved by: jlebon
See: https://github.com/projectatomic/rpm-ostree/issues/885
If we get a successful Apache directory listing HTML when fetching what we
intend to be a ref, we'd dump the HTML into the error.
I did some scanning of the pull code, and this was the only case
I saw offhand where we were dumping text out into an error. Which
makes sense, since most of our formats are binary, the exeptions I
think are just `repo/config` and `repo/refs/`.
Closes: #1015
Approved by: mbarnes
Using the error prefixing in the delta processing allows us to
do new code style. Also strip trailing whitespace.
Use error prefixing in a few other random places. I didn't
hunt for all of them, just testing out the new API.
Use `glnx_fchmod()`. Also note I dropped one `fchmod (tmpf, 0600)`
which is no longer necessary.
Update submodule: libglnx
Closes: #1011
Approved by: jlebon
Prep for `ostree_repo_new_at()`. Down the line perhaps
we should extend libcurl to accept a file descriptor for cookies,
but this works OK for now.
Closes: #1010
Approved by: jlebon
Prep for `ostree_repo_new_at()`. These commands were directly accessing
`repo->repodir`, which it turns out was unnecessary since the the APIs they then
used were fd-relative. Except actually there were bugs there, so fix all of the
cookie util code to actually use the passed `dfd` and not just hardcode
`AT_FDCWD`.
Also, libsoup can't handle this (its APIs require fully qualifed paths), and
there's not a really good reason to have two implementations now; historically
it was useful to cross-check them, but I don't think we need that.
While I'm here, port to new style.
Closes: #1010
Approved by: jlebon
(Note this PR was reverted in <https://github.com/ostreedev/ostree/pull/902>;
this version should be better)
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.
This second iteration of this patch fixes the bug we hit the first time;
embarassingly enough I broke `ostree remote list` finding system remotes.
The fix is to have `ostree_repo_open()` figure out whether it's the same
as `/ostree/repo` for now.
Down the line...we might consider having the `ostree remote` command line itself
instatiate an `OstreeSysroot` by default, but this maximizes compatibility; we
just have to pay a small cost that `ostree` usage outside of that case like
`ostree static-delta` in a releng Jenkins job or whatever will do this `stat()`
too.
Closes: https://github.com/ostreedev/ostree/issues/892Closes: #1008
Approved by: mbarnes
This came up in <https://github.com/ostreedev/ostree/pull/982>; when
we added more direct local importing, we did it synchronously.
This was actually quite a regression when doing local pulls between different
modes; in particular between a bare mode and `archive`, as we were suddenly
doing gzip {de,}compression in the main thread.
Down the line actually...a simpler fix is probably to change things so that the
local path is really only used when we know we can hardlink; everything else
would go though the fetcher codepath but with `file://`.
But this isn't a lot more code, and the speed/interactivity win is large.
Note we're only doing content async with this patch. We could do metadata as
well; we have the object already local. But the metadata code path is messier,
and metadata objects are smaller.
Another area where this comes up is that in e.g. Fedora releng, most operations
talk to a NetApp via NFS. So this has the classic network filesystem problem
that operations that are normally cheap like `stat()` can actually have
nontrivial latency. Doing as much as possible in threads is better there too.
Closes: #1006
Approved by: jlebon
Currently in Fedora we don't sign summaries, and every use of
`rpm-ostree` would emit to the journal an error when we failed
to fetch it.
Fix this by having `OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT` tell the fetcher
not to journal 404 errors. While fixing this, we had a mix of two booleans vs
the flags; fix things so we consistently use the flags in the fetcher and pull
code.
Closes: #1004
Approved by: mbarnes
We have variants of `test-basic` for all 4 modes, but not for pull-test, which
for some reason was named `pull-archive`, but mostly pulls *into* bare repos.
The test code was structured like the basic one where it called into a
`pull-test.sh`, so let's actually use it for 2/3 bare modes. (I tried to extend
it to `bare-user-only` but it failed, going to look at that after this).
This is related to https://github.com/ostreedev/ostree/issues/991Closes: #998
Approved by: jlebon
When we [switched to using checkout + force_copy](https://github.com/ostreedev/ostree/commit/e8efd1c8dcaad8fbd3b05c400972d237406263e7),
a side effect that went unnoticed at the time is that we started
setting directory mtimes to zero.
See the below bug where we long ago set the file times to zero, which got fixed,
so let's not regress things by setting the directory times to zero either. (Even
though AFAICS GNU tar doesn't complain about those)
This semantic is somewhat "overloaded" onto `force_copy`, but it avoids adding
yet another boolean; we don't have that many reserved boolean slots left. I
can't really think of many good use cases for `force_copy` *other* than the
`/etc` merge anyways.
https://bugzilla.redhat.com/show_bug.cgi?id=1229160
Closes: https://github.com/ostreedev/ostree/issues/995Closes: #997
Approved by: jlebon
There are a number of simple ports here. Prep for further work
in `/etc` merge.
I also stripped trailing whitespace globally.
Closes: #996
Approved by: jlebon
And in general, if for some reason we can't write `user.` xattrs, provide an
error immediately rather than doing it during a later pull. This way the failure
cause is a lot more obvious.
Related: https://github.com/ostreedev/ostree/issues/991Closes: #993
Approved by: jlebon
In the storage PR I was trying to do a `pull-local` of the whole
`/ostree/repo` on the system, which ended up triggering a `g_critical()`
in the collections code, since we tried to parse a remote-prefixed ref
`fedora:fedora/26/x86_64/atomic-host` as a ref.
I'm not sure offhand what our behavior in this case *should* be. I
think git only clones local refs, but I need to check.
This corner case arises only with `pull-local`. But in any case,
while we were previously saying this is programmer error, since it's
so easy to pass various unchecked input into the pull machinery,
make invalid refs an explicit error.
Closes: #992
Approved by: jlebon
For ostree-as-host, we're the superuser, so we'll blow past
any reserved free space by default. While deltas have size
metadata, if one happens to do a loose fetch, we can fill
up the disk.
Another case is flatpak: the system helper has similar concerns
here as ostree-as-host, and for `flatpak --user`, we also
want to be nice and avoid filling up the user's quota.
Closes: https://github.com/ostreedev/ostree/issues/962Closes: #987
Approved by: jlebon
This is prep for storage space checks, where we look at free
space after parsing the metadata, before we write anything.
We did length-limited writes in the fd-based input path, but not for the
`GInputStream` path which in practice is used for HTTP pulls.
Closes: #987
Approved by: jlebon
Some of the Jenkins jobs for Fedora Atomic Host broke after updating
to 2017.7, and it turns out that we regressed handling unreadable
files in `bare-user` mode. An example of this is `/etc/shadow`, which
ends up in the ostree-as-host content as `/usr/etc/shadow`.
Now there are better fixes here; we should probably delete it and create it
during the config merge if it doesn't exist. In general, having secret files in
ostree really isn't supported, so it doesn't make sense to include them.
But let's fix this regression - when operating as an unprivileged user we don't
have `CAP_DAC_OVERRIDE` and hence will fail to open un-user-readable objects.
(We still preserve the actual `0` mode of course in the xattr and will
apply it in `bare`)
Closes: #989
Approved by: jlebon
Previously, we only supported additions in the statoverride file;
it was mainly for adding the setuid bit without having that physically
on disk.
However, for testing a change to `bare-user` handling around *unreadable*
files (which happens for `/etc/shadow` in host content), I need a way
to write that into a repo in the test suite.
I'm not actually aware of a non-test-suite use case for this; a more
sophisticated user is going to be using the API directly, which can already do
this. But we need it for tests at least.
Closes: #989
Approved by: jlebon
I had thought `glnx_link_tmpfile_at()` actually consumed the tmpfile;
it does consume the *path* but not the fd. In the non-delta path
things were fine since we used the autocleanup.
But the delta code had a tmpfile allocated in its struct that got reused, and
hence leaked the fd. Fix this by making the commit API actually consume the
tmpfile fully, just like the path path.
Closes: #986
Approved by: jlebon
This is a lot like `git clone --reference`, but we chose "localcache" as the
term "reference" is already used.
The main use case I'm targeting this for is the Fedora Atomic Host installer
case where we embed the repo content in the installer, but we may want to
kickstart and download newer content. There, while we want to get a newer ref,
we can still use the local repo as an object cache, since we have it sitting
there in memory anyways.
Another case is where one has a host ostree (say e.g. Fedora Atomic
Workstation), and one wants to create a local archive mirror of FAH. Then one
can use `pull --reference /ostree/repo` and pull the common objects (e.g.
contents of `bash.rpm` etc.)
Closes: https://github.com/ostreedev/ostree/issues/975Closes: #982
Approved by: jlebon
It's more natural for a few calling places. Prep for patches to go the other
way, which in turn are prep for adding a commit filter v2 that takes `struct
stat`.
`ot_gfile_type_for_mode()` was only used in this function, so inline it here.
Closes: #974
Approved by: jlebon
Previously, `ostree pull` was silent if not on a tty. I don't
see a reason not to print the final status line at least. This
is prep for more work in the test suite, so I can write assertions
on the output.
But it should also be nicer for people who e.g. do an `ostree pull` in a Jenkins
job or whatever.
Closes: #981
Approved by: jlebon
The `-z2` is annoying now since it's really a legacy; we've long
since supported typing `archive`. Convert the docs fully and
explain that.
Also do some (but not all) of the tests just to encourage newer tests to use
`archive` too.
Closes: #980
Approved by: jlebon
Obviously very minor, but I noticed this while working on `pull --reference`. If
we have a local repo, we'll have already done a hardlink and copied the detached
metadata too, so there's no reason to request it again via the fetcher path.
Closes: #978
Approved by: jlebon
I'd mostly been avoiding this file since there's always patches outstanding, but
these few functions shouldn't conflict much.
Closes: #979
Approved by: jlebon
I added `glnx_open_anonymous_tmpfile()`, but then later noticed
that the usage of this was really to be combined with `mmap()`,
and we had two versions of that in the delta code. Add a helper.
(Bigger picture...how is this different from glibc's "mmap() of /dev/zero"
approach for large chunks? One advantage is the storage can be "swapped" to
`/var/tmp`, but still deleted automatically, rather than requiring swap space)
Closes: #973
Approved by: jlebon
There's lots of mechanically replacing `OtTmpFile` with `GLnxTmpfile`;
the biggest changes are in the commit path. Symlink commits are now
very clearly separated from regular files. Symlinks are `OtCleanupUnlinkat`,
and regular files are `GLnxTmpfile`.
The commit codepath separates those as `_ostree_repo_commit_path_final()` and
`_ostree_repo_commit_tmpf_final()`. A nice aspect of all of this is that they
both *consume* the temporary on success. This avoids an extra spurious
`unlink()` call.
One of the biggest bits of code motion is in `commit_loose_regfile_object()`,
which no longer needs to care about symlinks. For the most parth though it's
just removing conditionals.
Update submodule: libglnx
Closes: #958
Approved by: jlebon
Computing download/storage size for `archive` pulls is hard; there's
`OSTREE_REPO_COMMIT_MODIFIER_FLAGS_GENERATE_SIZES` which was from a
pre-static-deltas effort by Endless, but we aren't currently making use of this
much.
Static deltas were designed to solve this problem; we have the total
uncompressed size. Let's check free space before doing a delta pull.
Related: https://github.com/ostreedev/ostree/issues/962Closes: #963
Approved by: jlebon
The `OstreeRepoContentBareCommit` struct was basically an `OtTmpFile`, so let's
make it one. I moved the "convert to `GOutputStream`" logic into the callers,
since that bit can't fail; it makes the implementation much simpler since we can
just return the result of `ot_open_tmpfile_linkable_at()`.
Prep for `GLnxTmpfile` porting.
Closes: #957
Approved by: jlebon
The pull code also could make use of this in both the metadata and content
paths. I changed it to own the tempfile malloc (just like `GLnxTmpFile`), since
there's no reason to have different lifetimes for the filename and the file, and
that way we only have one variable rather than two.
The content path turns out to be a special case though, where
at least for mirroring archives, we directly pass the file *path*
down into `_ostree_repo_commit_loose_final()`.
This is prep for `GLnxTmpFile` porting.
Closes: #957
Approved by: jlebon
The variables here were duplicative; we don't need two booleans to distinguish
between symlinks and regular files. What we do need to handle is the "physical"
state versus the "object" state. Symlinks objects are stored as regular files in
`bare-user` and `archive`.
Prep for more cleanup.
Closes: #957
Approved by: jlebon
This is followon work from previous cleanups. Basically
`stat_bare_content_object()` was the `fstatat()` logic
and `ostree_repo_read_bare_fd()` was the `openat()` implementation;
they duplicated some bits to find the object in staging, recurse
into parent etc.
Further, I wanted an internal-only version of this API which didn't allocate
`GFileInfo`/`GInputStream` but used a plain `fd` and `struct stat` to avoid
mallocs.
The end version here I think looks a lot nicer, since we deduplicate the various
`open()` calls in the different cases for example.
Closes: #952
Approved by: jlebon
Prep for future cleanup patches (in particular I want an internal-only
version at first that uses a fd+`struct stat`) to avoid allocations.
The new version avoids lots of deep nesting of conditionals as well
by hoisting the "not found" handling to an early return.
There's a bit of code duplication between the two cases but it's
quite worth the result.
Closes: #951
Approved by: jlebon
Prep for a change in `ostree_repo_load_file()`. We would crash if a
caller had `out_file_info = NULL`, because we deref `ret_file_info`
below it.
Closes: #951
Approved by: jlebon
We don't want to inject this warning by default for every build
like the other ones in `configure.ac`, since it can be spruriously
wrong. But there's no reason not to have a more extended set of
warnings for well-known toolchains (e.g. f25).
Closes: #943
Approved by: jlebon
Colin Walters