Skip to content

Commit

Permalink
really disable fallback mounting to RO
Browse files Browse the repository at this point in the history
Passing the "rw" option is not enough. Careful reading of the manpage, verified
by tests and reading of the source code of util-linux reveals that disabling
the RO fallback only happens when you pass the "-w" flag.

Then, also fix "ro" mounts in case of XFS, as XFS attempts to write even if
passed the "ro" flag without the "norecovery" option.

Fixes: 6deb565 ("disable fallback to RO mounts for RW volumes")
Signed-off-by: Moritz Wanzenböck <moritz.wanzenboeck@linbit.com>
  • Loading branch information
WanzenBug committed Jan 2, 2025
1 parent 9388e1e commit 2b86f8c
Showing 1 changed file with 16 additions and 17 deletions.
33 changes: 16 additions & 17 deletions pkg/client/linstor.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"math"
"math/rand"
"os"
"os/exec"
"regexp"
"sort"
"strings"
Expand Down Expand Up @@ -64,7 +65,8 @@ type Linstor struct {
log *logrus.Entry
fallbackPrefix string
client *lc.HighLevelClient
mounter *mount.SafeFormatAndMount
mounter mount.Interface
resizer *mount.ResizeFs
labelBySP bool
}

Expand Down Expand Up @@ -98,10 +100,8 @@ func NewLinstor(options ...func(*Linstor) error) (*Linstor, error) {
"linstorCSIComponent": "client",
})

l.mounter = &mount.SafeFormatAndMount{
Interface: mount.New("/bin/mount"),
Exec: utilexec.New(),
}
l.mounter = mount.New("")
l.resizer = mount.NewResizeFs(utilexec.New())

l.log.WithFields(logrus.Fields{
"APIClient": fmt.Sprintf("%+v", l.client),
Expand Down Expand Up @@ -1954,17 +1954,19 @@ func (s *Linstor) Mount(ctx context.Context, source, target, fsType string, read
return fmt.Errorf("path %s is not a device", source) //nolint:goerr113
}

var mntFlags []string
if readonly {
mntOpts = append(mntOpts, "ro")
// xfs will try to write even on passing the "ro" option without the "norecovery" option
mntOpts = append(mntOpts, "ro", "norecovery")

// This requires DRBD 9.0.26+, older versions ignore this flag
err := s.setDevReadOnly(ctx, source)
if err != nil {
return fmt.Errorf("failed to set source device readonly: %w", err)
}
} else {
// Explicitly set rw option: otherwise mount may fall back to RO mount on promotion errors
mntOpts = append(mntOpts, "rw")
// Explicitly set -w option: otherwise mount may fall back to RO mount on promotion errors
mntFlags = append(mntFlags, "-w")

// We might be re-using an existing device that was set RO previously
err = s.setDevReadWrite(ctx, source)
Expand Down Expand Up @@ -1996,7 +1998,7 @@ func (s *Linstor) Mount(ctx context.Context, source, target, fsType string, read
}

if !isMounted {
err = s.mounter.Mount(source, target, fsType, mntOpts)
err = s.mounter.MountSensitiveWithoutSystemdWithMountFlags(source, target, fsType, mntOpts, nil, mntFlags)
if err != nil {
return err
}
Expand All @@ -2006,15 +2008,13 @@ func (s *Linstor) Mount(ctx context.Context, source, target, fsType string, read
return nil
}

resizerFs := mount.NewResizeFs(s.mounter.Exec)

needResize, err := resizerFs.NeedResize(source, target)
needResize, err := s.resizer.NeedResize(source, target)
if err != nil {
return fmt.Errorf("unable to determine if resize required: %w", err)
}

if needResize {
_, err := resizerFs.Resize(source, target)
_, err := s.resizer.Resize(source, target)
if err != nil {
unmountErr := s.Unmount(target)
if unmountErr != nil {
Expand All @@ -2029,12 +2029,12 @@ func (s *Linstor) Mount(ctx context.Context, source, target, fsType string, read
}

func (s *Linstor) setDevReadOnly(ctx context.Context, srcPath string) error {
_, err := s.mounter.Exec.CommandContext(ctx, "blockdev", "--setro", srcPath).CombinedOutput()
_, err := exec.CommandContext(ctx, "blockdev", "--setro", srcPath).CombinedOutput()
return err
}

func (s *Linstor) setDevReadWrite(ctx context.Context, srcPath string) error {
_, err := s.mounter.Exec.CommandContext(ctx, "blockdev", "--setrw", srcPath).CombinedOutput()
_, err := exec.CommandContext(ctx, "blockdev", "--setrw", srcPath).CombinedOutput()
return err
}

Expand Down Expand Up @@ -2157,8 +2157,7 @@ func (s *Linstor) NodeExpand(target string) error {
return fmt.Errorf("mount point '%s' not found: %w", target, os.ErrNotExist)
}

resizer := mount.NewResizeFs(s.mounter.Exec)
_, err = resizer.Resize(mounts[i].Device, target)
_, err = s.resizer.Resize(mounts[i].Device, target)
return err
}

Expand Down

0 comments on commit 2b86f8c

Please sign in to comment.