Merge pull request #580 from prashanthpai/issue-555

 volume-create: Fix few outstanding issues

e2e/volume_ops_test.go

@@ -117,6 +117,7 @@ func testVolumeExpand(t *testing.T) {
 			{NodeID: gds[0].PeerID(), Path: brickPaths[2]},
 			{NodeID: gds[1].PeerID(), Path: brickPaths[3]},
 		},
+		Force: true,
 	}
 	_, err := client.VolumeExpand(volname, expandReq)
 	r.Nil(err)

glustercli/cmd/volume.go

@@ -46,6 +46,9 @@ var (
 
 	// Stop Command Flags
 	flagStopCmdForce bool
+
+	// Expand Command Flags
+	flagExpandCmdForce bool
 )
 
 func init() {
@@ -80,6 +83,7 @@ func init() {
 
 	// Volume Expand
 	volumeExpandCmd.Flags().IntVarP(&flagCreateCmdReplicaCount, "replica", "", 0, "Replica Count")
+	volumeExpandCmd.Flags().BoolVarP(&flagExpandCmdForce, "force", "f", false, "Force")
 	volumeCmd.AddCommand(volumeExpandCmd)
 
 	RootCmd.AddCommand(volumeCmd)
@@ -505,6 +509,7 @@ var volumeExpandCmd = &cobra.Command{
 		vol, err := client.VolumeExpand(volname, api.VolExpandReq{
 			ReplicaCount: flagCreateCmdReplicaCount,
 			Bricks:       bricks, // string of format <UUID>:<path>
+			Force:        flagExpandCmdForce,
 		})
 		if err != nil {
 			log.WithFields(log.Fields{

glusterd2/brick/types.go

@@ -1,7 +1,10 @@
 package brick
 
 import (
+	"fmt"
+
 	"github.com/pborman/uuid"
+	"golang.org/x/sys/unix"
 )
 
 // Type is the type of Brick
@@ -44,3 +47,49 @@ func (b *Brickinfo) StringMap() map[string]string {
 
 	return m
 }
+
+// Validate checks if brick path is valid, if brick is a mount point,
+// if brick is on root partition and if it has xattr support.
+func (b *Brickinfo) Validate(check InitChecks) error {
+
+	var (
+		brickStat unix.Stat_t
+		err       error
+	)
+
+	if err = validatePathLength(b.Path); err != nil {
+		return err
+	}
+
+	if err = unix.Lstat(b.Path, &brickStat); err != nil {
+		return err
+	}
+
+	if (brickStat.Mode & unix.S_IFMT) != unix.S_IFDIR {
+		return fmt.Errorf("Brick path %s is not a directory", b.Path)
+	}
+
+	if check.IsMount {
+		if err = validateIsBrickMount(&brickStat, b.Path); err != nil {
+			return err
+		}
+	}
+
+	if check.IsOnRoot {
+		if err = validateIsOnRootDevice(&brickStat); err != nil {
+			return err
+		}
+	}
+
+	if err = validateXattrSupport(b.Path); err != nil {
+		return err
+	}
+
+	if check.IsInUse {
+		if err = validateBrickInUse(b.Path); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}

glusterd2/brick/validation.go

@@ -0,0 +1,92 @@
+package brick
+
+import (
+	"fmt"
+	"os"
+	"path"
+	"path/filepath"
+	"strings"
+	"syscall"
+
+	"github.com/gluster/glusterd2/pkg/errors"
+	"golang.org/x/sys/unix"
+)
+
+const (
+	testXattrKey     = "trusted.glusterfs.testing-xattr-support"
+	volumeIDXattrKey = "trusted.glusterfs.volume-id"
+	gfidXattrKey     = "trusted.gfid"
+)
+
+// InitChecks is a set of checks to be run on a brick
+type InitChecks struct {
+	IsInUse  bool
+	IsMount  bool
+	IsOnRoot bool
+}
+
+func validatePathLength(path string) error {
+
+	if len(filepath.Clean(path)) >= syscall.PathMax {
+		return errors.ErrBrickPathTooLong
+	}
+
+	subdirs := strings.Split(path, string(os.PathSeparator))
+	for _, subdir := range subdirs {
+		if len(subdir) >= syscall.PathMax {
+			return errors.ErrSubDirPathTooLong
+		}
+	}
+
+	return nil
+}
+
+func validateIsBrickMount(brickStat *unix.Stat_t, brickPath string) error {
+
+	var parentStat unix.Stat_t
+	if err := unix.Lstat(path.Dir(brickPath), &parentStat); err != nil {
+		return err
+	}
+
+	if brickStat.Dev != parentStat.Dev {
+		return errors.ErrBrickIsMountPoint
+	}
+
+	return nil
+}
+
+func validateIsOnRootDevice(brickStat *unix.Stat_t) error {
+
+	var rootStat unix.Stat_t
+	if err := unix.Lstat("/", &rootStat); err != nil {
+		return err
+	}
+
+	if brickStat.Dev == rootStat.Dev {
+		return errors.ErrBrickUnderRootPartition
+	}
+
+	return nil
+}
+
+func validateXattrSupport(brickPath string) error {
+	defer unix.Removexattr(brickPath, testXattrKey)
+	return unix.Setxattr(brickPath, testXattrKey, []byte("payload"), 0)
+}
+
+func validateBrickInUse(brickPath string) error {
+	keys := []string{gfidXattrKey, volumeIDXattrKey}
+	for path := brickPath; path != "/"; path = filepath.Dir(path) {
+		for _, key := range keys {
+			size, err := unix.Getxattr(path, key, nil)
+			if err != nil {
+				continue
+			} else if size > 0 {
+				return fmt.Errorf("Xattr %s already present on %s", key, path)
+			} else {
+				return nil
+			}
+		}
+	}
+	return nil
+}

glusterd2/commands/volumes/common.go

@@ -4,7 +4,11 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"os"
+	"path/filepath"
 
+	"github.com/gluster/glusterd2/glusterd2/brick"
+	"github.com/gluster/glusterd2/glusterd2/gdctx"
 	"github.com/gluster/glusterd2/glusterd2/servers/sunrpc"
 	"github.com/gluster/glusterd2/glusterd2/store"
 	"github.com/gluster/glusterd2/glusterd2/transaction"
@@ -13,8 +17,14 @@ import (
 	"github.com/gluster/glusterd2/glusterd2/xlator"
 	"github.com/gluster/glusterd2/glusterd2/xlator/options"
 	"github.com/gluster/glusterd2/pkg/api"
+
+	"github.com/pborman/uuid"
+	log "github.com/sirupsen/logrus"
+	"golang.org/x/sys/unix"
 )
 
+const volumeIDXattrKey = "trusted.glusterfs.volume-id"
+
 // validateOptions validates if the options and their values are valid and can
 // be set on a volume.
 func validateOptions(opts map[string]string) error {
@@ -101,22 +111,119 @@ func notifyVolfileChange(c transaction.TxnCtx) error {
 	return nil
 }
 
+func validateBricks(c transaction.TxnCtx) error {
+
+	var err error
+
+	var bricks []brick.Brickinfo
+	if err = c.Get("bricks", &bricks); err != nil {
+		return err
+	}
+
+	var checks brick.InitChecks
+	if err = c.Get("brick-checks", &checks); err != nil {
+		return err
+	}
+
+	for _, b := range bricks {
+		if !uuid.Equal(b.NodeID, gdctx.MyUUID) {
+			continue
+		}
+
+		if err = b.Validate(checks); err != nil {
+			c.Logger().WithError(err).WithField(
+				"brick", b.Path).Debug("Brick validation failed")
+			return err
+		}
+	}
+
+	return nil
+}
+
+func initBricks(c transaction.TxnCtx) error {
+
+	var err error
+
+	var bricks []brick.Brickinfo
+	if err = c.Get("bricks", &bricks); err != nil {
+		return err
+	}
+
+	var checks brick.InitChecks
+	if err = c.Get("brick-checks", &checks); err != nil {
+		return err
+	}
+
+	flags := 0
+	if checks.IsInUse {
+		// Perform a pure replace operation, which fails if the named
+		// attribute does not already exist.
+		flags = unix.XATTR_CREATE
+	}
+	for _, b := range bricks {
+		if !uuid.Equal(b.NodeID, gdctx.MyUUID) {
+			continue
+		}
+
+		err = unix.Setxattr(b.Path, volumeIDXattrKey, []byte(b.VolumeID), flags)
+		if err != nil {
+			log.WithError(err).WithFields(log.Fields{
+				"path": b.Path,
+				"key":  volumeIDXattrKey}).Error("Setxattr failed")
+			return err
+		}
+
+		path := filepath.Join(b.Path, ".glusterfs")
+		err = os.MkdirAll(path, os.ModeDir|os.ModePerm)
+		if err != nil {
+			log.WithError(err).WithField(
+				"path", path).Error("MkdirAll failed")
+			return err
+		}
+	}
+
+	return nil
+}
+
+func undoInitBricks(c transaction.TxnCtx) error {
+
+	var bricks []brick.Brickinfo
+	if err := c.Get("bricks", &bricks); err != nil {
+		return err
+	}
+
+	// FIXME: This is prone to races. See issue #314
+
+	for _, b := range bricks {
+		if !uuid.Equal(b.NodeID, gdctx.MyUUID) {
+			continue
+		}
+
+		unix.Removexattr(b.Path, volumeIDXattrKey)
+		os.Remove(filepath.Join(b.Path, ".glusterfs"))
+	}
+
+	return nil
+}
+
 func storeVolume(c transaction.TxnCtx) error {
 
 	var volinfo volume.Volinfo
 	if err := c.Get("volinfo", &volinfo); err != nil {
+		c.Logger().WithError(err).WithField(
+			"key", "volinfo").Debug("Failed to get key from store")
 		return err
 	}
 
 	if err := volume.AddOrUpdateVolumeFunc(&volinfo); err != nil {
 		c.Logger().WithError(err).WithField(
-			"volume", volinfo.Name).Debug("storeVolume: failed to store volume info")
+			"volume", volinfo.Name).Debug("failed to store volume info")
 		return err
 	}
 
 	if err := volgen.Generate(); err != nil {
 		c.Logger().WithError(err).WithField(
-			"volume", volinfo.Name).Debug("generateVolfiles: failed to generate volfiles")
+			"volume", volinfo.Name).Debug("failed to generate volfiles")
 		return err
 	}