From 407727e1fbd4a87d6ae7fff058f7a7898c82684b Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Tue, 13 Jun 2023 18:14:38 +0200 Subject: [PATCH 1/3] virtiofsd: Stop using deprecated `-f` option The rust implementation of virtiofsd always runs foreground and spits a deprecation warning when `-f` is passed. Signed-off-by: Greg Kurz (cherry picked from commit 2a15ad97888c8e784537b45cf61434ee5a8b8c47) Signed-off-by: Greg Kurz --- src/runtime/virtcontainers/virtiofsd.go | 2 -- src/runtime/virtcontainers/virtiofsd_test.go | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/runtime/virtcontainers/virtiofsd.go b/src/runtime/virtcontainers/virtiofsd.go index 6df62adf9d..bb5aaa319e 100644 --- a/src/runtime/virtcontainers/virtiofsd.go +++ b/src/runtime/virtcontainers/virtiofsd.go @@ -194,8 +194,6 @@ func (v *virtiofsd) args(FdSocketNumber uint) ([]string, error) { "-o", "source=" + v.sourcePath, // fd number of vhost-user socket fmt.Sprintf("--fd=%v", FdSocketNumber), - // foreground operation - "-f", } if len(v.extraArgs) != 0 { diff --git a/src/runtime/virtcontainers/virtiofsd_test.go b/src/runtime/virtcontainers/virtiofsd_test.go index a4252a2ba9..2fd096b380 100644 --- a/src/runtime/virtcontainers/virtiofsd_test.go +++ b/src/runtime/virtcontainers/virtiofsd_test.go @@ -79,12 +79,12 @@ func TestVirtiofsdArgs(t *testing.T) { cache: "none", } - expected := "--syslog -o cache=none -o no_posix_lock -o source=/run/kata-shared/foo --fd=123 -f" + expected := "--syslog -o cache=none -o no_posix_lock -o source=/run/kata-shared/foo --fd=123" args, err := v.args(123) assert.NoError(err) assert.Equal(expected, strings.Join(args, " ")) - expected = "--syslog -o cache=none -o no_posix_lock -o source=/run/kata-shared/foo --fd=456 -f" + expected = "--syslog -o cache=none -o no_posix_lock -o source=/run/kata-shared/foo --fd=456" args, err = v.args(456) assert.NoError(err) assert.Equal(expected, strings.Join(args, " ")) From 2e9125c32b2e9ae549a01c28fbe42d7406935e82 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Wed, 14 Jun 2023 14:56:30 +0200 Subject: [PATCH 2/3] virtiofsd: Drop `-o no_posix_lock` The C implementation of virtiofsd had some kind of limited support for remote POSIX locks that was causing some workflows to fail with kata. Commit 432f9bea6e8b2 hard coded `-o no_posix_lock` in order to enforce guest local POSIX locks and avoid the issues. We've switched to the rust implementation of virtiofsd since then, but it emits a warning about `-o` being deprecated. According to https://gitlab.com/virtio-fs/virtiofsd/-/issues/53 : The C implementation of the daemon has limited support for remote POSIX locks, restricted exclusively to non-blocking operations. We tried to implement the same level of functionality in #2, but we finally decided against it because, in practice most applications will fail if non-blocking operations aren't supported. Implementing support for non-blocking isn't trivial and will probably require extending the kernel interface before we can even start working on the daemon side. There is thus no justification to pass `-o no_posix_lock` anymore. Signed-off-by: Greg Kurz (cherry picked from commit 8e00dc694416d1bd962d130de3f1fa42737233f6) Signed-off-by: Greg Kurz --- src/runtime/virtcontainers/virtiofsd.go | 3 --- src/runtime/virtcontainers/virtiofsd_test.go | 4 ++-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/runtime/virtcontainers/virtiofsd.go b/src/runtime/virtcontainers/virtiofsd.go index bb5aaa319e..d45c08dca4 100644 --- a/src/runtime/virtcontainers/virtiofsd.go +++ b/src/runtime/virtcontainers/virtiofsd.go @@ -187,9 +187,6 @@ func (v *virtiofsd) args(FdSocketNumber uint) ([]string, error) { "--syslog", // cache mode for virtiofsd "-o", "cache=" + v.cache, - // disable posix locking in daemon: bunch of basic posix locks properties are broken - // apt-get update is broken if enabled - "-o", "no_posix_lock", // shared directory tree "-o", "source=" + v.sourcePath, // fd number of vhost-user socket diff --git a/src/runtime/virtcontainers/virtiofsd_test.go b/src/runtime/virtcontainers/virtiofsd_test.go index 2fd096b380..fbcdbec276 100644 --- a/src/runtime/virtcontainers/virtiofsd_test.go +++ b/src/runtime/virtcontainers/virtiofsd_test.go @@ -79,12 +79,12 @@ func TestVirtiofsdArgs(t *testing.T) { cache: "none", } - expected := "--syslog -o cache=none -o no_posix_lock -o source=/run/kata-shared/foo --fd=123" + expected := "--syslog -o cache=none -o source=/run/kata-shared/foo --fd=123" args, err := v.args(123) assert.NoError(err) assert.Equal(expected, strings.Join(args, " ")) - expected = "--syslog -o cache=none -o no_posix_lock -o source=/run/kata-shared/foo --fd=456" + expected = "--syslog -o cache=none -o source=/run/kata-shared/foo --fd=456" args, err = v.args(456) assert.NoError(err) assert.Equal(expected, strings.Join(args, " ")) From 993ecec93216bf925679f9c77f3438890cc8c30f Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Wed, 14 Jun 2023 12:33:45 +0200 Subject: [PATCH 3/3] virtiofsd: Convert legacy `-o` sub-options to their `--` replacement The `-o` option is the legacy way to configure virtiofsd, inherited from the C implementation. The rust implementation honours it for compatibility but it logs deprecation warnings. Let's use the replacement options in the go shim code. Also drop references to `-o` from the configuration TOML file. Fixes #7111 Signed-off-by: Greg Kurz (cherry picked from commit a43ea24dfc2611be3da5b358e079402db54fd7db) Signed-off-by: Greg Kurz --- src/runtime/Makefile | 2 +- src/runtime/config/configuration-clh.toml.in | 4 ++-- src/runtime/config/configuration-qemu.toml.in | 4 ++-- src/runtime/virtcontainers/virtiofsd.go | 4 ++-- src/runtime/virtcontainers/virtiofsd_test.go | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/runtime/Makefile b/src/runtime/Makefile index 99dde7e2b9..88676eaf08 100644 --- a/src/runtime/Makefile +++ b/src/runtime/Makefile @@ -210,7 +210,7 @@ DEFVIRTIOFSQUEUESIZE ?= 1024 # # see `virtiofsd -h` for possible options. # Make sure you quote args. -DEFVIRTIOFSEXTRAARGS ?= [\"--thread-pool-size=1\", \"-o\", \"announce_submounts\"] +DEFVIRTIOFSEXTRAARGS ?= [\"--thread-pool-size=1\", \"--announce-submounts\"] DEFENABLEIOTHREADS := false DEFENABLEVHOSTUSERSTORE := false DEFVHOSTUSERSTOREPATH := $(PKGRUNDIR)/vhost-user diff --git a/src/runtime/config/configuration-clh.toml.in b/src/runtime/config/configuration-clh.toml.in index df7cc7ac5d..9ee606fc40 100644 --- a/src/runtime/config/configuration-clh.toml.in +++ b/src/runtime/config/configuration-clh.toml.in @@ -145,9 +145,9 @@ virtio_fs_queue_size = @DEFVIRTIOFSQUEUESIZE@ # Extra args for virtiofsd daemon # # Format example: -# ["-o", "arg1=xxx,arg2", "-o", "hello world", "--arg3=yyy"] +# ["--arg1=xxx", "--arg2=yyy"] # Examples: -# Set virtiofsd log level to debug : ["-o", "log_level=debug"] or ["-d"] +# Set virtiofsd log level to debug : ["--log-level=debug"] # see `virtiofsd -h` for possible options. virtio_fs_extra_args = @DEFVIRTIOFSEXTRAARGS@ diff --git a/src/runtime/config/configuration-qemu.toml.in b/src/runtime/config/configuration-qemu.toml.in index 4fb5a8ba03..69ab1c0e5c 100644 --- a/src/runtime/config/configuration-qemu.toml.in +++ b/src/runtime/config/configuration-qemu.toml.in @@ -197,9 +197,9 @@ virtio_fs_queue_size = @DEFVIRTIOFSQUEUESIZE@ # Extra args for virtiofsd daemon # # Format example: -# ["-o", "arg1=xxx,arg2", "-o", "hello world", "--arg3=yyy"] +# ["--arg1=xxx", "--arg2=yyy"] # Examples: -# Set virtiofsd log level to debug : ["-o", "log_level=debug"] or ["-d"] +# Set virtiofsd log level to debug : ["--log-level=debug"] # # see `virtiofsd -h` for possible options. virtio_fs_extra_args = @DEFVIRTIOFSEXTRAARGS@ diff --git a/src/runtime/virtcontainers/virtiofsd.go b/src/runtime/virtcontainers/virtiofsd.go index d45c08dca4..3e02756eb3 100644 --- a/src/runtime/virtcontainers/virtiofsd.go +++ b/src/runtime/virtcontainers/virtiofsd.go @@ -186,9 +186,9 @@ func (v *virtiofsd) args(FdSocketNumber uint) ([]string, error) { // Send logs to syslog "--syslog", // cache mode for virtiofsd - "-o", "cache=" + v.cache, + "--cache=" + v.cache, // shared directory tree - "-o", "source=" + v.sourcePath, + "--shared-dir=" + v.sourcePath, // fd number of vhost-user socket fmt.Sprintf("--fd=%v", FdSocketNumber), } diff --git a/src/runtime/virtcontainers/virtiofsd_test.go b/src/runtime/virtcontainers/virtiofsd_test.go index fbcdbec276..c7d1e1e784 100644 --- a/src/runtime/virtcontainers/virtiofsd_test.go +++ b/src/runtime/virtcontainers/virtiofsd_test.go @@ -79,12 +79,12 @@ func TestVirtiofsdArgs(t *testing.T) { cache: "none", } - expected := "--syslog -o cache=none -o source=/run/kata-shared/foo --fd=123" + expected := "--syslog --cache=none --shared-dir=/run/kata-shared/foo --fd=123" args, err := v.args(123) assert.NoError(err) assert.Equal(expected, strings.Join(args, " ")) - expected = "--syslog -o cache=none -o source=/run/kata-shared/foo --fd=456" + expected = "--syslog --cache=none --shared-dir=/run/kata-shared/foo --fd=456" args, err = v.args(456) assert.NoError(err) assert.Equal(expected, strings.Join(args, " "))