From 60606647de7f128228e94a3a5bd4414b147d3594 Mon Sep 17 00:00:00 2001 From: Jose Carlos Venegas Munoz Date: Mon, 29 Jun 2020 01:16:08 -0700 Subject: [PATCH] virtiofsd: Improve logging [ port from runtime commit 882a82393305a4b11a77744b5fc77b98e42d15b9 ] Send virtiofsd logs to syslog in the same way that qemu implementation does. This requires not to wait for messages from virtiofsd stdout. This takes the qemu implementation approach. Give the socket fd to virtiofsd. Signed-off-by: Jose Carlos Venegas Munoz Signed-off-by: Peng Tao --- src/runtime/virtcontainers/virtiofsd.go | 62 ++++++++++++++++---- src/runtime/virtcontainers/virtiofsd_test.go | 13 +++- 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/src/runtime/virtcontainers/virtiofsd.go b/src/runtime/virtcontainers/virtiofsd.go index a22bef9e4d..fcad708c7b 100644 --- a/src/runtime/virtcontainers/virtiofsd.go +++ b/src/runtime/virtcontainers/virtiofsd.go @@ -10,8 +10,10 @@ import ( "context" "fmt" "io" + "net" "os" "os/exec" + "path/filepath" "strings" "syscall" "time" @@ -58,6 +60,26 @@ type virtiofsd struct { wait virtiofsdWaitFunc } +// Open socket on behalf of virtiofsd +// return file descriptor to be used by virtiofsd. +func (v *virtiofsd) getSocketFD() (*os.File, error) { + var listener *net.UnixListener + + if _, err := os.Stat(filepath.Dir(v.socketPath)); err != nil { + return nil, errors.Errorf("Socket directory does not exist %s", filepath.Dir(v.socketPath)) + } + + listener, err := net.ListenUnix("unix", &net.UnixAddr{Name: v.socketPath, Net: "unix"}) + if err != nil { + return nil, err + } + defer listener.Close() + + listener.SetUnlinkOnClose(false) + + return listener.File() +} + // Start the virtiofsd daemon func (v *virtiofsd) Start(ctx context.Context) (int, error) { span, _ := v.trace("Start") @@ -68,20 +90,28 @@ func (v *virtiofsd) Start(ctx context.Context) (int, error) { return pid, err } - args, err := v.args() + cmd := exec.Command(v.path) + + socketFD, err := v.getSocketFD() + if err != nil { + return 0, err + } + + cmd.ExtraFiles = append(cmd.ExtraFiles, socketFD) + + // Extra files start from 2 (0: stdin, 1: stdout, 2: stderr) + // Extra FDs for virtiofsd start from 3 + // Get the FD number for previous added socketFD + socketFdNumber := 2 + uint(len(cmd.ExtraFiles)) + args, err := v.args(socketFdNumber) if err != nil { return pid, err } + cmd.Args = append(cmd.Args, args...) v.Logger().WithField("path", v.path).Info() v.Logger().WithField("args", strings.Join(args, " ")).Info() - cmd := exec.Command(v.path, args...) - stderr, err := cmd.StderrPipe() - if err != nil { - return pid, fmt.Errorf("failed to get stderr from virtiofsd command, error: %s", err) - } - if err = utils.StartCmd(cmd); err != nil { return pid, err } @@ -96,7 +126,7 @@ func (v *virtiofsd) Start(ctx context.Context) (int, error) { v.wait = waitVirtiofsReady } - return cmd.Process.Pid, v.wait(cmd, stderr, v.debug) + return pid, socketFD.Close() } func (v *virtiofsd) Stop() error { @@ -115,7 +145,7 @@ func (v *virtiofsd) Stop() error { return nil } -func (v *virtiofsd) args() ([]string, error) { +func (v *virtiofsd) args(FdSocketNumber uint) ([]string, error) { if v.sourcePath == "" { return []string{}, errors.New("vitiofsd source path is empty") } @@ -125,11 +155,23 @@ func (v *virtiofsd) args() ([]string, error) { } args := []string{ + // Send logs to syslog + "--syslog", + // foreground operation "-f", + // 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, - "-o", "vhost_user_socket=" + v.socketPath, + // fd number of vhost-user socket + fmt.Sprintf("--fd=%v", FdSocketNumber), + } + + if v.debug { + args = append(args, "-o", "debug") } if len(v.extraArgs) != 0 { diff --git a/src/runtime/virtcontainers/virtiofsd_test.go b/src/runtime/virtcontainers/virtiofsd_test.go index ebac2644b4..e2ed02320e 100644 --- a/src/runtime/virtcontainers/virtiofsd_test.go +++ b/src/runtime/virtcontainers/virtiofsd_test.go @@ -33,11 +33,19 @@ func TestVirtiofsdStart(t *testing.T) { assert.NoError(err) defer os.RemoveAll(sourcePath) + socketDir, err := ioutil.TempDir("", "") + assert.NoError(err) + defer os.RemoveAll(sourcePath) + + socketPath := socketDir + "socket.s" + validConfig := fields{ - path: "/tmp/a/path", - socketPath: "/tmp/a/path/to/sock.sock", + path: "/usr/bin/virtiofsd-path", + socketPath: socketPath, sourcePath: sourcePath, } + NoDirectorySocket := validConfig + NoDirectorySocket.socketPath = "/tmp/path/to/virtiofsd/socket.sock" tests := []struct { name string @@ -45,6 +53,7 @@ func TestVirtiofsdStart(t *testing.T) { wantErr bool }{ {"empty config", fields{}, true}, + {"Directory socket does not exist", NoDirectorySocket, true}, {"valid config", validConfig, false}, } for _, tt := range tests {