diff --git a/cf-agent/nfs.c b/cf-agent/nfs.c index 4ba7d095c7..da8b5cd78a 100644 --- a/cf-agent/nfs.c +++ b/cf-agent/nfs.c @@ -50,9 +50,16 @@ static Item *FSTABLIST = NULL; /* GLOBAL_X */ static void GetHostAndSource(const char *buf, char *host, char *source); -static void AugmentMountInfo(Seq *list, char *host, char *source, char *mounton, char *options); +static bool ConflictingOptions(const char *a, const char *b); +static bool OptionPresent(const char *opt, const Seq *actual); +static bool ConflictingOptionPresent(const char *opt, const Seq *actual); +static char *OptionStringExpandDefaults(const char *opts); + +static void AugmentMountInfo(Seq *list, char *host, char *source, char *mounton, char *fstype, char *options); static bool MatchFSInFstab(char *match); static void DeleteThisItem(Item **liststart, Item *entry); +static char *GetFstabEntryOptions(const char *mountpt); +static void ReplaceFstabEntry(Item *liststart, const char *mountpt, const char *new_entry); static const char *const VMOUNTCOMM[] = { @@ -174,6 +181,208 @@ static void GetHostAndSource(const char *buf, char *host, char *source) source[source_index] = '\0'; } +/** + * True if 'a' and 'b' are mutually exclusive mount options that cannot both + * hold: ro/rw, hard/soft, sync/async, noatime/relatime, or a "no"-prefixed + * option and its bare form (e.g. noexec/exec). + */ +static bool ConflictingOptions(const char *a, const char *b) +{ + /* A "no"-prefixed option vs its bare form, in either direction. */ + if (StringEqualN(a, "no", 2) && StringEqual(a + 2, b)) + { + return true; + } + if (StringEqualN(b, "no", 2) && StringEqual(b + 2, a)) + { + return true; + } + + /* Inverse pairs that do not share the "no" prefix. */ + static const char *const pairs[][2] = { + { "noatime", "relatime" }, + { "hard", "soft" }, + { "sync", "async" }, + { "ro", "rw" }, + }; + for (size_t i = 0; i < sizeof(pairs) / sizeof(pairs[0]); i++) + { + if ((StringEqual(a, pairs[i][0]) && StringEqual(b, pairs[i][1])) || + (StringEqual(a, pairs[i][1]) && StringEqual(b, pairs[i][0]))) + { + return true; + } + } + return false; +} + +/** + * True if 'opt' is present in the live options, directly or via a + * tcp/udp<->proto= alias. + */ +static bool OptionPresent(const char *opt, const Seq *actual) +{ + const size_t len = SeqLength(actual); + for (size_t i = 0; i < len; i++) + { + const char *act = SeqAt(actual, i); + if (StringEqual(opt, act)) + { + return true; + } + if ((StringEqual(opt, "tcp") && StringEqual(act, "proto=tcp")) || + (StringEqual(opt, "udp") && StringEqual(act, "proto=udp")) || + (StringEqual(opt, "proto=tcp") && StringEqual(act, "tcp")) || + (StringEqual(opt, "proto=udp") && StringEqual(act, "udp"))) + { + return true; + } + } + return false; +} + +/** + * True if an option contradicting 'opt' is present in the live mount. + */ +static bool ConflictingOptionPresent(const char *opt, const Seq *actual) +{ + const size_t len = SeqLength(actual); + for (size_t i = 0; i < len; i++) + { + if (ConflictingOptions(opt, SeqAt(actual, i))) + { + return true; + } + } + return false; +} + +/** + * Expand a bare "defaults" in an option string to its positives + * rw,suid,dev,exec,async: util-linux ignores the flags implied by a bare + * "defaults" on a `mount -o remount,...`, so the explicit form is needed both to + * restore a drifted mount in place and to compare against the live options. + * Newly allocated (caller frees), NULL if opts empty. + */ +static char *OptionStringExpandDefaults(const char *opts) +{ + if ((opts == NULL) || (opts[0] == '\0')) + { + return NULL; + } + + Seq *in = SeqStringFromString(opts, ','); + const size_t len = SeqLength(in); + for (size_t i = 0; i < len; i++) + { + if (StringEqual(SeqAt(in, i), "defaults")) + { + SeqSet(in, i, xstrdup("rw,suid,dev,exec,async")); + } + } + char *result = StringJoin(in, ","); + SeqDestroy(in); + return result; +} + +/** + * Extract the options from the parenthesized tail of a mount(8) output line + * ("host:/source on /mountpoint type nfs (rw,noatime)"), without the enclosing + * parentheses and with surrounding whitespace trimmed. The closing parenthesis + * must not be left on the last option or it will never compare equal to the + * promised one. + * Newly allocated (caller frees), NULL if the line has no "(...)" section. + */ +static char *MountOptionsFromLine(const char *line) +{ + const char *paren = strchr(line, '('); + if (paren == NULL) + { + return NULL; + } + const char *end = strchr(paren, ')'); + if (end == NULL) + { + return NULL; + } + + /* TrimWhitespace() can return an interior pointer, which is not free()-able. */ + char *tail = xstrndup(paren + 1, (size_t) (end - paren - 1)); + char *opts = xstrdup(TrimWhitespace(tail)); + free(tail); + return opts; +} + +/** + * Whether the live mount (actual_opts, from /proc/mounts) satisfies the promise. + * Only named options are enforced; unnamed ones are left alone. The promise is + * resolved "last wins" like `mount -o` (defaults,ro is read-only, ro,rw is rw), + * with "defaults" expanded to rw,suid,dev,exec,async (auto/nouser aren't runtime + * state). Each surviving option must then hold: its inverse absent, and it + * present or a default-on flag (suid/dev/exec/async only show when negated). + */ +bool OptionsSubsetMatches(const char *promised_opts, const char *actual_opts) +{ + if (promised_opts == NULL || promised_opts[0] == '\0') + { + return true; + } + + Seq *actual = SeqStringFromString(actual_opts, ','); + + /* Expand a bare "defaults" to its positives (rw,suid,dev,exec,async), + * preserving order for the last-wins pass, then split into options. */ + char *expanded = OptionStringExpandDefaults(promised_opts); + Seq *eff = SeqStringFromString(expanded, ','); + free(expanded); + + /* On-by-default flags the kernel does not echo (only their negatives show). */ + static const char *const default_on[] = { "suid", "dev", "exec", "async" }; + + bool mismatch = false; + const size_t n_eff = SeqLength(eff); + for (size_t i = 0; (i < n_eff) && !mismatch; i++) + { + const char *x = SeqAt(eff, i); + + /* Last wins: skip this option if a later one overrides it (its inverse) + * or repeats it. */ + bool overridden = false; + for (size_t j = i + 1; j < n_eff; j++) + { + const char *y = SeqAt(eff, j); + if (ConflictingOptions(x, y)) + { + Log(LOG_LEVEL_VERBOSE, "Mount option '%s' overridden by later '%s'", x, y); + overridden = true; + break; + } + if (StringEqual(x, y)) + { + overridden = true; + break; + } + } + if (overridden) + { + continue; + } + + const bool is_default_on = IsStringInArray( + x, default_on, sizeof(default_on) / sizeof(default_on[0])); + + if (ConflictingOptionPresent(x, actual) || !(OptionPresent(x, actual) || is_default_on)) + { + mismatch = true; + } + } + + SeqDestroy(eff); + SeqDestroy(actual); + + return !mismatch; +} + bool LoadMountInfo(Seq *list) /* This is, in fact, the most portable way to read the mount info! */ /* Depressing, isn't it? */ @@ -346,22 +555,26 @@ bool LoadMountInfo(Seq *list) Log(LOG_LEVEL_DEBUG, "LoadMountInfo: host '%s', source '%s', mounton '%s'", host, source, mounton); + char *mountopts = MountOptionsFromLine(vbuff); + if (panfs) { - AugmentMountInfo(list, host, source, mounton, "panfs"); + AugmentMountInfo(list, host, source, mounton, "panfs", mountopts); } else if (nfs) { - AugmentMountInfo(list, host, source, mounton, "nfs"); + AugmentMountInfo(list, host, source, mounton, "nfs", mountopts); } else if (cifs) { - AugmentMountInfo(list, host, source, mounton, "cifs"); + AugmentMountInfo(list, host, source, mounton, "cifs", mountopts); } else { - AugmentMountInfo(list, host, source, mounton, NULL); + AugmentMountInfo(list, host, source, mounton, NULL, mountopts); } + + free(mountopts); } free(vbuff); @@ -373,7 +586,7 @@ bool LoadMountInfo(Seq *list) /*******************************************************************/ -static void AugmentMountInfo(Seq *list, char *host, char *source, char *mounton, char *options) +static void AugmentMountInfo(Seq *list, char *host, char *source, char *mounton, char *fstype, char *options) { Mount *entry = xcalloc(1, sizeof(Mount)); @@ -392,9 +605,19 @@ static void AugmentMountInfo(Seq *list, char *host, char *source, char *mounton, entry->mounton = xstrdup(mounton); } - if (options) + /* Store the fstype in options so IsForeignFileSystem can detect + * foreign filesystems via strstr(entry->options, "nfs"/"panfs"/"cifs"). */ + if (fstype != NULL) { - entry->options = xstrdup(options); + entry->options = xstrdup(fstype); + } + + /* Store the full kernel-resolved options in raw_opts. + * For unmounted filesystems (options == NULL or empty), raw_opts stays NULL + * and will be checked in FileSystemMountedCorrectly as "not mounted". */ + if (options != NULL && options[0] != '\0') + { + entry->raw_opts = xstrdup(options); } SeqAppend(list, entry); @@ -408,25 +631,11 @@ void DeleteMountInfo(Seq *list) { Mount *entry = SeqAt(list, i); - if (entry->host) - { - free(entry->host); - } - - if (entry->source) - { - free(entry->source); - } - - if (entry->mounton) - { - free(entry->mounton); - } - - if (entry->options) - { - free(entry->options); - } + free(entry->host); + free(entry->source); + free(entry->mounton); + free(entry->options); + free(entry->raw_opts); } SeqClear(list); @@ -534,6 +743,7 @@ int VerifyInFstab(EvalContext *ctx, char *name, const Attributes *a, const Promi if (!MatchFSInFstab(mountpt)) { + /* CFE-90: Entry not in fstab - add it */ AppendItem(&FSTABLIST, fstab, NULL); FSTAB_EDITS++; cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_CHANGE, pp, a, "Adding file system entry '%s' to '%s'", fstab, @@ -541,6 +751,29 @@ int VerifyInFstab(EvalContext *ctx, char *name, const Attributes *a, const Promi *result = PromiseResultUpdate(*result, PROMISE_RESULT_CHANGE); changes += 1; } + else + { + /* CFE-90: Entry exists - rewrite it if the options differ. The compare + * is exact (order-sensitive) on purpose: for duplicated/conflicting + * options the kernel uses the last one, so option order is significant + * in an fstab line and must not be normalized away. Since + * GetFstabEntryOptions now reads the real options field (it previously + * returned the fstype), this converges - a differently-ordered entry is + * rewritten once to the promised form, then matches - rather than being + * rewritten on every run. */ + char *existing_opts = GetFstabEntryOptions(mountpt); + if (existing_opts != NULL && !StringEqual(existing_opts, opts)) + { + /* Replace the entire fstab entry with the corrected options */ + ReplaceFstabEntry(FSTABLIST, mountpt, fstab); + FSTAB_EDITS++; + cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_CHANGE, pp, a, "Updating file system entry for '%s' in '%s' (options: '%s' -> '%s')", + mountpt, VFSTAB[VSYSTEMHARDCLASS], existing_opts, opts); + *result = PromiseResultUpdate(*result, PROMISE_RESULT_CHANGE); + changes += 1; + } + free(existing_opts); + } free(opts); return changes; @@ -618,7 +851,7 @@ int VerifyNotInFstab(EvalContext *ctx, char *name, const Attributes *a, const Pr if (strstr(line, "busy")) { - cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_INTERRUPTED, pp, a, "The device under '%s' cannot be removed from '%s'", + cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_INTERRUPTED, pp, a, "The device under '%s' cannot be removed from '%s'", mountpt, VFSTAB[VSYSTEMHARDCLASS]); *result = PromiseResultUpdate(*result, PROMISE_RESULT_INTERRUPTED); free(line); @@ -679,55 +912,68 @@ PromiseResult VerifyMount(EvalContext *ctx, char *name, const Attributes *a, con } PromiseResult result = PROMISE_RESULT_NOOP; - if (!DONTDO) + + /* CFE-3366: gate the mount on the promise action, not just DONTDO, so a + * dry-run (or action_policy => "warn") reports a warning and does not + * define promise_repaired for a run that changes nothing. */ + if (!MakingInternalChanges(ctx, pp, a, &result, "mount '%s' to keep promise", mountpt)) { - if (StringEqual(a->mount.mount_type, "panfs")) - { - snprintf(comm, CF_BUFSIZE, "%s -t panfs -o %s %s%s %s", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), opts, host, rmountpt, mountpt); - } - else if (StringEqual(a->mount.mount_type, "cifs")) - { - snprintf(comm, CF_BUFSIZE, "%s -t cifs -o %s %s%s %s", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), opts, host, rmountpt, mountpt); - } - else - { - snprintf(comm, CF_BUFSIZE, "%s -o %s %s:%s %s", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), opts, host, rmountpt, mountpt); - } + free(opts); + return result; + } - if ((pfp = cf_popen(comm, "r", true)) == NULL) - { - Log(LOG_LEVEL_ERR, "Failed to open pipe from '%s'", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS])); - return PROMISE_RESULT_FAIL; - } + if (StringEqual(a->mount.mount_type, "panfs")) + { + NDEBUG_UNUSED int ret = snprintf(comm, CF_BUFSIZE, "%s -t panfs -o %s %s%s %s", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), opts, host, rmountpt, mountpt); + assert(ret >= 0 && ret < CF_BUFSIZE); + } + else if (StringEqual(a->mount.mount_type, "cifs")) + { + NDEBUG_UNUSED int ret = snprintf(comm, CF_BUFSIZE, "%s -t cifs -o %s %s%s %s", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), opts, host, rmountpt, mountpt); + assert(ret >= 0 && ret < CF_BUFSIZE); + } + else + { + NDEBUG_UNUSED int ret = snprintf(comm, CF_BUFSIZE, "%s -o %s %s:%s %s", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), opts, host, rmountpt, mountpt); + assert(ret >= 0 && ret < CF_BUFSIZE); + } - size_t line_size = CF_BUFSIZE; - char *line = xmalloc(line_size); + if ((pfp = cf_popen(comm, "r", true)) == NULL) + { + Log(LOG_LEVEL_ERR, "Failed to open pipe from '%s'", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS])); + free(opts); + return PROMISE_RESULT_FAIL; + } - ssize_t res = CfReadLine(&line, &line_size, pfp); + size_t line_size = CF_BUFSIZE; + char *line = xmalloc(line_size); - if (res == -1) - { - if (!feof(pfp)) - { - Log(LOG_LEVEL_ERR, "Unable to read output of mount command. (fread: %s)", GetErrorStr()); - cf_pclose(pfp); - free(line); - return PROMISE_RESULT_FAIL; - } - } - else if ((strstr(line, "busy")) || (strstr(line, "Busy"))) + ssize_t res = CfReadLine(&line, &line_size, pfp); + + if (res == -1) + { + if (!feof(pfp)) { - cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_INTERRUPTED, pp, a, "The device under '%s' cannot be mounted", mountpt); - result = PromiseResultUpdate(result, PROMISE_RESULT_INTERRUPTED); + Log(LOG_LEVEL_ERR, "Unable to read output of mount command. (fread: %s)", GetErrorStr()); cf_pclose(pfp); free(line); - return 1; + free(opts); + return PROMISE_RESULT_FAIL; } - - free(line); + } + else if (StringContains(line, "busy") || StringContains(line, "Busy")) + { + cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_INTERRUPTED, pp, a, "The device under '%s' cannot be mounted", mountpt); + result = PromiseResultUpdate(result, PROMISE_RESULT_INTERRUPTED); cf_pclose(pfp); + free(line); + free(opts); + return result; } + free(line); + cf_pclose(pfp); + /* Since opts is either Rlist2String or xstrdup'd, we need to always free it */ free(opts); @@ -748,40 +994,52 @@ PromiseResult VerifyUnmount(EvalContext *ctx, char *name, const Attributes *a, c mountpt = name; PromiseResult result = PROMISE_RESULT_NOOP; - if (!DONTDO) + + /* CFE-3366: gate the unmount on the promise action, not just DONTDO, so a + * dry-run (or action_policy => "warn") reports a warning and does not + * define promise_repaired for a run that changes nothing. */ + if (!MakingInternalChanges(ctx, pp, a, &result, "unmount '%s' to keep promise", mountpt)) { - snprintf(comm, CF_BUFSIZE, "%s %s", VUNMOUNTCOMM[VSYSTEMHARDCLASS], mountpt); + return result; + } - if ((pfp = cf_popen(comm, "r", true)) == NULL) - { - Log(LOG_LEVEL_ERR, "Failed to open pipe from %s", VUNMOUNTCOMM[VSYSTEMHARDCLASS]); - return result; - } + NDEBUG_UNUSED int ret = snprintf(comm, CF_BUFSIZE, "%s %s", VUNMOUNTCOMM[VSYSTEMHARDCLASS], mountpt); + assert(ret >= 0 && ret < CF_BUFSIZE); - size_t line_size = CF_BUFSIZE; - char *line = xmalloc(line_size); + if ((pfp = cf_popen(comm, "r", true)) == NULL) + { + Log(LOG_LEVEL_ERR, "Failed to open pipe from %s", VUNMOUNTCOMM[VSYSTEMHARDCLASS]); + return result; + } - ssize_t res = CfReadLine(&line, &line_size, pfp); - if (res == -1) - { - cf_pclose(pfp); - free(line); + size_t line_size = CF_BUFSIZE; + char *line = xmalloc(line_size); - if (!feof(pfp)) - { - Log(LOG_LEVEL_ERR, "Unable to read output of unmount command. (fread: %s)", GetErrorStr()); - return result; - } - } - else if (res > 0 && ((strstr(line, "busy")) || (strstr(line, "Busy")))) + ssize_t res = CfReadLine(&line, &line_size, pfp); + if (res == -1) + { + /* CfReadLine() returns -1 both at end-of-output and on a read error. A + * successful unmount is silent, so EOF is the normal case here; only a + * genuine read error should be reported. feof() must be consulted + * before cf_pclose() closes and invalidates the stream. */ + bool read_error = !feof(pfp); + cf_pclose(pfp); + free(line); + + if (read_error) { - cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_INTERRUPTED, pp, a, "The device under '%s' cannot be unmounted", mountpt); - result = PromiseResultUpdate(result, PROMISE_RESULT_INTERRUPTED); - cf_pclose(pfp); - free(line); + Log(LOG_LEVEL_ERR, "Unable to read output of unmount command. (fread: %s)", GetErrorStr()); return result; } } + else if (res > 0 && (StringContains(line, "busy") || StringContains(line, "Busy"))) + { + cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_INTERRUPTED, pp, a, "The device under '%s' cannot be unmounted", mountpt); + result = PromiseResultUpdate(result, PROMISE_RESULT_INTERRUPTED); + cf_pclose(pfp); + free(line); + return result; + } cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_CHANGE, pp, a, "Unmounting '%s' to keep promise", mountpt); result = PromiseResultUpdate(result, PROMISE_RESULT_CHANGE); @@ -956,6 +1214,284 @@ static void DeleteThisItem(Item **liststart, Item *entry) } } +/*******************************************************************/ +/* CFE-90: Helper functions for fstab options comparison */ +/*******************************************************************/ + +/* fstab fields, 1-based: device, mount point, type, options, dump, pass. */ +#define FSTAB_FIELD_MOUNTPOINT 2 +#define FSTAB_FIELD_OPTIONS 4 + +/** + * Field 'n' (1-based) of a whitespace-separated fstab line. + * Newly allocated (caller frees), NULL for a comment line or one with fewer + * than 'n' fields. + */ +static char *FstabField(const char *line, size_t n) +{ + if ((line == NULL) || (line[0] == '#')) + { + return NULL; + } + + char *copy = xstrdup(line); + char *saveptr = NULL; + char *token = strtok_r(copy, " \t", &saveptr); + for (size_t i = 1; (i < n) && (token != NULL); i++) + { + token = strtok_r(NULL, " \t", &saveptr); + } + char *field = (token != NULL) ? xstrdup(token) : NULL; + + free(copy); + return field; +} + +/** + * True if the fstab line is the entry for 'mountpt'. + */ +static bool FstabEntryIsFor(const char *line, const char *mountpt) +{ + char *mp = FstabField(line, FSTAB_FIELD_MOUNTPOINT); + bool match = (mp != NULL) && StringEqual(mp, mountpt); + free(mp); + return match; +} + +/** + * Extract the options field from the fstab entry matching mountpt. + * Newly allocated (caller frees), NULL if there is no such entry. + */ +static char *GetFstabEntryOptions(const char *mountpt) +{ + for (Item *ip = FSTABLIST; ip != NULL; ip = ip->next) + { + if (FstabEntryIsFor(ip->name, mountpt)) + { + return FstabField(ip->name, FSTAB_FIELD_OPTIONS); + } + } + + return NULL; +} + +/** + * Replace the fstab entry for mountpt with new_entry. + */ +static void ReplaceFstabEntry(Item *liststart, const char *mountpt, const char *new_entry) +{ + for (Item *ip = liststart; ip != NULL; ip = ip->next) + { + if (FstabEntryIsFor(ip->name, mountpt)) + { + free(ip->name); + ip->name = xstrdup(new_entry); + return; + } + } +} + +/*******************************************************************/ +/* CFE-90: Remount reconciliation */ +/*******************************************************************/ + +/** + * Re-read the mount table from scratch (the cached global list is stale after + * a mount operation) and report whether the filesystem now mounted at 'name' + * satisfies the promise: correct source (when specified) and, when options are + * promised, a superset of the promised options. NFS-specific options + * (transport, NFS version, etc.) cannot be changed by a remount - see nfs(5) + * "THE REMOUNT OPTION": https://man7.org/linux/man-pages/man5/nfs.5.html + * The remount call can still return success without applying them, so we + * verify the resulting state rather than trust the command's exit status. + */ +static bool LiveMountConverged(const char *name, const Attributes *a) +{ + assert(a != NULL); + if (a == NULL) + { + return false; + } + Seq *tmp = SeqNew(100, free); + bool converged = false; + + if (LoadMountInfo(tmp)) + { + const size_t n_mounted = SeqLength(tmp); + for (size_t i = 0; i < n_mounted; i++) + { + Mount *mp = SeqAt(tmp, i); + if (mp == NULL || mp->mounton == NULL || !StringEqual(mp->mounton, name)) + { + continue; + } + + /* Something is mounted here - check source, then options. */ + if ((a->mount.mount_source != NULL) + && ((mp->source == NULL) || !StringEqual(mp->source, a->mount.mount_source))) + { + converged = false; + } + else if ((a->mount.mount_server != NULL) + && ((mp->host == NULL) || !StringEqual(mp->host, a->mount.mount_server))) + { + /* CFE-2350: the server is part of the mount identity, so a + * remounted-in-place mount that kept the old server has not + * converged - a remount in place cannot change the server, so + * reconciling it requires unmount_mount in remount_methods. */ + converged = false; + } + else if (a->mount.mount_options != NULL) + { + char *opts = Rlist2String(a->mount.mount_options, ","); + converged = (mp->raw_opts != NULL) && OptionsSubsetMatches(opts, mp->raw_opts); + free(opts); + } + else + { + converged = true; + } + break; + } + } + + DeleteMountInfo(tmp); + SeqDestroy(tmp); + return converged; +} + +/** + * CFE-90: Reconcile an already-mounted filesystem that has drifted from the + * promise, trying each a->mount.remount_methods mechanism in order (default: + * just "remount"; the disruptive "unmount_mount" is opt-in) and re-checking + * after each. Honors DONTDO; reports its own cfPS outcome. + */ +PromiseResult ReconcileMountOptions(EvalContext *ctx, char *name, const Attributes *a, const Promise *pp) +{ + assert(a != NULL); + if (a == NULL) + { + return PROMISE_RESULT_NOOP; + } + PromiseResult result = PROMISE_RESULT_NOOP; + char *opts = Rlist2String(a->mount.mount_options, ","); + int timeout = (a->mount.remount_timeout != CF_NOINT) ? a->mount.remount_timeout : RPCTIMEOUT; + + /* Ordered method list: promise-specified, else the default remount. */ + /* Borrows the const char *, does not own them. */ + Seq *methods = SeqNew( + (a->mount.remount_methods != NULL) ? RlistLen(a->mount.remount_methods) : 1, NULL); + if (a->mount.remount_methods != NULL) + { + for (const Rlist *rp = a->mount.remount_methods; rp != NULL; rp = rp->next) + { + SeqAppend(methods, RlistScalarValue(rp)); + } + } + else + { + /* Default: in-place remount only. unmount_mount tears the filesystem + * down and back up, so it is opt-in (needed for options a remount + * can't change, e.g. NFS-negotiated vers=/rsize= or the server). */ + SeqAppend(methods, "remount"); + } + + /* CFE-3366: gate on the promise action, not just DONTDO, so a dry-run or + * action_policy => "warn" reports a warning without defining + * promise_repaired on a run that changes nothing. */ + if (!MakingInternalChanges(ctx, pp, a, &result, + "reconcile mount '%s' to promised options '%s'", name, + (opts != NULL) ? opts : "")) + { + SeqDestroy(methods); + free(opts); + return result; + } + + bool converged = false; + const size_t n_methods = SeqLength(methods); + for (size_t i = 0; (i < n_methods) && !converged; i++) + { + const char *method = SeqAt(methods, i); + + if (StringEqual(method, "remount")) + { + char comm[CF_BUFSIZE]; + char *ropts = OptionStringExpandDefaults(opts); + if (ropts != NULL) + { + NDEBUG_UNUSED int ret = snprintf(comm, CF_BUFSIZE, "%s -o remount,%s %s", + CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), ropts, name); + assert(ret >= 0 && ret < CF_BUFSIZE); + } + else + { + NDEBUG_UNUSED int ret = snprintf(comm, CF_BUFSIZE, "%s -o remount %s", + CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), name); + assert(ret >= 0 && ret < CF_BUFSIZE); + } + free(ropts); + + Log(LOG_LEVEL_VERBOSE, "Reconciling '%s' via remount: %s", name, comm); + SetTimeOut(timeout); + + FILE *pfp = cf_popen(comm, "r", true); + if (pfp == NULL) + { + Log(LOG_LEVEL_ERR, "Failed to open pipe from '%s'", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS])); + } + else + { + size_t line_size = CF_BUFSIZE; + char *line = xmalloc(line_size); + while (CfReadLine(&line, &line_size, pfp) != -1) + { + /* drain command output */ + } + free(line); + cf_pclose(pfp); + } + } + else if (StringEqual(method, "unmount_mount")) + { + /* Reuse the tested helpers - both honor DONTDO and build the correct + * per-fstype command. This also handles a wrong-source mount, which + * an in-place remount cannot fix. */ + Log(LOG_LEVEL_VERBOSE, "Reconciling '%s' via unmount + mount", name); + SetTimeOut(timeout); + result = PromiseResultUpdate(result, VerifyUnmount(ctx, name, a, pp)); + result = PromiseResultUpdate(result, VerifyMount(ctx, name, a, pp)); + } + else + { + Log(LOG_LEVEL_WARNING, "Unknown remount_method '%s' for '%s' - skipping", method, name); + continue; + } + + /* Verify-after-act: confirm the live mount actually satisfies the promise. */ + if (LiveMountConverged(name, a)) + { + cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_CHANGE, pp, a, + "Reconciled mount '%s' to promised options '%s' via '%s'", name, + (opts != NULL) ? opts : "", method); + result = PromiseResultUpdate(result, PROMISE_RESULT_CHANGE); + converged = true; + } + } + + if (!converged) + { + cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_FAIL, pp, a, + "Could not reconcile mount '%s' to promised options '%s'", name, + (opts != NULL) ? opts : ""); + result = PromiseResultUpdate(result, PROMISE_RESULT_FAIL); + } + + SeqDestroy(methods); + free(opts); + return result; +} + void CleanupNFS(void) { Log(LOG_LEVEL_VERBOSE, "Number of changes observed in '%s' is %d", VFSTAB[VSYSTEMHARDCLASS], FSTAB_EDITS); diff --git a/cf-agent/nfs.h b/cf-agent/nfs.h index 03567c7b35..1ea51bde5b 100644 --- a/cf-agent/nfs.h +++ b/cf-agent/nfs.h @@ -29,6 +29,8 @@ #include // Seq bool LoadMountInfo(Seq *list); +bool OptionsSubsetMatches(const char *promised_opts, const char *actual_opts); +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); diff --git a/cf-agent/verify_storage.c b/cf-agent/verify_storage.c index 698585b9d8..77b910c662 100644 --- a/cf-agent/verify_storage.c +++ b/cf-agent/verify_storage.c @@ -101,18 +101,11 @@ PromiseResult VerifyStoragePromise(EvalContext *ctx, char *path, const Promise * /* No parameter conflicts here */ - if (a.mount.unmount) - { - if ((a.mount.mount_source)) - { - Log(LOG_LEVEL_VERBOSE, "An unmount promise indicates a mount-source information - probably an error"); - } - if ((a.mount.mount_server)) - { - Log(LOG_LEVEL_VERBOSE, "An unmount promise indicates a mount-server information - probably an error"); - } - } - else if (a.havemount) + /* CFE-2350: mount_source / mount_server on an unmount promise are not an + * error - they select which specific mount to unmount (e.g. one from a + * particular server) rather than every mount of a type. Only a *mount* + * promise needs both. */ + if (!a.mount.unmount && a.havemount) { if ((a.mount.mount_source == NULL) || (a.mount.mount_server == NULL)) { @@ -341,6 +334,11 @@ 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); + if (a == NULL) + { + return false; + } bool found = false; for (size_t i = 0; i < SeqLength(list); i++) @@ -366,11 +364,42 @@ static bool FileSystemMountedCorrectly(Seq *list, char *name, const Attributes * mp->host, mp->source, name); return false; } - else + + /* CFE-2350: the server is part of mount identity, but acting on a + * mismatch is disruptive (umount+mount), so only check it when the + * promise opts in via remount or unmount. */ + if ((a->mount.remount || a->mount.unmount) + && (a->mount.mount_server != NULL) + && ((mp->host == NULL) || !StringEqual(mp->host, a->mount.mount_server))) { - Log(LOG_LEVEL_VERBOSE, "File system '%s' seems to be mounted correctly", mp->source); - break; + Log(LOG_LEVEL_INFO, + "Filesystem on '%s' is from server '%s', not the promised server '%s'", + name, (mp->host != NULL) ? mp->host : "(unknown)", a->mount.mount_server); + return false; } + + /* CFE-90: option drift is only managed when 'remount' is on; without + * it a mount with the correct source is "mounted correctly" whatever + * the options (they still drive the initial mount and fstab). */ + if (a->mount.remount && a->mount.mount_options != NULL) + { + 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 != NULL) ? 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; } } @@ -378,8 +407,10 @@ static bool FileSystemMountedCorrectly(Seq *list, char *name, const Attributes * { 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 +471,11 @@ 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); + if (a == NULL) + { + return PROMISE_RESULT_NOOP; + } char dir[CF_BUFSIZE]; int changes = 0; @@ -454,13 +489,30 @@ 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)) + Seq *mounted_fs = GetGlobalMountedFSList(); + if (!FileSystemMountedCorrectly(mounted_fs, name, a)) { + /* Whether something is mounted at the promiser at all (vs. nothing + * mounted there). Computed for both mount and unmount: the unmount + * path uses it to tell "a different filesystem is mounted here" from + * "nothing is mounted here". */ + bool already_mounted = false; + const size_t n_mounted = SeqLength(mounted_fs); + for (size_t i = 0; i < n_mounted; i++) + { + Mount *mp = SeqAt(mounted_fs, i); + if (mp != NULL && mp->mounton != NULL && StringEqual(name, mp->mounton)) + { + already_mounted = true; + break; + } + } + if (!a->mount.unmount) { + /* Ensure the mount point exists before mounting or remounting. + * dir is "/.", so this creates the mount point directory. */ if (!MakeParentDirectory(dir, a->move_obstructions, NULL)) { // Could not create parent directory, assume this is okay, @@ -470,32 +522,59 @@ static PromiseResult VerifyMountPromise(EvalContext *ctx, char *name, const Attr dir); } - if (a->mount.editfstab) + 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); + } + + /* Persist to fstab regardless of the live outcome (reported + * above); it converges at the next remount/reboot. */ + 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 - mount just THIS filesystem instead of + * arming MountAll ('mount -a'), which would also mount every + * other unmounted fstab entry. Then persist to fstab. */ result = PromiseResultUpdate(result, VerifyMount(ctx, name, a, pp)); + if (a->mount.editfstab) + { + changes += VerifyInFstab(ctx, name, a, pp, &result); + } } } else { - if (a->mount.editfstab) + /* CFE-2350: a *different* filesystem here is not ours to unmount - + * leave it (and its fstab entry) alone; otherwise nothing of ours + * is mounted, so just ensure it is not in fstab. */ + if (already_mounted) + { + cfPS(ctx, LOG_LEVEL_VERBOSE, PROMISE_RESULT_NOOP, pp, a, + "A different filesystem is mounted on '%s' than the unmount promise targets; leaving it untouched", + name); + } + else if (a->mount.editfstab) { changes += VerifyNotInFstab(ctx, name, a, pp, &result); } } - - if (changes > 0) - { - CF_MOUNTALL = true; - } } else { @@ -509,11 +588,21 @@ 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: mounted correctly, but still maintain fstab (add a + * missing entry, correct drifted options) so the promise persists + * across reboots. Independent of 'remount' - keeping fstab correct + * is documented mount_options behavior; remounting live is not. */ + 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; } diff --git a/libpromises/attributes.c b/libpromises/attributes.c index ddc3138e8e..50851aaada 100644 --- a/libpromises/attributes.c +++ b/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; } diff --git a/libpromises/cf3.defs.h b/libpromises/cf3.defs.h index fd53bf8a68..b3bd138507 100644 --- a/libpromises/cf3.defs.h +++ b/libpromises/cf3.defs.h @@ -913,7 +913,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; @@ -1292,6 +1293,9 @@ typedef struct Rlist *mount_options; int editfstab; int unmount; + int remount; + Rlist *remount_methods; + int remount_timeout; } StorageMount; typedef struct diff --git a/libpromises/mod_storage.c b/libpromises/mod_storage.c index cd663a0f4a..4ed8f51b5a 100644 --- a/libpromises/mod_storage.c +++ b/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", SYNTAX_STATUS_NORMAL), + ConstraintSyntaxNewInt("remount_timeout", CF_VALRANGE, "Timeout in seconds for each remount_method. Default value: 60 seconds", SYNTAX_STATUS_NORMAL), ConstraintSyntaxNewNull() }; diff --git a/tests/unit/nfs_test.c b/tests/unit/nfs_test.c index 8e5d8ab78a..2e0c403e81 100644 --- a/tests/unit/nfs_test.c +++ b/tests/unit/nfs_test.c @@ -33,11 +33,156 @@ static void test_MatchFSInFstab(void) assert_false(MatchFSInFstab("/mnt/fileserver3/vol1")); } +static void test_OptionsSubsetMatches(void) +{ + /* Empty/NULL promise is always satisfied. */ + assert_true(OptionsSubsetMatches(NULL, "rw,noatime")); + assert_true(OptionsSubsetMatches("", "rw,noatime")); + + /* Subset: all promised present, kernel-added options ignored. */ + assert_true(OptionsSubsetMatches("rw,noatime", + "rw,noatime,vers=4.2,rsize=524288,wsize=524288,hard,proto=tcp,addr=10.0.0.1")); + + /* Order-insensitive. */ + assert_true(OptionsSubsetMatches("noatime,rw", "rw,noatime,vers=4.2")); + + /* A promised option that is simply absent -> mismatch. */ + assert_false(OptionsSubsetMatches("rw,noatime,acl", "rw,noatime,vers=4.2")); + + /* Inverse pairs contradict. */ + assert_false(OptionsSubsetMatches("noatime", "rw,relatime,vers=4.2")); + assert_false(OptionsSubsetMatches("ro", "rw,relatime")); + assert_false(OptionsSubsetMatches("rw", "ro,relatime")); + assert_false(OptionsSubsetMatches("hard", "rw,soft")); + assert_false(OptionsSubsetMatches("sync", "rw,async")); + + /* Generic "no" vs "" contradiction. */ + assert_false(OptionsSubsetMatches("nodev", "rw,dev")); + assert_false(OptionsSubsetMatches("atime", "rw,noatime")); + + /* Protocol aliases, both directions. */ + assert_true(OptionsSubsetMatches("tcp", "rw,proto=tcp,vers=4.2")); + assert_true(OptionsSubsetMatches("proto=tcp", "rw,tcp")); + assert_true(OptionsSubsetMatches("udp", "rw,proto=udp")); + + /* "defaults" is never echoed by the kernel; it holds iff none of the + * negatives that would violate it (ro/nosuid/nodev/noexec/sync) are + * present. atime/relatime and kernel-added options are irrelevant to it. */ + assert_true(OptionsSubsetMatches("defaults", "rw,relatime,vers=4.2,hard")); + assert_true(OptionsSubsetMatches("defaults,noatime", "rw,noatime,vers=4.2")); + assert_true(OptionsSubsetMatches("defaults", "rw,noatime,relatime,vers=4.2")); + assert_false(OptionsSubsetMatches("defaults", "ro,relatime,vers=4.2")); + assert_false(OptionsSubsetMatches("defaults", "rw,nosuid,vers=4.2")); + assert_false(OptionsSubsetMatches("defaults", "rw,noexec")); + assert_false(OptionsSubsetMatches("defaults", "rw,sync")); + + /* Last wins: a later option overrides an earlier conflicting one, exactly + * as `mount -o` applies the list (defaults,ro is read-only; ro,rw is rw). */ + assert_true(OptionsSubsetMatches("defaults,ro", "ro,relatime,vers=4.2")); /* ro overrides defaults' rw */ + assert_false(OptionsSubsetMatches("defaults,ro", "rw,relatime,vers=4.2")); /* wants ro, mount is rw */ + assert_true(OptionsSubsetMatches("ro,rw", "rw,relatime")); /* rw wins */ + assert_false(OptionsSubsetMatches("ro,rw", "ro,relatime")); /* wants rw, mount is ro */ + assert_true(OptionsSubsetMatches("rw,ro", "ro,relatime")); /* ro wins */ + assert_true(OptionsSubsetMatches("defaults,nosuid", "rw,nosuid,vers=4.2")); /* nosuid overrides defaults' suid */ +} + +static void test_OptionStringExpandDefaults(void) +{ + char *s; + + /* "defaults" expands to its checkable positives so an in-place remount + * restores a mount that drifted to ro/nosuid/etc (util-linux does not honor + * the options implied by "defaults" on a remount). */ + s = OptionStringExpandDefaults("defaults"); + assert_true(StringEqual(s, "rw,suid,dev,exec,async")); + free(s); + s = OptionStringExpandDefaults("defaults,noatime"); + assert_true(StringEqual(s, "rw,suid,dev,exec,async,noatime")); + free(s); + /* Everything else is passed through unchanged and order-preserved. */ + s = OptionStringExpandDefaults("rw,noatime"); + assert_true(StringEqual(s, "rw,noatime")); + free(s); + s = OptionStringExpandDefaults("ro"); + assert_true(StringEqual(s, "ro")); + free(s); + assert_true(OptionStringExpandDefaults("") == NULL); + assert_true(OptionStringExpandDefaults(NULL) == NULL); +} + +static void test_MountOptionsFromLine(void) +{ + char *s; + + /* The closing parenthesis must not survive on the last option: it would + * never compare equal to the promised one, so a correctly mounted + * filesystem would be reported as drifted (and, with remount enabled, + * remounted on every run without ever converging). */ + s = MountOptionsFromLine("srv:/vol on /mnt/x type nfs (rw,noatime)"); + assert_true(StringEqual(s, "rw,noatime")); + free(s); + s = MountOptionsFromLine("/dev/sda1 on /boot type ext4 (ro,nosuid)"); + assert_true(StringEqual(s, "ro,nosuid")); + free(s); + + /* A long NFS line keeps its last (kernel-negotiated) option intact. */ + s = MountOptionsFromLine( + "10.0.0.1:/export on /mnt/data type nfs4 (rw,relatime,vers=4.2,hard,addr=10.0.0.1)"); + assert_true(StringEqual(s, "rw,relatime,vers=4.2,hard,addr=10.0.0.1")); + free(s); + + /* Whitespace in the parentheses is not part of an option. The leading case + * also catches an interior pointer being returned: the free() then aborts. */ + s = MountOptionsFromLine("srv:/vol on /mnt/x type nfs (rw,noatime \t)"); + assert_true(StringEqual(s, "rw,noatime")); + free(s); + s = MountOptionsFromLine("srv:/vol on /mnt/x type nfs ( \trw,noatime )"); + assert_true(StringEqual(s, "rw,noatime")); + free(s); + + /* Only whitespace between the parentheses: empty, not NULL. */ + s = MountOptionsFromLine("srv:/vol on /mnt/x type nfs ( )"); + assert_true(StringEqual(s, "")); + free(s); + + /* Nothing to extract: no parentheses, and an unterminated one. */ + assert_true(MountOptionsFromLine("srv:/vol on /mnt/x type nfs") == NULL); + assert_true(MountOptionsFromLine("srv:/vol on /mnt/x type nfs (rw") == NULL); +} + +static void test_GetFstabEntryOptions(void) +{ + /* Own entries, so this does not depend on another test having run. */ + AppendItem(&FSTABLIST, "srv:/vol/a\t/mnt/opts/tabs\tnfs\trw,intr,noatime\t0 0", NULL); + AppendItem(&FSTABLIST, "UUID=1234 /mnt/opts/spaces ext4 errors=remount-ro 0 1", NULL); + AppendItem(&FSTABLIST, "#srv:/vol/c /mnt/opts/commented nfs rw,noatime 0 0", NULL); + AppendItem(&FSTABLIST, "srv:/vol/d /mnt/opts/short", NULL); + + /* Field 4 is the options — not the fstype in field 3. Tabs and runs of + * spaces both separate fields. */ + char *opts = GetFstabEntryOptions("/mnt/opts/tabs"); + assert_true(StringEqual(opts, "rw,intr,noatime")); + free(opts); + opts = GetFstabEntryOptions("/mnt/opts/spaces"); + assert_true(StringEqual(opts, "errors=remount-ro")); + free(opts); + + /* A commented-out entry is not an entry; nor is a truncated one or an + * unknown mount point. */ + assert_true(GetFstabEntryOptions("/mnt/opts/commented") == NULL); + assert_true(GetFstabEntryOptions("/mnt/opts/short") == NULL); + assert_true(GetFstabEntryOptions("/mnt/opts/nosuchmount") == NULL); +} + int main() { PRINT_TEST_BANNER(); const UnitTest tests[] = { unit_test(test_MatchFSInFstab), + unit_test(test_OptionsSubsetMatches), + unit_test(test_OptionStringExpandDefaults), + unit_test(test_MountOptionsFromLine), + unit_test(test_GetFstabEntryOptions), }; return run_tests(tests);