rpc/dataconn + build: support GOOS={solaris,illumos}

.circleci/config.yml

@@ -52,7 +52,7 @@ jobs:
           paths:
             - "/usr/include/google/protobuf"
             
-      - run: sudo apt update && sudo apt install python3 python3-pip libgirepository1.0-dev
+      - run: sudo apt update && sudo apt install python3 python3-pip libgirepository1.0-dev gawk
       - run: ./lazy.sh devsetup
 
       - run: make zrepl-bin

Makefile

@@ -93,6 +93,7 @@ clean: docs-clean
 .PHONY: bins-all lint test vet zrepl-bin platformtest-bin
 
 BINS_ALL_TARGETS := zrepl-bin platformtest-bin vet lint
+GO_SUPPORTS_ILLUMOS := $(shell $(GO) version | gawk -F '.' '/^go version /{split($$0, comps, " "); split(comps[3], v, "."); if (v[1] == "go1" && v[2] >= 13) { print "illumos"; } else { print "noillumos"; }}')
 bins-all:
 	$(MAKE) $(BINS_ALL_TARGETS) GOOS=freebsd   GOARCH=amd64
 	$(MAKE) $(BINS_ALL_TARGETS) GOOS=freebsd   GOARCH=386
@@ -100,6 +101,14 @@ bins-all:
 	$(MAKE) $(BINS_ALL_TARGETS) GOOS=linux     GOARCH=arm64
 	$(MAKE) $(BINS_ALL_TARGETS) GOOS=linux     GOARCH=386
 	$(MAKE) $(BINS_ALL_TARGETS) GOOS=darwin    GOARCH=amd64
+	$(MAKE) $(BINS_ALL_TARGETS) GOOS=solaris   GOARCH=amd64
+ifeq ($(GO_SUPPORTS_ILLUMOS), illumos)
+	$(MAKE) $(BINS_ALL_TARGETS) GOOS=illumos   GOARCH=amd64
+else ifeq ($(GO_SUPPORTS_ILLUMOS), noillumos)
+	@echo "SKIPPING ILLUMOS BUILD BECAUSE GO VERSION DOESN'T SUPPORT IT"
+else
+	@echo "CANNOT DETERMINE WHETHER GO VERSION SUPPORTS GOOS=illumos"; exit 1
+endif
 
 lint:
 	$(GO_ENV_VARS) $(GOLANGCI_LINT) run ./...

rpc/dataconn/timeoutconn/timeoutconn.go

@@ -14,7 +14,6 @@ import (
 	"sync/atomic"
 	"syscall"
 	"time"
-	"unsafe"
 )
 
 type Wire interface {
@@ -155,25 +154,6 @@ type SyscallConner interface {
 
 var _ SyscallConner = (*net.TCPConn)(nil)
 
-func buildIovecs(buffers net.Buffers) (totalLen int64, vecs []syscall.Iovec) {
-	vecs = make([]syscall.Iovec, 0, len(buffers))
-	for i := range buffers {
-		totalLen += int64(len(buffers[i]))
-		if len(buffers[i]) == 0 {
-			continue
-		}
-
-		v := syscall.Iovec{
-			Base: &buffers[i][0],
-		}
-		// syscall.Iovec.Len has platform-dependent size, thus use SetLen
-		v.SetLen(len(buffers[i]))
-
-		vecs = append(vecs, v)
-	}
-	return totalLen, vecs
-}
-
 // Reads the given buffers full:
 // Think of io.ReadvFull, but for net.Buffers + using the readv syscall.
 //
@@ -184,25 +164,10 @@ func buildIovecs(buffers net.Buffers) (totalLen int64, vecs []syscall.Iovec) {
 // then + io.EOF is returned. This behavior is different to io.ReadFull
 // which returns io.ErrUnexpectedEOF.
 func (c Conn) ReadvFull(buffers net.Buffers) (n int64, err error) {
-	totalLen, iovecs := buildIovecs(buffers)
+	return c.readv(buffers)
-	if debugReadvNoShortReadsAssertEnable {
-		defer debugReadvNoShortReadsAssert(totalLen, n, err)
-	}
-	scc, ok := c.Wire.(SyscallConner)
-	if !ok {
-		return c.readvFallback(buffers)
-	}
-	raw, err := scc.SyscallConn()
-	if err == SyscallConnNotSupported {
-		return c.readvFallback(buffers)
-	}
-	if err != nil {
-		return 0, err
-	}
-	n, err = c.readv(raw, iovecs)
-	return
 }
 
+// invoked by c.readv if readv system call cannot be used
 func (c Conn) readvFallback(nbuffers net.Buffers) (n int64, err error) {
 	buffers := [][]byte(nbuffers)
 	for i := range buffers {
@@ -226,91 +191,3 @@ func (c Conn) readvFallback(nbuffers net.Buffers) (n int64, err error) {
 	}
 	return n, nil
 }
-
-func (c Conn) readv(rawConn syscall.RawConn, iovecs []syscall.Iovec) (n int64, err error) {
-	for len(iovecs) > 0 {
-		if err := c.renewReadDeadline(); err != nil {
-			return n, err
-		}
-		oneN, oneErr := c.doOneReadv(rawConn, &iovecs)
-		n += oneN
-		if netErr, ok := oneErr.(net.Error); ok && netErr.Timeout() && oneN > 0 { // TODO likely not working
-			continue
-		} else if oneErr == nil && oneN > 0 {
-			continue
-		} else {
-			return n, oneErr
-		}
-	}
-	return n, nil
-}
-
-func (c Conn) doOneReadv(rawConn syscall.RawConn, iovecs *[]syscall.Iovec) (n int64, err error) {
-	rawReadErr := rawConn.Read(func(fd uintptr) (done bool) {
-		// iovecs, n and err must not be shadowed!
-
-		// NOTE: unsafe.Pointer safety rules
-		// 		https://tip.golang.org/pkg/unsafe/#Pointer
-		//
-		//		(4) Conversion of a Pointer to a uintptr when calling syscall.Syscall.
-		// 		...
-		//		uintptr() conversions must appear within the syscall.Syscall argument list.
-		//      (even though we are not the escape analysis Likely not )
-		thisReadN, _, errno := syscall.Syscall(
-			syscall.SYS_READV,
-			fd,
-			uintptr(unsafe.Pointer(&(*iovecs)[0])),
-			uintptr(len(*iovecs)),
-		)
-		if thisReadN == ^uintptr(0) {
-			if errno == syscall.EAGAIN {
-				return false
-			}
-			err = syscall.Errno(errno)
-			return true
-		}
-		if int(thisReadN) < 0 {
-			panic("unexpected return value")
-		}
-		n += int64(thisReadN) // TODO check overflow
-
-		// shift iovecs forward
-		for left := int(thisReadN); left > 0; {
-			// conversion to uint does not change value, see TestIovecLenFieldIsMachineUint, and left > 0
-			thisIovecConsumedCompletely := uint((*iovecs)[0].Len) <= uint(left)
-			if thisIovecConsumedCompletely {
-				// Update left, cannot go below 0 due to
-				// a) definition of thisIovecConsumedCompletely
-				// b) left > 0 due to loop invariant
-				// Convertion .Len to int64 is thus also safe now, because it is < left < INT_MAX
-				left -= int((*iovecs)[0].Len)
-				*iovecs = (*iovecs)[1:]
-			} else {
-				// trim this iovec to remaining length
-
-				// NOTE: unsafe.Pointer safety rules
-				// 		https://tip.golang.org/pkg/unsafe/#Pointer
-				// 		(3) Conversion of a Pointer to a uintptr and back, with arithmetic.
-				// 		...
-				//		Note that both conversions must appear in the same expression,
-				//		with only the intervening arithmetic between them:
-				(*iovecs)[0].Base = (*byte)(unsafe.Pointer(uintptr(unsafe.Pointer((*iovecs)[0].Base)) + uintptr(left)))
-				curVecNewLength := uint((*iovecs)[0].Len) - uint(left) // casts to uint do not change value
-				(*iovecs)[0].SetLen(int(curVecNewLength))              // int and uint have the same size, no change of value
-
-				break // inner
-			}
-		}
-		if thisReadN == 0 {
-			err = io.EOF
-			return true
-		}
-		return true
-	})
-
-	if rawReadErr != nil {
-		err = rawReadErr
-	}
-
-	return n, err
-}

rpc/dataconn/timeoutconn/timeoutconn_debug.go

@@ -1,19 +0,0 @@
-package timeoutconn
-
-import (
-	"fmt"
-	"io"
-)
-
-const debugReadvNoShortReadsAssertEnable = false
-
-func debugReadvNoShortReadsAssert(expectedLen, returnedLen int64, returnedErr error) {
-	readShort := expectedLen != returnedLen
-	if !readShort {
-		return
-	}
-	if returnedErr != io.EOF {
-		return
-	}
-	panic(fmt.Sprintf("ReadvFull short and error is not EOF%v\n", returnedErr))
-}

rpc/dataconn/timeoutconn/timeoutconn_readv_unsupported.go

@@ -0,0 +1,11 @@
+// +build illumos solaris
+
+package timeoutconn
+
+import "net"
+
+func (c Conn) readv(buffers net.Buffers) (n int64, err error) {
+	// Go does not expose the SYS_READV symbol for Solaris / Illumos - do they have it?
+	// Anyhow, use the fallback
+	return c.readvFallback(buffers)
+}

rpc/dataconn/timeoutconn/timoutconn_readv.go

@@ -0,0 +1,133 @@
+// +build !illumos
+// +build !solaris
+
+package timeoutconn
+
+import (
+	"io"
+	"net"
+	"syscall"
+	"unsafe"
+)
+
+func buildIovecs(buffers net.Buffers) (totalLen int64, vecs []syscall.Iovec) {
+	vecs = make([]syscall.Iovec, 0, len(buffers))
+	for i := range buffers {
+		totalLen += int64(len(buffers[i]))
+		if len(buffers[i]) == 0 {
+			continue
+		}
+
+		v := syscall.Iovec{
+			Base: &buffers[i][0],
+		}
+		// syscall.Iovec.Len has platform-dependent size, thus use SetLen
+		v.SetLen(len(buffers[i]))
+
+		vecs = append(vecs, v)
+	}
+	return totalLen, vecs
+}
+
+func (c Conn) readv(buffers net.Buffers) (n int64, err error) {
+
+	scc, ok := c.Wire.(SyscallConner)
+	if !ok {
+		return c.readvFallback(buffers)
+	}
+	rawConn, err := scc.SyscallConn()
+	if err == SyscallConnNotSupported {
+		return c.readvFallback(buffers)
+	}
+	if err != nil {
+		return 0, err
+	}
+
+	_, iovecs := buildIovecs(buffers)
+
+	for len(iovecs) > 0 {
+		if err := c.renewReadDeadline(); err != nil {
+			return n, err
+		}
+		oneN, oneErr := c.doOneReadv(rawConn, &iovecs)
+		n += oneN
+		if netErr, ok := oneErr.(net.Error); ok && netErr.Timeout() && oneN > 0 { // TODO likely not working
+			continue
+		} else if oneErr == nil && oneN > 0 {
+			continue
+		} else {
+			return n, oneErr
+		}
+	}
+	return n, nil
+}
+
+func (c Conn) doOneReadv(rawConn syscall.RawConn, iovecs *[]syscall.Iovec) (n int64, err error) {
+	rawReadErr := rawConn.Read(func(fd uintptr) (done bool) {
+		// iovecs, n and err must not be shadowed!
+
+		// NOTE: unsafe.Pointer safety rules
+		// 		https://tip.golang.org/pkg/unsafe/#Pointer
+		//
+		//		(4) Conversion of a Pointer to a uintptr when calling syscall.Syscall.
+		// 		...
+		//		uintptr() conversions must appear within the syscall.Syscall argument list.
+		//      (even though we are not the escape analysis Likely not )
+		thisReadN, _, errno := syscall.Syscall(
+			syscall.SYS_READV,
+			fd,
+			uintptr(unsafe.Pointer(&(*iovecs)[0])),
+			uintptr(len(*iovecs)),
+		)
+		if thisReadN == ^uintptr(0) {
+			if errno == syscall.EAGAIN {
+				return false
+			}
+			err = syscall.Errno(errno)
+			return true
+		}
+		if int(thisReadN) < 0 {
+			panic("unexpected return value")
+		}
+		n += int64(thisReadN) // TODO check overflow
+
+		// shift iovecs forward
+		for left := int(thisReadN); left > 0; {
+			// conversion to uint does not change value, see TestIovecLenFieldIsMachineUint, and left > 0
+			thisIovecConsumedCompletely := uint((*iovecs)[0].Len) <= uint(left)
+			if thisIovecConsumedCompletely {
+				// Update left, cannot go below 0 due to
+				// a) definition of thisIovecConsumedCompletely
+				// b) left > 0 due to loop invariant
+				// Convertion .Len to int64 is thus also safe now, because it is < left < INT_MAX
+				left -= int((*iovecs)[0].Len)
+				*iovecs = (*iovecs)[1:]
+			} else {
+				// trim this iovec to remaining length
+
+				// NOTE: unsafe.Pointer safety rules
+				// 		https://tip.golang.org/pkg/unsafe/#Pointer
+				// 		(3) Conversion of a Pointer to a uintptr and back, with arithmetic.
+				// 		...
+				//		Note that both conversions must appear in the same expression,
+				//		with only the intervening arithmetic between them:
+				(*iovecs)[0].Base = (*byte)(unsafe.Pointer(uintptr(unsafe.Pointer((*iovecs)[0].Base)) + uintptr(left)))
+				curVecNewLength := uint((*iovecs)[0].Len) - uint(left) // casts to uint do not change value
+				(*iovecs)[0].SetLen(int(curVecNewLength))              // int and uint have the same size, no change of value
+
+				break // inner
+			}
+		}
+		if thisReadN == 0 {
+			err = io.EOF
+			return true
+		}
+		return true
+	})
+
+	if rawReadErr != nil {
+		err = rawReadErr
+	}
+
+	return n, err
+}