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,11 +367,33 @@ 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;
         }
     }
 
@@ -440,7 +463,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 +477,16 @@ 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))
     {
+        /* Declared at this scope so the CF_MOUNTALL guard below can see it. */
+        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 +496,61 @@ 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++)
+            {
+                Mount *mp = SeqAt(GetGlobalMountedFSList(), i);
+                if (mp != NULL && mp->mounton != NULL && strcmp(name, mp->mounton) == 0)
+                {
+                    already_mounted = true;
+                    break;
+                }
+            }
+
+            if (already_mounted)
             {
-                changes += VerifyInFstab(ctx, name, a, pp, &result);
+                /* 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
-                result = PromiseResultUpdate(result, VerifyMount(ctx, name, a, pp));
+                /* Not mounted at all - mount it (historical behavior). */
+                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
+                    result = PromiseResultUpdate(result, VerifyMount(ctx, name, a, pp));
+                }
             }
         }
         else
@@ -492,7 +561,9 @@ static PromiseResult VerifyMountPromise(EvalContext *ctx, char *name, const Attr
             }
         }
 
-        if (changes > 0)
+        /* mount -a can only help filesystems that are NOT already mounted; it
+         * never remounts or changes options on a live mount. */
+        if (changes > 0 && !already_mounted)
         {
             CF_MOUNTALL = true;
         }
@@ -513,7 +584,6 @@ static PromiseResult VerifyMountPromise(EvalContext *ctx, char *name, const Attr
         }
     }
 
-    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()
 };