moby / sys

Apache License 2.0
73 stars 43 forks source link

mount: can't remount,rw #178

Open Darkness4 opened 3 weeks ago

Darkness4 commented 3 weeks ago

Hello, I've migrated from docker/docker/pkg/mount to moby/sys/mount, and I've run in a small issue.

I'm running this test:

package main

import (
    "os"

    "github.com/moby/sys/mount"
)

func main() {
    _ = os.Mkdir("/tmp/test", 0755)
    err := mount.Mount("tmpfs", "/tmp/test", "tmpfs", "ro")
    if err != nil {
        panic(err)
    }

    err = mount.Mount("", "/tmp/test", "none", "remount,rw")
    if err != nil {
        panic(err)
    }
}

And "remount,rw" doesn't get executed. Not sure if this is expected behavior.

It doesn't enter any case in this function: https://github.com/moby/sys/blob/638aa7cf269163105f1db1fc649b3c57acde4ba6/mount/mounter_linux.go#L30-L72

kolyshkin commented 2 days ago

Hello, I've migrated from docker/docker/pkg/mount to moby/sys/mount, and I've run in a small issue.

This implied you've found a regression (IOW your code worked correctly when the docker/docker/pkg/mount was used. It looks like it's not -- I tested with github.com/moby/moby v17.12.0-ce-rc1.0.20190717161051-705d9623b7c1+incompatible and it gives the same result ("remount,rw" is ignored).

Yet it seems the bug is valid, it's just not a regression. Looking...

kolyshkin commented 2 days ago

My conclusion is, this package did not envision such a use case, but hopefully it can be added without breaking anything else.

Can you give us some more details about your use case?

In the meantime, a workaround for your use case is to specify some text-only option, such as size=....

Darkness4 commented 12 hours ago

Hello, I maintaining a fork of k3os. I'm not the original maintainer so I don't really know the original motivation of using the docker/docker/pkg/mount package, but they used the package to remount /k3os/system with rw during upgrades, which is immutable by default (mounted ro).

As proper "workaround", I simply used the syscall package to remount: https://github.com/Darkness4/k3os/commit/2214405f67679d57d47bc320908718ea2ff077e8#diff-05da710ec6495dd56e5e44533a90c845f5fe090ad706dea8b496b8b9e6dc084dL62