Really, all `ostree admin finalize-staged` needs is access to `/sysroot`
and `/boot`. So let's activate it right after `local-fs.target` so that
it gets deactivated later in the shutdown process. This should allow us
to conflict with less services still running and possibly writing things
to `/etc`.
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1672283Closes: #1840
Approved by: cgwalters
On at least one user's computer, g_getenv("http_proxy") returns the
empty string, so check for that and treat it as no proxy rather than
printing a warning.
See https://github.com/flatpak/flatpak/issues/2790Closes: #1835
Approved by: cgwalters
Currently the P2P code requires you to trust every remote you have
configured to the same extent, because a remote controlled by a
malicious actor can serve updates to refs (such as Flatpak apps)
installed from other remotes.[1] The way this attack would play out is
that the malicious remote would deploy the same collection ID as the
victim remote, and would then be able to serve updates for it.
One possible remedy would be to make it an error to configure remotes
such that two have the same collection ID but differing GPG keys. I
attempted to do that in Flatpak[2] but it proved difficult because it is
valid to configure two remotes with the same collection ID, and they may
then each want to update their keyrings which wouldn't happen
atomically.
Another potential solution I've considered is to add a `trusted-remotes`
option to ostree_repo_find_remotes_async() which would dictate which
keyring to use when pulling each ref. However the
ostree_repo_finder_resolve_async() API would still remain vulnerable,
and changing that would require rewriting a large chunk of libostree's
P2P support.
So this commit represents a third attempt at mitigating this security
hole, namely to have the client specify which remote to use for GPG
verification at pull time. This way the pull will fail if the commits
are signed with anything other than the keys we actually trust to serve
updates.
This is implemented as an option "ref-keyring-map" for
ostree_repo_pull_from_remotes_async() and
ostree_repo_pull_with_options() which dictates the remote to be used for
GPG verification of each collection-ref. I think specifying a keyring
remote for each ref is better than specifying a remote for each
OstreeRepoFinderResult, because there are some edge cases where a result
could serve updates to refs which were installed from more than one
remote.
The PR to make Flatpak use this new option is here[3].
[1] https://github.com/flatpak/flatpak/issues/1447
[2] https://github.com/flatpak/flatpak/pull/2601
[3] https://github.com/flatpak/flatpak/pull/2705Closes: #1810
Approved by: cgwalters
When writing a delta to a file this may not always be recorded
in the filename, and it's useful data.
Ref: https://mail.gnome.org/archives/ostree-list/2019-February/msg00000.html
This also required teaching `show` to accept a file path.
Note...for some reason `test-deltas.sh` breaks when run from
a tty - we get `SIGTTIN` which implies something is reading from
the tty but it wasn't obvious to me what.
Closes: #1823
Approved by: jlebon
In Silverblue right now, the boot menu title looks like this:
Fedora 29.20190301.0 (Workstation Edition) 29.20190301.0 (ostree)
This is because RPM-OSTree's `mutate-os-release` feature is enabled,
which injects the OSTree version string directly into `VERSION` and
`PRETTY_NAME`. So appending the version string again is a bit redundant.
Let's just do a simple substring check here before adding the version to
the title.
Closes: #1829
Approved by: cgwalters
The sysroot.bootloader key configures the bootloader
that OSTree uses when deploying a sysroot. Having this key
allows specifying behavior not to use the default bootloader
backend code, which is preferable when creating a first
deployment from the sysroot (#1774).
As of now, the key can take the values "auto" or "none". If
the key is not given, the value defaults to "auto".
"auto" causes _ostree_sysroot_query_bootloader() to be used
when writing a new deployment, which is the original behavior
that dynamically detects which bootloader to use.
"none" avoids querying the bootloader dynamically. The BLS
config fragments are still written to
sysroot/boot/loader/entries for use by higher-level software.
More values can be supported in future to specify a single
bootloader, different behavior for the bootloader code, or
a list of bootloaders to try.
Resolves: #1774Closes: #1814
Approved by: jlebon
Add ot_keyfile_get_value_with_default_group_optional() which allows
getting values from keys where the group is optional in the config
file. This is preparatory to add the sysroot.bootloader repo config
key, where the sysroot group is optional.
Closes: #1814
Approved by: jlebon
Rename ot_keyfile_get_string_as_list() to
ot_keyfile_get_string_list_with_separator_choice() which expresses
more clearly why the function is needed. Also shorten the
function comment.
Closes: #1814
Approved by: jlebon
Currently it's not an error to provide too many arguments to an ostree
config command. Change it so we print usage information in that case,
and update the unit tests.
Closes: #1743
Approved by: cgwalters
It seems cleaner to make the GKeyFile a g_autoptr variable and just
return rather than using the "goto out;" idiom.
Closes: #1743
Approved by: cgwalters
Currently there's a way to set a key to the empty string but there's no
way to unset it completely (remove the key from the group). This might
be helpful for instance if you want to temporarily set
"core.lock-timeout-secs" to a specific value for the duration of one
operation and then return it to the default after that operation
completes.
This commit implements an "unset" operation for the config command, adds
a unit test, and updates the man page.
Closes: #1743
Approved by: cgwalters
Currently the behavior of ostree_repo_resolve_rev() is that it tries to
resolve a ref to a commit by checking the refs/ directories, but also by
checking for in-memory ref-checksum pairs which are part of an
in-progress transaction and also by checking the parent repo if one
exists. Currently ostree_repo_resolve_collection_ref() only checks the
refs/ directories, so this commit makes its behavior analagous since it
is the analagous API which supports collection-refs.
The impetus for this was that currently Flatpak uses
ostree_repo_resolve_rev() to load a commit after doing a P2P pull in
flatpak_dir_do_resolve_p2p_refs(), but that assumes the ref came from
the same remote that originally provided it, which might not be the case
if more than one remote has the same collection ID configured. And
changing Flatpak to use ostree_repo_resolve_collection_ref() doesn't
work without this patch.
Closes: #1821
Approved by: pwithnall
This uses the OSTREE_REPO_REMOTE_CHANGE_REPLACE operation to add a
remote or replace an existing one. This is roughly the opposite of
--if-not-exists and will raise an error if both options are passed.
Closes: #1166
Approved by: cgwalters
Add the OSTREE_REPO_REMOTE_CHANGE_REPLACE operation to the
OstreeRepoRemoteChange enum. This operation will add a remote or replace
an existing one. It respects the location of the remote configuration
file when replacing and the remotes config dir settings when adding a
new remote.
Closes: #1166
Approved by: cgwalters
We have a `http2=[0|1]` remote config option; let's have the
`--disable-http2` build option define the default for that. This way
it's easy to still enable http2 for testing even if
we have it disabled by default.
Closes: #1798
Approved by: jlebon
Similar as available for u-boot (ce2995e1dc)
and syslinux (c5112c25e4), enable parsing
and writing devicetree filename into grub.cfg.
This is required by arm64-based devices running edk2 instead of u-boot
as the main bootloader (e.g. 96boards HiKey and HiKey960).
Signed-off-by: Ricardo Salveti <ricardo@foundries.io>
Closes: #1790
Approved by: cgwalters
Even with the previous docstring, I didn't understand at first the
relationship between the `ostree-grub-generator` script and
`ostree-bootloader-grub2.c`. Throw some more docs to clarify things a
bit.
Closes: #1791
Approved by: cgwalters
We want a case where we can disable the min-free-space check. Initially,
it felt like to add a OSTREE_REPO_PULL_FLAGS_DISABLE_FREE_SPACE_CHECK but
the problem is prepare_transaction() does not have a OstreeRepoPullFlags
parameter which we can parse right here. On top of it, prepare_transaction()
enforces min-free-space check and won't let the transaction proceed if
the check failed.
This is pretty bad in conjunction with "inherit-transaction" as what
Flatpak uses. There is no way to disable this check unless we remove
it altogether from prepare_transaction.
This issue came out to light when flatpak wasn't able to write metadata
after fetching from remote:
[uajain@localhost ~]$ flatpak remote-info flathub org.kde.Platform//5.9
error: min-free-space-size 500MB would be exceeded
Metadata objects helps in housekeeping and restricting them means
restricting crucial UX (like search, new updates) functionalities
in clients like gnome-software. The error banners originated from
these issues are also abrupt and not much helpful to the user. This
is the specific instance of the issue this patches tries to address.
See https://github.com/flatpak/flatpak/issues/2139 for discussion.
Closes: #1779
Approved by: mwleeds
The way _ostree_repo_import_object() is written, a hardlink copy is only
attempted if the source repo is trusted, so update the docs for
ostree_repo_import_object_from_with_trust() to reflect that.
Closes: #1777
Approved by: cgwalters
This allows specifying gpgpath as list of
paths that can point to a file or a directory. If a directory path
is given, paths to all regular files in the directory are added
to the remote as gpg ascii keys. If the path is not a directory,
the file is directly added (whether regular file, empty - errors
will be reported later when verifying gpg keys e.g. when pulling).
Adding the gpgkeypath property looks like:
ostree --repo=repo remote add --set=gpgpath="/path/key1.asc,/path/keys.d" R1 https://example.com/some/remote/ostree/repoCloses#773Closes: #1773
Approved by: cgwalters
When falling back to copying objects when importing them into a
bare-user repo, we only actually need to transfer over the
`user.ostreemeta` xattr.
This allows the destination repo to be on a separate filesystem that
might not even support `security.selinux`. (I hit this while importing
over virtio-9p).
Closes: #1771
Approved by: cgwalters
I found this useful while hacking on rpm-ostree but I think it might be
useful enough to upstream. This stat is really helpful for validating
that a pipeline is hitting the devino cache sweet spot.
Closes: #1772
Approved by: cgwalters
It might be "local", but e.g. we may be crossing filesystems. So there
are valid use cases for only wanting to pull the commit metadata with
`pull-local`.
Closes: #1769
Approved by: cgwalters
This is the alias version of #1749. I.e. we want to make sure that one
can't even create an alias which would end up dangling.
See also: https://pagure.io/releng/issue/7891Closes: #1768
Approved by: sinnykumari
if a file ".wh..wh..opq" is present in a directory, delete anything
from lower layers that is already in that directory.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Closes: #1486
Approved by: cgwalters
Wrap the `Version` key in the YAML-compatible output of
`ostree --version` with quotes so that it's parsed as a string. The
issues with the previous approach in a nutshell:
```
In [5]: yaml.load("asdf: 2018.10")
Out[5]: {'asdf': 2018.1}
```
It's treating the version number as a floating-point. Now, this is
technically a backwards incompatible change, but given that the previous
approach is inherently broken for our needs, I don't see a way around
breaking it now.
Closes: #1761
Approved by: cgwalters
This renames a config key to make its semantics more obvious. Despite
what the commit message says, it only applies when a set of repo finders
is not specified (either on the command line or in a library API call).
This also renames the corresponding ostree_repo_get function. We can do
this since it hasn't been released yet.
Closes: #1763
Approved by: pwithnall
Rather than manually starting the `ostree-finalize-staged.service` unit,
we can leverage systemd's path units for this. It fits quite nicely too,
given that we already have a path we drop iif we have a staged
deployment.
To give some time for the preset to make it to systems, we don't yet
drop the explicit call to `systemctl start`. Though we do make it
conditional based on a DEBUG env var so that we can actually test it in
CI for now. Once we're sure this has propagated, we can drop the
`systemctl start` path and the env var together.
Closes: #1740
Approved by: cgwalters
This commit disables searching on the local network for refs, unless
explicitly requested by the user either by changing the value of the
"core.repo-finders" config option, or by passing an OstreeRepoFinderAvahi to
ostree_repo_find_remotes_async() / ostree_repo_finder_resolve_async(),
or by specifying "lan" in the --finders option of the find-remotes
command.
The primary reason for this is that ostree_repo_find_remotes_async()
takes about 40% longer to complete with the LAN finder enabled, and that
API is used widely (e.g. in every flatpak operation). It's also probable
that some users don't want ostree doing potentially unexpected traffic
on the local network, even though everything pulled from a peer is GPG
verified.
Flathub will soon deploy collection IDs to everyone[1] so these code
paths will soon see a lot more use and that's why this change is being
made now.
Endless is the only potential user of the LAN updates feature, and we
can revert this patch on our fork of ostree. For it to be used outside
Endless OS we will need to upstream eos-updater-avahi and
eos-update-server into ostree.
[1] https://github.com/flathub/flathub/issues/676Closes: #1758
Approved by: cgwalters
Currently libostree essentially has two modes when it's pulling refs:
the "legacy" code paths pull only from the Internet, and the code paths
that are aware of collection IDs try to pull from the Internet, the
local network, and mounted filesystems (such as USB drives). The problem
is that while we eventually want to migrate everyone to using collection
IDs, we don't want to force checking LAN and USB sources if the user
just wants to pull from the Internet, since the LAN/USB code paths can
have privacy[1], security[2], and performance[3] implications.
So this commit implements a new repo config option called "repo-finders"
which can be configured to, for example, "config;lan;mount;" to check
all three sources or "config;mount;" to disable searching the LAN. The
set of values mirror those used for the --finders option of the
find-remotes command. This configuration affects pulls in three places:
1. the ostree_repo_find_remotes_async() API, regardless of whether or
not the user of the API provided a list of OstreeRepoFinders
2. the ostree_repo_finder_resolve_async() /
ostree_repo_finder_resolve_all_async() API
3. the find-remotes command
This feature is especially important right now since we soon want to
have Flathub publish a metadata key which will have Flatpak clients
update the remote config to add a collection ID.[4]
This effectively fixes https://github.com/flatpak/flatpak/issues/1863
but I'll patch Flatpak too, so it doesn't pass finders to libostree only
to then have them be removed.
[1] https://github.com/flatpak/flatpak/issues/1863#issuecomment-404128824
[2] https://github.com/ostreedev/ostree/issues/1527
[3] Based on how long the "ostree find-remotes" command takes to
complete, having the LAN finder enabled slows down that step of the
pull process by about 40%. See also
https://github.com/flatpak/flatpak/issues/1862
[4] https://github.com/flathub/flathub/issues/676Closes: #1758
Approved by: cgwalters
Previously, we were preparing the root very late in the boot process;
right before we switch root. The issue with that is that most services
in the initrd that run `After=initrd-root-fs.target` expect that
`/sysroot` already points to the rootfs we'll be pivoting to. Running
this late violates that assumption.
This patch fixes this by making `ostree-prepare-root.service` instead
run right after `sysroot.mount` (the physical sysroot mounted by
systemd) but still before `initrd-root-fs.target` (which is the target
signalling that `/sysroot` is now valid and ready).
This should make it easier to integrate OSTree with other initrd
services such as Ignition.
Related: https://github.com/dustymabe/ignition-dracut/issues/20Closes: #1759
Approved by: cgwalters
For the same reasons as #1697. This is especially important in services
that are likely to be used as an `After/Before=` target in other units.
`ostree-prepare-root.service` is one such service.
Closes: #1759
Approved by: cgwalters
See https://github.com/projectatomic/rpm-ostree/issues/1568
Basically for people on e.g. rotational media, the default 90
second timeout can be too small.
We're in a tough situation here, because delaying shutdown
can be problematic too if the user is trying to shut down their
laptop to put in a backpack, etc.
There's potential optimizations here to make; I think we
could pre-copy the kernel/initramfs for example.
I suspect for some people the grub2 os-prober is a factor here too,
if that tries to e.g. inspect attached USB rotational hard drives.
But hopefully we'll get rid of that soon.
Closes: #1755
Approved by: jlebon
I was debugging some rpm-ostree work and saw:
`openat: No such file or directory`
and it wasn't immediately obvious it was stderr from `rofiles-fuse`.
Use the `err` API which is better in many ways; in this case
it automatically prefixes with `argv0`.
Closes: #1747
Approved by: jlebon
The idea is that if the process is running as root, it can change
ownership of newly written files to match the owner of the repo.
Unfortunately, it currently applies in the other direction, too - a
non-root user writing to a root owned repository. If the repo is
writable by the user but owned by root, it can still create files and
directories there, but it can't change ownership of them.
This feature comes from
https://bugzilla.gnome.org/show_bug.cgi?id=738954. As it turns out, this
feature was never completed. It only works on content objects and not
metadata objects, refs, deltas, summaries, etc. Rather than try to fix
all of those, remove the feature until someone has interest in
completing it.
Closes: #1754
Approved by: cgwalters
Actually testing the patch to add `--force-copy-zerosized` to
rpm-ostree tripped over the fact that it uses `--union-identical`,
and we just hit an assertion failure with that combination.
Fix this by copying over the logic we have for the hardlink case.
Closes: #1753
Approved by: jlebon
In rpm-ostree we've hit a few cases where hardlinking zero-sized
files causes us problems. The most prominent is lock files in
`/usr/etc`, such as `/usr/etc/selinux/semanage.LOCK`. If there
are two zero-sized lock files to grab, but they're hardlinked,
then locking will fail.
Another case here is if one is using ostree inside a container
and don't have access to FUSE (i.e. `rofiles-fuse`), then the
ostree hardlinking can cause files that aren't ordinarily hardlinked
to become so, and mutation of one mutates all. An example where
this is concerning is Python `__init__.py` files.
Now, these lock files should clearly not be in the tree to begin
with, but - we're not gaining a huge amount by hardlinking these
files either, so let's add an option to disable it.
Closes: #1752
Approved by: jlebon
Deleting a ref with aliases makes them dangling. In such
cases, display an error message to the user.
Fixes#1597
Signed-off-by: Sinny Kumari <sinny@redhat.com>
Closes: #1749
Approved by: cgwalters
Write to the journal when starting to finalize a staged deployment.
Combined with the "Transaction completed" message we already emit, this
makes it easy later on to determine whether the operation was successful
by inspecting the journal. This will be used by `rpm-ostree status`.
Closes: #1750
Approved by: cgwalters
It's a neat way to point folks to the documentation (of course, better
would be to have man pages for each of those services). Also
consistently use Title Case everywhere.
Closes: #1750
Approved by: cgwalters
There are use cases for libostree as a local content store
for content derived or delivered via other mechanisms (e.g. OCI
images, RPMs, etc.). rpm-ostree today imports RPMs into OSTree
branches, and puts the RPM header value as commit metadata.
Some of these can be quite large because the header includes
permissions for each file. Similarly, some OCI metadata is large.
Since there's no security issues with this, support committing
such content.
We still by default limit the size of metadata fetches, although
for good measure we make this configurable too via a new
`max-metadata-size` value.
Closes: https://github.com/ostreedev/ostree/issues/1721Closes: #1744
Approved by: jlebon
Currently the locking code checks if the value -1 was set for the config
key "lock-timeout-secs" and if so, a thread trying to acquire a lock
will block indefinitely. Positive values specify how long to attempt to
acquire a lock in a non-blocking way (the attempt is made once every
second). But when the value is read from the config file,
g_ascii_strtoull() is used, which converts it to an unsigned integer.
This commit makes libostree use g_ascii_strtoll() instead, so that it's
possible to set that key to -1 as intended.
Closes: #1737
Approved by: jlebon
Currently on Endless OS, the OSTree ref for the operating system is
something like os/eos/amd64/eos3, so that's what gets passed to `ostree
create-usb` when copying the OS to a USB drive (for offline updates).
However, when eos-updater checks for updates it pulls the metadata for a
candidate commit and in so doing updates that eos3 ref to point to the
partial commit being examined as a potential update rather than the
deployed commit. This causes `ostree create-usb` to fail with an error
like "No such metadata object
7fb045cb2d1f1f3a81bfc157c6128ff443eb56350315b9536bdb56aee0659863.dirtree".
OSTree creates deployment refs that look like "ostree/1/1/0" to maintain
a pointer to the deployed commit, but create-usb can't use these because
it shows up in the summary as just a ref, not a collection-ref.
So this commit adds a --commit option to the create-usb command, so we
can use the appropriate ref but copy the deployed commit rather than a
(potentially partial) update commit.
Closes: #1735
Approved by: cgwalters
Copying the xattrs on metadata objects is wrong in general, we
don't "own" them. Notably this would fail in the situation of
doing a pull from e.g. a `bare-user` source to a destination
that was on a different mount point (so we couldn't hardlink),
and the source had e.g. a `security.selinux` attribute.
Closes: #1734Closes: #1736
Approved by: jlebon
Change the create-usb command so that it always creates the destination
repository using the "archive" mode, rather than using archive mode when
xattrs aren't supported and bare-user otherwise. This has a few
advantages:
1. The archive mode works with FAT filesystems, which is what most
USB drives are, and which doesn't support xattrs.
2. At least in some quick testing I did, archive mode is about
twice as performant as bare-user mode, in terms of how long it takes for
the create-usb command to complete.
3. This ensures that a tool can safely change the permissions on
".ostree/repo" and subdirectories after create-usb completes, which is
important for Endless since otherwise you can't use `ostree create-usb`
as root and then `flatpak create-usb` as a non-root user on the same USB
drive (or in other words copy OS updates and apps to the same USB).
Closes: #1733
Approved by: cgwalters
Just include the whole URL that failed if libcurl failed with something
elementary like CURLE_COULDNT_CONNECT or CURLE_COULDNT_RESOLVE_HOST.
Closes: #1731Closes: #1732
Approved by: cgwalters
There's some subtlety to this, we don't handle all cases.
But the 99% cases are using `--sysroot deploy` to create an
initial deployment, and then doing upgrades from inside
a booted deployment.
It was only the latter case that didn't work with a separate `/var`.
Fixing all of them would probably require libostree to learn
how to e.g. look at `/etc/fstab` (or worse, systemd mount units?)
and handle the mounting. I don't think we want to do anything
like that right now, since there are no active drivers for the
use case.
Closes: https://github.com/ostreedev/ostree/issues/1729Closes: #1730
Approved by: akiernan
Earlier, the actual reserved space (in blocks) were calculated inside the
transaction codepath ostree_repo_prepare_transaction(). However, while
reworking on ostree_repo_get_min_free_space_bytes() API, it was realized that
this calculation can be done independently from the transaction's codepaths, hence
enabling the usage for ostree_repo_get_min_free_space_bytes() API irrespective
of whether there is an ongoing transaction or not.
https://github.com/ostreedev/ostree/issues/1720Closes: #1722
Approved by: pwithnall
This commit defines a metadata key that tells clients to update their
remote config to add a collection ID. This functionality is currently
implemented in Flatpak for the key "xa.collection-id", but there are two
good reasons for moving the key to OSTree:
1) Servers such as Flathub shouldn't set xa.collection-id in their
metadata now or in the medium term future, because many users are still
using old versions of Flatpak and OSTree[1] which would hit various
bugs[2][3][4] on the P2P code paths that are enabled by collection IDs.
Defining a new key means that only clients running recent
(as-yet-unreleased) versions of Flatpak and OSTree will pay attention to
it and deploy the collection ID, leaving the users on old versions
unaffected.
2) OSTree is as "invested" in collection IDs as Flatpak, so there's no
reason the key should be defined in Flatpak rather than here. According
to Philip Withnall, the reason the key was put in Flatpak originally was
that at the time there was uncertainty about tying OSTree to collection
IDs.
[1] https://ahayzen.com/direct/flathub.html#downloadsbyflatpakstacked
[2] https://github.com/ostreedev/ostree/commit/e4e6d85ea
[3] https://github.com/flatpak/flatpak/commit/5813639f
[4] https://github.com/flatpak/flatpak/commit/5b21a5b7Closes: #1726
Approved by: pwithnall
There is no API method to remove a file or subdirectory from a MutableTree
besides directly manipulating the GHashTable returned by _get_files or
_get_subdirs. This isn't possible from an introspection binding that transforms
the returned GHashTable, and may also leave the tree checksum in an invalid
state. Introduce a new method so that removing files or subdirectories is
safe, and possible from bindings.
Closes: #1724
Approved by: jlebon
This fixes typos and grammar in the docs for OstreeRepo, and copies the
information about OSTREE_REPO_MODE_BARE_USER_ONLY from ostree-core.h
Closes: #1725
Approved by: jlebon
In the OstreeRepoFinderAvahi implementation,
ostree_avahi_service_build_repo_finder_result() is where the DNS-SD
records are processed and turned into OstreeRepoFinderResult objects.
Each result object is supposed to have a hash table mapping refs to
checksums, so this is accomplished by first adding a placeholder (a ref
mapping to a NULL checksum) for each ref matched by the bloom filter,
and later filling in the checksums using the remote's summary file,
which happens in get_checksums(). The problem is that there's no
guarantee all the checksums will be resolved (non-NULL), so the
ostree_repo_finder_result_new() call then hits an assertion failure in
is_valid_collection_ref_map() leading to a crash (in the case that one
or more refs had NULL checksums).
There are at least two situations where the ref checksum might not be
found in the peer remote's summary file:
1) The bloom filter match was a false positive. This is going to happen
sometimes by design.
2) The peer remote's summary is out of sync with its DNS-SD records.
This shouldn't normally happen but it's still good to be robust to the
possibility; in Endless OS nothing guarantees the atomicity of updating
the summary and DNS-SD records.
This commit changes libostree to be robust to the possibility of refs
missing from the peer remote's summary, by removing any that still have
a NULL checksum associated with them after the summary has been fetched
and processed.
The other OstreeRepoFinder implementations don't have this issue because
they use summary files directly and therefore always have access to the
checksum.
Closes: #1717
Approved by: pwithnall
when converted to bytes
In a subsequent commit, we add a public API to read the value of
min-free-space-* value in bytes. The value for free space check
is enforced in terms of block size instead of bytes. Therefore,
for consistency we check while preparing the transaction that the
value doesn't overflow when converted to bytes.
https://phabricator.endlessm.com/T23694Closes: #1715
Approved by: cgwalters
Debian and Debian-derived systems have their GRUB configuration file in
/boot/grub/grub.cfg, rather than /boot/grub2/grub.cfg. Detecting this
file is necessary to correctly generate GRUB boot configuration on
Debian systems.
Closes: #1714
Approved by: cgwalters
Fetching value from a repo config using 'ostree config
get SECTIONNAME.KEYNAME' didn't work in some cases like
when having dots in Group Name entry.
As per Desktop entry file specification, Group Name
may contain all ASCII characters except for [ and ]
and control characters.
Link - https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-1.1.html
Having --group option will help user to clearly specify
Group Name and get desired result.
It also adds test for ostree config get|set and bash
completion for --group option
Fixes https://github.com/ostreedev/ostree/issues/1565Closes: #1696
Approved by: cgwalters
If a ref already exists, we are likely only a few commits behind the
current head of the ref, so it is probably better for bandwidth
consumption to pull the individual objects rather than the from-scratch
delta.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1709
Approved by: cgwalters
Since 9dc6ddce08 it has not been true that
'new_config' was simply ref'd: it's serialized, and then re-parsed into
a new GKeyFile.
Closes: #1707
Approved by: jlebon
Add an invalid-cache test error flag to ensure that the code that checks
for and recovers from a corrupted summary cache is hit. This helps make
sure that the recovery path is actually used without resorting to
G_MESSAGES_DEBUG.
Closes: #1698
Approved by: cgwalters
If for some reason the cached summary doesn't match the cached signature
then fetch the remote summary and verify again. Since commit c4c2b5eb
this is unlikely to happen since the summary will only be cached if it
matches the signature. However, if the summary cache has been corrupted
for any other reason then it's best to be safe and fetch the remote
summary again.
This is essentially the corollary to c4c2b5eb. Where that commit helps
you from getting into the corrupted summary cache in the first place,
this helps you get out of it. Without this the client can get wedged
until a prune or the remote server republishes the summary.
Closes: #1698
Approved by: cgwalters
copy_option() unnecessarily passed ownership of the value
to g_variant_dict_insert_value, but that already refs, so it was leaked.
Closes: #1702
Approved by: cgwalters
We need to have the g_auto(GLnxDirFdIterator) inside the loop, or
we don't correctly clean up when iterating several times.
Closes: #1700
Approved by: cgwalters
Now that we have `auto-update-summary`, there is no point in having
`commit-update-summary`. The latter also only had an effect through
the `commit` CLI command, whereas the former is embedded directly in
libostree.
There is one corner case that slips through: `commit` would update the
summary file even if orphan commits were created, which we no longer do
here. I can't imagine anyone relying on this, so it seems safe to drop.
Closes: #1689Closes: #1693
Approved by: mwleeds
Mildly bikeshed, though I find the name `auto-update-summary` to be
easier to grok than `change-update-summary`. I think it's because it can
be read as "verb-verb-noun" rather than "noun-verb-noun".
Closes: #1693
Approved by: mwleeds
Normally, a configured remote will only serve refs with one associated
collection ID, but temporary remotes such as USB drives or LAN peers can
serve refs from multiple collection IDs which may use different GPG
keyrings. So the OstreeRepoFinderMount and OstreeRepoFinderAvahi classes
create dynamic OstreeRemote objects for each (uri, keyring) pair. So if
for example the USB mounted at /mnt/usb serves content from the
configured remotes "eos-apps" and "eos-sdk", the OstreeRepoFinderResult
array returned by ostree_repo_find_remotes_async() will have one result
with a remote called something like
file_mnt_usb_eos-apps.trustedkeys.gpg and the list of refs on the USB
that came from eos-apps, and another result with a remote
file_mnt_usb_eos-sdk.trustedkeys.gpg and the list of refs from eos-sdk.
Unfortunately while OstreeRepoFinderMount and OstreeRepoFinderAvahi
correctly only include refs in a result if the ref uses the associated
keyring, the find_remotes_cb() function used to clean up the set of
results looks at the remote summary file and includes every ref that's
in the intersection with the requested refs, regardless of whether it
uses a different remote's keyring. This leads to an error when you try
to pull from a USB containing refs from different collection IDs: the
pull using the wrong collection ID will error out with "Refspec not
found" and the result with the correct keyring will then be ignored "as
it has no relevant refs or they have already been pulled." So the pull
ultimately fails.
This commit fixes the issue by filtering refs coming from a dynamic
remote, so that only ones with the collection ID associated with the
keyring remote are examined. This only needs to be done for dynamic
remotes because you should be able to pull any ref from a configured
remote using its keyring. It's also only done when looking at the
collection map in the summary file, because LAN/USB remotes won't have a
"main" collection ID set (OSTREE_SUMMARY_COLLECTION_ID).
Closes: #1695
Approved by: pwithnall
This is standard practice for units like this; e.g. it's what
`systemd-remount-fs.service` does. I think it may be part of
or the whole cause for
https://github.com/projectatomic/rpm-ostree/issues/1471
I haven't reproduced the problem exactly but it seems to me that
if the unit starts and is GC'd, then when systemd goes to execute
a later unit it might end up restarting it.
A noticeable side effect of this is that `systemctl status ostree-remount`
exits with code `0` as expected.
Closes: #1697
Approved by: jlebon
This commits adds and implements a boolean repo config option called
"change-update-summary" which updates the summary file every time a ref
changes (additions, updates, and deletions).
The main impetus for this feature is that the `ostree create-usb` and
`flatpak create-usb` commands depend on the repo summary being up to
date. On the command line you can work around this by asking the user to
run `ostree summary --update` but in the case of GNOME Software calling
out to `flatpak create-usb` this wouldn't work because it's running as a
user and the repo is owned by root. That strategy also means flatpak
can't update the repo metadata refs for fear of invalidating the
summary.
Another use case for this relates to LAN updates. Specifically, the
component of eos-updater that generates DNS-SD records advertising ostree
refs depends on the repo summary being up to date.
Since ostree_repo_regenerate_summary() now takes an exclusive lock, this
should be safe to enable. However it's not enabled by default because of
the performance cost, and because it's more useful on clients than
servers (which likely have another mechanism for updating the summary).
Fixes https://github.com/ostreedev/ostree/issues/1664Closes: #1681
Approved by: jlebon
This ensures that commits aren't deleted and refs aren't added, removed,
or updated while the summary is being generated. This is in preparation
for adding a repo config option that will automatically regenerate the
summary on every ref change.
Closes: #1681
Approved by: jlebon
Using `MAX(0, $x)` here is useless since we're comparing against an
unsigned integer. Just unpack this and only subtract if it's safe to do
so.
Also, explicitly check for `fd >= 0` rather than just `!= -1` to be sure
it's a valid fd. And finally, explicitly check the return value of
`g_input_stream_read_all` as is done everywhere else in the tree and
make it clear that we're purposely ignoring the return value of `_flush`
here, but not in other places.
Discovered by Coverity.
Closes: #1692
Approved by: cgwalters
I initially was going to add a `G_DEFINE_AUTOPTR_CLEANUP_FUNC` for
`FetchStaticDeltaData`, but it honestly didn't seem worth mucking around
ownership everywhere and potentially getting it wrong.
Discovered by Coverity.
Closes: #1692
Approved by: cgwalters
In `write_metadata_object()`, make sure when creating tombstone commits
that we're actually passed an expected checksum to use.
In `write_dir_entry_to_mtree_internal()`, sanity check that `dfd_iter`
is indeed not `NULL` before trying to dereference it.
Discovered by Coverity.
Closes: #1692
Approved by: cgwalters
Since min_free_space_size_mb is considered before min_free_space_percent
in min_free_space_calculate_reserved_blocks(), it has to be considered
first when generating the error message in order for it to be accurate.
Closes: #1691
Approved by: jlebon
Previously, we would error out if both of the options were mentioned
in the config file (even if one of them is disabled with 0). There
were few suggestions that this behavior was not quite right.
Therefore, instead of throwing error and exiting, it's preferred to
warn the user. Hence, the solution that worked out is:
* Allow both options to exist simulateneously
* Check each config's value and decide:
* If both are present and are non-zero, warn the user. Also, prefer
to use min-free-space-size over the another.
* If both are absent, then use -percent=3% as fallback
* Every other case is valid hence, no warning
https://phabricator.endlessm.com/T13698
(cherry picked from commit be68991cf80f0aa1da7d36ab6e1d2c4d6c7cd3fb)
Signed-off-by: Robert McQueen <rob@endlessm.com>
Closes: #1685
Approved by: cgwalters
This is the inverse of https://github.com/ostreedev/ostree/pull/1558
aka commits cadece6c4f398ca61d21e497bd6e3fbb549f9cf6 and
3358698c86d80821d81443c906621c92672f99fb
Needed to fix `rpm-ostree kargs` test suite with default staging; skipping
a test here for now as eventually what we'll do is turn on the rpm-ostree
suite fully here.
Closes: #1677
Approved by: jlebon
Running `ostree commit --tree=ref=a --tree=dir=b` involves reading the
whole of a into an `OstreeMutableTree` before composing `b` on top. This
is inefficient if a is a complete rootfs and b is just touching one file.
We process O(size of a + size of b) directories rather than
O(number of touched dirs).
This commit makes `ostree commit` more efficient at composing multiple
directories together. With `ostree_mutable_tree_fill_empty_from_dirtree`
we create a lazy `OstreeMutableTree` which only reads the underlying
information from disk when needed. We don't need to read all the
subdirectories just to get the checksum of a tree already checked into the
repo.
This provides great speedups when composing a rootfs out of multiple other
rootfs as we do in our build system. We compose multiple containers
together with:
ostree commit --tree=ref=base-rootfs --tree=ref=container1 --tree=ref=container2
and it is much faster now.
As a test I ran
time ostree --repo=... commit --orphan --tree=ref=big-rootfs --tree=dir=modified_etc
Where modified_etc contained a modified sudoers file under /etc. I used
`strace` to count syscalls and I seperatly took timing measurements. To
test with a cold cache I ran
sync && echo 3 | sudo tee /proc/sys/vm/drop_caches
Results:
| | Before | After |
| -------------------- | ------ | ----- |
| Time (cold cache) | 8.1s | 0.12s |
| Time (warm cache) | 3.7s | 0.08s |
| `openat` calls | 53589 | 246 |
| `fgetxattr` calls | 78916 | 0 |
I'm not sure if this will have some negative interaction with the
`_ostree_repo_commit_modifier_apply` which is short-circuited here. I
think it was disabled for `--tree=ref=` anyway, but I'm not certain. All
the tests pass anyway.
I originally implemented this in terms of the `OstreeRepoFile` APIs, but
it was *way* less efficient, opening and reading many files unnecessarily.
Closes: #1643
Approved by: cgwalters
When ostree-prepare-root is pid 1, ostree-prepare-boot defers creation of
/run/ostree-booted, which happens in ostree-remount, but that's too late
if we need ostree-system-generator to bind /var. Add the creation of the
/run/ostree-booted marker to ostree-system-generator based on the
existence of the ostree= kernel command line argument (which matches the
condition that ostree-remount uses).
Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
Closes: #1675
Approved by: cgwalters
It is important that we use user-friendly error strings. The reason
being error strings are seen by users such as in GNOME Software's
error banner.
Closes: #1671
Approved by: jlebon
For `rpm-ostree ex livefs` we have a use case of pushing a rollback
deployment. There's no reason this should require deleting the staged
deployment (and doing so actually breaks livefs which tries to access
it as a data source).
I was initially very conservative here, but I think it ends up
being fairly easy to retain the staged deployment. We need to handle
two cases:
First, when the staged is *intentionally* deleted; here, we just need
to unlink the `/run` file, and then everything will be sync'd up after
reloading.
Second, (as in the livefs case) where we're retaining it,
e.g. adding a deployment to the end. What I realized here is that
we can have the code keep `new_deployments` as view without staged,
and then when we do the final reload we'll end up re-reading it from
disk anyways.
Closes: #1672
Approved by: jlebon
This exposes a way to specify from the command line the number
of times to retry each download after a network error. If a negative
value is given, then the default number of retries (5) is used. If 0
is given, then errors are returned without retrying.
closes#1659Closes: #1669
Approved by: jlebon
I made a logical error in #1617 which resulted in the exact *opposite*
behaviour we want when `/var` is a separate mount.
Split this out and lower the number of negations to make it more obvious
that it's correct.
Closes: #1667Closes: #1668
Approved by: cgwalters
Follow systemd units in using emergency.target, not emergency.service
(which is the sole unit, by default, in emergency.target) so we can
easily reconfigure the units which are actived when entering
emergency mode.
Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
Closes: #1665
Approved by: cgwalters
This implements a TODO item from
`ostree_mutable_tree_get_contents_checksum`. We now no-longer invalidate
the dirtree contents checksum at `get_contents_checksum` time - we
invalidate it when the mtree is modified. This is implemented by keeping
a pointer to the parent directory in each `OstreeMutableTree`. This gives
us stronger invariants on `contents_checksum`.
For even stronger guarantees about invariants we could make
`ostree_repo_write_mtree` or similar a member of `OstreeMutableTree` and
remove `ostree_mutable_tree_set_metadata_checksum`.
I think I've fixed a bug here too. We now invalidate parent's contents
checksum when our metadata checksum changes, whereas we didn't before.
Closes: #1655
Approved by: cgwalters
If ostree_repo_prepare_transaction() fails, we should reset the
repository’s state so that the failed call was essentially idempotent.
Do that by calling ostree_repo_abort_transaction() on the failure path.
Typically, the way for preparing a transaction to fail is for its
GCancellable to be triggered, rather than because any of the operations
involved in preparing a transaction are particularly failure prone.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1647
Approved by: cgwalters
min-free-space-* act as a gating condition whether to we want hold onto caches in
repo/tmp. If it is found that the free-disk space is going below this threshold,
we flag it as an error and cleanup current boot's staging directory.
Closes: #1602
Approved by: jlebon
Here's a subtle bug in abort_transaction():
One of the policies of cleaning up is to skip the current boot's staging
directory. The responsible function for this is cleanup_tmpdir() which tries
to lock each of the tmpdir before deleting it. When it comes to the current
boot's staging dir, it tries to lock the directory(again!) but fails as there
is already a lockfile present. Just because the current boot's staging dir was
meant to be skipped, the bug never surfaced up and wasn't catastrohpic.
if (!_ostree_repo_try_lock_tmpdir (dfd, path, &lockfile, &did_lock, error))
return FALSE;
if (!did_lock)
return TRUE; /* Note early return */
...
if (g_str_has_prefix (path, self->stagedir_prefix))
return TRUE; /* Note early return */
The actual check for skipping staging dir for current boot was never reached
because the function returned at did_lock failure.
Therefore, execute cleanup_tmpdir() after releasing the lockfile in
abort_transaction() so that cleanup_tmpdir gets a chance to lock current boot's
staging directory and succeed.
Closes: #1602
Approved by: jlebon
Currently the BLS snippets are named ostree-$ID-$VARIANT_ID-$index.conf,
but the BLS config files are actually sorted by using the version field
which is the inverse of the index.
In most places, _ostree_sysroot_read_boot_loader_configs() is used to
get the BLS files and this function already returns them sorted by the
version field. The only place where the index trailing number is used is
in the ostree-grub-generator script that lists the BLS files to populate
the grub config file.
But for some bootloaders the BLS filename is the criteria for sorting by
taking the filename as a string version. So on these bootloaders the BLS
entries will be listed in the reverse order.
To avoid that, change the BLS snippets filename to have the version field
instead of the index and also to have the version before deployment name.
Make the filenames to be of the form ostree-$version-$ID-$VARIANT_ID.conf
so the version is before the deployment name.
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Closes: #1654
Approved by: cgwalters
If a function has a guint "out argument", passing a pointer to a gsize
is not, in general, valid. On an ILP64 platform there is no problem
since guint and gsize are identical, but on an LP64 platform it will
overwrite only the first word of the gsize, leaving the second word
unaffected. On little-endian machines, if the second word is
zero-initialized (as it is here), the result is numerically equal to
the guint, but on big-endian machines the result is around 4 billion
times what it should be, resulting in
ostree_repo_finder_config_resolve_async() reading past the end of
the array and causing undefined behaviour.
In practice this caused assertion failures (and consequently test
failures) on Debian's s390x (z/Architecture), ppc64 (64-bit PowerPC)
and sparc64 (64-bit SPARC) ports.
Closes: #1640
Signed-off-by: Simon McVittie <smcv@debian.org>
Closes: #1641
Approved by: cgwalters
The ostree-grub-generator populates the grub.cfg menu entries using the
BLS config files. But it uses the ls command that by default sorts the
entries alphabetically, so the order won't be correct if there are more
than 10 deployments, i.e:
$ ls -1 /boot/loader/entries/
ostree-fedora-workstation-0.conf
ostree-fedora-workstation-10.conf
ostree-fedora-workstation-1.conf
...
So instead the -v option should be used to make ls use version sorting:
$ ls -1 -v /boot/loader/entries/
ostree-fedora-workstation-0.conf
ostree-fedora-workstation-1.conf
...
ostree-fedora-workstation-10.conf
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Closes: #1653
Approved by: cgwalters
A prelude to my understanding. Unfortunately `OstreeMutableTree` provides
little encapsulation, as each member has setters† so it's difficult to come
up with a list of invariants.
† `files` and `subdirs` only have getters, but the getters return mutable
references to the internals, so we still can't reason about invariants.
Closes: #1645
Approved by: jlebon
We special-case AVAHI_ERR_NO_DAEMON to not cause warnings, but if
we pass AVAHI_CLIENT_NO_FAIL to avahi_client_new, we never actually
see AVAHI_ERR_NO_DAEMON. Instead, we will get AVAHI_ERR_BAD_STATE
when we try to use the client.
Closes: #1618
Signed-off-by: Simon McVittie <smcv@debian.org>
Closes: #1639
Approved by: cgwalters
During the pull, there is an explicit check for free space on disk
vs. the size of uncompressed delta; But while writing the new content
objects that are generated, they have to honor min-free-space-* checks
too. We enforce this check in _bare_content_commit as that is where
we can know the final size of the new content object.
Closes: #1614
Approved by: jlebon
We were referencing the txn bits outside of the lock in the error
path. Generally shouldn't matter, but e.g. Rust wouldn't let us do this, and
race detector tooling will warn about it.
Closes: #1632
Approved by: jlebon
I generally like having variables include their units where applicable;
timer variables having `_secs` or `_ms`, etc.
Closes: #1632
Approved by: jlebon
Systemd units using ConditionNeedsUpdate run if the mtime of .updated in
the specified directory is newer than /usr. Since /usr has an mtime of
0, there's no way to have an older .updated file. Systemd units
typically specify ConditionNeedsUpdate=/etc or ConditionNeedsUpdate=/var
to support stateless systems like ostree.
Remove the file from the new deployment's /etc and the OS's /var
regardless of where they came from to ensure that these systemd units
run when booting new deployments. This will provide a method to run
services only on upgrade.
Closes: #1628https://bugzilla.gnome.org/show_bug.cgi?id=752950Closes: #1631
Approved by: cgwalters
Currently when I run `ostree prune` it hits a seg fault when the
hash_func is used (in this case g_str_hash) from the call stack
_ostree_repo_prune_tmp() -> g_hash_table_contains() ->
g_hash_table_lookup_node(). So the key, in this case dent->d_name, must
be corrupt in some way.
glnx_dirfd_iterator_next_dent() uses readdir() to get the dirent struct.
And according to the man page for readdir(3), "POSIX.1 explicitly notes
that this field should not be used as an lvalue" (in reference to
d_name). So this commit avoids modifying d_name in place and copies it
instead. This seems to avoid the seg fault.
Closes: #1627
Approved by: jlebon
Jonathan Lebon
Laurent Bonnans
Matthew Leeds
Colin Walters
Robert Fairley
Robert Fairley
Dan Nicholson
Simon McVittie
Ricardo Salveti
Matthias Clasen
Rafael Fonseca
Umang Jain
Luca Bruno
Alexander Larsson
Giuseppe Scrivano
Sinny Kumari
Robert McQueen
Felix Krull
Philip Withnall
Will Thompson
bubblemelon
William Manley
Alex Kiernan
Alex Kiernan
Javier Martinez Canillas