Get subvolume mounting/unmounting working way better

3 files changed, 75 insertions, 47 deletions

diff --git a/cmd/btrfs-mount/fuseutil.go b/cmd/btrfs-mount/fuseutil.go
new file mode 100644
index 0000000..c89e4b2
--- /dev/null
+++ b/cmd/btrfs-mount/fuseutil.go
@@ -0,0 +1,51 @@
+package main
+
+import (
+	"context"
+	"sync/atomic"
+
+	"github.com/datawire/dlib/dcontext"
+	"github.com/datawire/dlib/dgroup"
+	"github.com/datawire/dlib/dlog"
+	"github.com/jacobsa/fuse"
+)
+
+func Mount(ctx context.Context, mountpoint string, server fuse.Server, cfg *fuse.MountConfig) error {
+	grp := dgroup.NewGroup(ctx, dgroup.GroupConfig{
+		// Allow mountHandle.Join() returning to cause the
+		// "unmount" goroutine to quit.
+		ShutdownOnNonError: true,
+	})
+	mounted := uint32(1)
+	grp.Go("unmount", func(ctx context.Context) error {
+		<-ctx.Done()
+		var err error
+		var gotNil bool
+		// Keep retrying, because the FS might be busy.
+		for atomic.LoadUint32(&mounted) != 0 {
+			if _err := fuse.Unmount(mountpoint); _err == nil {
+				gotNil = true
+			} else if !gotNil {
+				err = _err
+			}
+		}
+		if gotNil {
+			return nil
+		}
+		return err
+	})
+	grp.Go("mount", func(ctx context.Context) error {
+		defer atomic.StoreUint32(&mounted, 0)
+
+		cfg.OpContext = ctx
+		cfg.ErrorLogger = dlog.StdLogger(ctx, dlog.LogLevelError)
+		cfg.DebugLogger = dlog.StdLogger(ctx, dlog.LogLevelDebug)
+
+		mountHandle, err := fuse.Mount(mountpoint, server, cfg)
+		if err != nil {
+			return err
+		}
+		return mountHandle.Join(dcontext.HardContext(ctx))
+	})
+	return grp.Wait()
+}
diff --git a/cmd/btrfs-mount/main.go b/cmd/btrfs-mount/main.go
index 4451e7a..ed22d7d 100644
--- a/cmd/btrfs-mount/main.go
+++ b/cmd/btrfs-mount/main.go
@@ -5,11 +5,9 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"sync/atomic"
 
 	"github.com/datawire/dlib/dgroup"
 	"github.com/datawire/dlib/dlog"
-	"github.com/jacobsa/fuse"
 	"github.com/sirupsen/logrus"
 
 	"lukeshu.com/btrfs-tools/pkg/btrfs"
@@ -57,28 +55,13 @@ func Main(ctx context.Context, mountpoint string, imgfilenames ...string) (err e
 		maybeSetErr(fs.Close())
 	}()
 
-	mounted := uint32(1)
-	grp := dgroup.NewGroup(ctx, dgroup.GroupConfig{
-		ShutdownOnNonError: true,
-	})
-	grp.Go("shutdown", func(ctx context.Context) error {
-		<-ctx.Done()
-		if atomic.LoadUint32(&mounted) == 0 {
-			return nil
-		}
-		return fuse.Unmount(os.Args[1])
-	})
-	grp.Go("mount", func(ctx context.Context) error {
-		defer atomic.StoreUint32(&mounted, 0)
-		rootSubvol := &Subvolume{
-			Subvolume: btrfs.Subvolume{
-				FS:     fs,
-				TreeID: btrfs.FS_TREE_OBJECTID,
-			},
-			DeviceName: tryAbs(imgfilenames[0]),
-			Mountpoint: mountpoint,
-		}
-		return rootSubvol.Run(ctx, false)
-	})
-	return grp.Wait()
+	rootSubvol := &Subvolume{
+		Subvolume: btrfs.Subvolume{
+			FS:     fs,
+			TreeID: btrfs.FS_TREE_OBJECTID,
+		},
+		DeviceName: tryAbs(imgfilenames[0]),
+		Mountpoint: mountpoint,
+	}
+	return rootSubvol.Run(ctx)
 }
diff --git a/cmd/btrfs-mount/subvol_fuse.go b/cmd/btrfs-mount/subvol_fuse.go
index 522da85..a96153a 100644
--- a/cmd/btrfs-mount/subvol_fuse.go
+++ b/cmd/btrfs-mount/subvol_fuse.go
@@ -10,9 +10,7 @@ import (
 	"sync/atomic"
 	"syscall"
 
-	"github.com/datawire/dlib/dcontext"
 	"github.com/datawire/dlib/dgroup"
-	"github.com/datawire/dlib/dlog"
 	"github.com/jacobsa/fuse"
 	"github.com/jacobsa/fuse/fuseops"
 	"github.com/jacobsa/fuse/fuseutil"
@@ -46,14 +44,10 @@ type Subvolume struct {
 	grp      *dgroup.Group
 }
 
-func (sv *Subvolume) Run(ctx context.Context, isSubvol bool) error {
+func (sv *Subvolume) Run(ctx context.Context) error {
 	sv.grp = dgroup.NewGroup(ctx, dgroup.GroupConfig{})
 	sv.grp.Go("self", func(ctx context.Context) error {
 		cfg := &fuse.MountConfig{
-			OpContext:   ctx,
-			ErrorLogger: dlog.StdLogger(ctx, dlog.LogLevelError),
-			DebugLogger: dlog.StdLogger(ctx, dlog.LogLevelDebug),
-
 			FSName:  sv.DeviceName,
 			Subtype: "btrfs",
 
@@ -63,17 +57,7 @@ func (sv *Subvolume) Run(ctx context.Context, isSubvol bool) error {
 				"allow_other": "",
 			},
 		}
-		if isSubvol {
-			//cfg.Options["nonempty"] = ""
-		}
-		mount, err := fuse.Mount(
-			sv.Mountpoint,
-			fuseutil.NewFileSystemServer(sv),
-			cfg)
-		if err != nil {
-			return err
-		}
-		return mount.Join(dcontext.HardContext(ctx))
+		return Mount(ctx, sv.Mountpoint, fuseutil.NewFileSystemServer(sv), cfg)
 	})
 	return sv.grp.Wait()
 }
@@ -135,7 +119,7 @@ func (sv *Subvolume) LoadDir(inode btrfs.ObjID) (val *btrfs.Dir, err error) {
 							DeviceName: sv.DeviceName,
 							Mountpoint: filepath.Join(sv.Mountpoint, subMountpoint[1:]),
 						}
-						return subSv.Run(ctx, true)
+						return subSv.Run(ctx)
 					})
 				}
 			}
@@ -186,13 +170,23 @@ func (sv *Subvolume) LookUpInode(_ context.Context, op *fuseops.LookUpInodeOp) e
 		return syscall.ENOENT
 	}
 	if entry.Location.ItemType != btrfsitem.INODE_ITEM_KEY {
+		// Subvolume
+		//
+		// Because each subvolume has its own pool of inodes
+		// (as in 2 different subvolumes can have files with
+		// te same inode number), so to represent that to FUSE
+		// we need to have this be a full separate mountpoint.
+		//
+		// I'd want to return EIO or EINTR or something here,
+		// but both the FUSE userspace tools and the kernel
+		// itself stat the mountpoint before mounting it, so
+		// we've got to return something bogus here to let
+		// that mount happen.
 		op.Entry = fuseops.ChildInodeEntry{
 			Child: 2, // an inode number that a real file will never have
 			Attributes: fuseops.InodeAttributes{
 				Nlink: 1,
 				Mode:  uint32(linux.ModeFmtDir | 0700),
-				//Uid:   1000, // TODO
-				//Gid:   1000, // TODO
 			},
 		}
 		return nil