deploy: Remove global `sync` by default

Our previous change here was not actually sufficient for
the ceph case, because what (I think) is happening is that
our other `syncfs()` invocation is getting blocked on some
kernel mutexes that are used in `sync`, and that's causing the
process to fully block.

We should not be dependent on a full filesystem `sync`, only
on the sync of the sysroot and boot filesystems.

Anyone who *does* want this behavior could inject an override
for `ostree-finalize-staged.service` that overrides `ExecStop`
to add a run of `sync`.

diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
index 1f37909dbe5e647741c04a4d276aa5b57f6c50d6..0546aac4488261fa5c46518cbc20239dadbbcf6d 100644 (file)
--- a/src/libostree/ostree-sysroot-deploy.c
+++ b/src/libostree/ostree-sysroot-deploy.c
@@ -1634,44 +1634,8 @@ typedef struct
 {
   guint64 root_syncfs_msec;
   guint64 boot_syncfs_msec;
-  guint64 extra_syncfs_msec;
 } SyncStats;
 
-typedef struct
-{
-  int ref_count; /* atomic */
-  bool success;
-  GMutex mutex;
-  GCond cond;
-} SyncData;
-
-static void
-sync_data_unref (SyncData *data)
-{
-  if (!g_atomic_int_dec_and_test (&data->ref_count))
-    return;
-  g_mutex_clear (&data->mutex);
-  g_cond_clear (&data->cond);
-  g_free (data);
-}
-
-// An asynchronous sync() call.  See below.
-static void *
-sync_in_thread (void *ptr)
-{
-  SyncData *syncdata = ptr;
-  ot_journal_print (LOG_INFO, "Starting global sync()");
-  sync ();
-  ot_journal_print (LOG_INFO, "Completed global sync()");
-  g_mutex_lock (&syncdata->mutex);
-  // Signal success
-  syncdata->success = true;
-  g_cond_broadcast (&syncdata->cond);
-  g_mutex_unlock (&syncdata->mutex);
-  sync_data_unref (syncdata);
-  return NULL;
-}
-
 /* First, sync the root directory as well as /var and /boot which may
  * be separate mount points.  Then *in addition*, do a global
  * `sync()`.
@@ -1702,52 +1666,6 @@ full_system_sync (OstreeSysroot *self, SyncStats *out_stats, GCancellable *cance
   end_msec = g_get_monotonic_time () / 1000;
   out_stats->boot_syncfs_msec = (end_msec - start_msec);
 
-  /* And now out of an excess of conservativism, we still invoke
-   * sync().  The advantage of still using `syncfs()` above is that we
-   * actually get error codes out of that API, and we more clearly
-   * delineate what we actually want to sync in the future when this
-   * global sync call is removed.
-   *
-   * Due to https://bugzilla.redhat.com/show_bug.cgi?id=2003532 which is
-   * a bug in the interaction between Ceph and systemd, we have a capped
-   * timeout of 5s on the sync here.  In general, we don't actually want
-   * to "own" flushing data on *all* mounted filesystems as part of
-   * our process.  Again, we're only keeping the `sync` here out of
-   * conservatisim.  We could likely just remove it and e.g. systemd
-   * would take care of sync as part of unmounting individual filesystems.
-   */
-  start_msec = g_get_monotonic_time () / 1000;
-  if (!(self->opt_flags & OSTREE_SYSROOT_GLOBAL_OPT_SKIP_SYNC))
-    {
-      SyncData *syncdata = g_new (SyncData, 1);
-      syncdata->ref_count = 2; // One for us, one for thread
-      syncdata->success = FALSE;
-      g_mutex_init (&syncdata->mutex);
-      g_cond_init (&syncdata->cond);
-      g_mutex_lock (&syncdata->mutex);
-      GThread *worker = g_thread_new ("ostree sync", sync_in_thread, syncdata);
-      // Wait up to 5 seconds for sync() to finish
-      gint64 end_time = g_get_monotonic_time () + (5 * G_TIME_SPAN_SECOND);
-      while (!syncdata->success)
-        {
-          if (!g_cond_wait_until (&syncdata->cond, &syncdata->mutex, end_time))
-            {
-              ot_journal_print (LOG_INFO, "Timed out waiting for global sync()");
-              break;
-            }
-        }
-      g_mutex_unlock (&syncdata->mutex);
-      sync_data_unref (syncdata);
-      // We can't join the thread here, for two reasons.  First, the whole
-      // point here is to avoid blocking on `sync`.  The other reason is
-      // today there is no return value on success from `sync`, so there's
-      // no point to joining the thread even if we had a nonblocking mechanism
-      // to do so.
-      g_thread_unref (worker);
-    }
-  end_msec = g_get_monotonic_time () / 1000;
-  out_stats->extra_syncfs_msec = (end_msec - start_msec);
-
   return TRUE;
 }
 
@@ -3018,8 +2936,7 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, GPtrArray *n
         bootloader_is_atomic ? "yes" : "no", "OSTREE_DID_BOOTSWAP=%s",
         requires_new_bootversion ? "yes" : "no", "OSTREE_N_DEPLOYMENTS=%u", new_deployments->len,
         "OSTREE_SYNCFS_ROOT_MSEC=%" G_GUINT64_FORMAT, syncstats.root_syncfs_msec,
-        "OSTREE_SYNCFS_BOOT_MSEC=%" G_GUINT64_FORMAT, syncstats.boot_syncfs_msec,
-        "OSTREE_SYNCFS_EXTRA_MSEC=%" G_GUINT64_FORMAT, syncstats.extra_syncfs_msec, NULL);
+        "OSTREE_SYNCFS_BOOT_MSEC=%" G_GUINT64_FORMAT, syncstats.boot_syncfs_msec, NULL);
     _ostree_sysroot_emit_journal_msg (self, msg);
   }