diff --git a/src/runtime/config/configuration-clh.toml.in b/src/runtime/config/configuration-clh.toml.in index cedf2303ad..fbf8385721 100644 --- a/src/runtime/config/configuration-clh.toml.in +++ b/src/runtime/config/configuration-clh.toml.in @@ -147,7 +147,7 @@ virtio_fs_extra_args = @DEFVIRTIOFSEXTRAARGS@ # Cache mode: # -# - none +# - never # Metadata, data, and pathname lookup are not cached in guest. They are # always fetched from host and any changes are immediately pushed to host. # diff --git a/src/runtime/config/configuration-qemu.toml.in b/src/runtime/config/configuration-qemu.toml.in index f7e70a6d53..26b7e2f087 100644 --- a/src/runtime/config/configuration-qemu.toml.in +++ b/src/runtime/config/configuration-qemu.toml.in @@ -205,7 +205,7 @@ virtio_fs_extra_args = @DEFVIRTIOFSEXTRAARGS@ # Cache mode: # -# - none +# - never # Metadata, data, and pathname lookup are not cached in guest. They are # always fetched from host and any changes are immediately pushed to host. # diff --git a/src/runtime/pkg/katautils/config_test.go b/src/runtime/pkg/katautils/config_test.go index 335f077fbb..156c312cfc 100644 --- a/src/runtime/pkg/katautils/config_test.go +++ b/src/runtime/pkg/katautils/config_test.go @@ -1242,9 +1242,9 @@ func TestDefaultVirtioFSCache(t *testing.T) { cache = h.defaultVirtioFSCache() assert.Equal("always", cache) - h.VirtioFSCache = "none" + h.VirtioFSCache = "never" cache = h.defaultVirtioFSCache() - assert.Equal("none", cache) + assert.Equal("never", cache) } func TestDefaultFirmware(t *testing.T) { diff --git a/src/runtime/pkg/oci/utils_test.go b/src/runtime/pkg/oci/utils_test.go index b5b15ad871..7b2c10f2f0 100644 --- a/src/runtime/pkg/oci/utils_test.go +++ b/src/runtime/pkg/oci/utils_test.go @@ -650,7 +650,7 @@ func TestAddHypervisorAnnotations(t *testing.T) { ocispec.Annotations[vcAnnotations.BlockDeviceCacheNoflush] = "true" ocispec.Annotations[vcAnnotations.SharedFS] = "virtio-fs" ocispec.Annotations[vcAnnotations.VirtioFSDaemon] = "/bin/false" - ocispec.Annotations[vcAnnotations.VirtioFSCache] = "/home/cache" + ocispec.Annotations[vcAnnotations.VirtioFSCache] = "auto" ocispec.Annotations[vcAnnotations.VirtioFSExtraArgs] = "[ \"arg0\", \"arg1\" ]" ocispec.Annotations[vcAnnotations.Msize9p] = "512" ocispec.Annotations[vcAnnotations.MachineType] = "q35" @@ -688,7 +688,7 @@ func TestAddHypervisorAnnotations(t *testing.T) { assert.Equal(config.HypervisorConfig.BlockDeviceCacheNoflush, true) assert.Equal(config.HypervisorConfig.SharedFS, "virtio-fs") assert.Equal(config.HypervisorConfig.VirtioFSDaemon, "/bin/false") - assert.Equal(config.HypervisorConfig.VirtioFSCache, "/home/cache") + assert.Equal(config.HypervisorConfig.VirtioFSCache, "auto") assert.ElementsMatch(config.HypervisorConfig.VirtioFSExtraArgs, [2]string{"arg0", "arg1"}) assert.Equal(config.HypervisorConfig.Msize9p, uint32(512)) assert.Equal(config.HypervisorConfig.HypervisorMachineType, "q35") diff --git a/src/runtime/virtcontainers/documentation/api/1.0/api.md b/src/runtime/virtcontainers/documentation/api/1.0/api.md index c7215cc97c..9255a96ca9 100644 --- a/src/runtime/virtcontainers/documentation/api/1.0/api.md +++ b/src/runtime/virtcontainers/documentation/api/1.0/api.md @@ -219,7 +219,7 @@ type HypervisorConfig struct { // VirtioFSDaemonList is the list of valid virtiofs names for annotations VirtioFSDaemonList []string - // VirtioFSCache cache mode for fs version cache or "none" + // VirtioFSCache cache mode for fs version cache VirtioFSCache string // VirtioFSExtraArgs passes options to virtiofsd daemon diff --git a/src/runtime/virtcontainers/hypervisor.go b/src/runtime/virtcontainers/hypervisor.go index b8482e7b4b..eb6ed3fd36 100644 --- a/src/runtime/virtcontainers/hypervisor.go +++ b/src/runtime/virtcontainers/hypervisor.go @@ -325,7 +325,7 @@ type HypervisorConfig struct { // VirtioFSDaemon is the virtio-fs vhost-user daemon path VirtioFSDaemon string - // VirtioFSCache cache mode for fs version cache or "none" + // VirtioFSCache cache mode for fs version cache VirtioFSCache string // File based memory backend root directory diff --git a/src/runtime/virtcontainers/kata_agent.go b/src/runtime/virtcontainers/kata_agent.go index 5746759542..9de1661efd 100644 --- a/src/runtime/virtcontainers/kata_agent.go +++ b/src/runtime/virtcontainers/kata_agent.go @@ -34,6 +34,7 @@ import ( "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils" "context" + "github.com/gogo/protobuf/proto" "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/selinux/go-selinux" @@ -87,7 +88,7 @@ var ( type9pFs = "9p" typeVirtioFS = "virtiofs" typeOverlayFS = "overlay" - typeVirtioFSNoCache = "none" + typeVirtioFSNoCache = "never" kata9pDevType = "9p" kataMmioBlkDevType = "mmioblk" kataBlkDevType = "blk" @@ -801,7 +802,7 @@ func setupStorages(ctx context.Context, sandbox *Sandbox) []*grpc.Storage { if sharedFS == config.VirtioFS || sharedFS == config.VirtioFSNydus { // If virtio-fs uses either of the two cache options 'auto, always', // the guest directory can be mounted with option 'dax' allowing it to - // directly map contents from the host. When set to 'none', the mount + // directly map contents from the host. When set to 'never', the mount // options should not contain 'dax' lest the virtio-fs daemon crashing // with an invalid address reference. if sandbox.config.HypervisorConfig.VirtioFSCache != typeVirtioFSNoCache { diff --git a/src/runtime/virtcontainers/persist/api/config.go b/src/runtime/virtcontainers/persist/api/config.go index 44ba820643..9b5af5668d 100644 --- a/src/runtime/virtcontainers/persist/api/config.go +++ b/src/runtime/virtcontainers/persist/api/config.go @@ -70,7 +70,7 @@ type HypervisorConfig struct { // VirtioFSDaemon is the virtio-fs vhost-user daemon path VirtioFSDaemon string - // VirtioFSCache cache mode for fs version cache or "none" + // VirtioFSCache cache mode for fs version cache VirtioFSCache string // File based memory backend root directory diff --git a/src/runtime/virtcontainers/pkg/annotations/annotations.go b/src/runtime/virtcontainers/pkg/annotations/annotations.go index 67c81cb1f8..5d36926c5e 100644 --- a/src/runtime/virtcontainers/pkg/annotations/annotations.go +++ b/src/runtime/virtcontainers/pkg/annotations/annotations.go @@ -190,7 +190,7 @@ const ( // VirtioFSDaemon is a sandbox annotations to specify virtio-fs vhost-user daemon path VirtioFSDaemon = kataAnnotHypervisorPrefix + "virtio_fs_daemon" - // VirtioFSCache is a sandbox annotation to specify the cache mode for fs version cache or "none" + // VirtioFSCache is a sandbox annotation to specify the cache mode for fs version cache VirtioFSCache = kataAnnotHypervisorPrefix + "virtio_fs_cache" // VirtioFSCacheSize is a sandbox annotation to specify the DAX cache size in MiB diff --git a/src/runtime/virtcontainers/virtiofsd.go b/src/runtime/virtcontainers/virtiofsd.go index 0d8ba1c190..08f7dfd6f8 100644 --- a/src/runtime/virtcontainers/virtiofsd.go +++ b/src/runtime/virtcontainers/virtiofsd.go @@ -29,11 +29,12 @@ var virtiofsdTracingTags = map[string]string{ } var ( - errVirtiofsdDaemonPathEmpty = errors.New("virtiofsd daemon path is empty") - errVirtiofsdSocketPathEmpty = errors.New("virtiofsd socket path is empty") - errVirtiofsdSourcePathEmpty = errors.New("virtiofsd source path is empty") - errVirtiofsdSourceNotAvailable = errors.New("virtiofsd source path not available") - errUnimplemented = errors.New("unimplemented") + errVirtiofsdDaemonPathEmpty = errors.New("virtiofsd daemon path is empty") + errVirtiofsdSocketPathEmpty = errors.New("virtiofsd socket path is empty") + errVirtiofsdSourcePathEmpty = errors.New("virtiofsd source path is empty") + errVirtiofsdInvalidVirtiofsCacheMode = func(mode string) error { return errors.Errorf("Invalid virtio-fs cache mode: %s", mode) } + errVirtiofsdSourceNotAvailable = errors.New("virtiofsd source path not available") + errUnimplemented = errors.New("unimplemented") ) type VirtiofsDaemon interface { @@ -63,7 +64,7 @@ type virtiofsd struct { path string // socketPath where daemon will serve socketPath string - // cache size for virtiofsd + // cache mode for virtiofsd cache string // sourcePath path that daemon will help to share sourcePath string @@ -213,6 +214,16 @@ func (v *virtiofsd) valid() error { if _, err := os.Stat(v.sourcePath); err != nil { return errVirtiofsdSourceNotAvailable } + + if v.cache == "" { + v.cache = "auto" + } else if v.cache == "none" { + v.Logger().Warn("virtio-fs cache mode `none` is deprecated since Kata Containers 2.5.0 and will be removed in the future release, please use `never` instead. For more details please refer to https://github.com/kata-containers/kata-containers/issues/4234.") + v.cache = "never" + } else if v.cache != "auto" && v.cache != "always" && v.cache != "never" { + return errVirtiofsdInvalidVirtiofsCacheMode(v.cache) + } + return nil } diff --git a/src/runtime/virtcontainers/virtiofsd_test.go b/src/runtime/virtcontainers/virtiofsd_test.go index 3705a559b9..a4252a2ba9 100644 --- a/src/runtime/virtcontainers/virtiofsd_test.go +++ b/src/runtime/virtcontainers/virtiofsd_test.go @@ -91,7 +91,7 @@ func TestVirtiofsdArgs(t *testing.T) { } func TestValid(t *testing.T) { - assert := assert.New(t) + a := assert.New(t) sourcePath := t.TempDir() socketDir := t.TempDir() @@ -103,31 +103,61 @@ func TestValid(t *testing.T) { path: "/usr/bin/virtiofsd", sourcePath: sourcePath, socketPath: socketPath, + cache: "auto", } } - // valid case - v := newVirtiofsdFunc() - err := v.valid() - assert.NoError(err) + type fieldFunc func(v *virtiofsd) + type assertFunc func(name string, assert *assert.Assertions, v *virtiofsd) - v = newVirtiofsdFunc() - v.path = "" - err = v.valid() - assert.Equal(errVirtiofsdDaemonPathEmpty, err) + // nolint: govet + tests := []struct { + name string + f fieldFunc + wantErr error + customAssert assertFunc + }{ + {"valid case", nil, nil, nil}, + {"no path", func(v *virtiofsd) { + v.path = "" + }, errVirtiofsdDaemonPathEmpty, nil}, + {"no sourcePath", func(v *virtiofsd) { + v.sourcePath = "" + }, errVirtiofsdSourcePathEmpty, nil}, + {"no socketPath", func(v *virtiofsd) { + v.socketPath = "" + }, errVirtiofsdSocketPathEmpty, nil}, + {"source is not available", func(v *virtiofsd) { + v.sourcePath = "/foo/bar" + }, errVirtiofsdSourceNotAvailable, nil}, + {"replace cache mode none by never", func(v *virtiofsd) { + v.cache = "none" + }, nil, func(name string, a *assert.Assertions, v *virtiofsd) { + a.Equal("never", v.cache, "test case %+s, cache mode none should be replaced by never", name) + }}, + {"invald cache mode: replace none by never", func(v *virtiofsd) { + v.cache = "foo" + }, errVirtiofsdInvalidVirtiofsCacheMode("foo"), nil}, + } - v = newVirtiofsdFunc() - v.sourcePath = "" - err = v.valid() - assert.Equal(errVirtiofsdSourcePathEmpty, err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v := newVirtiofsdFunc() + if tt.f != nil { + tt.f(v) + } + err := v.valid() + if tt.wantErr != nil && err == nil { + t.Errorf("test case %+s: virtiofsd.valid() should get error `%+v`, but got nil", tt.name, tt.wantErr) + } else if tt.wantErr == nil && err != nil { + t.Errorf("test case %+s: virtiofsd.valid() should get no erro, but got `%+v`", tt.name, err) + } else if tt.wantErr != nil && err != nil { + a.Equal(err.Error(), tt.wantErr.Error(), "test case %+s", tt.name) + } - v = newVirtiofsdFunc() - v.socketPath = "" - err = v.valid() - assert.Equal(errVirtiofsdSocketPathEmpty, err) - - v = newVirtiofsdFunc() - v.sourcePath = "/foo/bar" - err = v.valid() - assert.Equal(errVirtiofsdSourceNotAvailable, err) + if tt.customAssert != nil { + tt.customAssert(tt.name, a, v) + } + }) + } }