-
Notifications
You must be signed in to change notification settings - Fork 122
Add notification messages framework #35
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -240,7 +240,19 @@ func (c *Connection) beginOp( | |||
| // should not record any state keyed on their ID. | ||||
| // | ||||
| // Cf. https://github.com/osxfuse/osxfuse/issues/208 | ||||
| if opCode != fusekernel.OpForget { | ||||
|
|
||||
| // Special case: For notify ops ,it is issued by the userspace filesystem to | ||||
| // fuse kernel. The ops's kernel opCode is 0 in kernel' viewpoint or kernel not | ||||
| // care whether what the iopcode is .But the ops have the notifycode from 1 | ||||
| // to 6 now. For adapt the notfiy ops to the BeginOP ,we fake an opcode as | ||||
| // notfiycode + 100. Now opcode range of ops from kernel including origin | ||||
| // from userspace and just from kernel is < 100. The notify ops's opcode | ||||
| // range is [101-106]. | ||||
| // Then we can process all ops consistently. | ||||
| // Cf. func (c *Connection) SetNotifyContext(op interface{}) | ||||
| // (context.Context, error) { | ||||
|
|
||||
| if opCode != fusekernel.OpForget && opCode < 100 { | ||||
| var cancel func() | ||||
| ctx, cancel = context.WithCancel(ctx) | ||||
| c.recordCancelFunc(fuseID, cancel) | ||||
|
|
@@ -410,6 +422,38 @@ func (c *Connection) ReadOp() (ctx context.Context, op interface{}, err error) { | |||
| } | ||||
| } | ||||
|
|
||||
| // The SetNotifyContext set context according with value of the notify op's | ||||
| // fuseops.Notify*Op. | ||||
| func (c *Connection) SetNotifyContext(op interface{}) (context.Context, error) { | ||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add documentation for all new exported symbols.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||||
|
|
||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove the blank line here. (Please run gofmt if you haven't.) |
||||
| outMsg := c.getOutMessage() | ||||
|
|
||||
| err := c.buildNotify(outMsg, op) | ||||
| if err != nil { | ||||
| return nil, err | ||||
| } | ||||
|
|
||||
| ctx := context.Background() | ||||
|
|
||||
| switch op.(type) { | ||||
| case *fuseops.NotifyInvalInodeOp: | ||||
| ctx = c.beginOp(100+uint32(fusekernel.NotifyCodeInvalInode), 0) | ||||
|
|
||||
| case *fuseops.NotifyInvalEntryOp: | ||||
| ctx = c.beginOp(100+uint32(fusekernel.NotifyCodeInvalEntry), 0) | ||||
|
|
||||
| case *fuseops.NotifyDeleteOp: | ||||
| ctx = c.beginOp(100+uint32(fusekernel.NotifyCodeDelete), 0) | ||||
|
|
||||
| default: | ||||
| panic(fmt.Sprintf("Unexpected op: %#v", op)) | ||||
| } | ||||
|
|
||||
| ctx = context.WithValue(ctx, contextKey, opState{nil, outMsg, op}) | ||||
| return ctx, nil | ||||
|
|
||||
| } | ||||
|
|
||||
| // Skip errors that happen as a matter of course, since they spook users. | ||||
| func (c *Connection) shouldLogError( | ||||
| op interface{}, | ||||
|
|
@@ -497,6 +541,29 @@ func (c *Connection) Reply(ctx context.Context, opErr error) { | |||
| } | ||||
| } | ||||
|
|
||||
| // The NotifyKernel is same as Reply func of Connection.But the diff is | ||||
| // that the func only send to kernel. | ||||
| func (c *Connection) NotifyKernel(ctx context.Context) { | ||||
|
|
||||
| // we should get outmsg from context | ||||
| var key interface{} = contextKey | ||||
| foo := ctx.Value(key) | ||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain why we need to pull this value out of the context instead of just passing it as a parameter to NotifyKernel?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just do like the reply func. Of course you can also just pass the value of the context as a parameter to NotifyKernel.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. The reply mechanism needs to use a context because it passes a value ( Line 406 in 659cc51
This is not the case here AIUI, so please change the function signature to not take a context, but just take the opState parameter directly.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I rethink this case, even the notify func is diff with the normal op which need reply function. But the context use in notify func is same as the reply func. Just the context is created by our func (s *fileSystemServer) Invalidateentry/inode. And now we call unity all notify op and the normal op. Maybe i am not understandering the issue on detail.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, you’re saying having both functions (NotifyKernel and Reply) take arguments via a context is good because they are similar, yes? While similarity is good, I would argue that in this case, it’s clearer if a context is not used unless absolutely necessary. Since we’re creating a context but then immediately call the function (instead of passing the context somewhere else), I don’t think we need to use a context. Contexts are used (aside from deadlines and cancellation) to propagate data across API boundaries. In this case, we don’t need to cross API boundaries, so we shouldn’t use a context if we can use a simple variable instead. |
||||
| state, ok := foo.(opState) | ||||
| if !ok { | ||||
| panic(fmt.Sprintf("Reply called with invalid context: %#v", ctx)) | ||||
| } | ||||
|
|
||||
| outMsg := state.outMsg | ||||
| defer c.putOutMessage(outMsg) | ||||
|
|
||||
| c.debugLogger.Println("dev fd is:unique:notifycode ", c.dev.Fd(), outMsg.OutHeader().Unique, outMsg.OutHeader().Error) | ||||
| err := c.writeMessage(outMsg.Bytes()) | ||||
| if err != nil && c.errorLogger != nil { | ||||
| c.errorLogger.Printf("writeMessage: %v %v", err, outMsg.Bytes()) | ||||
| } | ||||
|
|
||||
| } | ||||
|
|
||||
| // Close the connection. Must not be called until operations that were read | ||||
| // from the connection have been responded to. | ||||
| func (c *Connection) close() (err error) { | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -865,3 +865,62 @@ type SetXattrOp struct { | |
| // simply replace the value if the attribute exists. | ||
| Flags uint32 | ||
| } | ||
|
|
||
| // Notify IO readiness event | ||
| type NotifyPollOp struct { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's especially important to have good documentation for the new Op structs. See the other ops in this package for an example of the kind of thing I'm looking for.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| } | ||
|
|
||
| // Notify to invalidate cache for an inode. | ||
| // Added in FUSE protocol version 7.12. If the kernel does not support this | ||
| // (or a newer) version, the op will return -ENOSYS and do nothing | ||
| type NotifyInvalInodeOp struct { | ||
| // inode to invalidatej | ||
| Ino InodeID | ||
|
|
||
| // the offset in the inode where to start invalidating or negative to invalidate attributes only | ||
| Off int64 | ||
|
|
||
| // the amount of cache to invalidate or 0 for all | ||
| Len int64 | ||
| } | ||
|
|
||
| // Notify to invalidate parent attributes and the dentry matching parent/name | ||
| // Added in FUSE protocol version 7.12. If the kernel does not support this | ||
| // (or a newer) version, the op will return -ENOSYS and do nothing | ||
| type NotifyInvalEntryOp struct { | ||
| // the inode number | ||
| Parent InodeID | ||
|
|
||
| // the child entry file name | ||
| Name string | ||
| } | ||
|
|
||
| // Store data to the kernel buffers | ||
| // Cf:http://libfuse.github.io/doxygen/fuse__lowlevel_8h.html#a9cb974af9745294ff446d11cba2422f1 | ||
| type NotifyStoreOp struct { | ||
| } | ||
|
|
||
| // Retrieve data from the kernel buffers | ||
| type NotifyRetrieveOp struct { | ||
| } | ||
|
|
||
| // This op behaves like NotifyInvalEntryOp with the following additional | ||
| // effect (at least as of Linux kernel 4.8): | ||
|
|
||
| // If the provided child inode matches the inode that is currently | ||
| // associated with the cached dentry, and if there are any inotify | ||
| // watches registered for the dentry, then the watchers are informed | ||
| // that the dentry has been deleted. | ||
| // Added in FUSE protocol version 7.18. If the kernel does not | ||
| // support this (or a newer) version, op will return -ENOSYS and do nothing. | ||
|
|
||
| type NotifyDeleteOp struct { | ||
| // the inode number | ||
| Parent InodeID | ||
|
|
||
| // the child entry's inode | ||
| Child InodeID | ||
|
|
||
| // the child entry file name | ||
| Name string | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ import ( | |
| "context" | ||
| "fmt" | ||
| "os" | ||
| "github.com/jacobsa/fuse/fuseops" | ||
| ) | ||
|
|
||
| // Server is an interface for any type that knows how to serve ops read from a | ||
|
|
@@ -27,6 +28,11 @@ type Server interface { | |
| // until all operations have been responded to. Must not be called more than | ||
| // once. | ||
| ServeOps(*Connection) | ||
| InvalidateEntry(fuseops.InodeID, string) error | ||
| InvalidateInode(fuseops.InodeID, int64, int64) error | ||
| NotifyDelete(fuseops.InodeID, fuseops.InodeID, string) error | ||
| SetMfs(*MountedFileSystem) | ||
| GetMfs() *MountedFileSystem | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a lot of extra methods to be adding to the very simple Server interface, which could plausibly have been proxied in client code.. |
||
|
|
||
| // Mount attempts to mount a file system on the given directory, using the | ||
|
|
@@ -84,7 +90,10 @@ func Mount( | |
| return | ||
| } | ||
|
|
||
| // Serve the connection in the background. When done, set the join status. | ||
| mfs.Conn = connection | ||
| server.SetMfs(mfs) | ||
|
|
||
| // Serve the connection in the background. When done, set the join status | ||
| go func() { | ||
| server.ServeOps(connection) | ||
| mfs.joinStatus = connection.close() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For notify ops ,it is issued by the userspace filesystem to
fuse kernel. The ops's kernel opCode is 0 in kernel' viewpoint or kernel not
care whether what the opcode is .But the ops have the notifycode from 1
to 6 now. For adapt the notfiy ops to the BeginOP ,we fake an opcode as
notfiycode + 100. Now opcode range of ops from kernel including origin
from userspace and just from kernel is < 100. The notify ops's opcode
range is [101-106].
Then we can process all ops consistently.
Cf. func (c *Connection) SetNotifyContext(op interface{})
(context.Context, error) {
see pr code.