diff --git a/cmd/main.go b/cmd/main.go index e15bce80..8ac8da0c 100644 --- a/cmd/main.go +++ b/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(), ) diff --git a/pkg/kvm/blk.go b/pkg/kvm/blk.go index 1468a3ea..bb3fbe48 100644 --- a/pkg/kvm/blk.go +++ b/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) diff --git a/pkg/kvm/blk_test.go b/pkg/kvm/blk_test.go index 8f9ed95e..1b159ab6 100644 --- a/pkg/kvm/blk_test.go +++ b/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 diff --git a/pkg/kvm/kvm_test.go b/pkg/kvm/kvm_test.go index 91b0c2fe..26810fb6 100644 --- a/pkg/kvm/kvm_test.go +++ b/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 { diff --git a/pkg/kvm/nvme.go b/pkg/kvm/nvme.go index 33237d1a..6f75ad62 100644 --- a/pkg/kvm/nvme.go +++ b/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 diff --git a/pkg/kvm/nvme_test.go b/pkg/kvm/nvme_test.go index 34e0dbad..05ca0480 100644 --- a/pkg/kvm/nvme_test.go +++ b/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 diff --git a/pkg/kvm/transport.go b/pkg/kvm/transport.go index 2dabf925..6c48c27a 100644 --- a/pkg/kvm/transport.go +++ b/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), } } diff --git a/pkg/kvm/transport_test.go b/pkg/kvm/transport_test.go index c068bdb5..9ef9b789 100644 --- a/pkg/kvm/transport_test.go +++ b/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) {