runtime: change cache mode from none to never

New Rust virtiofsd's `cache` mode doesn't support `none` mode,
we should use `never` to replace it.

Fixes: #6018

Signed-off-by: Bin Liu <bin@hyper.sh>

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

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

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 {

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

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

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
 }
 

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)
+			}
+		})
+	}
 }