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

Getting rwalk did not find file for any Twalk call #31

Open
zakkor opened this issue Sep 10, 2022 · 3 comments · May be fixed by #34
Open

Getting rwalk did not find file for any Twalk call #31

zakkor opened this issue Sep 10, 2022 · 3 comments · May be fixed by #34

Comments

@zakkor
Copy link
Contributor

zakkor commented Sep 10, 2022

Using the provided example noop FS:

package main

import (
	"io"
	"log"
	"os"
	"time"

	"aqwari.net/net/styx"
)

type emptyDir struct{}

func (emptyDir) Readdir(n int) ([]os.FileInfo, error) { return nil, io.EOF }
func (emptyDir) Mode() os.FileMode                    { return os.ModeDir | 0777 }
func (emptyDir) IsDir() bool                          { return true }
func (emptyDir) ModTime() time.Time                   { return time.Now() }
func (emptyDir) Name() string                         { return "" }
func (emptyDir) Size() int64                          { return 0 }
func (emptyDir) Sys() interface{}                     { return nil }

func main() {
	// Run a file server that creates directories (and only directories)
	// on-demand, as a client walks to them.
	h := styx.HandlerFunc(func(s *styx.Session) {
		for s.Next() {
			switch t := s.Request().(type) {
			case styx.Tstat:
				t.Rstat(emptyDir{}, nil)
			case styx.Twalk:
				t.Rwalk(emptyDir{}, nil)
			case styx.Topen:
				t.Ropen(emptyDir{}, nil)
			}
		}
	})
	log.Fatal(styx.ListenAndServe(":564", h))
}

Running the following client command:

$ 9p -a localhost:564 stat a
9p: dirstat: rwalk did not find file

Perhaps my understanding of how this package works is incorrect, but I expected any sort of stat call to successfully return an empty dir.

I am guessing styx wants (either intentionally, or by accident) the file to be created first using Tcreate? But what about the scenario where you are starting up a server which already has some files pre-populated inside it?

@zakkor
Copy link
Contributor Author

zakkor commented Sep 10, 2022

The same error manifests itself in jsonfs as well:

$ ./jsonfs -a localhost:5640 example.json
$ 9p -a localhost:5640 ls /data
dirstat /data: rwalk did not find file

It seems like this is an issue that has been introduced in #30 - reverting part of those changes restores the expected behaviour.

With those PR changes reverted:

$ 9p -a localhost:5640 ls /data
items
itemsPerPage
startIndex
totalItems
updated

@droyo
Copy link
Owner

droyo commented Oct 11, 2022

This is a bit tricky. I feel that the change in #30 is more "correct", in the sense that the styx package should not be implicitly allocating Qids for successful walks. I would argue that the error here is in jsonfs, and the noop example above; they do not populate qids for existing files.

At the same time, that is not their fault, as the styx package provides no public API to directly allocate a qid for an existing file that was not created by Tcreate, other than Rwalk before PR #30. I think when I wrote this package I just brushed Qids aside as another identifier that only needs to stay unique for the lifetime of the server process.

I am open to suggestions here. Should we add a new method to be used when "loading" an existing filesystem? Restore (and document) the implicit behavior? What does plan 9 do?

@zakkor
Copy link
Contributor Author

zakkor commented Oct 11, 2022

It's tricky indeed - with styx being a more high-level package, I thought it was an intentional design that Rwalk automatically creates Qids when needed, which results in client code pretty much never needing to know anything about Qids or to manage them at all. I think this is a pretty cool feature.

I'm not an expert, but while it's true that Rwalk isn't supposed to create Qids, it seems like styx's T-messages and R-methods aren't really 9p messages, they're sort of a higher level, slightly more abstract concept, and they do a bit extra.

There could be a separate public API introduced for defining Qids for preexisting files, but I'm not sure what benefit that would give users of this package. You'd be able to give files any Qid you want, but would you actually want to set other Qids compared to what Rwalk is automatically setting? I know I wouldn't, but then again I am not the most knowledgeable on how 9p works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants