Skip to content

fix(filesystem): add comma separator in mountOptionsString - #3507

Open
Ray0907 wants to merge 1 commit into
prometheus:masterfrom
Ray0907:fix/filesystem-readonly-mount-options
Open

fix(filesystem): add comma separator in mountOptionsString#3507
Ray0907 wants to merge 1 commit into
prometheus:masterfrom
Ray0907:fix/filesystem-readonly-mount-options

Conversation

@Ray0907

@Ray0907 Ray0907 commented Dec 22, 2025

Copy link
Copy Markdown
Contributor

mountOptionsString was concatenating options without commas, breaking
readonly dete

Fixes: #3484

Signed-off-by: Ray Tien ray.tien0907@gmail.com

  mountOptionsString was concatenating options without commas, breaking
  readonly dete

  Signed-off-by: Ray Tien <ray.tien0907@gmail.com>
@discordianfish

Copy link
Copy Markdown
Member

Needs sign off. Also consider preallocating the string array. Not that its needed but why not.

SuperQ pushed a commit that referenced this pull request Jun 9, 2026
* fix(filesystem): add comma separator in mountOptionsString

mountOptionsString() was concatenating mount options without a
separator, so isFilesystemReadOnly()'s strings.Split(opts, ",") could
never find "ro" when it appeared alongside other options. As a
result node_filesystem_readonly incorrectly reported 0 for any
multi-option read-only mount, including ZFS filesystems mounted
read-only and ext4 filesystems after a kernel emergency
remount-ro triggered by errors=remount-ro.

Also preallocate the slice with len(m) capacity, as suggested in
review on #3507, to avoid repeated reallocations during append.

Fixes: #3484
Fixes: #3576
Co-authored-by: Ray Tien <ray.tien0907@gmail.com>
Signed-off-by: LeandroSaldivarmrf <leandro.saldivar@marfeel.com>

* test(filesystem): add regression tests for mountOptionsString

Signed-off-by: LeandroSaldivarmrf <leandro.saldivar@marfeel.com>

---------

Signed-off-by: LeandroSaldivarmrf <leandro.saldivar@marfeel.com>
Co-authored-by: Ray Tien <ray.tien0907@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

node_filesystem_readonly does not report readonly bind mounts correctly

2 participants