From b31456f2cbf07cd840425d22285d1c04bcc58b81 Mon Sep 17 00:00:00 2001 From: Justin Cormack Date: Wed, 12 Jul 2017 15:17:02 +0100 Subject: [PATCH] In the case of remounting with changed data, need to call mount The case where we are trying to do a remount with changed filesystem specific options was missing, we need to call `mount` as well here to change those options. See #33844 for where we need this, as we change `tmpfs` options. Signed-off-by: Justin Cormack (cherry picked from commit 3a1ab5b479ce843648cf676fbaaf2bec9e040dce) Signed-off-by: Ying --- components/engine/pkg/mount/mounter_linux.go | 5 +- .../engine/pkg/mount/mounter_linux_test.go | 71 ++++++++++++++----- 2 files changed, 55 insertions(+), 21 deletions(-) diff --git a/components/engine/pkg/mount/mounter_linux.go b/components/engine/pkg/mount/mounter_linux.go index 3ef2ce6f0dd..2e0570be03a 100644 --- a/components/engine/pkg/mount/mounter_linux.go +++ b/components/engine/pkg/mount/mounter_linux.go @@ -29,8 +29,9 @@ func isremount(device string, flags uintptr) bool { func mount(device, target, mType string, flags uintptr, data string) error { oflags := flags &^ ptypes - if !isremount(device, flags) { - // Initial call applying all non-propagation flags. + if !isremount(device, flags) || data != "" { + // Initial call applying all non-propagation flags for mount + // or remount with changed data if err := syscall.Mount(device, target, mType, oflags, data); err != nil { return err } diff --git a/components/engine/pkg/mount/mounter_linux_test.go b/components/engine/pkg/mount/mounter_linux_test.go index 9c74b1f51db..47c03b36314 100644 --- a/components/engine/pkg/mount/mounter_linux_test.go +++ b/components/engine/pkg/mount/mounter_linux_test.go @@ -26,7 +26,7 @@ func TestMount(t *testing.T) { t.Fatal(err) } defer ensureUnmount(t, source) - validateMount(t, source, "", "") + validateMount(t, source, "", "", "") if t.Failed() { t.FailNow() } @@ -43,27 +43,31 @@ func TestMount(t *testing.T) { options string expectedOpts string expectedOptional string + expectedVFS string }{ // No options - {"tmpfs", "tmpfs", "", "", ""}, + {"tmpfs", "tmpfs", "", "", "", ""}, // Default rw / ro test - {source, "", "bind", "", ""}, - {source, "", "bind,private", "", ""}, - {source, "", "bind,shared", "", "shared"}, - {source, "", "bind,slave", "", "master"}, - {source, "", "bind,unbindable", "", "unbindable"}, + {source, "", "bind", "", "", ""}, + {source, "", "bind,private", "", "", ""}, + {source, "", "bind,shared", "", "shared", ""}, + {source, "", "bind,slave", "", "master", ""}, + {source, "", "bind,unbindable", "", "unbindable", ""}, // Read Write tests - {source, "", "bind,rw", "rw", ""}, - {source, "", "bind,rw,private", "rw", ""}, - {source, "", "bind,rw,shared", "rw", "shared"}, - {source, "", "bind,rw,slave", "rw", "master"}, - {source, "", "bind,rw,unbindable", "rw", "unbindable"}, + {source, "", "bind,rw", "rw", "", ""}, + {source, "", "bind,rw,private", "rw", "", ""}, + {source, "", "bind,rw,shared", "rw", "shared", ""}, + {source, "", "bind,rw,slave", "rw", "master", ""}, + {source, "", "bind,rw,unbindable", "rw", "unbindable", ""}, // Read Only tests - {source, "", "bind,ro", "ro", ""}, - {source, "", "bind,ro,private", "ro", ""}, - {source, "", "bind,ro,shared", "ro", "shared"}, - {source, "", "bind,ro,slave", "ro", "master"}, - {source, "", "bind,ro,unbindable", "ro", "unbindable"}, + {source, "", "bind,ro", "ro", "", ""}, + {source, "", "bind,ro,private", "ro", "", ""}, + {source, "", "bind,ro,shared", "ro", "shared", ""}, + {source, "", "bind,ro,slave", "ro", "master", ""}, + {source, "", "bind,ro,unbindable", "ro", "unbindable", ""}, + // Remount tests to change per filesystem options + {"", "", "remount,size=128k", "rw", "", "rw,size=128k"}, + {"", "", "remount,ro,size=128k", "ro", "", "ro,size=128k"}, } for _, tc := range tests { @@ -87,11 +91,17 @@ func TestMount(t *testing.T) { } }() } + if strings.Contains(tc.options, "remount") { + // create a new mount to remount first + if err := Mount("tmpfs", target, "tmpfs", ""); err != nil { + t.Fatal(err) + } + } if err := Mount(tc.source, target, tc.ftype, tc.options); err != nil { t.Fatal(err) } defer ensureUnmount(t, target) - validateMount(t, target, tc.expectedOpts, tc.expectedOptional) + validateMount(t, target, tc.expectedOpts, tc.expectedOptional, tc.expectedVFS) }) } } @@ -104,7 +114,7 @@ func ensureUnmount(t *testing.T, mnt string) { } // validateMount checks that mnt has the given options -func validateMount(t *testing.T, mnt string, opts, optional string) { +func validateMount(t *testing.T, mnt string, opts, optional, vfs string) { info, err := GetMounts() if err != nil { t.Fatal(err) @@ -124,6 +134,13 @@ func validateMount(t *testing.T, mnt string, opts, optional string) { } } + wantedVFS := make(map[string]struct{}) + if vfs != "" { + for _, opt := range strings.Split(vfs, ",") { + wantedVFS[opt] = struct{}{} + } + } + mnts := make(map[int]*Info, len(info)) for _, mi := range info { mnts[mi.ID] = mi @@ -177,6 +194,22 @@ func validateMount(t *testing.T, mnt string, opts, optional string) { t.Errorf("missing optional field %q found %q", field, mi.Optional) } + // Validate VFS if set + if vfs != "" { + if mi.VfsOpts != "" { + for _, opt := range strings.Split(mi.VfsOpts, ",") { + opt = clean(opt) + if !has(wantedVFS, opt) { + t.Errorf("unexpected mount option %q expected %q", opt, vfs) + } + delete(wantedVFS, opt) + } + } + for opt := range wantedVFS { + t.Errorf("missing mount option %q found %q", opt, mi.VfsOpts) + } + } + return }