Skip to content

fuse/ipfs: seeking#694

Merged
jbenet merged 3 commits into
masterfrom
fuse-seek
Jan 30, 2015
Merged

fuse/ipfs: seeking#694
jbenet merged 3 commits into
masterfrom
fuse-seek

Conversation

@jbenet
Copy link
Copy Markdown
Member

@jbenet jbenet commented Jan 29, 2015

This commit changed the "ReadAll" to do proper read requests.
Seeking in fuse mounted fs now works. Note: this is why opening a
mounted video didnt work... we just didnt look at this code in
months.

@jbenet jbenet added the status/in-progress In progress label Jan 29, 2015
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hrm... I need to modify seek to perform No-op's if you seek to the files current offset.

@whyrusleeping
Copy link
Copy Markdown
Member

LGTM

@jbenet
Copy link
Copy Markdown
Member Author

jbenet commented Jan 30, 2015

@whyrusleeping can you check the ipns part?

This commit changed the "ReadAll" to do proper read requests.
Seeking in fuse mounted fs now works. Note: this is why opening a
mounted video didnt work... we just didnt look at this code in
months.
Comment thread fuse/ipns/ipns_unix.go
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I dont like that we have to do this... after the alpha im going to do a heavy refactor on the entire ipns subsystem and make it a lot better.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

that we have to do what?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

create a new dagReader on each read. Its not a light operation.

@whyrusleeping
Copy link
Copy Markdown
Member

The ipns stuff looks good because we disallow writing for now. Once writing is reenabled we will have to change some things.

in other words, LGTM

@jbenet
Copy link
Copy Markdown
Member Author

jbenet commented Jan 30, 2015

okay

jbenet added a commit that referenced this pull request Jan 30, 2015
@jbenet jbenet merged commit 33eb147 into master Jan 30, 2015
@jbenet jbenet removed the status/in-progress In progress label Jan 30, 2015
@jbenet jbenet deleted the fuse-seek branch January 30, 2015 00:22
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh crap, i missed this. We dont need to construct a new reader. The Node object has one in it already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants