After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 755595 - deploy: Races in creating symlinks may still exist
deploy: Races in creating symlinks may still exist
Status: RESOLVED WONTFIX
Product: ostree
Classification: Infrastructure
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: OSTree maintainer(s)
OSTree maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-09-24 21:41 UTC by John Hiesey
Modified: 2018-08-17 18:59 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description John Hiesey 2015-09-24 21:41:45 UTC
Although I closed bug #754913 since the issue I reported there was (at least mostly) fixed by https://git.gnome.org/browse/ostree/commit/?id=76a976817fb22c419557c6e010638b7ed2c2549b I'm still concerned that the current code may still miss some cases.

Here's what I'm worried about:
1. I don't see anything guaranteeing the directory containing the symlink created here: https://github.com/GNOME/ostree/blob/c61151d6503e02fd73507784e730facff90b95d8/src/libostree/ostree-sysroot-deploy.c#L1102 is synced.

One place that calls swap_bootlinks() does have a full_system_sync() after it, but the other one here: https://github.com/GNOME/ostree/blob/c61151d6503e02fd73507784e730facff90b95d8/src/libostree/ostree-sysroot-deploy.c#L1574 doesn't.


2. ot_gfile_atomic_symlink_swap() calls fsync() on the directory containing the symlink, but not on the symlink file itself. This article: http://blog.httrack.com/blog/2013/11/15/everything-you-always-wanted-to-know-about-fsync/ makes it seem like there is simply no way to sync the actual data blocks of a symlink that contain the target string. Unless there is something preventing such an issue, couldn't that result in the symlink existing but being invalid (e.g. containing the empty string)?


(1) should be easy enough to fix if it's a real issue. For (2), it seems like syncing the entire filesystem with syncfs() might be the only truly reliable solution.
Comment 1 Colin Walters 2015-10-02 22:38:01 UTC
We can likely take the hit of using syncfs() always.  But I'd like to set up a test framework for this to be really sure.
Comment 2 Colin Walters 2015-10-13 01:39:28 UTC
Compile and unit tested patch in https://github.com/GNOME/ostree/pull/152
Comment 3 Cosimo Cecchi 2015-10-27 00:06:06 UTC
The PR was merged - anything left for this issue?
Comment 4 Colin Walters 2015-10-27 01:25:47 UTC
Yeah, though I'd like to keep this open until we land some more tests.  I see two high level approaches:

1) hybrid qemu testing - run code on both host and guest, force poweroff vm
   Advantages:
     - More closely resembles real world systems
     - gnome-continuous has code to do this type of test
   Disadvantage: Harder to run in unit test scenario "make check"

2) A test FUSE filesystem that explicitly discards all data predating a syncfs()
   Disadvantages: 
      - I'm not aware of something like this, though it might not be too hard to write
   Advantages:
      - A lot easier to run in unit-type tests.
Comment 5 André Klapper 2018-08-17 18:59:18 UTC
OSTree has moved to Github a while ago.
Furthermore, GNOME Bugzilla will be shut down and replaced by gitlab.gnome.org.

If the problem reported in this Bugzilla ticket is still valid, please report it to https://github.com/ostreedev/ostree/issues instead. Thank you!

Closing this report as WONTFIX as part of Bugzilla Housekeeping.