fix: umount: delete the right metapath - #39

Closed
hallyn wants to merge 1 commit into
project-machine:mainfrom
hallyn:2025-04-22/umountfix
Closed

fix: umount: delete the right metapath#39
hallyn wants to merge 1 commit into
project-machine:mainfrom
hallyn:2025-04-22/umountfix

Conversation

@hallyn

@hallynhallyn commented Apr 23, 2025

Copy link
Copy Markdown
Contributor

We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath. So doing

atomfs mount oci:a dest
atomfs umount dest
atomfs mount oci:a dest

would fail as /run/atomfs/meta/ID/destpath still existed.

@hallyn
fix: umount: delete the right metapath
62af879
We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath.
So doing
atomfs mount oci:a dest
atomfs umount dest
atomfs mount oci:a dest
would fail as /run/atomfs/meta/ID/destpath still existed.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
@hallyn
hallynforce-pushed the 2025-04-22/umountfix branch from 143ecf8 to 62af879CompareApril 24, 2025 15:25
mikemccracken
mikemccracken requested changes Apr 24, 2025
View reviewed changes

@mikemccrackenmikemccracken left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't really understand this issue. Maybe common.ReplacePathSeparators() has an issue with the path you're sending? I'll try to repro with a test.

In any case I don't think this is the right fix. The /run/atomfs/meta/$nsid is shared by everything you atomfs mount within the same mountNS. This change is doing os.RemoveAll('/run/atomfs/') , which will remove stuff from other mounts too.

mikemccracken
mikemccracken reviewed Apr 24, 2025
View reviewed changes
Comment threadpkg/molecule/molecule.go
}
}

mountNSName, err := common.GetMountNSName()

@mikemccrackenmikemccrackenApr 24, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't see how GetMountNSName() could be returning a string with duplicate path info like you mention in the summary.

GetMountNSName() just looks at the /proc/self/ns/mnt link and strips the result, it should only return a namespace ID number (as a string), like 4026531841.

@hallynhallynApr 26, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

FWIW I just ran with a patch which prints out the values of mountNSName, common.ReplacePathSeparators(dest)), and common.RuntimeDir(metadir).

Please do that and let us know what you get. It's possible somethign in my context was weird, but what I did was: incus shell vm1; su - ubuntu; lxc-usernsexec -s -- bash; and inside that shell I did atomfs mount.

@mikemccrackenmikemccracken mentioned this pull request Apr 24, 2025
Draft
@hallyn

hallyn commented Jun 10, 2025

Copy link
Copy Markdown
ContributorAuthor

Hm, at the time I was 99% sure I was correct, and it did work on my
system, but since tests failed anyway, let me make a note to revisit
this :)

@hallyn

hallyn commented Jul 4, 2025

Copy link
Copy Markdown
ContributorAuthor

@mikemccracken

Ah, finally, I get it.

This patch is not quite right. There is a bug though. But your test in #41 manages to skirt around it!

The bug is in RuntimeDir((metadir string).

When the ATOMFS_TEST_RUN_DIR variable is set, then it does the right thing. It returns just that env variable, e.g. /tmp/meta. The bats tests always set that env variable, so the test passes.

But if ATOMFS_TEST_RUN_DIR is not set, and metadir is passed in (which unmount always does), then it returns the passed-in metadir. Unmount passes in the destination-specific metadir, e.g. /tmp/meta/meta/MOUNTNSNUM/destdir. Then, as my patch description pointed out, UmountWithMetadir appends meta/MOUNTNSNUM/destdir to that variable again, giving us /tmp/meta/meta/MOUNTNSNUM/destdir/meta/MOUNTNSNUM/destdir.

@hallyn

hallyn commented Jul 4, 2025

Copy link
Copy Markdown
ContributorAuthor

(if it helps, here is some debug output from manual run:)

root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/run/atomfs/meta/4026531841/tmp-zzz/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# export ATOMFS_TEST_RUN_DIR=/tmp/meta1
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
Error: "/tmp/meta1/meta/4026531841/tmp-zzz" exists: cowardly refusing to mess with it
couldn't mount molecule at mntpt "/tmp/zzz"
main.doMount
/home/ubuntu/atomfs/cmd/atomfs/mount.go:113
github.com/urfave/cli.HandleAction
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:524
github.com/urfave/cli.Command.Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/command.go:175
github.com/urfave/cli.(*App).Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:277
main.main
/home/ubuntu/atomfs/cmd/atomfs/main.go:38
runtime.main
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/proc.go:272
runtime.goexit
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/asm_amd64.s:1700
root@atomfs:/home/ubuntu/p1# rm -rf /tmp/meta1/meta/
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/tmp/meta1/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/tmp/meta1"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/tmp/meta1/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz

@mikemccracken

mikemccracken commented Jul 8, 2025

Copy link
Copy Markdown
Contributor

thanks for the extra detail - I will take a look soon, just wanted to ack that I saw this

@hallynhallyn closed this Jul 13, 2025
@hallyn

hallyn commented Jul 13, 2025

Copy link
Copy Markdown
ContributorAuthor

Replaced by PR #44

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

Reviewers

@mikemccrackenmikemccrackenmikemccracken requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hallyn@mikemccracken
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
fix: umount: delete the right metapath - #39
Closed
hallyn wants to merge 1 commit into
project-machine:mainfrom 
hallyn:2025-04-22/umountfix
Closed
fix: umount: delete the right metapath#39
hallyn wants to merge 1 commit into
project-machine:mainfrom 
hallyn:2025-04-22/umountfix
Conversation
@hallyn
@hallynhallyn
commented
Apr 23, 2025

Copy link


Copy Markdown

Contributor
We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath. So doing
atomfs mount oci:a dest

atomfs umount dest

atomfs mount oci:a dest
would fail as /run/atomfs/meta/ID/destpath still existed.
@hallyn
fix: umount: delete the right metapath
62af879
We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath.
So doing
atomfs mount oci:a dest
atomfs umount dest
atomfs mount oci:a dest
would fail as /run/atomfs/meta/ID/destpath still existed.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
@hallyn
hallynforce-pushed
the
2025-04-22/umountfix
branch
from
143ecf8 to
62af879CompareApril 24, 2025 15:25
mikemccracken
mikemccracken
requested changes
Apr 24, 2025
View reviewed changes
@mikemccrackenmikemccracken
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I don't really understand this issue. Maybe common.ReplacePathSeparators() has an issue with the path you're sending? I'll try to repro with a test.
In any case I don't think this is the right fix. The /run/atomfs/meta/$nsid is shared by everything you atomfs mount within the same mountNS. This change is doing os.RemoveAll('/run/atomfs/') , which will remove stuff from other mounts too.
mikemccracken
mikemccracken
reviewed
Apr 24, 2025
View reviewed changes
Comment threadpkg/molecule/molecule.go
 }
	}

	mountNSName, err := common.GetMountNSName()
@mikemccrackenmikemccrackenApr 24, 2025

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I don't see how GetMountNSName() could be returning a string with duplicate path info like you mention in the summary.
GetMountNSName() just looks at the /proc/self/ns/mnt link and strips the result, it should only return a namespace ID number (as a string), like 4026531841.
@hallynhallynApr 26, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

FWIW I just ran with a patch which prints out the values of mountNSName, common.ReplacePathSeparators(dest)), and common.RuntimeDir(metadir).
Please do that and let us know what you get. It's possible somethign in my context was weird, but what I did was: incus shell vm1; su - ubuntu; lxc-usernsexec -s -- bash; and inside that shell I did atomfs mount.
@mikemccrackenmikemccracken
mentioned this pull request
Apr 24, 2025
 Draft

@hallyn
hallyn
commented
Jun 10, 2025

Copy link


Copy Markdown

ContributorAuthor
Hm, at the time I was 99% sure I was correct, and it did work on my

system, but since tests failed anyway, let me make a note to revisit

this :)
@hallyn
hallyn
commented
Jul 4, 2025

Copy link


Copy Markdown

ContributorAuthor
@mikemccracken
Ah, finally, I get it.
This patch is not quite right. There is a bug though. But your test in #41 manages to skirt around it!
The bug is in RuntimeDir((metadir string).
When the ATOMFS_TEST_RUN_DIR variable is set, then it does the right thing. It returns just that env variable, e.g. /tmp/meta. The bats tests always set that env variable, so the test passes.
But if ATOMFS_TEST_RUN_DIR is not set, and metadir is passed in (which unmount always does), then it returns the passed-in metadir. Unmount passes in the destination-specific metadir, e.g. /tmp/meta/meta/MOUNTNSNUM/destdir. Then, as my patch description pointed out, UmountWithMetadir appends meta/MOUNTNSNUM/destdir to that variable again, giving us /tmp/meta/meta/MOUNTNSNUM/destdir/meta/MOUNTNSNUM/destdir.
@hallyn
hallyn
commented
Jul 4, 2025

Copy link


Copy Markdown

ContributorAuthor
(if it helps, here is some debug output from manual run:)
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/run/atomfs/meta/4026531841/tmp-zzz/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# export ATOMFS_TEST_RUN_DIR=/tmp/meta1
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
Error: "/tmp/meta1/meta/4026531841/tmp-zzz" exists: cowardly refusing to mess with it
couldn't mount molecule at mntpt "/tmp/zzz"
main.doMount
/home/ubuntu/atomfs/cmd/atomfs/mount.go:113
github.com/urfave/cli.HandleAction
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:524
github.com/urfave/cli.Command.Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/command.go:175
github.com/urfave/cli.(*App).Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:277
main.main
/home/ubuntu/atomfs/cmd/atomfs/main.go:38
runtime.main
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/proc.go:272
runtime.goexit
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/asm_amd64.s:1700
root@atomfs:/home/ubuntu/p1# rm -rf /tmp/meta1/meta/
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/tmp/meta1/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/tmp/meta1"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/tmp/meta1/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz

@mikemccracken
mikemccracken
commented
Jul 8, 2025

Copy link


Copy Markdown

Contributor
thanks for the extra detail - I will take a look soon, just wanted to ack that I saw this
@hallynhallyn
closed this
Jul 13, 2025
@hallyn
hallyn
commented
Jul 13, 2025

Copy link


Copy Markdown

ContributorAuthor
Replaced by PR #44
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@mikemccrackenmikemccrackenmikemccracken requested changes

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@hallyn@mikemccracken

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix: umount: delete the right metapath - #39
Closed
hallyn wants to merge 1 commit into
project-machine:mainfrom 
hallyn:2025-04-22/umountfix
Closed
fix: umount: delete the right metapath#39
hallyn wants to merge 1 commit into
project-machine:mainfrom 
hallyn:2025-04-22/umountfix
Conversation
@hallyn
@hallynhallyn
commented
Apr 23, 2025

Copy link


Copy Markdown

Contributor
We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath. So doing
atomfs mount oci:a dest

atomfs umount dest

atomfs mount oci:a dest
would fail as /run/atomfs/meta/ID/destpath still existed.
@hallyn
fix: umount: delete the right metapath
62af879
We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath.
So doing
atomfs mount oci:a dest
atomfs umount dest
atomfs mount oci:a dest
would fail as /run/atomfs/meta/ID/destpath still existed.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
@hallyn
hallynforce-pushed
the
2025-04-22/umountfix
branch
from
143ecf8 to
62af879CompareApril 24, 2025 15:25
mikemccracken
mikemccracken
requested changes
Apr 24, 2025
View reviewed changes
@mikemccrackenmikemccracken
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I don't really understand this issue. Maybe common.ReplacePathSeparators() has an issue with the path you're sending? I'll try to repro with a test.
In any case I don't think this is the right fix. The /run/atomfs/meta/$nsid is shared by everything you atomfs mount within the same mountNS. This change is doing os.RemoveAll('/run/atomfs/') , which will remove stuff from other mounts too.
mikemccracken
mikemccracken
reviewed
Apr 24, 2025
View reviewed changes
Comment threadpkg/molecule/molecule.go
 }
	}

	mountNSName, err := common.GetMountNSName()
@mikemccrackenmikemccrackenApr 24, 2025

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I don't see how GetMountNSName() could be returning a string with duplicate path info like you mention in the summary.
GetMountNSName() just looks at the /proc/self/ns/mnt link and strips the result, it should only return a namespace ID number (as a string), like 4026531841.
@hallynhallynApr 26, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

FWIW I just ran with a patch which prints out the values of mountNSName, common.ReplacePathSeparators(dest)), and common.RuntimeDir(metadir).
Please do that and let us know what you get. It's possible somethign in my context was weird, but what I did was: incus shell vm1; su - ubuntu; lxc-usernsexec -s -- bash; and inside that shell I did atomfs mount.
@mikemccrackenmikemccracken
mentioned this pull request
Apr 24, 2025
 Draft

@hallyn
hallyn
commented
Jun 10, 2025

Copy link


Copy Markdown

ContributorAuthor
Hm, at the time I was 99% sure I was correct, and it did work on my

system, but since tests failed anyway, let me make a note to revisit

this :)
@hallyn
hallyn
commented
Jul 4, 2025

Copy link


Copy Markdown

ContributorAuthor
@mikemccracken
Ah, finally, I get it.
This patch is not quite right. There is a bug though. But your test in #41 manages to skirt around it!
The bug is in RuntimeDir((metadir string).
When the ATOMFS_TEST_RUN_DIR variable is set, then it does the right thing. It returns just that env variable, e.g. /tmp/meta. The bats tests always set that env variable, so the test passes.
But if ATOMFS_TEST_RUN_DIR is not set, and metadir is passed in (which unmount always does), then it returns the passed-in metadir. Unmount passes in the destination-specific metadir, e.g. /tmp/meta/meta/MOUNTNSNUM/destdir. Then, as my patch description pointed out, UmountWithMetadir appends meta/MOUNTNSNUM/destdir to that variable again, giving us /tmp/meta/meta/MOUNTNSNUM/destdir/meta/MOUNTNSNUM/destdir.
@hallyn
hallyn
commented
Jul 4, 2025

Copy link


Copy Markdown

ContributorAuthor
(if it helps, here is some debug output from manual run:)
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/run/atomfs/meta/4026531841/tmp-zzz/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# export ATOMFS_TEST_RUN_DIR=/tmp/meta1
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
Error: "/tmp/meta1/meta/4026531841/tmp-zzz" exists: cowardly refusing to mess with it
couldn't mount molecule at mntpt "/tmp/zzz"
main.doMount
/home/ubuntu/atomfs/cmd/atomfs/mount.go:113
github.com/urfave/cli.HandleAction
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:524
github.com/urfave/cli.Command.Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/command.go:175
github.com/urfave/cli.(*App).Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:277
main.main
/home/ubuntu/atomfs/cmd/atomfs/main.go:38
runtime.main
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/proc.go:272
runtime.goexit
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/asm_amd64.s:1700
root@atomfs:/home/ubuntu/p1# rm -rf /tmp/meta1/meta/
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/tmp/meta1/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/tmp/meta1"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/tmp/meta1/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz

@mikemccracken
mikemccracken
commented
Jul 8, 2025

Copy link


Copy Markdown

Contributor
thanks for the extra detail - I will take a look soon, just wanted to ack that I saw this
@hallynhallyn
closed this
Jul 13, 2025
@hallyn
hallyn
commented
Jul 13, 2025

Copy link


Copy Markdown

ContributorAuthor
Replaced by PR #44
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@mikemccrackenmikemccrackenmikemccracken requested changes

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@hallyn@mikemccracken

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix: umount: delete the right metapath - #39
Closed
hallyn wants to merge 1 commit into
project-machine:mainfrom 
hallyn:2025-04-22/umountfix
Closed
fix: umount: delete the right metapath#39
hallyn wants to merge 1 commit into
project-machine:mainfrom 
hallyn:2025-04-22/umountfix
Conversation
@hallyn
@hallynhallyn
commented
Apr 23, 2025

Copy link


Copy Markdown

Contributor
We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath. So doing
atomfs mount oci:a dest

atomfs umount dest

atomfs mount oci:a dest
would fail as /run/atomfs/meta/ID/destpath still existed.
@hallyn
fix: umount: delete the right metapath
62af879
We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath.
So doing
atomfs mount oci:a dest
atomfs umount dest
atomfs mount oci:a dest
would fail as /run/atomfs/meta/ID/destpath still existed.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
@hallyn
hallynforce-pushed
the
2025-04-22/umountfix
branch
from
143ecf8 to
62af879CompareApril 24, 2025 15:25
mikemccracken
mikemccracken
requested changes
Apr 24, 2025
View reviewed changes
@mikemccrackenmikemccracken
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I don't really understand this issue. Maybe common.ReplacePathSeparators() has an issue with the path you're sending? I'll try to repro with a test.
In any case I don't think this is the right fix. The /run/atomfs/meta/$nsid is shared by everything you atomfs mount within the same mountNS. This change is doing os.RemoveAll('/run/atomfs/') , which will remove stuff from other mounts too.
mikemccracken
mikemccracken
reviewed
Apr 24, 2025
View reviewed changes
Comment threadpkg/molecule/molecule.go
 }
	}

	mountNSName, err := common.GetMountNSName()
@mikemccrackenmikemccrackenApr 24, 2025

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I don't see how GetMountNSName() could be returning a string with duplicate path info like you mention in the summary.
GetMountNSName() just looks at the /proc/self/ns/mnt link and strips the result, it should only return a namespace ID number (as a string), like 4026531841.
@hallynhallynApr 26, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

FWIW I just ran with a patch which prints out the values of mountNSName, common.ReplacePathSeparators(dest)), and common.RuntimeDir(metadir).
Please do that and let us know what you get. It's possible somethign in my context was weird, but what I did was: incus shell vm1; su - ubuntu; lxc-usernsexec -s -- bash; and inside that shell I did atomfs mount.
@mikemccrackenmikemccracken
mentioned this pull request
Apr 24, 2025
 Draft

@hallyn
hallyn
commented
Jun 10, 2025

Copy link


Copy Markdown

ContributorAuthor
Hm, at the time I was 99% sure I was correct, and it did work on my

system, but since tests failed anyway, let me make a note to revisit

this :)
@hallyn
hallyn
commented
Jul 4, 2025

Copy link


Copy Markdown

ContributorAuthor
@mikemccracken
Ah, finally, I get it.
This patch is not quite right. There is a bug though. But your test in #41 manages to skirt around it!
The bug is in RuntimeDir((metadir string).
When the ATOMFS_TEST_RUN_DIR variable is set, then it does the right thing. It returns just that env variable, e.g. /tmp/meta. The bats tests always set that env variable, so the test passes.
But if ATOMFS_TEST_RUN_DIR is not set, and metadir is passed in (which unmount always does), then it returns the passed-in metadir. Unmount passes in the destination-specific metadir, e.g. /tmp/meta/meta/MOUNTNSNUM/destdir. Then, as my patch description pointed out, UmountWithMetadir appends meta/MOUNTNSNUM/destdir to that variable again, giving us /tmp/meta/meta/MOUNTNSNUM/destdir/meta/MOUNTNSNUM/destdir.
@hallyn
hallyn
commented
Jul 4, 2025

Copy link


Copy Markdown

ContributorAuthor
(if it helps, here is some debug output from manual run:)
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/run/atomfs/meta/4026531841/tmp-zzz/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# export ATOMFS_TEST_RUN_DIR=/tmp/meta1
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
Error: "/tmp/meta1/meta/4026531841/tmp-zzz" exists: cowardly refusing to mess with it
couldn't mount molecule at mntpt "/tmp/zzz"
main.doMount
/home/ubuntu/atomfs/cmd/atomfs/mount.go:113
github.com/urfave/cli.HandleAction
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:524
github.com/urfave/cli.Command.Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/command.go:175
github.com/urfave/cli.(*App).Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:277
main.main
/home/ubuntu/atomfs/cmd/atomfs/main.go:38
runtime.main
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/proc.go:272
runtime.goexit
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/asm_amd64.s:1700
root@atomfs:/home/ubuntu/p1# rm -rf /tmp/meta1/meta/
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/tmp/meta1/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/tmp/meta1"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/tmp/meta1/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz

@mikemccracken
mikemccracken
commented
Jul 8, 2025

Copy link


Copy Markdown

Contributor
thanks for the extra detail - I will take a look soon, just wanted to ack that I saw this
@hallynhallyn
closed this
Jul 13, 2025
@hallyn
hallyn
commented
Jul 13, 2025

Copy link


Copy Markdown

ContributorAuthor
Replaced by PR #44
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@mikemccrackenmikemccrackenmikemccracken requested changes

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@hallyn@mikemccracken

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
fix: umount: delete the right metapath - #39
Closed
hallyn wants to merge 1 commit into
project-machine:mainfrom 
hallyn:2025-04-22/umountfix
Closed
fix: umount: delete the right metapath#39
hallyn wants to merge 1 commit into
project-machine:mainfrom 
hallyn:2025-04-22/umountfix
Conversation
@hallyn
@hallynhallyn
commented
Apr 23, 2025

Copy link


Copy Markdown

Contributor
We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath. So doing
atomfs mount oci:a dest

atomfs umount dest

atomfs mount oci:a dest
would fail as /run/atomfs/meta/ID/destpath still existed.
@hallyn
fix: umount: delete the right metapath
62af879
We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath.
So doing
atomfs mount oci:a dest
atomfs umount dest
atomfs mount oci:a dest
would fail as /run/atomfs/meta/ID/destpath still existed.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
@hallyn
hallynforce-pushed
the
2025-04-22/umountfix
branch
from
143ecf8 to
62af879CompareApril 24, 2025 15:25
mikemccracken
mikemccracken
requested changes
Apr 24, 2025
View reviewed changes
@mikemccrackenmikemccracken
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I don't really understand this issue. Maybe common.ReplacePathSeparators() has an issue with the path you're sending? I'll try to repro with a test.
In any case I don't think this is the right fix. The /run/atomfs/meta/$nsid is shared by everything you atomfs mount within the same mountNS. This change is doing os.RemoveAll('/run/atomfs/') , which will remove stuff from other mounts too.
mikemccracken
mikemccracken
reviewed
Apr 24, 2025
View reviewed changes
Comment threadpkg/molecule/molecule.go
 }
	}

	mountNSName, err := common.GetMountNSName()
@mikemccrackenmikemccrackenApr 24, 2025

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I don't see how GetMountNSName() could be returning a string with duplicate path info like you mention in the summary.
GetMountNSName() just looks at the /proc/self/ns/mnt link and strips the result, it should only return a namespace ID number (as a string), like 4026531841.
@hallynhallynApr 26, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

FWIW I just ran with a patch which prints out the values of mountNSName, common.ReplacePathSeparators(dest)), and common.RuntimeDir(metadir).
Please do that and let us know what you get. It's possible somethign in my context was weird, but what I did was: incus shell vm1; su - ubuntu; lxc-usernsexec -s -- bash; and inside that shell I did atomfs mount.
@mikemccrackenmikemccracken
mentioned this pull request
Apr 24, 2025
 Draft

@hallyn
hallyn
commented
Jun 10, 2025

Copy link


Copy Markdown

ContributorAuthor
Hm, at the time I was 99% sure I was correct, and it did work on my

system, but since tests failed anyway, let me make a note to revisit

this :)
@hallyn
hallyn
commented
Jul 4, 2025

Copy link


Copy Markdown

ContributorAuthor
@mikemccracken
Ah, finally, I get it.
This patch is not quite right. There is a bug though. But your test in #41 manages to skirt around it!
The bug is in RuntimeDir((metadir string).
When the ATOMFS_TEST_RUN_DIR variable is set, then it does the right thing. It returns just that env variable, e.g. /tmp/meta. The bats tests always set that env variable, so the test passes.
But if ATOMFS_TEST_RUN_DIR is not set, and metadir is passed in (which unmount always does), then it returns the passed-in metadir. Unmount passes in the destination-specific metadir, e.g. /tmp/meta/meta/MOUNTNSNUM/destdir. Then, as my patch description pointed out, UmountWithMetadir appends meta/MOUNTNSNUM/destdir to that variable again, giving us /tmp/meta/meta/MOUNTNSNUM/destdir/meta/MOUNTNSNUM/destdir.
@hallyn
hallyn
commented
Jul 4, 2025

Copy link


Copy Markdown

ContributorAuthor
(if it helps, here is some debug output from manual run:)
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/run/atomfs/meta/4026531841/tmp-zzz/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# export ATOMFS_TEST_RUN_DIR=/tmp/meta1
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
Error: "/tmp/meta1/meta/4026531841/tmp-zzz" exists: cowardly refusing to mess with it
couldn't mount molecule at mntpt "/tmp/zzz"
main.doMount
/home/ubuntu/atomfs/cmd/atomfs/mount.go:113
github.com/urfave/cli.HandleAction
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:524
github.com/urfave/cli.Command.Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/command.go:175
github.com/urfave/cli.(*App).Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:277
main.main
/home/ubuntu/atomfs/cmd/atomfs/main.go:38
runtime.main
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/proc.go:272
runtime.goexit
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/asm_amd64.s:1700
root@atomfs:/home/ubuntu/p1# rm -rf /tmp/meta1/meta/
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/tmp/meta1/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/tmp/meta1"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/tmp/meta1/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz

@mikemccracken
mikemccracken
commented
Jul 8, 2025

Copy link


Copy Markdown

Contributor
thanks for the extra detail - I will take a look soon, just wanted to ack that I saw this
@hallynhallyn
closed this
Jul 13, 2025
@hallyn
hallyn
commented
Jul 13, 2025

Copy link


Copy Markdown

ContributorAuthor
Replaced by PR #44
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@mikemccrackenmikemccrackenmikemccracken requested changes

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@hallyn@mikemccracken

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix: umount: delete the right metapath - #39
Closed
hallyn wants to merge 1 commit into
project-machine:mainfrom 
hallyn:2025-04-22/umountfix
Closed
fix: umount: delete the right metapath#39
hallyn wants to merge 1 commit into
project-machine:mainfrom 
hallyn:2025-04-22/umountfix
Conversation
@hallyn
@hallynhallyn
commented
Apr 23, 2025

Copy link


Copy Markdown

Contributor
We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath. So doing
atomfs mount oci:a dest

atomfs umount dest

atomfs mount oci:a dest
would fail as /run/atomfs/meta/ID/destpath still existed.
@hallyn
fix: umount: delete the right metapath
62af879
We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath.
So doing
atomfs mount oci:a dest
atomfs umount dest
atomfs mount oci:a dest
would fail as /run/atomfs/meta/ID/destpath still existed.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
@hallyn
hallynforce-pushed
the
2025-04-22/umountfix
branch
from
143ecf8 to
62af879CompareApril 24, 2025 15:25
mikemccracken
mikemccracken
requested changes
Apr 24, 2025
View reviewed changes
@mikemccrackenmikemccracken
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I don't really understand this issue. Maybe common.ReplacePathSeparators() has an issue with the path you're sending? I'll try to repro with a test.
In any case I don't think this is the right fix. The /run/atomfs/meta/$nsid is shared by everything you atomfs mount within the same mountNS. This change is doing os.RemoveAll('/run/atomfs/') , which will remove stuff from other mounts too.
mikemccracken
mikemccracken
reviewed
Apr 24, 2025
View reviewed changes
Comment threadpkg/molecule/molecule.go
 }
	}

	mountNSName, err := common.GetMountNSName()
@mikemccrackenmikemccrackenApr 24, 2025

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I don't see how GetMountNSName() could be returning a string with duplicate path info like you mention in the summary.
GetMountNSName() just looks at the /proc/self/ns/mnt link and strips the result, it should only return a namespace ID number (as a string), like 4026531841.
@hallynhallynApr 26, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

FWIW I just ran with a patch which prints out the values of mountNSName, common.ReplacePathSeparators(dest)), and common.RuntimeDir(metadir).
Please do that and let us know what you get. It's possible somethign in my context was weird, but what I did was: incus shell vm1; su - ubuntu; lxc-usernsexec -s -- bash; and inside that shell I did atomfs mount.
@mikemccrackenmikemccracken
mentioned this pull request
Apr 24, 2025
 Draft

@hallyn
hallyn
commented
Jun 10, 2025

Copy link


Copy Markdown

ContributorAuthor
Hm, at the time I was 99% sure I was correct, and it did work on my

system, but since tests failed anyway, let me make a note to revisit

this :)
@hallyn
hallyn
commented
Jul 4, 2025

Copy link


Copy Markdown

ContributorAuthor
@mikemccracken
Ah, finally, I get it.
This patch is not quite right. There is a bug though. But your test in #41 manages to skirt around it!
The bug is in RuntimeDir((metadir string).
When the ATOMFS_TEST_RUN_DIR variable is set, then it does the right thing. It returns just that env variable, e.g. /tmp/meta. The bats tests always set that env variable, so the test passes.
But if ATOMFS_TEST_RUN_DIR is not set, and metadir is passed in (which unmount always does), then it returns the passed-in metadir. Unmount passes in the destination-specific metadir, e.g. /tmp/meta/meta/MOUNTNSNUM/destdir. Then, as my patch description pointed out, UmountWithMetadir appends meta/MOUNTNSNUM/destdir to that variable again, giving us /tmp/meta/meta/MOUNTNSNUM/destdir/meta/MOUNTNSNUM/destdir.
@hallyn
hallyn
commented
Jul 4, 2025

Copy link


Copy Markdown

ContributorAuthor
(if it helps, here is some debug output from manual run:)
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/run/atomfs/meta/4026531841/tmp-zzz/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# export ATOMFS_TEST_RUN_DIR=/tmp/meta1
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
Error: "/tmp/meta1/meta/4026531841/tmp-zzz" exists: cowardly refusing to mess with it
couldn't mount molecule at mntpt "/tmp/zzz"
main.doMount
/home/ubuntu/atomfs/cmd/atomfs/mount.go:113
github.com/urfave/cli.HandleAction
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:524
github.com/urfave/cli.Command.Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/command.go:175
github.com/urfave/cli.(*App).Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:277
main.main
/home/ubuntu/atomfs/cmd/atomfs/main.go:38
runtime.main
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/proc.go:272
runtime.goexit
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/asm_amd64.s:1700
root@atomfs:/home/ubuntu/p1# rm -rf /tmp/meta1/meta/
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/tmp/meta1/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/tmp/meta1"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/tmp/meta1/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz

@mikemccracken
mikemccracken
commented
Jul 8, 2025

Copy link


Copy Markdown

Contributor
thanks for the extra detail - I will take a look soon, just wanted to ack that I saw this
@hallynhallyn
closed this
Jul 13, 2025
@hallyn
hallyn
commented
Jul 13, 2025

Copy link


Copy Markdown

ContributorAuthor
Replaced by PR #44
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@mikemccrackenmikemccrackenmikemccracken requested changes

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@hallyn@mikemccracken

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix: umount: delete the right metapath - #39
Closed
hallyn wants to merge 1 commit into
project-machine:mainfrom 
hallyn:2025-04-22/umountfix
Closed
fix: umount: delete the right metapath#39
hallyn wants to merge 1 commit into
project-machine:mainfrom 
hallyn:2025-04-22/umountfix
Conversation
@hallyn
@hallynhallyn
commented
Apr 23, 2025

Copy link


Copy Markdown

Contributor
We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath. So doing
atomfs mount oci:a dest

atomfs umount dest

atomfs mount oci:a dest
would fail as /run/atomfs/meta/ID/destpath still existed.
@hallyn
fix: umount: delete the right metapath
62af879
We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath.
So doing
atomfs mount oci:a dest
atomfs umount dest
atomfs mount oci:a dest
would fail as /run/atomfs/meta/ID/destpath still existed.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
@hallyn
hallynforce-pushed
the
2025-04-22/umountfix
branch
from
143ecf8 to
62af879CompareApril 24, 2025 15:25
mikemccracken
mikemccracken
requested changes
Apr 24, 2025
View reviewed changes
@mikemccrackenmikemccracken
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I don't really understand this issue. Maybe common.ReplacePathSeparators() has an issue with the path you're sending? I'll try to repro with a test.
In any case I don't think this is the right fix. The /run/atomfs/meta/$nsid is shared by everything you atomfs mount within the same mountNS. This change is doing os.RemoveAll('/run/atomfs/') , which will remove stuff from other mounts too.
mikemccracken
mikemccracken
reviewed
Apr 24, 2025
View reviewed changes
Comment threadpkg/molecule/molecule.go
 }
	}

	mountNSName, err := common.GetMountNSName()
@mikemccrackenmikemccrackenApr 24, 2025

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I don't see how GetMountNSName() could be returning a string with duplicate path info like you mention in the summary.
GetMountNSName() just looks at the /proc/self/ns/mnt link and strips the result, it should only return a namespace ID number (as a string), like 4026531841.
@hallynhallynApr 26, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

FWIW I just ran with a patch which prints out the values of mountNSName, common.ReplacePathSeparators(dest)), and common.RuntimeDir(metadir).
Please do that and let us know what you get. It's possible somethign in my context was weird, but what I did was: incus shell vm1; su - ubuntu; lxc-usernsexec -s -- bash; and inside that shell I did atomfs mount.
@mikemccrackenmikemccracken
mentioned this pull request
Apr 24, 2025
 Draft

@hallyn
hallyn
commented
Jun 10, 2025

Copy link


Copy Markdown

ContributorAuthor
Hm, at the time I was 99% sure I was correct, and it did work on my

system, but since tests failed anyway, let me make a note to revisit

this :)
@hallyn
hallyn
commented
Jul 4, 2025

Copy link


Copy Markdown

ContributorAuthor
@mikemccracken
Ah, finally, I get it.
This patch is not quite right. There is a bug though. But your test in #41 manages to skirt around it!
The bug is in RuntimeDir((metadir string).
When the ATOMFS_TEST_RUN_DIR variable is set, then it does the right thing. It returns just that env variable, e.g. /tmp/meta. The bats tests always set that env variable, so the test passes.
But if ATOMFS_TEST_RUN_DIR is not set, and metadir is passed in (which unmount always does), then it returns the passed-in metadir. Unmount passes in the destination-specific metadir, e.g. /tmp/meta/meta/MOUNTNSNUM/destdir. Then, as my patch description pointed out, UmountWithMetadir appends meta/MOUNTNSNUM/destdir to that variable again, giving us /tmp/meta/meta/MOUNTNSNUM/destdir/meta/MOUNTNSNUM/destdir.
@hallyn
hallyn
commented
Jul 4, 2025

Copy link


Copy Markdown

ContributorAuthor
(if it helps, here is some debug output from manual run:)
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/run/atomfs/meta/4026531841/tmp-zzz/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# export ATOMFS_TEST_RUN_DIR=/tmp/meta1
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
Error: "/tmp/meta1/meta/4026531841/tmp-zzz" exists: cowardly refusing to mess with it
couldn't mount molecule at mntpt "/tmp/zzz"
main.doMount
/home/ubuntu/atomfs/cmd/atomfs/mount.go:113
github.com/urfave/cli.HandleAction
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:524
github.com/urfave/cli.Command.Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/command.go:175
github.com/urfave/cli.(*App).Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:277
main.main
/home/ubuntu/atomfs/cmd/atomfs/main.go:38
runtime.main
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/proc.go:272
runtime.goexit
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/asm_amd64.s:1700
root@atomfs:/home/ubuntu/p1# rm -rf /tmp/meta1/meta/
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/tmp/meta1/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/tmp/meta1"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/tmp/meta1/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz

@mikemccracken
mikemccracken
commented
Jul 8, 2025

Copy link


Copy Markdown

Contributor
thanks for the extra detail - I will take a look soon, just wanted to ack that I saw this
@hallynhallyn
closed this
Jul 13, 2025
@hallyn
hallyn
commented
Jul 13, 2025

Copy link


Copy Markdown

ContributorAuthor
Replaced by PR #44
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@mikemccrackenmikemccrackenmikemccracken requested changes

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@hallyn@mikemccracken

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
fix: umount: delete the right metapath - #39
Closed
hallyn wants to merge 1 commit into
project-machine:mainfrom 
hallyn:2025-04-22/umountfix
Closed
fix: umount: delete the right metapath#39
hallyn wants to merge 1 commit into
project-machine:mainfrom 
hallyn:2025-04-22/umountfix
Conversation
@hallyn
@hallynhallyn
commented
Apr 23, 2025

Copy link


Copy Markdown

Contributor
We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath. So doing
atomfs mount oci:a dest

atomfs umount dest

atomfs mount oci:a dest
would fail as /run/atomfs/meta/ID/destpath still existed.
@hallyn
fix: umount: delete the right metapath
62af879
We were deleting /run/atomfs/meta/ID/destpath/meta/ID/destpath.
So doing
atomfs mount oci:a dest
atomfs umount dest
atomfs mount oci:a dest
would fail as /run/atomfs/meta/ID/destpath still existed.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
@hallyn
hallynforce-pushed
the
2025-04-22/umountfix
branch
from
143ecf8 to
62af879CompareApril 24, 2025 15:25
mikemccracken
mikemccracken
requested changes
Apr 24, 2025
View reviewed changes
@mikemccrackenmikemccracken
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I don't really understand this issue. Maybe common.ReplacePathSeparators() has an issue with the path you're sending? I'll try to repro with a test.
In any case I don't think this is the right fix. The /run/atomfs/meta/$nsid is shared by everything you atomfs mount within the same mountNS. This change is doing os.RemoveAll('/run/atomfs/') , which will remove stuff from other mounts too.
mikemccracken
mikemccracken
reviewed
Apr 24, 2025
View reviewed changes
Comment threadpkg/molecule/molecule.go
 }
	}

	mountNSName, err := common.GetMountNSName()
@mikemccrackenmikemccrackenApr 24, 2025

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I don't see how GetMountNSName() could be returning a string with duplicate path info like you mention in the summary.
GetMountNSName() just looks at the /proc/self/ns/mnt link and strips the result, it should only return a namespace ID number (as a string), like 4026531841.
@hallynhallynApr 26, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

FWIW I just ran with a patch which prints out the values of mountNSName, common.ReplacePathSeparators(dest)), and common.RuntimeDir(metadir).
Please do that and let us know what you get. It's possible somethign in my context was weird, but what I did was: incus shell vm1; su - ubuntu; lxc-usernsexec -s -- bash; and inside that shell I did atomfs mount.
@mikemccrackenmikemccracken
mentioned this pull request
Apr 24, 2025
 Draft

@hallyn
hallyn
commented
Jun 10, 2025

Copy link


Copy Markdown

ContributorAuthor
Hm, at the time I was 99% sure I was correct, and it did work on my

system, but since tests failed anyway, let me make a note to revisit

this :)
@hallyn
hallyn
commented
Jul 4, 2025

Copy link


Copy Markdown

ContributorAuthor
@mikemccracken
Ah, finally, I get it.
This patch is not quite right. There is a bug though. But your test in #41 manages to skirt around it!
The bug is in RuntimeDir((metadir string).
When the ATOMFS_TEST_RUN_DIR variable is set, then it does the right thing. It returns just that env variable, e.g. /tmp/meta. The bats tests always set that env variable, so the test passes.
But if ATOMFS_TEST_RUN_DIR is not set, and metadir is passed in (which unmount always does), then it returns the passed-in metadir. Unmount passes in the destination-specific metadir, e.g. /tmp/meta/meta/MOUNTNSNUM/destdir. Then, as my patch description pointed out, UmountWithMetadir appends meta/MOUNTNSNUM/destdir to that variable again, giving us /tmp/meta/meta/MOUNTNSNUM/destdir/meta/MOUNTNSNUM/destdir.
@hallyn
hallyn
commented
Jul 4, 2025

Copy link


Copy Markdown

ContributorAuthor
(if it helps, here is some debug output from manual run:)
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/run/atomfs/meta/4026531841/tmp-zzz"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/run/atomfs/meta/4026531841/tmp-zzz/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# export ATOMFS_TEST_RUN_DIR=/tmp/meta1
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
Error: "/tmp/meta1/meta/4026531841/tmp-zzz" exists: cowardly refusing to mess with it
couldn't mount molecule at mntpt "/tmp/zzz"
main.doMount
/home/ubuntu/atomfs/cmd/atomfs/mount.go:113
github.com/urfave/cli.HandleAction
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:524
github.com/urfave/cli.Command.Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/command.go:175
github.com/urfave/cli.(*App).Run
/home/ubuntu/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:277
main.main
/home/ubuntu/atomfs/cmd/atomfs/main.go:38
runtime.main
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/proc.go:272
runtime.goexit
/home/ubuntu/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.linux-amd64/src/runtime/asm_amd64.s:1700
root@atomfs:/home/ubuntu/p1# rm -rf /tmp/meta1/meta/
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs umount /tmp/zzz
XXX: metadir is "/tmp/meta1/meta/4026531841/tmp-zzz"
XXX: processed metadir is "/tmp/meta1"
XXX: mountNSName is "4026531841"
XXX: rest is "tmp-zzz"
XXX: removing "/tmp/meta1/meta/4026531841/tmp-zzz"
root@atomfs:/home/ubuntu/p1# ../atomfs/bin/atomfs mount oci:m-squashfs /tmp/zzz

@mikemccracken
mikemccracken
commented
Jul 8, 2025

Copy link


Copy Markdown

Contributor
thanks for the extra detail - I will take a look soon, just wanted to ack that I saw this
@hallynhallyn
closed this
Jul 13, 2025
@hallyn
hallyn
commented
Jul 13, 2025

Copy link


Copy Markdown

ContributorAuthor
Replaced by PR #44
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@mikemccrackenmikemccrackenmikemccracken requested changes

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@hallyn@mikemccracken