fix: umount: delete the right metapath

[hallyn]

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.

[mikemccracken]

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]

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.

[hallyn]

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.

[hallyn]

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]

@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]

(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]

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

[hallyn]

Replaced by PR #44