From 2a5bddcfdd00cca4eaf13fba3dfcc0f21e249746 Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Mon, 14 Feb 2022 14:22:33 +0100 Subject: [PATCH] Implement locking of files when setting and reading xattrs To lock, a file with the ending .flock is created beside the to be locked node (which also can be a directory) which is locked with a Lock. Other ocis processes respect that. --- go.mod | 1 + go.sum | 2 + .../utils/decomposedfs/xattrs/xattrs.go | 129 ++++++++++++++---- 3 files changed, 107 insertions(+), 25 deletions(-) diff --git a/go.mod b/go.mod index e61ef61a4d..20ab718e7e 100644 --- a/go.mod +++ b/go.mod @@ -28,6 +28,7 @@ require ( github.com/go-openapi/errors v0.20.1 // indirect github.com/go-openapi/strfmt v0.20.2 // indirect github.com/go-sql-driver/mysql v1.6.0 + github.com/gofrs/flock v0.8.1 // indirect github.com/golang-jwt/jwt v3.2.2+incompatible github.com/golang/protobuf v1.5.2 github.com/gomodule/redigo v1.8.8 diff --git a/go.sum b/go.sum index fea989cc02..f91f4bf868 100644 --- a/go.sum +++ b/go.sum @@ -419,6 +419,8 @@ github.com/gobwas/pool v0.2.1/go.mod h1:q8bcK0KcYlCgd9e7WYLm9LpyS+YeLd8JVDW6Wezm github.com/gobwas/ws v1.0.4/go.mod h1:szmBTxLgaFppYjEmNtny/v3w89xOydFnnZMcgRRu/EM= github.com/godbus/dbus/v5 v5.0.3/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= +github.com/gofrs/flock v0.8.1 h1:+gYjHKf32LDeiEEFhQaotPbLuUXjY5ZqxKgXy7n59aw= +github.com/gofrs/flock v0.8.1/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU= github.com/gofrs/uuid v3.2.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM= github.com/gofrs/uuid v4.0.0+incompatible h1:1SD/1F5pU8p29ybwgQSwpQk+mwdRrXCYuPhW6m+TnJw= github.com/gofrs/uuid v4.0.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM= diff --git a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go index fe730cbe93..00ea65d44b 100644 --- a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go +++ b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go @@ -19,13 +19,13 @@ package xattrs import ( - "os" "strconv" "strings" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/gofrs/flock" + "github.com/pkg/errors" "github.com/pkg/xattr" - "golang.org/x/sys/unix" ) // Declare a list of xattr keys @@ -112,29 +112,97 @@ func refFromCS3(b []byte) (*provider.Reference, error) { }, nil } +// flockFile returns the flock filename for a given file name +// it returns an empty string if the input is empty +func flockFile(file string) string { + var n string + if len(file) > 0 { + n = file + ".flock" + } + return n +} + +// acquireWriteLog acquires a lock on a file or directory. +// if the parameter write is true, it gets an exclusive write lock, otherwise a shared read lock. +// The function returns a Flock object, unlocking has to be done in the calling function. +func acquireLock(file string, write bool) (*flock.Flock, error) { + var err error + + // Create the a file to carry the log + n := flockFile(file) + if len(n) == 0 { + return nil, errors.New("Path empty") + } + // Acquire the write log on the target node first. + lock := flock.New(n) + + if write { + _, err = lock.TryLock() + } else { + _, err = lock.TryRLock() + } + + if err != nil { + return nil, err + } + return lock, nil +} + // CopyMetadata copies all extended attributes from source to target. // The optional filter function can be used to filter by attribute name, e.g. by checking a prefix -func CopyMetadata(s, t string, filter func(attributeName string) bool) error { - var attrs []string +// For the source file, a shared lock is acquired. For the target, an exclusive +// write lock is acquired. +func CopyMetadata(src, target string, filter func(attributeName string) bool) error { var err error - if attrs, err = xattr.List(s); err != nil { - return err + var writeLock, readLock *flock.Flock + + // Acquire the write log on the target node first. + writeLock, err = acquireLock(target, true) + + if err != nil { + return errors.Wrap(err, "xattrs: Unable to lock target to write") } - for i := range attrs { - if filter == nil || filter(attrs[i]) { - b, err := xattr.Get(s, attrs[i]) - if err != nil { - return err + defer func() { + err = writeLock.Unlock() + }() + + // now try to get a shared lock on the source + readLock, err = acquireLock(src, false) + + if err != nil { + return errors.Wrap(err, "xattrs: Unable to lock file for read") + } + defer func() { + err = readLock.Unlock() + }() + + // both locks are established. Copy. + var attrNameList []string + if attrNameList, err = xattr.List(src); err != nil { + return errors.Wrap(err, "Can not get xattr listing on src") + } + + // FIXME: this loop assumes that all reads and writes work. The case that one val + // is copied, but any others not is not handled properly. + for idx := range attrNameList { + attrName := attrNameList[idx] + if filter == nil || filter(attrName) { + var attrVal []byte + if attrVal, err = xattr.Get(src, attrName); err != nil { + return errors.Wrapf(err, "Can not read src attribute named %s", attrName) } - if err := xattr.Set(t, attrs[i], b); err != nil { - return err + if err = xattr.Set(target, attrName, attrVal); err != nil { + return errors.Wrapf(err, "Can not write target attribute named %s", attrName) } } } + return nil } // Set an extended attribute key to the given value +// No file locking is involved here as writing a single xattr is +// considered to be atomic. func Set(filePath string, key string, val string) error { if err := xattr.Set(filePath, key, []byte(val)); err != nil { @@ -144,24 +212,24 @@ func Set(filePath string, key string, val string) error { } // SetMultiple allows setting multiple key value pairs at once -// the changes are protected with an flock +// the changes are protected with an file lock +// If the file lock can not be acquired the function returns a +// lock error. func SetMultiple(filePath string, attribs map[string]string) error { // h, err := lockedfile.OpenFile(filePath, os.O_WRONLY, 0) // 0? Open File only workn for files ... but we want to lock dirs ... or symlinks // or we append .lock to the file and use https://github.com/gofrs/flock - h, err := os.Open(filePath) - if err != nil { - return err - } - defer h.Close() - err = unix.Flock(int(h.Fd()), unix.LOCK_EX) + fileLock, err := acquireLock(filePath, true) + if err != nil { - return err + return errors.Wrap(err, "xattrs: Can not acquired write log") } - defer unix.Flock(int(h.Fd()), unix.LOCK_UN) + defer func() { + err = fileLock.Unlock() + }() for key, val := range attribs { - if err := xattr.FSet(h, key, []byte(val)); err != nil { + if err := xattr.Set(filePath, key, []byte(val)); err != nil { return err } } @@ -169,8 +237,9 @@ func SetMultiple(filePath string, attribs map[string]string) error { } // Get an extended attribute value for the given key +// No file locking is involved here as reading a single xattr is +// considered to be atomic. func Get(filePath, key string) (string, error) { - v, err := xattr.Get(filePath, key) if err != nil { return "", err @@ -192,8 +261,18 @@ func GetInt64(filePath, key string) (int64, error) { return v, nil } -// All reads all extended attributes for a node +// All reads all extended attributes for a node, protected by a +// shared file lock func All(filePath string) (map[string]string, error) { + fileLock, err := acquireLock(filePath, false) + + if err != nil { + return nil, errors.Wrap(err, "xattrs: Unable to lock file for read") + } + defer func() { + err = fileLock.Unlock() + }() + attrNames, err := xattr.List(filePath) if err != nil { return nil, err