In glibc 2.27, a wrapper for `copy_file_range` was added[1]. The
function is now always defined, either using a userspace fallback or
just returning `ENOSYS` if the kernel doesn't support it. This throws
off our preprocessor conditionals. Work around this by just renaming our
implementation differently. This is similar to what systemd did[2].
Hit this when trying to build on F28, which rebased to glibc 2.27.
[1] https://sourceware.org/git/?p=glibc.git;a=commit;h=bad7a0c81f501fbbcc79af9eaa4b8254441c4a1f
[2] 5187dd2c40
glibc 2.27 added support for memfd_create. Unfortunately flatpak-builder,
or rather the included libglnx library, also has such a function to
wrap the corresponding syscall. It correctly tries to detect it in
the configure script to disabled the wrapper in case glibc provides
it. However it doesn't work due to a missing include.
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890722
Share its static var goodness with clients. This will be used in
rpm-ostree from various places which sometimes do use a `GLnxConsole`
and sometimes don't, so it's more convenient to make it its own
function.
The idea was clearly to avoid useless updates, but we never actually *set* these
values. Drop the code for now to avoid confusion, I'll reimplement this in a
better way.
At the top of glnx-fdio.h there's this comment:
/* When we include libgen.h because we need
* dirname() we immediately undefine
* basename() since libgen.h defines it as
* a macro to the XDG version which is really
* broken. */
and then it does #undef basename to try to
gain access to non-default basename implementation.
The problem is that this trick doesn't work on
some systems:
./libglnx/glnx-fdio.h: In function 'glnx_basename':
./libglnx/glnx-fdio.h:46:11: error: 'basename'
undeclared (first use in this function)
return (basename) (path);
Anyway, basename() is like 3 lines of code to
implement, so this commit just does that instead
of relying on glibc for it.
This caused GCC 6.3.0 -Winline to complain:
../../../ext/libglnx/glnx-errors.h:169:1: warning: function
‘glnx_throw_errno_prefix’ can never be inlined because it uses
variable argument lists [-Winline]
glnx_throw_errno_prefix (GError **error, const char *fmt, ...)
^~~~~~~~~~~~~~~~~~~~~~~
../../../ext/libglnx/glnx-errors.h:169:1: warning: inlining failed
in call to ‘glnx_throw_errno_prefix’: function not inlinable
[-Winline]
Without this, including glnx-missing-syscall.h raises this warning:
../ext/libglnx/glnx-missing-syscall.h:121:6: warning: "HAVE_DECL_COPY_FILE_RANGE" is not defined [-Wundef]
#if !HAVE_DECL_COPY_FILE_RANGE
^~~~~~~~~~~~~~~~~~~~~~~~~
`g-ir-scanner` is unaware of this GNUC extension and complains.
Saw that while building ostree.
While we're here, fix up a few other things:
- Tell the compiler the stat buffer is unused (I didn't see
a warning, just doing this on general principle)
- Return from `glnx_throw_errno_prefix()` directly; we do
preserve errno there, let's feel free to rely on it.
Often, the caller doesn't actually care about the details of the stat
struct itself, but just whether the entry exists or not. It does work
to just pass `NULL` directly to glibc in a quick test, but given that
the argument is tagged as `__nonnull` and that the documentation does
not explicitly specify this is supported, let's do this safely.
I'd like to have the checks for `EBADF` as well as the
"assign to -1" in more places. The cleanup function we
had for `glnx_fd_close` is actually what we want.
Let's rename the cleanup macro to `glnx_autofd` to better match
other autocleanups like `g_autofree`.
Then we can use `glnx_fd_close()` as a replacement for plain Unix `close()`. I
left the `glnx_close_fd` macro, but it's obviously confusing now with the
former. We'll eventually remove it.
Planning to use memfd_create() in flatpak and rpm-ostree, which both use
bubblewrap, and want to pass read-only data via file descriptor to the
container. Passing via `O_TMPFILE` requires `O_RDWR` (read and write),
and passing via a pipe would require buffering.
The systemd `missing.h` has grown enormously; I only cherry-picked the bits for
memfd.
This makes us more friendly to being embedded in a GObject or
the like that's fully zero-initialized, rather than relying on the special
`-1` value for the fd.
Calls to `glnx_release_lock_file()` become idempotent, so it's easy to call it
unconditionally in an object finalizer.
Particularly if `AT_FDCWD` is used, we need to open
in the target dir, otherwise we can get `EXDEV` when trying
to do the final link.
(Theoretically we can cross a mountpoint even with fd-relative
though this is a lot less likely)
Having our tests forced into a `goto out` style is seriously annoying
since we can't write tests like we write production code. Add
a macro that checks for the error being NULL.
This doesn't fully solve the problem since the test functions are
still forced into `void` returns; at some point I may extend
GLib to have `g_test_add_err_func()`.
We have a use case in libostree's staging dirs where we try to reuse
them across multiple ostree txns, but we want the fd-relative bits
here.
Extend the tmpdir API to make deletion optional. While here, also extend the API
to support checking for errors when deleting for projects like libostree that
want to do so consistently.
Also while here, add a change to set the fd to `-1` after clearing to be extra
defensive.
I was working on rpm-ostree unified core, and hit the fact that
`glnx_file_copy_at()` had the same bug with `fsetxattr()` and files whose mode
is <= `0400` (e.g. `000` in the case of `/etc/shadow`) that libostree did a
while ago. Basically, Linux currently allows `write()` on non-writable open files
but not `fsetxattr()`. This situation is masked for privileged (i.e.
`CAP_DAC_OVERRIDE`) code.
Looking at this, I think it's cleaner to convert to `O_TMPFILE` here,
since that code already handles setting the tmpfile to mode `0600`. Now,
this *is* a behavior change in the corner case of existing files which
are symbolic links. Previously we'd do an `open(O_TRUNC)` which would follow
the link.
But in the big picture, I think the use cases for `open(O_TRUNC)` are really
rare - I audited all callers of this in ostree/rpm-ostree/flatpak, and all of
them will be fine with this behavior change. For example, the ostree `/etc`
merge code already explicitly unlinks the target beforehand. Other cases like
supporting `repo/pubring.gpg` in an ostree repo being a symlink...eh, just no.
Making this change allows us to convert to new style, and brings all of the
general benefits of using `O_TMPFILE` too.
While reading a strace I noticed a double close in the tests; this was because
we were missing an assignment to `-1` in the tests. However, let's make
supporting this clearer by explicitly supporting the fd being `-1` while still
setting the `initialized` variable to `FALSE`. We also add the `EBADF` assertion
checking.
I noticed while reading the manpage for `linkat()` that `O_TMPFILE`
supports `O_EXCL` to mean exactly what we're doing with the anonymous
tmpfile API.
Change the code to start using it; this required refactoring the internals since
we had a check to be sure the caller wasn't passing `O_EXCL` for the
non-anonymous path which we want to keep.
Presumably the storage system could do smarter things if it knows a file will
always be anonymous, e.g. it doesn't need to journal its data.
This is a very common pattern in both ostree/rpm-ostree. Make a better API for
this. I thought a lot about simply zeroing out `struct stat` but that feels
dangerous; none of the values have seem obviously `cannot be zero`.
Basically all of the ostree/rpm-ostree callers want to both create and open, so
let's merge `glnx_mkdtempat()` and `glnx_mkdtempat_open()`.
Second, all of them want to do `glnx_shutil_rm_rf_at()` on cleanup, so we do the
same thing we did with `GLnxTmpfile` and create `GLnxTmpDir` that has a cleanup
attribute.
The cleanup this results in for rpm-ostree is pretty substantial.
FICLONE is the new alias for the formerly btrfs-specific ioctl; XFS
has experimental patches to support it.
Further, we should use copy_file_range() for the case where we're only doing a
limited copy. Both NFS and XFS (with reflink enabled) understand it.
Part of the reason I'm doing this is so that ostree's `/etc` merge will start
using XFS reflinks. But another major reason is to take the next step after and
copy this code into GLib as well, so that all of the general GLib users will
benefit; e.g. Nautilus will transparently do server copy offloads with NFS home
directories.
See also this coreutils thread about `copy_file_range()`:
<https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24399>. I don't care about file
holes for our use cases, so it's fine.
Other changes while I'm here:
- Tweak the sendfile() case to match the newly inlined logic for cfr
- Add a TEMP_FAILURE_RETRY() around the read()
We should be able to rely upstream on everything *except* `glnx_unref_object`
which requires the library itself to depend on a newer glib, which isn't true
for e.g. RHEL7 libsoup.
libostree was almost ready for this; just a few patches to push
it to completion in
https://github.com/ostreedev/ostree/pull/1042
systemd does this by default. I think we should treat this as a fatal error
since it can cause really painful-to-debug problems if we don't just get
EBADF but actually close something else's fd due to a race.
Minor tweak to the new `GLNX_AUTO_PREFIX_ERROR`. Since the common case
is that there's no errors, let's bring down the same check that
`g_prefix_error` does to avoid a function call most of the time.
Another one where we have a lot of inlines in ostree at least. Not the same as
`glnx_shutil_mkdir_p_at()` since in these cases we don't want automatic
intermediate dirs, and it's cheaper to just call `mkdirat()` and handle `EEXIST`
rather than do a `stat()` first.
This is kind of long overdue. Reasons are the same as the other wrappers. I
debated adding `O_NOFOLLOW` support but the use cases for that are pretty
obscure, callers who want that can just use the syscall directly for now.
