Skip to content

Commit

Permalink
Change ForceUnmount to UnmountOnEnoent (#2465)
Browse files Browse the repository at this point in the history
* PWX-38618: Rename option to UnmountOnEnoent

Signed-off-by: pnookala-px <[email protected]>

* fix test compile

Signed-off-by: pnookala-px <[email protected]>

---------

Signed-off-by: pnookala-px <[email protected]>
  • Loading branch information
pnookala-px authored Aug 22, 2024
1 parent f41c790 commit 014bae4
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 11 deletions.
12 changes: 6 additions & 6 deletions pkg/mount/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -646,9 +646,9 @@ func (m *Mounter) Unmount(
// fuse mounts show-up with this key as device.
device = value
}
forceUnmount := false
if _, ok := opts[options.OptionsForceUnmount]; ok {
forceUnmount = true
unmountOnEnoent := false
if _, ok := opts[options.OptionsUnmountOnEnoent]; ok {
unmountOnEnoent = true
}
info, ok := m.mounts[device]
if !ok {
Expand All @@ -657,9 +657,9 @@ func (m *Mounter) Unmount(
m.printMountTable()
m.Unlock()
err := ErrEnoent
// If forceUnmount, we need to attempt Unmount even if entry is not there
// If unmountOnEnoent, we need to attempt Unmount even if entry is not there
// in the mount table.
if forceUnmount {
if unmountOnEnoent {
unmountErr := m.mountImpl.Unmount(path, flags, timeout)
if unmountErr == nil {
logrus.Infof("Unmount of path [%s] successful even though entry didn't exist in mount-table", path)
Expand Down Expand Up @@ -699,7 +699,7 @@ func (m *Mounter) Unmount(
return nil
}
err := ErrEnoent
if forceUnmount {
if unmountOnEnoent {
logrus.Warnf("Device %q is not mounted at path %q as per mount table, still attempt the Unmount", device, path)
unmountErr := m.mountImpl.Unmount(path, flags, timeout)
if unmountErr == nil {
Expand Down
25 changes: 22 additions & 3 deletions pkg/mount/mount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ func TestRawMounter(t *testing.T) {
func allTests(t *testing.T, source, dest string) {
load(t, source, dest)
mountTest(t, source, dest)
forceUnmountTest(t, source, dest)
enoentUnmountTest(t, source, dest)
doubleUnmountTest(t, source, dest)
enoentUnmountTestWithoutOptions(t, source, dest)
mountTestParallel(t, source, dest)
inspect(t, source, dest)
reload(t, source, dest)
Expand Down Expand Up @@ -98,14 +100,31 @@ func mountTest(t *testing.T, source, dest string) {
require.NoError(t, err, "Failed in unmount")
}

func forceUnmountTest(t *testing.T, source, dest string) {
func enoentUnmountTest(t *testing.T, source, dest string) {
opts := make(map[string]string)
opts[options.OptionsForceUnmount] = "true"
opts[options.OptionsUnmountOnEnoent] = "true"
syscall.Mount(source, dest, "", syscall.MS_BIND, "")
err := m.Unmount(source, dest, 0, 0, opts)
require.NoError(t, err, "Failed in unmount")
}

func doubleUnmountTest(t *testing.T, source, dest string) {
err := m.Mount(0, source, dest, "", syscall.MS_BIND, "", 0, nil)
require.NoError(t, err, "Failed in mount")
err = m.Unmount(source, dest, 0, 0, nil)
require.NoError(t, err, "Failed in unmount")
err = m.Unmount(source, dest, 0, 0, nil)
require.Error(t, err, "Failed in second unmount, expected an error")
}

func enoentUnmountTestWithoutOptions(t *testing.T, source, dest string) {
syscall.Mount(source, dest, "", syscall.MS_BIND, "")
err := m.Unmount(source, dest, 0, 0, nil)
require.Error(t, err, "Failed in unmount, expected an error")
syscall.Unmount(dest, 0)
}


// mountTestParallel runs mount and unmount in parallel with serveral dirs
// in addition, we trigger failed unmount to test race condition in the case
// source directory is not found in the cache
Expand Down
4 changes: 2 additions & 2 deletions pkg/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ const (
// - Attach
// It indicates which IO path to use to complete user IO
OptionsFastpath = "FASTPATH_STATE"
// OptionsForceUnmount is an option to the following Opentstorage Volume API
// OptionsUnmountOnEnoent is an option to the following Opentstorage Volume API
// - Unmount
// It is used to issue an umount system call to the requested path even
// if the entry for the path is not present in the mount table
OptionsForceUnmount = "FORCE_UNMOUNT"
OptionsUnmountOnEnoent = "UNMOUNT_ON_ENOENT"
)

// IsBoolOptionSet checks if a boolean option key is set
Expand Down

0 comments on commit 014bae4

Please sign in to comment.