[merged] libglnx porting: Migrate to new tempfile code #369
Conversation
|
Committed the libglnx dependency (though more review is welcome) and rebased 🏄 |
|
I saw locally |
|
In the big picture, I am now thinking that |
|
Hum interesting... |
|
This turns out to be that the untrusted-delta codepath explicitly tries to re-open the tempfile path, mmap it, and checksum it. We can't do that with The right thing instead is to compute the checksum while writing, just like we do in |
When reworking the ostree core [to use O_TMPFILE](ostreedev#369), I hit an issue in the way the untrusted delta codepath ends up trying to re-open the file to checksum it. That's not possible with `O_TMPFILE` since the fd (which we opened `O_WRONLY`) is the only accessible reference to the content. Fix this by changing the delta processing code to update a checksum as we're doing writes, which is also faster, and ends up simplifying the code as well. What would be an even larger simplification here is if we e.g. used a separate thread calling `write_object()` or something like that; the main issue I see there is somehow bridging the fact that function wants a `GInputStream*` but the delta code is generating stream of writes.
|
☔ The latest upstream changes (presumably f3cbe86) made this pull request unmergeable. Please resolve the merge conflicts. |
When reworking the ostree core [to use O_TMPFILE](ostreedev#369), I hit an issue in the way the untrusted delta codepath ends up trying to re-open the file to checksum it. That's not possible with `O_TMPFILE` since the fd (which we opened `O_WRONLY`) is the only accessible reference to the content. Fix this by changing the delta processing code to update a checksum as we're doing writes, which is also faster, and ends up simplifying the code as well. What would be an even larger simplification here is if we e.g. used a separate thread calling `write_object()` or something like that; the main issue I see there is somehow bridging the fact that function wants a `GInputStream*` but the delta code is generating stream of writes.
When reworking the ostree core [to use O_TMPFILE](#369), I hit an issue in the way the untrusted delta codepath ends up trying to re-open the file to checksum it. That's not possible with `O_TMPFILE` since the fd (which we opened `O_WRONLY`) is the only accessible reference to the content. Fix this by changing the delta processing code to update a checksum as we're doing writes, which is also faster, and ends up simplifying the code as well. What would be an even larger simplification here is if we e.g. used a separate thread calling `write_object()` or something like that; the main issue I see there is somehow bridging the fact that function wants a `GInputStream*` but the delta code is generating stream of writes. Closes: #392 Approved by: jlebon
|
@cgwalters Can you rebase? |
In general this is even cleaner now, though it was better after I extracted a helper function for the "write tempfile with contents" bits that were shared between metadata and regular file codepaths.
|
🏄 |
|
One risk of this is that now there are two pretty different ways to handle tmpfiles, and specifically CentOS 7.2 |
|
OK, these makes sense. I was a bit confused why libglnx needed the missing syscall tidbit, but I see the RHEL/CentOS kernels have Overall, the O_TMPFILE approach is really nice. And this sets us up even better for when (hopefully) It might be interesting to add a new type in libglnx to encapsulate the pattern a bit more and have e.g. a variation on |
|
☀️ Test successful - status-atomicjenkins |
rh-atomic-bot
changed the title
|
This regresses use of ostree on filesystems that don't support O_TMPFILE, such as ubifs, which is the root filesystem on quite a few eMMC/NAND block devices such as the ones used in ARM or "reduced functionality" Intel compatible platforms. |
In general this is even cleaner now, though it was better after I
extracted a helper function for the "write tempfile with contents"
bits that were shared between metadata and regular file codepaths.
Depends on GNOME/libglnx#17