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/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.
 #

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.
 #

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) {

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")

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