CFE-90: reconcile storage mount options when they drift (opt-in remount)

cf-agent/nfs.h

@@ -28,7 +28,20 @@
 #include <cf3.defs.h>
 #include <sequence.h> // Seq
 
-bool LoadMountInfo(Seq *list);
+extern bool LoadMountInfo(Seq *list);
+
+/* Option subset matching for CFE-90 mount option verification.
+ * Checks whether all options in 'promised_opts' are present in
+ * 'actual_opts' (kernel-resolved options).  Kernel-added NFS
+ * auto-negotiated options are ignored.  Returns true if all
+ * user-specified options are satisfied. */
+extern bool OptionsSubsetMatches(const char *promised_opts, const char *actual_opts);
+
+/* CFE-90: Reconcile an already-mounted filesystem whose source or options
+ * diverge from the promise, using the mechanisms in a->mount.remount_methods
+ * (in order, verifying after each).  Only invoked when remount is enabled. */
+PromiseResult ReconcileMountOptions(EvalContext *ctx, char *name, const Attributes *a, const Promise *pp);
+
 void DeleteMountInfo(Seq *list);
 int VerifyNotInFstab(EvalContext *ctx, char *name, const Attributes *a, const Promise *pp, PromiseResult *result);
 int VerifyInFstab(EvalContext *ctx, char *name, const Attributes *a, const Promise *pp, PromiseResult *result);

cf-agent/verify_storage.c

@@ -341,6 +341,7 @@ static PromiseResult VolumeScanArrivals(ARG_UNUSED char *file, ARG_UNUSED const
 #if !defined(__MINGW32__)
 static bool FileSystemMountedCorrectly(Seq *list, char *name, const Attributes *a)
 {
+    assert(a != NULL);
     bool found = false;
 
     for (size_t i = 0; i < SeqLength(list); i++)
@@ -366,20 +367,44 @@ static bool FileSystemMountedCorrectly(Seq *list, char *name, const Attributes *
                       mp->host, mp->source, name);
                 return false;
             }
-            else
+
+            /* CFE-90: The live mount's options are only managed when 'remount'
+             * is enabled.  Without it, a mount with the correct source is
+             * "mounted correctly" regardless of option drift (options still
+             * govern the initial mount and, with edit_fstab, the fstab entry).
+             * When enabled, use subset matching: all promised options must be
+             * present in the actual (kernel-resolved) set; kernel-added
+             * auto-negotiated options (vers=, rsize=, timeo=, ...) are ignored. */
+            if (a->mount.remount && a->mount.mount_options != NULL)
             {
-                Log(LOG_LEVEL_VERBOSE, "File system '%s' seems to be mounted correctly", mp->source);
-                break;
+                char *opts = Rlist2String(a->mount.mount_options, ",");
+                if (mp->raw_opts == NULL || mp->raw_opts[0] == '\0'
+                    || !OptionsSubsetMatches(opts, mp->raw_opts))
+                {
+                    Log(LOG_LEVEL_INFO,
+                        "Mount options for '%s' do not match promise (actual: '%s', promised: '%s')",
+                        name,
+                        mp->raw_opts ? mp->raw_opts : "(none)",
+                        opts);
+                    free(opts);
+                    return false;
+                }
+                free(opts);
             }
+
+            Log(LOG_LEVEL_VERBOSE, "File system '%s' seems to be mounted correctly", mp->source);
+            break;
         }
     }
 
     if (!found)
     {
         if (!a->mount.unmount)
         {
+            /* CFE-1863: do not arm MountAll ('mount -a') here - the caller
+             * mounts this one filesystem surgically.  CF_MOUNTALL is reserved
+             * for the explicit 'mountfilesystems' agent control. */
             Log(LOG_LEVEL_VERBOSE, "File system '%s' seems not to be mounted correctly", name);
-            CF_MOUNTALL = true;
         }
     }
 
@@ -440,7 +465,7 @@ static bool IsForeignFileSystem(struct stat *childstat, char *dir)
 
 static PromiseResult VerifyMountPromise(EvalContext *ctx, char *name, const Attributes *a, const Promise *pp)
 {
-    char *options;
+    assert(a != NULL);
     char dir[CF_BUFSIZE];
     int changes = 0;
 
@@ -454,13 +479,17 @@ static PromiseResult VerifyMountPromise(EvalContext *ctx, char *name, const Attr
         return PROMISE_RESULT_INTERRUPTED;
     }
 
-    options = Rlist2String(a->mount.mount_options, ",");
-
     PromiseResult result = PROMISE_RESULT_NOOP;
     if (!FileSystemMountedCorrectly(GetGlobalMountedFSList(), name, a))
     {
+        /* Whether something is already mounted at the promiser (vs. nothing
+         * mounted there at all). */
+        bool already_mounted = false;
+
         if (!a->mount.unmount)
         {
+            /* Ensure the mount point exists before mounting or remounting.
+             * dir is "<name>/.", so this creates the mount point directory. */
             if (!MakeParentDirectory(dir, a->move_obstructions, NULL))
             {
                 // Could not create parent directory, assume this is okay,
@@ -470,18 +499,59 @@ static PromiseResult VerifyMountPromise(EvalContext *ctx, char *name, const Attr
                     dir);
             }
 
-            if (a->mount.editfstab)
+            /* CFE-90: distinguish "not mounted at all" from "mounted, but not as
+             * promised" (wrong source, or drifted options with remount enabled). */
+            for (size_t i = 0; i < SeqLength(GetGlobalMountedFSList()); i++)
             {
-                changes += VerifyInFstab(ctx, name, a, pp, &result);
+                Mount *mp = SeqAt(GetGlobalMountedFSList(), i);
+                if (mp != NULL && mp->mounton != NULL && strcmp(name, mp->mounton) == 0)
+                {
+                    already_mounted = true;
+                    break;
+                }
+            }
+
+            if (already_mounted)
+            {
+                /* CFE-90: mounted but not as promised.  Correct the live mount
+                 * first (only when remount is enabled), then update fstab. */
+                if (a->mount.remount)
+                {
+                    result = PromiseResultUpdate(result, ReconcileMountOptions(ctx, name, a, pp));
+                    changes++;
+                }
+                else
+                {
+                    /* Reachable only for a wrong-source mount: option drift with
+                     * remount disabled is reported as mounted correctly. */
+                    cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_FAIL, pp, a,
+                         "A different filesystem is mounted on '%s' than promised; enable 'remount' to correct", name);
+                    result = PromiseResultUpdate(result, PROMISE_RESULT_FAIL);
+                }
+
+                /* Live first, then persist the intent to fstab regardless of
+                 * whether the live reconciliation succeeded (it converges the
+                 * system at the next remount/reboot; the live outcome is already
+                 * reported separately above). */
+                if (a->mount.editfstab)
+                {
+                    changes += VerifyInFstab(ctx, name, a, pp, &result);
+                }
             }
             else
             {
-                cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_FAIL, pp, a,
-                     "Filesystem '%s' was not mounted as promised, and no edits were promised in '%s'", name,
-                     VFSTAB[VSYSTEMHARDCLASS]);
-                result = PromiseResultUpdate(result, PROMISE_RESULT_FAIL);
-                // Mount explicitly
+                /* CFE-1863: Not mounted at all - mount THIS filesystem
+                 * surgically rather than arming MountAll, which runs
+                 * 'mount -a'/'mount -va' and would also mount every other
+                 * unmounted fstab entry, including unrelated devices and
+                 * foreign filesystem types.  Mount the live filesystem first,
+                 * then persist the intent to fstab (when edit_fstab), mirroring
+                 * the reconcile path's "prove-it-live-then-persist" ordering. */
                 result = PromiseResultUpdate(result, VerifyMount(ctx, name, a, pp));
+                if (a->mount.editfstab)
+                {
+                    changes += VerifyInFstab(ctx, name, a, pp, &result);
+                }
             }
         }
         else
@@ -491,11 +561,6 @@ static PromiseResult VerifyMountPromise(EvalContext *ctx, char *name, const Attr
                 changes += VerifyNotInFstab(ctx, name, a, pp, &result);
             }
         }
-
-        if (changes > 0)
-        {
-            CF_MOUNTALL = true;
-        }
     }
     else
     {
@@ -509,11 +574,24 @@ static PromiseResult VerifyMountPromise(EvalContext *ctx, char *name, const Attr
         }
         else
         {
-            cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_NOOP, pp, a, "Filesystem '%s' seems to be mounted as promised", name);
+            /* CFE-1539: The live mount already satisfies the promise, but fstab
+             * is still maintained so the promise persists across reboots: a
+             * missing entry is (re)added and drifted options are corrected.
+             * This is deliberately independent of the opt-in live 'remount' -
+             * keeping fstab correct is the historically documented behavior of
+             * mount_options, whereas remounting a live filesystem is the
+             * disruptive part gated by 'remount'. */
+            if (a->mount.editfstab)
+            {
+                changes += VerifyInFstab(ctx, name, a, pp, &result);
+            }
+            if (changes == 0)
+            {
+                cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_NOOP, pp, a, "Filesystem '%s' seems to be mounted as promised", name);
+            }
         }
     }
 
-    free(options);
     return result;
 }
 

libpromises/attributes.c

@@ -1694,6 +1694,9 @@ StorageMount GetMountConstraints(const EvalContext *ctx, const Promise *pp)
     m.mount_options = PromiseGetConstraintAsList(ctx, "mount_options", pp);
     m.editfstab = PromiseGetConstraintAsBoolean(ctx, "edit_fstab", pp);
     m.unmount = PromiseGetConstraintAsBoolean(ctx, "unmount", pp);
+    m.remount = PromiseGetConstraintAsBoolean(ctx, "remount", pp);
+    m.remount_methods = PromiseGetConstraintAsList(ctx, "remount_methods", pp);
+    m.remount_timeout = PromiseGetConstraintAsInt(ctx, "remount_timeout", pp);
 
     return m;
 }

libpromises/cf3.defs.h

@@ -911,7 +911,8 @@ typedef struct
     char *host;
     char *source;
     char *mounton;
-    char *options;
+    char *options;          /* fstype string (e.g. "nfs", "panfs", "cifs") for foreign-FS detection */
+    char *raw_opts;         /* full kernel-resolved options from /proc/mounts */
     int unmount;
 } Mount;
 
@@ -1290,6 +1291,9 @@ typedef struct
     Rlist *mount_options;
     int editfstab;
     int unmount;
+    int remount;
+    Rlist *remount_methods;
+    int remount_timeout;
 } StorageMount;
 
 typedef struct

libpromises/mod_storage.c

@@ -48,6 +48,9 @@ static const ConstraintSyntax mount_constraints[] =
     ConstraintSyntaxNewString("mount_server", "", "Hostname or IP or remote file system server", SYNTAX_STATUS_NORMAL),
     ConstraintSyntaxNewStringList("mount_options", "", "List of option strings to add to the file system table (\"fstab\")", SYNTAX_STATUS_NORMAL),
     ConstraintSyntaxNewBool("unmount", "true/false unmount a previously mounted filesystem. Default value: false", SYNTAX_STATUS_NORMAL),
+    ConstraintSyntaxNewBool("remount", "true/false correct the options of an already-mounted filesystem when they differ from the promise. Default value: false", SYNTAX_STATUS_NORMAL),
+    ConstraintSyntaxNewOptionList("remount_methods", "remount,unmount_mount", "Ordered list of mechanisms to reconcile a mounted filesystem with the promise (tried in order). Default: remount then unmount_mount", SYNTAX_STATUS_NORMAL),
+    ConstraintSyntaxNewInt("remount_timeout", CF_VALRANGE, "Timeout in seconds bounding the unmount/mount reconciliation of a busy or unreachable filesystem", SYNTAX_STATUS_NORMAL),
     ConstraintSyntaxNewNull()
 };