feat(cmds): files: add new-root command to change the MFS root #8648
Conversation
schomatis
force-pushed
the
schomatis/feat/files-cmd/mfs-new-root
branch
from
5aa084e
to
247816a
Compare
|
Hello, I don't have the necessary knowledge to analyse this PR but since I'm the author of #7183, my question is : will it be possible to use this command without the daemon running ? In my case the root MFS corresponding file was missing in the blocks folder at startup and it prevented the daemon to start. Hence the need to have a replace-root working without daemon. |
As written now, yes |
|
Correct, thanks. Right now the idea is for this command to be used when the MFS filesystem is corrupted and the daemon fails to start (either hangs or refuses) and you need to run this standalone to fix it. If you run it alongside the daemon it will fail as it will try to get a lock on the repo which is already in use. Will add some more documentation around that. |
| "rm": filesRmCmd, | ||
| "flush": filesFlushCmd, | ||
| "chcid": filesChcidCmd, | ||
| "replace-root": filesReplaceRoot, |
There was a problem hiding this comment.
💭 loose thoughts
- since we already use unix terms like
mkdirmvcprm, perhaps this one should be namedchroot? - another idea: perhaps we could add a second (optional) argument to already existing
chcid, which would allow in-place swapping of CIDs for any path (not just root)
There was a problem hiding this comment.
since we already use unix terms like mkdir mv cp rm, perhaps this one should be named chroot ?
The current name came from an informal Slack thread, but I'm fine with whatever is decided. My 2 cents on chroot is that it might imply some temporary change related to the running daemon/process when in fact we're doing a long-term modification of the single filesystem available (since we don't have any jail/sandbox abstractions).
another idea: perhaps we could add a second (optional) argument to already existing chcid, which would allow in-place swapping of CIDs for any path (not just root)
This sounds like it is beyond the scope of this issue which is fixing a corrupted filesystem preventing daemon startup (corruption in sub-paths wouldn't have the same effect). The idea is valid on itself though, maybe we should discuss it in another issue as a feature request.
|
@schomatis : what are the next steps here? |
|
@BigLep As noted in the OP: awaiting review. |
Perhaps @hsanjuan is busy, @schomatis perhaps you could review in lieu? |
|
No, I can't review my own PR. |
Sorry I meant @BigLep! |
|
@schomatis : ACK. I was confused because the PR was left as draft. I have marked it as ready for review. |
|
@BigLep Ok, I see the confusion. The 'Ready for review' button is very misleading. It should actually be 'Ready for merge'. The draft state is a way to signal a WIP PR to make sure you won't accidentally merge it as is. If the IPFS team interprets drafts differently please let me know and I'll adjust my signalling. |
| It is an advanced command that you normally do *not* want to run except when | ||
| the filesystem has been corrupted and the daemon refuses to run. | ||
|
|
||
| FIXME: Add an example of the daemon not running once https://github.com/ipfs/go-ipfs/issues/7183 |
| [nothing; empty dir] | ||
| $ ipfs files stat / --hash | ||
| QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn | ||
| # FIXME(BLOCKING): Need the following to somehow "start" the root dir, otherwise |
| Arguments: []cmds.Argument{ | ||
| cmds.StringArg("new-root", true, false, "New root to use."), | ||
| }, | ||
| // FIXME(BLOCKING): Can/should we do this with the repo running? |
| // FIXME(BLOCKING): Check (a) that this CID exists *locally* and (b) | ||
| // that it's a dir. |
| // FIXME(BLOCKING): Do we need this if we're closing the repo at the end | ||
| // of the command? Likely not. |
Some minor fixes pending but it should get a review at this point.
(Sharness test also pending.)
Connected to #7183.
Closes #8100.