Fix: async (background) mount may cause problems

CHANGELOG.md

@@ -2,6 +2,9 @@
 **Bug Fixes**
 - [#1080](https://github.com/Azure/azure-storage-fuse/issues/1080) HNS rename flow does not encode source path correctly.
 - [#1081](https://github.com/Azure/azure-storage-fuse/issues/1081) Blobfuse will exit with non-zero status code if allow_other option is used but not enabled in fuse config.
+- [#1079](https://github.com/Azure/azure-storage-fuse/issues/1079) Shell returns before child process mounts the container and if user tries to bind the mount it leads to inconsistent state.
+- If mount fails in forked child, blobfuse2 will return back with status error code.
+
 
 ## 2.0.2 (2022-02-23)
 **Bug Fixes**

README.md

@@ -99,8 +99,9 @@ To learn about a specific command, just include the name of the command (For exa
     * `--read-only=true`: Mount container in read-only mode.
     * `--default-working-dir`: The default working directory to store log files and other blobfuse2 related information.
     * `--disable-version-check=true`: Disable the blobfuse2 version check.
-    * `----secure-config=true` : Config file is encrypted suing 'blobfuse2 secure` command.
-    * `----passphrase=<STRING>` : Passphrase used to encrypt/decrypt config file.
+    * `--secure-config=true` : Config file is encrypted suing 'blobfuse2 secure` command.
+    * `--passphrase=<STRING>` : Passphrase used to encrypt/decrypt config file.
+    * `--wait-for-mount=<TIMEOUT IN SECONDS>` : Let parent process wait for given timeout before exit to ensure child has started. 
 - Attribute cache options
     * `--attr-cache-timeout=<TIMEOUT IN SECONDS>`: The timeout for the attribute cache entries.
     * `--no-symlinks=true`: To improve performance disable symlink support.

cmd/mount.go

@@ -42,13 +42,15 @@ import (
 	"net/http"
 	_ "net/http/pprof"
 	"os"
+	"os/signal"
 	"path/filepath"
 	"runtime"
 	"runtime/debug"
 	"runtime/pprof"
 	"strconv"
 	"strings"
 	"syscall"
+	"time"
 
 	"github.com/Azure/azure-storage-fuse/v2/common"
 	"github.com/Azure/azure-storage-fuse/v2/common/config"
@@ -85,6 +87,7 @@ type mountOptions struct {
 	ProfilerPort      int            `config:"profiler-port"`
 	ProfilerIP        string         `config:"profiler-ip"`
 	MonitorOpt        monitorOptions `config:"health_monitor"`
+	WaitForMount      time.Duration  `config:"wait-for-mount"`
 
 	// v1 support
 	Streaming      bool     `config:"streaming"`
@@ -401,24 +404,70 @@ var mountCmd = &cobra.Command{
 			}
 
 			ctx, _ := context.WithCancel(context.Background()) //nolint
-			daemon.SetSigHandler(sigusrHandler(pipeline, ctx), syscall.SIGUSR1, syscall.SIGUSR2)
+
+			// Signal handlers for parent and child to communicate success or failures in mount
+			var sigusr2, sigchild chan os.Signal
+			if !daemon.WasReborn() { // execute in parent only
+				sigusr2 = make(chan os.Signal, 1)
+				signal.Notify(sigusr2, syscall.SIGUSR2)
+
+				sigchild = make(chan os.Signal, 1)
+				signal.Notify(sigchild, syscall.SIGCHLD)
+			} else { // execute in child only
+				daemon.SetSigHandler(sigusrHandler(pipeline, ctx), syscall.SIGUSR1, syscall.SIGUSR2)
+				go func() {
+					_ = daemon.ServeSignals()
+				}()
+			}
+
 			child, err := dmnCtx.Reborn()
 			if err != nil {
 				log.Err("mount : failed to daemonize application [%v]", err)
 				return Destroy(fmt.Sprintf("failed to daemonize application [%s]", err.Error()))
 			}
 
 			log.Debug("mount: foreground disabled, child = %v", daemon.WasReborn())
-			if child == nil {
+			if child == nil { // execute in child only
 				defer dmnCtx.Release() // nolint
 				setGOConfig()
 				go startDynamicProfiler()
 
 				err = runPipeline(pipeline, ctx)
 				if err != nil {
+					pid := os.Getpid()
+					fname := fmt.Sprintf("/tmp/blobfuse2.%v", pid)
+					if err = ioutil.WriteFile(fname, []byte(err.Error()), 0777); err != nil {
+						log.Err("mount: failed to save mount failure logs [%s]", err.Error())
+					}
+
 					return err
 				}
+
+			} else { // execute in parent only
+				select {
+				case <-sigusr2:
+					log.Info("mount: Child [%v] mounted successfully at %s", child.Pid, options.MountPath)
+
+				case <-sigchild:
+					// Get error string from the child
+					log.Info("mount: Child [%v] terminated from %s", child.Pid, options.MountPath)
+
+					fname := fmt.Sprintf("/tmp/blobfuse2.%v", child.Pid)
+
+					buff, err := ioutil.ReadFile(fname)
+					if err != nil {
+						log.Err("mount: failed to read child [%v] failure logs [%s]", child.Pid, err.Error())
+						return Destroy(fmt.Sprintf("failed to mount, please check logs [%s]", err.Error()))
+					} else {
+						_ = os.Remove(fname)
+						return Destroy(string(buff))
+					}
+
+				case <-time.After(options.WaitForMount):
+					log.Info("mount: Child [%v : %s] status check timeout", child.Pid, options.MountPath)
+				}
 			}
+
 		} else {
 			if options.CPUProfile != "" {
 				os.Remove(options.CPUProfile)
@@ -638,6 +687,8 @@ func init() {
 	config.BindPFlag("libfuse-options", mountCmd.PersistentFlags().ShorthandLookup("o"))
 	mountCmd.PersistentFlags().ShorthandLookup("o").Hidden = true
 
+	mountCmd.PersistentFlags().DurationVar(&options.WaitForMount, "wait-for-mount", 5*time.Second, "Let parent process wait for given timeout before exit")
+
 	config.AttachToFlagSet(mountCmd.PersistentFlags())
 	config.AttachFlagCompletions(mountCmd)
 	config.AddConfigChangeEventListener(config.ConfigChangeEventHandlerFunc(OnConfigChange))

common/util.go

@@ -47,6 +47,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"syscall"
 
 	"gopkg.in/ini.v1"
 )
@@ -265,3 +266,17 @@ func ExpandPath(path string) string {
 
 	return os.ExpandEnv(path)
 }
+
+// NotifyMountToParent : Send a signal to parent process about successful mount
+func NotifyMountToParent() error {
+	ppid := syscall.Getppid()
+	if ppid > 1 {
+		if err := syscall.Kill(ppid, syscall.SIGUSR2); err != nil {
+			return err
+		}
+	} else {
+		return fmt.Errorf("failed to get parent pid, received : %v", ppid)
+	}
+
+	return nil
+}

component/libfuse/libfuse2_handler.go

@@ -1,3 +1,4 @@
+//go:build fuse2
 // +build fuse2
 
 /*
@@ -241,6 +242,12 @@ func (lf *Libfuse) destroyFuse() error {
 //export libfuse2_init
 func libfuse2_init(conn *C.fuse_conn_info_t) (res unsafe.Pointer) {
 	log.Trace("Libfuse::libfuse2_init : init")
+
+	log.Info("Libfuse::NotifyMountToParent : Notifying parent for successful mount")
+	if err := common.NotifyMountToParent(); err != nil {
+		log.Err("Libfuse::NotifyMountToParent : Failed to notify parent, error: [%v]", err)
+	}
+
 	C.populate_uid_gid()
 
 	log.Info("Libfuse::libfuse2_init : Kernel Caps : %d", conn.capable)
@@ -320,6 +327,7 @@ func (lf *Libfuse) fillStat(attr *internal.ObjAttr, stbuf *C.stat_t) {
 // otherwise, perform necessary permission checking
 
 // libfuse2_getattr gets file attributes
+//
 //export libfuse2_getattr
 func libfuse2_getattr(path *C.char, stbuf *C.stat_t) C.int {
 	name := trimFusePath(path)
@@ -350,6 +358,7 @@ func libfuse2_getattr(path *C.char, stbuf *C.stat_t) C.int {
 }
 
 // File Operations
+//
 //export libfuse_statfs
 func libfuse_statfs(path *C.char, buf *C.statvfs_t) C.int {
 	name := trimFusePath(path)
@@ -383,6 +392,7 @@ func libfuse_statfs(path *C.char, buf *C.statvfs_t) C.int {
 // Directory Operations
 
 // libfuse_mkdir creates a directory
+//
 //export libfuse_mkdir
 func libfuse_mkdir(path *C.char, mode C.mode_t) C.int {
 	name := trimFusePath(path)
@@ -402,6 +412,7 @@ func libfuse_mkdir(path *C.char, mode C.mode_t) C.int {
 }
 
 // libfuse_opendir opens handle to given directory
+//
 //export libfuse_opendir
 func libfuse_opendir(path *C.char, fi *C.fuse_file_info_t) C.int {
 	name := trimFusePath(path)
@@ -430,6 +441,7 @@ func libfuse_opendir(path *C.char, fi *C.fuse_file_info_t) C.int {
 }
 
 // libfuse_releasedir opens handle to given directory
+//
 //export libfuse_releasedir
 func libfuse_releasedir(path *C.char, fi *C.fuse_file_info_t) C.int {
 	handle := (*handlemap.Handle)(unsafe.Pointer(uintptr(fi.fh)))
@@ -441,6 +453,7 @@ func libfuse_releasedir(path *C.char, fi *C.fuse_file_info_t) C.int {
 }
 
 // libfuse2_readdir reads a directory
+//
 //export libfuse2_readdir
 func libfuse2_readdir(_ *C.char, buf unsafe.Pointer, filler C.fuse_fill_dir_t, off C.off_t, fi *C.fuse_file_info_t) C.int {
 	handle := (*handlemap.Handle)(unsafe.Pointer(uintptr(fi.fh)))
@@ -507,6 +520,7 @@ func libfuse2_readdir(_ *C.char, buf unsafe.Pointer, filler C.fuse_fill_dir_t, o
 }
 
 // libfuse_rmdir deletes a directory, which must be empty.
+//
 //export libfuse_rmdir
 func libfuse_rmdir(path *C.char) C.int {
 	name := trimFusePath(path)
@@ -537,6 +551,7 @@ func libfuse_rmdir(path *C.char) C.int {
 // File Operations
 
 // libfuse_create creates a file with the specified mode and then opens it.
+//
 //export libfuse_create
 func libfuse_create(path *C.char, mode C.mode_t, fi *C.fuse_file_info_t) C.int {
 	name := trimFusePath(path)
@@ -570,6 +585,7 @@ func libfuse_create(path *C.char, mode C.mode_t, fi *C.fuse_file_info_t) C.int {
 }
 
 // libfuse_open opens a file
+//
 //export libfuse_open
 func libfuse_open(path *C.char, fi *C.fuse_file_info_t) C.int {
 	name := trimFusePath(path)
@@ -618,6 +634,7 @@ func libfuse_open(path *C.char, fi *C.fuse_file_info_t) C.int {
 }
 
 // libfuse_read reads data from an open file
+//
 //export libfuse_read
 func libfuse_read(path *C.char, buf *C.char, size C.size_t, off C.off_t, fi *C.fuse_file_info_t) C.int {
 	fileHandle := (*C.file_handle_t)(unsafe.Pointer(uintptr(fi.fh)))
@@ -653,6 +670,7 @@ func libfuse_read(path *C.char, buf *C.char, size C.size_t, off C.off_t, fi *C.f
 }
 
 // libfuse_write writes data to an open file
+//
 //export libfuse_write
 func libfuse_write(path *C.char, buf *C.char, size C.size_t, off C.off_t, fi *C.fuse_file_info_t) C.int {
 	fileHandle := (*C.file_handle_t)(unsafe.Pointer(uintptr(fi.fh)))
@@ -677,6 +695,7 @@ func libfuse_write(path *C.char, buf *C.char, size C.size_t, off C.off_t, fi *C.
 }
 
 // libfuse_flush possibly flushes cached data
+//
 //export libfuse_flush
 func libfuse_flush(path *C.char, fi *C.fuse_file_info_t) C.int {
 	fileHandle := (*C.file_handle_t)(unsafe.Pointer(uintptr(fi.fh)))
@@ -703,6 +722,7 @@ func libfuse_flush(path *C.char, fi *C.fuse_file_info_t) C.int {
 }
 
 // libfuse2_truncate changes the size of a file
+//
 //export libfuse2_truncate
 func libfuse2_truncate(path *C.char, off C.off_t) C.int {
 	name := trimFusePath(path)
@@ -726,6 +746,7 @@ func libfuse2_truncate(path *C.char, off C.off_t) C.int {
 }
 
 // libfuse_release releases an open file
+//
 //export libfuse_release
 func libfuse_release(path *C.char, fi *C.fuse_file_info_t) C.int {
 	fileHandle := (*C.file_handle_t)(unsafe.Pointer(uintptr(fi.fh)))
@@ -753,6 +774,7 @@ func libfuse_release(path *C.char, fi *C.fuse_file_info_t) C.int {
 }
 
 // libfuse_unlink removes a file
+//
 //export libfuse_unlink
 func libfuse_unlink(path *C.char) C.int {
 	name := trimFusePath(path)
@@ -778,6 +800,7 @@ func libfuse_unlink(path *C.char) C.int {
 // https://man7.org/linux/man-pages/man2/rename.2.html
 // errors handled: EISDIR, ENOENT, ENOTDIR, ENOTEMPTY, EEXIST
 // TODO: handle EACCESS, EINVAL?
+//
 //export libfuse2_rename
 func libfuse2_rename(src *C.char, dst *C.char) C.int {
 	srcPath := trimFusePath(src)
@@ -849,6 +872,7 @@ func libfuse2_rename(src *C.char, dst *C.char) C.int {
 // Symlink Operations
 
 // libfuse_symlink creates a symbolic link
+//
 //export libfuse_symlink
 func libfuse_symlink(target *C.char, link *C.char) C.int {
 	name := trimFusePath(link)
@@ -870,6 +894,7 @@ func libfuse_symlink(target *C.char, link *C.char) C.int {
 }
 
 // libfuse_readlink reads the target of a symbolic link
+//
 //export libfuse_readlink
 func libfuse_readlink(path *C.char, buf *C.char, size C.size_t) C.int {
 	name := trimFusePath(path)
@@ -895,6 +920,7 @@ func libfuse_readlink(path *C.char, buf *C.char, size C.size_t) C.int {
 }
 
 // libfuse_fsync synchronizes file contents
+//
 //export libfuse_fsync
 func libfuse_fsync(path *C.char, datasync C.int, fi *C.fuse_file_info_t) C.int {
 	if fi.fh == 0 {
@@ -922,6 +948,7 @@ func libfuse_fsync(path *C.char, datasync C.int, fi *C.fuse_file_info_t) C.int {
 }
 
 // libfuse_fsyncdir synchronizes directory contents
+//
 //export libfuse_fsyncdir
 func libfuse_fsyncdir(path *C.char, datasync C.int, fi *C.fuse_file_info_t) C.int {
 	name := trimFusePath(path)
@@ -945,6 +972,7 @@ func libfuse_fsyncdir(path *C.char, datasync C.int, fi *C.fuse_file_info_t) C.in
 }
 
 // libfuse2_chmod changes permission bits of a file
+//
 //export libfuse2_chmod
 func libfuse2_chmod(path *C.char, mode C.mode_t) C.int {
 	name := trimFusePath(path)
@@ -971,6 +999,7 @@ func libfuse2_chmod(path *C.char, mode C.mode_t) C.int {
 }
 
 // libfuse2_chown changes the owner and group of a file
+//
 //export libfuse2_chown
 func libfuse2_chown(path *C.char, uid C.uid_t, gid C.gid_t) C.int {
 	name := trimFusePath(path)
@@ -981,6 +1010,7 @@ func libfuse2_chown(path *C.char, uid C.uid_t, gid C.gid_t) C.int {
 }
 
 // libfuse2_utimens changes the access and modification times of a file
+//
 //export libfuse2_utimens
 func libfuse2_utimens(path *C.char, tv *C.timespec_t) C.int {
 	name := trimFusePath(path)
@@ -993,6 +1023,7 @@ func libfuse2_utimens(path *C.char, tv *C.timespec_t) C.int {
 }
 
 // blobfuse_cache_update refresh the file-cache policy for this file
+//
 //export blobfuse_cache_update
 func blobfuse_cache_update(path *C.char) C.int {
 	name := trimFusePath(path)