Merge pull request #17 from peter-wangxu/fix_nits

Add extend volume feature

README.md

@@ -21,6 +21,7 @@ efforts needed when connecting/disconnecting to storage backend.
         * [Connect to a LUN on specific target](#connect-to-a-lun-on-specific-target)
         * [Connect and rescan all LUNs from a target](#connect-and-rescan-all-luns-from-a-target)
         * [Disconnect a device from remote system](#disconnect-a-lun-from-storage-system)
+        * [Extend a connected device](#extend-a-connected-device)
         * [Get help](#get-help)
 * [Testing](#testing)
     * [Unit test](#unit-test)
@@ -129,6 +130,28 @@ conn.TargetPortals = []int{10}
 deviceInfo, _ : = iscsi.DisconnectVolume(conn)
 ```
 
+#### Extend a already connected device
+
+Sometimes, the device can be extend on the storage system, while the device size
+is not awared from the host side, in this case, a host side rescan is needed.
+
+```go
+package main
+
+import (
+        "github.com/peter-wangxu/goock/connector"
+)
+
+iscsi := connector.New()
+
+conn := connector.ConnectionProperty{}
+conn.TargetPortals = []string{"192.168.1.30"}
+conn.TargetIqns = []string{"iqn.xxxxxxxxxxxxxx"}
+conn.TargetPortals = []int{10}
+
+deviceInfo, _ : = iscsi.ExtendVolume(conn)
+```
+
 ### As a client tool
 
 Goock is also client tool, which can be used from shell. When the host is connecting with
@@ -167,6 +190,17 @@ goock connect <target IP>
  goock disconnect <Target IP> <LUN ID>
  ```
 
+#### Extend a connected device
+
+```bash
+goock extend <Target IP> <LUN ID>
+```
+or
+
+```bash
+goock extend /dev/sdx
+```
+
 #### Get help
 
 ```bash

client/base.go

@@ -61,15 +61,13 @@ func HandleExtend(args ...string) error {
 	var err error
 	if len(args) <= 0 {
 		err = fmt.Errorf("Need device name or Target IP with LUN ID.")
-	}
-	if len(args) == 1 {
+	} else if len(args) == 1 {
 		// User only supplies the local device name
-
+		err = fmt.Errorf("Currently device name is not supported.")
 	} else {
 		// User specify TargetIP with LUN ID
+		err = HandleISCSIExtend(args...)
 	}
-	if err != nil {
-		return err
-	}
-	return HandleISCSIExtend(args...)
+	return err
+
 }

client/base_test.go

@@ -24,5 +24,5 @@ func TestHandleExtendEmpty(t *testing.T) {
 
 func TestHandleExtendLocal(t *testing.T) {
 	err := HandleExtend("/dev/sdm")
-	assert.Nil(t, err)
+	assert.Error(t, err)
 }

client/iscsi.go

@@ -113,7 +113,17 @@ func HandleISCSIDisconnect(args ...string) error {
 }
 
 func HandleISCSIExtend(args ...string) error {
-	return nil
+	targetIp := args[0]
+	lunIds, err := ValidateLunId(args[1:])
+
+	sessions := iscsiConnector.DiscoverPortal(targetIp)
+	if err == nil {
+		for _, lun := range lunIds {
+			property := Session2ConnectionProperty(sessions, lun)
+			iscsiConnector.ExtendVolume(property)
+		}
+	}
+	return err
 }
 
 func FetchVolumeInfo(sessions []model.ISCSISession, lun int) (connector.VolumeInfo, error) {
@@ -152,5 +162,8 @@ func ValidateLunId(lunIds []string) ([]int, error) {
 		i, _ := strconv.Atoi(lun)
 		ret = append(ret, i)
 	}
+	if len(ret) <= 0 {
+		log.Warnf("No lun ID specified, correct and retry.")
+	}
 	return ret, err
 }

client/iscsi_test.go

@@ -128,9 +128,23 @@ func TestHandleDisISCSIConnectViaDevice(t *testing.T) {
 	err := HandleISCSIDisconnect("/dev/sdb")
 	assert.Error(t, err)
 }
+
+func TestHandleExtend(t *testing.T) {
+	fake := &FakeISCSIConnector{}
+	SetISCSIConnector(fake)
+	err := HandleISCSIExtend("192.168.1.30", "1")
+	assert.Nil(t, err)
+}
+
 func TestBeautifyVolumeInfo(t *testing.T) {
 	info := connector.VolumeInfo{Paths: []string{"/dev/disk/by-path/xxxxxxxxxxxxxxxx", "/dev/disk/by-path/yyyyyyyyyyyyyyyyyy"},
 		MultipathId: "351160160b6e00e5a50060160b6e00e5a", Wwn: "351160160b6e00e5a50060160b6e00e5a",
 		Multipath: "/dev/mapper/351160160b6e00e5a50060160b6e00e5a"}
 	BeautifyVolumeInfo(info)
 }
+
+func TestValidateLunId(t *testing.T) {
+	lunids, err := ValidateLunId([]string{"a", "b"})
+	assert.Error(t, err)
+	assert.Len(t, lunids, 0)
+}

connector/iscsi.go

@@ -178,14 +178,22 @@ func (iscsi *ISCSIConnector) filterTargets(sessions []model.ISCSISession, connec
 
 // Update the local kernel's size information
 func (iscsi *ISCSIConnector) ExtendVolume(connectionProperty ConnectionProperty) error {
-
+	var err error
 	paths := iscsi.getVolumePaths(connectionProperty)
-	for _, path := range paths {
-		linux.ExtendDevice(path)
+	paths, _ = goockutil.FilterPath(paths)
+
+	if len(paths) > 0 {
+		// Flush size of each single path
+		for _, path := range paths {
+			linux.ExtendDevice(path)
+		}
+		// Flush size for multipath descriptor
+		mpathId := linux.GetWWN(paths[0])
+		err = linux.ResizeMpath(mpathId)
+	} else {
+		err = fmt.Errorf("Unable to find any path to extend.")
 	}
-	mpathId := linux.GetWWN(paths[0])
-	linux.ResizeMpath(mpathId)
-	return nil
+	return err
 }
 
 // Attach the volume from the remote to the local

connector/iscsi_test.go

@@ -127,4 +127,44 @@ func TestISCSIConnector_DisconnectVolume(t *testing.T) {
 
 func TestISCSIConnector_DisconnectVolumeNoMultipath(t *testing.T) {
 	//TODO(peter) wait for test data feeding feature
+
+}
+
+func TestISCSIConnector_ExtendVolume(t *testing.T) {
+	linux.SetExecutor(test.NewMockExecutor())
+	model.SetExecutor(test.NewMockExecutor())
+	iscsi := NewISCSIConnector()
+	fakeProperty := ConnectionProperty{}
+	fakeProperty.TargetIqns = []string{
+		"iqn.1992-04.com.emc:cx.apm00141313414.a17",
+		"iqn.1992-04.com.emc:cx.apm00141313414.b17",
+	}
+	fakeProperty.TargetPortals = []string{
+		"10.168.7.14:3260",
+		"10.168.7.15:3260",
+	}
+	fakeProperty.TargetLuns = []int{
+		19,
+		19,
+	}
+	err := iscsi.ExtendVolume(fakeProperty)
+	assert.Nil(t, err)
+}
+
+func TestISCSIConnector_ExtendVolumeNoAnyPath(t *testing.T) {
+	linux.SetExecutor(test.NewMockExecutor())
+	model.SetExecutor(test.NewMockExecutor())
+	iscsi := NewISCSIConnector()
+	fakeProperty := ConnectionProperty{}
+	fakeProperty.TargetIqns = []string{
+		"iqn.1992-04.com.emc:cx.apm55555555555.b17",
+	}
+	fakeProperty.TargetPortals = []string{
+		"10.168.7.199:3260",
+	}
+	fakeProperty.TargetLuns = []int{
+		19,
+	}
+	err := iscsi.ExtendVolume(fakeProperty)
+	assert.Error(t, err)
 }

linux/scsi.go

@@ -18,6 +18,7 @@ package linux
 import (
 	"fmt"
 	"github.com/peter-wangxu/goock/model"
+	"github.com/sirupsen/logrus"
 	"path/filepath"
 	"regexp"
 	"strconv"
@@ -68,9 +69,9 @@ func CheckReadWrite(path string, wwn string) bool {
 func GetDeviceSize(path string) int {
 	output, err := executor.Command("blockdev", "--getsize64", path).CombinedOutput()
 	if nil != err {
-		log.WithError(err).Warn("Unable to get size of device %s", path)
+		log.WithError(err).Warnf("Unable to get size of device %s", path)
 	}
-	trimmed := strings.Trim(string(output), " ")
+	trimmed := strings.TrimSpace(string(output))
 	if trimmed == "" {
 		return 0
 	}
@@ -99,32 +100,68 @@ func ScanSCSIBus(path string, content string) error {
 // Use echo 1 > /sys/block/%s/device/delete to force delete the device
 func RemoveSCSIDevice(path string) {
 	if strings.Contains(path, string(filepath.Separator)) {
+		// Before remove the device from host, flush buffers to disk
+		FlushDeviceIO(path)
 		// Get the file name from the full path, ex : /dev/sdb -> sdb
 		_, path = filepath.Split(path)
+	} else {
+		FlushDeviceIO(fmt.Sprintf("/dev/%s", path))
 	}
+
 	path = fmt.Sprintf("/sys/block/%s/device/delete", path)
 	cmd := executor.Command("tee", "-a", path)
 	cmd.SetStdin(strings.NewReader("1"))
 	out, _ := cmd.CombinedOutput()
 	log.Debugf("Remove device [%s] with output : [%s]", path, out)
 }
 
-//TODO Add echo_scsi_command for use
-func ExtendDevice(path string) error {
+// path = "/dev/sdb" or "
+// "/dev/disk/by-path/ip-10.244.213.177:3260-iscsi-iqn.1992-04.com.emc:cx.fnm00150600267.a0-lun-10"
+func FlushDeviceIO(path string) error {
+	cmd := executor.Command("blockdev", "-v", "--flushbufs", path)
+	_, err := cmd.CombinedOutput()
+	return err
+}
+
+// Commands example:
+// echo 1 > /sys/bus/scsi/drivers/sd/9:0:0:6/rescan
+func ExtendDevice(path string) (int, error) {
 
-	return nil
+	info, err := GetDeviceInfo(path)
+	if err != nil {
+		return 0, fmt.Errorf("Unable to extend device %s, device info not found", path)
+	}
+	deviceId := info.GetDeviceIdentifier()
+	rescanPath := fmt.Sprintf("/sys/bus/scsi/drivers/sd/%s/rescan", deviceId)
+	deviceSize := GetDeviceSize(path)
+	log.WithFields(logrus.Fields{
+		"path":     path,
+		"device":   deviceId,
+		"original": deviceSize,
+	}).Debug("Begin to extend the device.")
+
+	cmd := executor.Command("tee", "-a", rescanPath)
+	cmd.SetStdin(strings.NewReader("1"))
+	out, err := cmd.CombinedOutput()
+	newSize := GetDeviceSize(path)
+	log.WithFields(logrus.Fields{
+		"path":    path,
+		"newSize": newSize,
+		"output":  out,
+	}).Info("Extend device finished.")
+	return newSize, err
 }
 
 // output:
 // sudo sg_scan /dev/disk/by-path/pci-0000:05:00.1-fc-0x5006016d09200925-lun-0
 // /dev/disk/by-path/pci-0000:05:00.1-fc-0x5006016d09200925-lun-0: scsi9 channel=0 id=0 lun=0 [em]
-func GetDeviceInfo(path string) model.DeviceInfo {
+func GetDeviceInfo(path string) (model.DeviceInfo, error) {
 	devices := model.NewDeviceInfo(path)
 	if len(devices) <= 0 {
 		log.Warn("Unable to get device info for device ", path)
-		return model.DeviceInfo{}
+		return model.DeviceInfo{}, fmt.Errorf("Unable to get device info.")
 	}
-	return devices[0]
+	return devices[0], nil
 }
 
 // TODO Add the commands here

linux/scsi_test.go

@@ -57,13 +57,42 @@ func TestRemoveSCSIDeviceWithPath(t *testing.T) {
 	SetExecutor(test.NewMockExecutor())
 	RemoveSCSIDevice("/dev/sdx")
 }
+
+func TestFlushDeviceIO(t *testing.T) {
+	SetExecutor(test.NewMockExecutor())
+	err := FlushDeviceIO("/dev/sdm")
+	assert.Nil(t, err)
+}
+
+func TestExtendDevice(t *testing.T) {
+	SetExecutor(test.NewMockExecutor())
+	model.SetExecutor(test.NewMockExecutor())
+
+	newSize, err := ExtendDevice("/dev/sdg")
+
+	assert.Nil(t, err)
+	assert.Equal(t, 2147483648, newSize)
+
+}
+
+func TestExtendDeviceNoDeviceInfo(t *testing.T) {
+	model.SetExecutor(test.NewMockExecutor())
+	_, err := ExtendDevice("/dev/unknown")
+	assert.Error(t, err)
+}
+
 func TestGetDeviceInfo(t *testing.T) {
 	model.SetExecutor(test.NewMockExecutor())
-	info := GetDeviceInfo("/dev/sdb")
+	info, err := GetDeviceInfo("/dev/sdb")
+	assert.Nil(t, err)
 	assert.Equal(t, "/dev/sdb", info.Device)
+	assert.Equal(t, "scsi0", info.Host)
+	assert.Equal(t, 0, info.GetHostId())
+	assert.Equal(t, "0:1:0:0", info.GetDeviceIdentifier())
 }
 func TestGetDeviceInfoNotFound(t *testing.T) {
 	model.SetExecutor(test.NewMockExecutor())
-	info := GetDeviceInfo("/dev/sdx")
+	info, err := GetDeviceInfo("/dev/sdx")
+	assert.Error(t, err)
 	assert.Equal(t, "", info.Device)
 }

model/model.go

@@ -16,6 +16,7 @@ limitations under the License.
 package model
 
 import (
+	"fmt"
 	"github.com/peter-wangxu/goock/exec"
 	"github.com/sirupsen/logrus"
 	"reflect"
@@ -481,11 +482,9 @@ type DeviceInfo struct {
 	params  []string
 	Device  string
 	Host    string
-	// numbered host
-	HostNumber int
-	Channel    int
-	Target     int
-	Lun        int
+	Channel int
+	Target  int
+	Lun     int
 }
 
 func (d *DeviceInfo) GetPattern() interface{} {
@@ -536,6 +535,16 @@ func (d *DeviceInfo) Parse() []DeviceInfo {
 	return list
 }
 
+func (d *DeviceInfo) GetHostId() int {
+	i, _ := strconv.Atoi(strings.Replace(d.Host, "scsi", "", -1))
+	return i
+}
+
+// Returns a string with format: <host>:<channel>:<Target>:<Lun>
+func (d *DeviceInfo) GetDeviceIdentifier() string {
+	return fmt.Sprintf("%d:%d:%d:%d", d.GetHostId(), d.Channel, d.Target, d.Lun)
+}
+
 func NewDeviceInfo(path string) []DeviceInfo {
 	rS := &DeviceInfo{parser: &LineParser{Delimiter: "\\n+"}, params: []string{path}}
 	return rS.Parse()