refactor(kvm): transition to spdk client

Signed-off-by: Artsiom Koltun <[email protected]>

cmd/main.go

@@ -13,8 +13,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/opiproject/gospdk/spdk"
-
 	"github.com/opiproject/opi-spdk-bridge/pkg/backend"
 	"github.com/opiproject/opi-spdk-bridge/pkg/frontend"
 	"github.com/opiproject/opi-spdk-bridge/pkg/kvm"
@@ -140,7 +138,6 @@ func runGrpcServer(grpcPort int, useKvm bool, store gokv.Store, spdkAddress, qmp
 	)
 	s := grpc.NewServer(serverOptions...)
 
-	jsonRPC := spdk.NewClient(spdkAddress)
 	protocol := client.TCP
 	if _, _, err := net.SplitHostPort(spdkAddress); err != nil {
 		protocol = client.Unix
@@ -164,7 +161,7 @@ func runGrpcServer(grpcPort int, useKvm bool, store gokv.Store, spdkAddress, qmp
 			store,
 			map[pb.NvmeTransportType]frontend.NvmeTransport{
 				pb.NvmeTransportType_NVME_TRANSPORT_TYPE_TCP:  frontend.NewNvmeTCPTransport(spdkClient),
-				pb.NvmeTransportType_NVME_TRANSPORT_TYPE_PCIE: kvm.NewNvmeVfiouserTransport(ctrlrDir, jsonRPC),
+				pb.NvmeTransportType_NVME_TRANSPORT_TYPE_PCIE: kvm.NewNvmeVfiouserTransport(ctrlrDir, spdkClient),
 			},
 			frontend.NewVhostUserBlkTransport(),
 		)

pkg/kvm/blk.go

@@ -10,6 +10,8 @@ import (
 	"path/filepath"
 
 	pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/types/known/emptypb"
 )
 
@@ -29,7 +31,7 @@ func (s *Server) CreateVirtioBlk(ctx context.Context, in *pb.CreateVirtioBlkRequ
 	out, err := s.Server.CreateVirtioBlk(ctx, in)
 	if err != nil {
 		log.Println("Error running cmd on opi-spdk bridge:", err)
-		return out, err
+		return out, status.Error(codes.Unknown, err.Error())
 	}
 
 	mon, err := newMonitor(s.qmpAddress, s.protocol, s.timeout, s.pollDevicePresenceStep)

pkg/kvm/blk_test.go

@@ -9,8 +9,8 @@ import (
 	"testing"
 
 	"github.com/philippgille/gokv/gomap"
+	"github.com/spdk/spdk/go/rpc/client"
 
-	"github.com/opiproject/gospdk/spdk"
 	pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go"
 	"github.com/opiproject/opi-spdk-bridge/pkg/frontend"
 	"github.com/opiproject/opi-spdk-bridge/pkg/utils"
@@ -42,7 +42,7 @@ func TestCreateVirtioBlk(t *testing.T) {
 	t.Cleanup(checkGlobalTestProtoObjectsNotChanged(t, t.Name()))
 
 	tests := map[string]struct {
-		jsonRPC              spdk.JSONRPC
+		jsonRPC              client.IClient
 		errCode              codes.Code
 		errMsg               string
 		nonDefaultQmpAddress string
@@ -65,8 +65,8 @@ func TestCreateVirtioBlk(t *testing.T) {
 		"spdk failed to create virtio-blk": {
 			in:      testCreateVirtioBlkRequest,
 			jsonRPC: alwaysFailingJSONRPC,
-			errCode: status.Convert(errStub).Code(),
-			errMsg:  status.Convert(errStub).Message(),
+			errCode: codes.Unknown,
+			errMsg:  "vhost_create_blk_controller: stub error",
 		},
 		"qemu chardev add failed": {
 			in:      testCreateVirtioBlkRequest,
@@ -207,7 +207,7 @@ func TestCreateVirtioBlk(t *testing.T) {
 func TestDeleteVirtioBlk(t *testing.T) {
 	t.Cleanup(checkGlobalTestProtoObjectsNotChanged(t, t.Name()))
 	tests := map[string]struct {
-		jsonRPC              spdk.JSONRPC
+		jsonRPC              client.IClient
 		errCode              codes.Code
 		errMsg               string
 		nonDefaultQmpAddress string

pkg/kvm/kvm_test.go

@@ -5,7 +5,7 @@
 package kvm
 
 import (
-	"context"
+	"errors"
 	"fmt"
 	"log"
 	"net"
@@ -17,18 +17,16 @@ import (
 	"testing"
 	"time"
 
-	"github.com/opiproject/gospdk/spdk"
+	"github.com/opiproject/opi-spdk-bridge/pkg/spdk"
 	"github.com/opiproject/opi-spdk-bridge/pkg/utils"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
+	"github.com/spdk/spdk/go/rpc/client"
 )
 
 const qmpID = `"id":"`
 
 var (
-	errStub                 = status.Error(codes.Internal, "stub error")
 	alwaysSuccessfulJSONRPC = &stubJSONRRPC{err: nil}
-	alwaysFailingJSONRPC    = &stubJSONRRPC{err: errStub}
+	alwaysFailingJSONRPC    = &stubJSONRRPC{err: errors.New("stub error")}
 
 	genericQmpError = `{"error": {"class": "GenericError", "desc": "some error"}}` + "\n"
 	genericQmpOk    = `{"return": {}}` + "\n"
@@ -53,49 +51,27 @@ type stubJSONRRPC struct {
 }
 
 // build time check that struct implements interface
-var _ spdk.JSONRPC = (*stubJSONRRPC)(nil)
+var _ client.IClient = (*stubJSONRRPC)(nil)
 
-func (s *stubJSONRRPC) GetID() uint64 {
-	return 0
-}
-
-func (s *stubJSONRRPC) StartUnixListener() net.Listener {
-	return nil
-}
-
-func (s *stubJSONRRPC) GetVersion(_ context.Context) string {
-	return ""
-}
+func (s *stubJSONRRPC) Call(method string, arg any) (*client.Response, error) {
+	s.arg = arg
 
-func (s *stubJSONRRPC) Call(_ context.Context, method string, arg, result interface{}) error {
+	response := &client.Response{}
 	if method == "vhost_create_blk_controller" {
 		if s.err == nil {
-			resultCreateVirtioBLk, ok := result.(*spdk.VhostCreateBlkControllerResult)
-			if !ok {
-				log.Panicf("Unexpected type for virtio-blk device creation result")
-			}
-			*resultCreateVirtioBLk = spdk.VhostCreateBlkControllerResult(true)
+			response.Result = spdk.VhostCreateBlkControllerResult(true)
 		}
 	} else if method == "vhost_delete_controller" {
 		if s.err == nil {
-			resultDeleteVirtioBLk, ok := result.(*spdk.VhostDeleteControllerResult)
-			if !ok {
-				log.Panicf("Unexpected type for virtio-blk device deletion result")
-			}
-			*resultDeleteVirtioBLk = spdk.VhostDeleteControllerResult(true)
+			response.Result = spdk.VhostDeleteControllerResult(true)
 		}
 	} else if method == "nvmf_subsystem_add_listener" || method == "nvmf_subsystem_remove_listener" {
 		if s.err == nil {
-			resultCreateNvmeController, ok := result.(*spdk.NvmfSubsystemAddListenerResult)
-			if !ok {
-				log.Panicf("Unexpected type for add subsystem listener result")
-			}
-			*resultCreateNvmeController = spdk.NvmfSubsystemAddListenerResult(true)
+			response.Result = spdk.NvmfSubsystemAddListenerResult(true)
 		}
 	}
-	s.arg = arg
 
-	return s.err
+	return response, s.err
 }
 
 type mockCall struct {

pkg/kvm/nvme.go

@@ -12,6 +12,8 @@ import (
 	"os"
 	"path/filepath"
 
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/types/known/emptypb"
 
 	pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go"
@@ -55,7 +57,7 @@ func (s *Server) CreateNvmeController(ctx context.Context, in *pb.CreateNvmeCont
 	if err != nil {
 		log.Println("Error running cmd on opi-spdk bridge:", err)
 		_ = deleteControllerDir(s.ctrlrDir, dirName)
-		return out, err
+		return out, status.Error(codes.Unknown, err.Error())
 	}
 	name := out.Name
 

pkg/kvm/nvme_test.go

@@ -14,8 +14,8 @@ import (
 	"testing"
 
 	"github.com/philippgille/gokv/gomap"
+	"github.com/spdk/spdk/go/rpc/client"
 
-	"github.com/opiproject/gospdk/spdk"
 	pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go"
 	"github.com/opiproject/opi-spdk-bridge/pkg/frontend"
 	"github.com/opiproject/opi-spdk-bridge/pkg/utils"
@@ -71,7 +71,7 @@ func TestCreateNvmeController(t *testing.T) {
 	t.Cleanup(checkGlobalTestProtoObjectsNotChanged(t, t.Name()))
 
 	tests := map[string]struct {
-		jsonRPC                       spdk.JSONRPC
+		jsonRPC                       client.IClient
 		nonDefaultQmpAddress          string
 		ctrlrDirExistsBeforeOperation bool
 		ctrlrDirExistsAfterOperation  bool
@@ -101,8 +101,8 @@ func TestCreateNvmeController(t *testing.T) {
 			jsonRPC:                       alwaysFailingJSONRPC,
 			ctrlrDirExistsBeforeOperation: false,
 			ctrlrDirExistsAfterOperation:  false,
-			errCode:                       status.Convert(errStub).Code(),
-			errMsg:                        status.Convert(errStub).Message(),
+			errCode:                       codes.Unknown,
+			errMsg:                        "nvmf_subsystem_add_listener: stub error",
 		},
 		"qemu Nvme controller add failed": {
 			in:                            testCreateNvmeControllerRequest,
@@ -316,7 +316,7 @@ func TestCreateNvmeController(t *testing.T) {
 func TestDeleteNvmeController(t *testing.T) {
 	t.Cleanup(checkGlobalTestProtoObjectsNotChanged(t, t.Name()))
 	tests := map[string]struct {
-		jsonRPC              spdk.JSONRPC
+		jsonRPC              client.IClient
 		nonDefaultQmpAddress string
 
 		ctrlrDirExistsBeforeOperation bool

pkg/kvm/transport.go

@@ -10,24 +10,25 @@ import (
 	"log"
 	"os"
 
-	"github.com/opiproject/gospdk/spdk"
 	pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go"
 	"github.com/opiproject/opi-spdk-bridge/pkg/frontend"
+	"github.com/opiproject/opi-spdk-bridge/pkg/spdk"
 	"github.com/opiproject/opi-spdk-bridge/pkg/utils"
+	"github.com/spdk/spdk/go/rpc/client"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
 
 type nvmeVfiouserTransport struct {
 	ctrlrDir string
-	rpc      spdk.JSONRPC
+	rpc      *spdk.SpdkClientAdapter
 }
 
 // build time check that struct implements interface
 var _ frontend.NvmeTransport = (*nvmeVfiouserTransport)(nil)
 
 // NewNvmeVfiouserTransport creates a new instance of nvmeVfiouserTransport
-func NewNvmeVfiouserTransport(ctrlrDir string, rpc spdk.JSONRPC) frontend.NvmeTransport {
+func NewNvmeVfiouserTransport(ctrlrDir string, spdkClient client.IClient) frontend.NvmeTransport {
 	if ctrlrDir == "" {
 		log.Panicf("ctrlrDir cannot be empty")
 	}
@@ -40,13 +41,13 @@ func NewNvmeVfiouserTransport(ctrlrDir string, rpc spdk.JSONRPC) frontend.NvmeTr
 		log.Panicf("%v is not a directory", ctrlrDir)
 	}
 
-	if rpc == nil {
-		log.Panicf("rpc cannot be nil")
+	if spdkClient == nil {
+		log.Panicf("spdkClient cannot be nil")
 	}
 
 	return &nvmeVfiouserTransport{
 		ctrlrDir: ctrlrDir,
-		rpc:      rpc,
+		rpc:      spdk.NewSpdkClientAdapter(spdkClient),
 	}
 }
 

pkg/kvm/transport_test.go

@@ -11,16 +11,17 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/opiproject/gospdk/spdk"
 	pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go"
+	"github.com/opiproject/opi-spdk-bridge/pkg/spdk"
 	"github.com/opiproject/opi-spdk-bridge/pkg/utils"
+	"github.com/spdk/spdk/go/rpc/client"
 	"google.golang.org/protobuf/types/known/wrapperspb"
 )
 
 func TestNewNvmeVfiouserTransport(t *testing.T) {
 	tests := map[string]struct {
 		ctrlrDir  string
-		rpc       spdk.JSONRPC
+		rpc       client.IClient
 		wantPanic bool
 	}{
 		"valid controller dir": {
@@ -62,7 +63,7 @@ func TestNewNvmeVfiouserTransport(t *testing.T) {
 			gotTransport := NewNvmeVfiouserTransport(tt.ctrlrDir, tt.rpc)
 			wantTransport := &nvmeVfiouserTransport{
 				ctrlrDir: tt.ctrlrDir,
-				rpc:      tt.rpc,
+				rpc:      spdk.NewSpdkClientAdapter(tt.rpc),
 			}
 
 			if !reflect.DeepEqual(gotTransport, wantTransport) {