Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add experimental daemon plugin for 9P2000.L support #6612

Closed
wants to merge 102 commits into from
Closed
Show file tree
Hide file tree
Changes from 78 commits
Commits
Show all changes
102 commits
Select commit Hold shift + click to select a range
2a02a91
plugin/fs: initial experiment
djdv Sep 9, 2019
ef75786
plugin/fs: Basic Readdir
djdv Jul 26, 2019
43a38b6
plugin/fs: Basic ReadAt
djdv Aug 14, 2019
7a80532
plugin/fs: Fix a bunch of path related issues
djdv Aug 15, 2019
2d99302
plugin/fs: Fix a bunch of metadata related issues
djdv Aug 22, 2019
75618a5
plugin/fs: linting
djdv Aug 22, 2019
07534a3
plugin/fs: use config
djdv Aug 26, 2019
0d28489
plugin/fs: Better init/config
djdv Aug 29, 2019
7bb358f
plugin/fs: Betterer init/config
djdv Aug 30, 2019
6a17332
plugin/fs: consistent protocol identifier
djdv Aug 30, 2019
090f7cd
plugin/fs: text secession
djdv Aug 30, 2019
19d5332
squashme-deps
djdv Sep 2, 2019
353007e
plugin/fs: client fixes
djdv Sep 4, 2019
6e7ea19
plugin/fs: add PinFS test
djdv Sep 4, 2019
28c5cff
plugin/fs: add root test + change pin test
djdv Sep 4, 2019
81b9a06
IPFS-test WIP
djdv Sep 4, 2019
864dbfe
deps
djdv Sep 5, 2019
35eee21
fix fs test on linux
djdv Sep 6, 2019
483845c
CR feedback + docs WIP, linting, fix freebsd builds
djdv Sep 7, 2019
3876302
openbsd builds
djdv Sep 7, 2019
d9fd656
fix doc format
djdv Sep 7, 2019
f48f4a8
stop tracking client-lib
djdv Sep 7, 2019
4bb5712
docfix
djdv Sep 7, 2019
760d3c6
wait for fs service to return on close + remove listener on Windows i…
djdv Sep 7, 2019
6e87be8
change how IPFS reads directory streams
djdv Sep 9, 2019
7ff0d1a
fix readdir offset + add test
djdv Sep 9, 2019
99df410
fix: use environment variable when set
djdv Sep 9, 2019
dc6287d
docs: wrong port + add more to the client example
djdv Sep 9, 2019
b8c4a3c
markdown fixes
djdv Sep 9, 2019
0997454
the linebreaks too
djdv Sep 9, 2019
3ee383a
doc: add a table of the root index mapping
djdv Sep 9, 2019
15e894a
CR bundle (split later)
djdv Sep 11, 2019
e5ec672
qid-fix
djdv Sep 12, 2019
c653fc5
clone-fix
djdv Sep 12, 2019
b6d1506
change config handling (needs tests)
djdv Sep 12, 2019
fbf5bca
delay listener init
djdv Sep 12, 2019
888c33d
use multicodec standard
djdv Sep 12, 2019
3017586
don't implicitly timestamp core objects
djdv Sep 12, 2019
6507b34
use a standard blocksize
djdv Sep 12, 2019
4a471d6
stop listening sooner
djdv Sep 12, 2019
afd9cb1
ipfs read fixes
djdv Sep 13, 2019
e8c1a5e
docs: experimental stability before feature reality
djdv Sep 13, 2019
a2a9e8b
readdir improvments
djdv Sep 13, 2019
0a6110d
readdir end of stream fix
djdv Sep 13, 2019
1dd3125
store ipfs reference on open for use in read
djdv Sep 13, 2019
7b10b5f
better constrain/contextualize operation calls
djdv Sep 13, 2019
82780a1
change root construction
djdv Sep 14, 2019
4c9a790
better walk logic (and more; todo:split commit)
djdv Sep 15, 2019
fb20a44
ipns initial
djdv Sep 15, 2019
5ad516c
log linting
djdv Sep 16, 2019
02f69b8
walk logic - this is better but still broken
djdv Sep 17, 2019
eaff0c9
misc cleanup and context changes
djdv Sep 18, 2019
3ab6029
context consistency
djdv Sep 18, 2019
b6dee76
less broken stat
djdv Sep 18, 2019
770bbce
small lint
djdv Sep 19, 2019
4717f73
Revert "change config handling (needs tests)"
djdv Sep 19, 2019
21b2d2c
rework config, remove windows manet hacks, remove ipns
djdv Sep 21, 2019
62560e4
fix read offset and return values
djdv Sep 22, 2019
11c5add
change mount docs, context WIP
djdv Sep 22, 2019
dd3a894
walk refactor
djdv Sep 24, 2019
1636e1b
unbreak tests
djdv Sep 24, 2019
63f6d27
possible fix for re-attachment
djdv Sep 25, 2019
97f36a9
socket path template nit
djdv Sep 25, 2019
3c56f5f
change attacher-constructors and walk clone-semantics
djdv Sep 26, 2019
4c1ad5d
add basic root tests
djdv Sep 26, 2019
e3db71a
remove runtime check, already static checked
djdv Sep 27, 2019
b2a43b8
fix attach for other roots too
djdv Sep 27, 2019
9cc06ae
traversal/construction refactor
djdv Oct 3, 2019
e79e371
ipns refactor WIP
djdv Oct 9, 2019
d1a1170
change/fix node derive logic
djdv Oct 9, 2019
be83005
update experimental doc
djdv Oct 9, 2019
fbac898
linting
djdv Oct 9, 2019
62cf47e
test: don't compare directory size
djdv Oct 9, 2019
e8ccc6b
list ipns in the root
djdv Oct 10, 2019
6d92612
context refactor wip
djdv Oct 14, 2019
294c3ea
actually cancel the context + lint
djdv Oct 14, 2019
8ce5e51
actually use the context too
djdv Oct 14, 2019
8328485
don't readdir forever during Linux tests
djdv Oct 15, 2019
db0bcba
don't lose err value
djdv Oct 15, 2019
7bb94a4
readdir fixes
djdv Oct 15, 2019
e029c7d
gomod deps
djdv Oct 15, 2019
8cee086
changes and tests around plugin close
djdv Oct 21, 2019
2a495c7
lol unsaved buffers
djdv Oct 21, 2019
161d86a
expose WalkRef and generalize Backtrack
djdv Oct 21, 2019
9ef8e6a
extract WalkRef and cleanup related use
djdv Oct 21, 2019
545d666
unexport NinePath
djdv Oct 21, 2019
9024b95
inheritance changes related to the base clases
djdv Oct 21, 2019
a0465f4
small lint and docs
djdv Oct 21, 2019
2aead06
change attach clone method
djdv Oct 22, 2019
ec9f8ea
unbreak root attrs
djdv Oct 22, 2019
c744a00
cleanup attachers
djdv Oct 22, 2019
65b677f
slightly saner test strings
djdv Oct 23, 2019
7ce0e9b
test open
djdv Oct 23, 2019
4c6b8ee
allow ipfs root to be read
djdv Oct 23, 2019
6e654db
fix context for proxy roots
djdv Oct 23, 2019
3501592
fix
djdv Oct 23, 2019
07db2b9
doc pass WIP
djdv Oct 23, 2019
984f155
make sure test defers gets called
djdv Oct 25, 2019
0fa5983
try not to leak
djdv Oct 25, 2019
c9faaa7
change Plugin Close procedure
djdv Oct 25, 2019
c7cfd71
change Plugin Close procedure part 2
djdv Oct 25, 2019
c294ac8
don't swallow Read return value
djdv Oct 25, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions docs/experimental-features.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ the above issue.
- [AutoRelay](#autorelay)
- [TLS 1.3 Handshake](#tls-13-as-default-handshake-protocol)
- [Strategic Providing](#strategic-providing)
- [IPFS filesystem API plugin](#filesystem-api-plugin)

---

Expand Down Expand Up @@ -705,3 +706,42 @@ ipfs config --json Experimental.StrategicProviding true
- [ ] provide roots
- [ ] provide all
- [ ] provide strategic

## Filesystem API Plugin

### State

Experimental, not included by default.

This daemon plugin wraps the IPFS node and exposes file system services over a multiaddr listener.
Currently we use the 9P2000.L protocol, and offer read-only support for `/ipfs` requests. With support for other (writable) systems coming next.

You may connect to this service using the [`v9fs`](https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/plain/Documentation/filesystems/9p.txt) client used in the Linux kernel.
By default we listen for requests on a Unix domain socket.
`mount -t 9p -o trans=unix $IPFS_PATH/filesystem.9p.sock ~/ipfs-mount`

To configure the listening address and more, see the [package documentation](https://godoc.org/github.com/ipfs/go-ipfs/plugin/plugins/filesystem)
See the [v9fs documentation](https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/plain/Documentation/filesystems/9p.txt) for instructions on using transports, such as TCP.
i.e.
```
> export IPFS_FS_ADDR=/ip4/127.0.0.1/tcp/564
> ipfs daemon &
> mount -t 9p 127.0.0.1 ~/ipfs-mount
> ...
> umount ~/ipfs-mount
> ipfs shutdown
```

### How to enable

See the Plugin documentation [here](https://github.com/ipfs/go-ipfs/blob/master/docs/plugins.md#installing-plugins).
You will likely want to add the plugin to the `go-ipfs` plugin preload list
`filesystem github.com/ipfs/go-ipfs/plugin/plugins/filesystem *`

### Road to being a real feature

- [ ] Needs testing
- Technically correct (`go test`, sharness, etc.)
- [ ] Needs finalizing
- Config structure and environment variable names become stable/frozen
- Practically/API correct (does this service fulfill the practical needs of users/clients?)
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ require (
github.com/Kubuxu/gocovmerge v0.0.0-20161216165753-7ecaa51963cd
github.com/blang/semver v3.5.1+incompatible
github.com/bren2010/proquint v0.0.0-20160323162903-38337c27106d
github.com/djdv/p9 v0.0.0-20190907154417-3fdf0632585f
github.com/dustin/go-humanize v1.0.0
github.com/elgris/jsondiff v0.0.0-20160530203242-765b5c24c302
github.com/fatih/color v1.7.0 // indirect
Expand Down Expand Up @@ -87,6 +88,7 @@ require (
github.com/libp2p/go-maddr-filter v0.0.5
github.com/mattn/go-runewidth v0.0.4 // indirect
github.com/mitchellh/go-homedir v1.1.0
github.com/mitchellh/mapstructure v1.1.2
github.com/mr-tron/base58 v1.1.2
github.com/multiformats/go-multiaddr v0.0.4
github.com/multiformats/go-multiaddr-dns v0.0.3
Expand All @@ -109,7 +111,7 @@ require (
go.uber.org/goleak v0.10.0 // indirect
go.uber.org/multierr v1.1.0 // indirect
go4.org v0.0.0-20190313082347-94abd6928b1d // indirect
golang.org/x/sys v0.0.0-20190626221950-04f50cda93cb
golang.org/x/sys v0.0.0-20190903213830-1f305c863dab
golang.org/x/tools v0.0.0-20190621195816-6e04913cbbac // indirect
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 // indirect
gopkg.in/cheggaaa/pb.v1 v1.0.28
Expand Down
9 changes: 9 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ github.com/Stebalien/go-bitfield v0.0.0-20180330043415-076a62f9ce6e/go.mod h1:3o
github.com/Stebalien/go-bitfield v0.0.1 h1:X3kbSSPUaJK60wV2hjOPZwmpljr6VGCqdq4cBLhbQBo=
github.com/Stebalien/go-bitfield v0.0.1/go.mod h1:GNjFpasyUVkHMsfEOk8EFLJ9syQ6SI+XWrX9Wf2XH0s=
github.com/aead/siphash v1.0.1/go.mod h1:Nywa3cDsYNNK3gaciGTWPwHt0wlpNV15vwmswBAUSII=
github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc h1:cAKDfWh5VpdgMhJosfJnn5/FoN2SRZ4p7fJNX58YPaU=
github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf h1:qet1QNfXsQxTZqLG4oE62mJzwPIB8+Tee4RNCL9ulrY=
github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
Expand Down Expand Up @@ -70,6 +72,8 @@ github.com/dgryski/go-farm v0.0.0-20190104051053-3adb47b1fb0f/go.mod h1:SqUrOPUn
github.com/dgryski/go-farm v0.0.0-20190423205320-6a90982ecee2 h1:tdlZCpZ/P9DhczCTSixgIKmwPv6+wP5DGjqLYw5SUiA=
github.com/dgryski/go-farm v0.0.0-20190423205320-6a90982ecee2/go.mod h1:SqUrOPUnsFjfmXRMNPybcSiG0BgUW2AuFH8PAnS2iTw=
github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no=
github.com/djdv/p9 v0.0.0-20190907154417-3fdf0632585f h1:CUjmKarvWU1sBD/hUbKDes68c9ApZcmHnYaxRL9wvgc=
github.com/djdv/p9 v0.0.0-20190907154417-3fdf0632585f/go.mod h1:58+5oyIFRxi9jt7uQrdqpmUryJDTsxY8N8GxhnNocLQ=
github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo=
github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
github.com/elgris/jsondiff v0.0.0-20160530203242-765b5c24c302 h1:QV0ZrfBLpFc2KDk+a4LJefDczXnonRwrYrQJY/9L4dA=
Expand Down Expand Up @@ -193,6 +197,8 @@ github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4=
github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ=
github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI=
github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
github.com/hugelgupf/socketpair v0.0.0-20190730060125-05d35a94e714 h1:/jC7qQFrv8CrSJVmaolDVOxTfS9kc36uB6H40kdbQq8=
github.com/hugelgupf/socketpair v0.0.0-20190730060125-05d35a94e714/go.mod h1:2Goc3h8EklBH5mspfHFxBnEoURQCGzQQH1ga9Myjvis=
github.com/huin/goupnp v0.0.0-20180415215157-1395d1447324/go.mod h1:MZ2ZmwcBpvOoJ22IJsc7va19ZwoheaBk43rKg12SKag=
github.com/huin/goupnp v1.0.0 h1:wg75sLpL6DZqwHQN6E1Cfk6mtfzS45z8OV+ic+DtHRo=
github.com/huin/goupnp v1.0.0/go.mod h1:n9v9KO1tAxYH82qOn+UTIFQDmx5n1Zxd/ClZDMX7Bnc=
Expand Down Expand Up @@ -861,6 +867,8 @@ golang.org/x/sys v0.0.0-20190526052359-791d8a0f4d09/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20190610200419-93c9922d18ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20190626221950-04f50cda93cb h1:fgwFCsaw9buMuxNd6+DQfAuSFqbNiQZpcgJQAgJsK6k=
golang.org/x/sys v0.0.0-20190626221950-04f50cda93cb/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20190903213830-1f305c863dab h1:2WmrFWBmUPXp+o/5X0nS66SLRS6DKwZlgFD76BKThvc=
golang.org/x/sys v0.0.0-20190903213830-1f305c863dab/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.0.0-20170915090833-1cbadb444a80/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.2 h1:tW2bmiBqwgJj/UpqtC8EpXEZVYOwU0yG4iWbprSVAcs=
Expand Down Expand Up @@ -891,6 +899,7 @@ google.golang.org/genproto v0.0.0-20180831171423-11092d34479b/go.mod h1:JiN7NxoA
google.golang.org/genproto v0.0.0-20190307195333-5fe7a883aa19/go.mod h1:VzzqZJRnGkLBvHegQrXjBqPurQTc5/KpmUdxsrq26oE=
google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
gopkg.in/airbrake/gobrake.v2 v2.0.9/go.mod h1:/h5ZAUhDkGaJfjzjKLSjv6zCL6O0LLBxU4K+aSYdM/U=
gopkg.in/alecthomas/kingpin.v2 v2.2.6 h1:jMFz6MfLP0/4fUyZle81rXUoxOBFi19VUFKVDOQfozc=
gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY=
Expand Down
12 changes: 12 additions & 0 deletions plugin/plugins/filesystem/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
Package filesystem is an experimental package that implements the go-ipfs daemon plugin interface
and defines the plugin's config structure. The plugin itself exposes file system services over a multiaddr listener.

By default, we try to expose the IPFS namespace using the 9P2000.L protocol, over a unix domain socket
(located at $IPFS_PATH/filesystem.9P.sock)

To set the multiaddr listen address, you may use the environment variable $IPFS_FS_ADDR, or set the option in the node's config file
via `ipfs config --json 'Plugins.Plugins.filesystem.Config "Config":{"Service":{"9P":"/ip4/127.0.0.1/tcp/564"}}'`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling this returns Error: failed to get config value: " key has no attributes" when using the default config.
Is there some flag we can pass to force creation of keys that don't exist? If not should we add one?
Otherwise, we need more instructing for the user. As it stands they'd need to manually add the Plugin section + our plugin's config within it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet. Really, we should change how we do config type checking (see https://github.com/ipfs/go-ipfs/blob/7e7b76259f655c5aa321e622a38619587a5a02a8/repo/fsrepo/fsrepo.go#L589). Basically, instead of manually type checking like that, we should just set the key and see if it serializes correctly.

The tricky part will be making sure the error doesn't suck.

To disable this plugin entirely, use: `ipfs config --json Plugins.Plugins.filesystem.Disabled true`
*/
package filesystem
138 changes: 138 additions & 0 deletions plugin/plugins/filesystem/filesystem.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
package filesystem

import (
"context"
"os"
"path/filepath"
"strings"

"github.com/djdv/p9/p9"
plugin "github.com/ipfs/go-ipfs/plugin"
fsnodes "github.com/ipfs/go-ipfs/plugin/plugins/filesystem/nodes"
nodeopts "github.com/ipfs/go-ipfs/plugin/plugins/filesystem/nodes/options"
logging "github.com/ipfs/go-log"
coreiface "github.com/ipfs/interface-go-ipfs-core"
"github.com/mitchellh/mapstructure"
"github.com/multiformats/go-multiaddr"
manet "github.com/multiformats/go-multiaddr-net"
)

var (
_ plugin.PluginDaemon = (*FileSystemPlugin)(nil) // impl check

// Plugins is an exported list of plugins that will be loaded by go-ipfs.
Plugins = []plugin.Plugin{
&FileSystemPlugin{}, //TODO: individually name implementations: &P9{}
}

logger logging.EventLogger
)

func init() {
logger = logging.Logger("plugin/filesystem")
}

type FileSystemPlugin struct {
ctx context.Context
cancel context.CancelFunc

addr multiaddr.Multiaddr
listener manet.Listener
errorChan chan error
}

func (*FileSystemPlugin) Name() string {
return PluginName
}

func (*FileSystemPlugin) Version() string {
return PluginVersion
}

func (fs *FileSystemPlugin) Init(env *plugin.Environment) error {
djdv marked this conversation as resolved.
Show resolved Hide resolved
logger.Info("Initializing 9P resource server...")

// stabilise repo path; our template depends on this
if !filepath.IsAbs(env.Repo) {
absRepo, err := filepath.Abs(env.Repo)
if err != nil {
return err
}
env.Repo = absRepo
}

cfg := &Config{}
// load config from file or initialize it
if env.Config != nil {
if err := mapstructure.Decode(env.Config, cfg); err != nil {
return err
}
} else {
cfg = defaultConfig()
}

var addrString string
// allow environment variable to override config values
if envAddr := os.ExpandEnv(EnvAddr); envAddr != "" {
addrString = EnvAddr
} else {
addrString = cfg.Service[defaultService]
}

// expand string templates and initialize listening addr
templateRepoPath := env.Repo
if strings.HasPrefix(addrString, "/unix") {
// prevent template from expanding to double slashed paths like `/unix//home/...`
// but allow it to expand to `/unix/C:\Users...`
templateRepoPath = strings.TrimPrefix(templateRepoPath, "/")
}

addrString = os.Expand(addrString, configVarMapper(templateRepoPath))

ma, err := multiaddr.NewMultiaddr(addrString)
if err != nil {
return err
}
fs.addr = ma

logger.Info("9P resource server okay for launch")
return nil
}

func (fs *FileSystemPlugin) Start(core coreiface.CoreAPI) error {
logger.Info("Starting 9P resource server...")

// make sure sockets are not in use already (if we're using them)
err := removeUnixSockets(fs.addr)
if err != nil {
return err
}

fs.ctx, fs.cancel = context.WithCancel(context.Background())
fs.errorChan = make(chan error, 1)

// launch the listener
listener, err := manet.Listen(fs.addr)
if err != nil {
logger.Errorf("9P listen error: %s\n", err)
return err
}
fs.listener = listener

// construct and launch the 9P resource server
opts := []nodeopts.AttachOption{nodeopts.Logger(logging.Logger("9root"))}
s := p9.NewServer(fsnodes.RootAttacher(fs.ctx, core, opts...))
go func() {
fs.errorChan <- s.Serve(manet.NetListener(fs.listener))
}()

logger.Infof("9P service is listening on %s\n", fs.listener.Addr())
return nil
}

func (fs *FileSystemPlugin) Close() error {
Stebalien marked this conversation as resolved.
Show resolved Hide resolved
logger.Info("9P server requested to close")
fs.listener.Close()
djdv marked this conversation as resolved.
Show resolved Hide resolved
fs.cancel()
return <-fs.errorChan
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this should probably be idempotent over multiple close calls (instead of just hanging forever). The usual way to do this is to set a variable and then close a "closed" chan. To read the error, wait for the closed chan to close then read the error from the variable (no lock necessary).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, I don't know how we're actually using this. If it's not idempotent, we should document that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right.
I made changes here:
8cee086#diff-8d243ab8514fa1b77d41df37a81641c9R44
8cee086#diff-8d243ab8514fa1b77d41df37a81641c9R137
8cee086#diff-8d243ab8514fa1b77d41df37a81641c9R166

However, I also had to add in a bool to signify close intent. I'm wondering if there's a better way to handle this 8cee086#diff-8d243ab8514fa1b77d41df37a81641c9R146 specifically.
We only want to drop the Accept error on Close.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually use the flag (bool) approach. However, it needs to be atomic. What if we:

  1. Canceled the context first.
  2. Then closed the listener.
  3. Then, when checking to see if we care about the error, we can ignore it if the context has been canceled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be better, thanks for that. c9faaa7

I also did this, but I'm not sure if it's an improvement.
Instead of assigning directly to the object and rubbing out the error conditionally, we assign it conditionally. Maybe a little more convoluted than the former.
c7cfd71

Not sure.

}
Loading