diff --git a/cmd/main.go b/cmd/main.go index 8b9603df..7883b9c2 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -66,16 +66,20 @@ func main() { if useKvm { log.Println("Creating KVM server.") - frontendServer := frontend.NewServerWithSubsystemListener(jsonRPC, - kvm.NewVfiouserSubsystemListener(ctrlrDir)) + frontendServer := frontend.NewCustomizedServer(jsonRPC, + kvm.NewVfiouserSubsystemListener(ctrlrDir), + frontend.NewVhostUserBlkTransport(), + ) kvmServer := kvm.NewServer(frontendServer, qmpAddress, ctrlrDir, buses) pb.RegisterFrontendNvmeServiceServer(s, kvmServer) pb.RegisterFrontendVirtioBlkServiceServer(s, kvmServer) pb.RegisterFrontendVirtioScsiServiceServer(s, kvmServer) } else { - frontendServer := frontend.NewServerWithSubsystemListener(jsonRPC, - frontend.NewTCPSubsystemListener(tcpTransportListenAddr)) + frontendServer := frontend.NewCustomizedServer(jsonRPC, + frontend.NewTCPSubsystemListener(tcpTransportListenAddr), + frontend.NewVhostUserBlkTransport(), + ) pb.RegisterFrontendNvmeServiceServer(s, frontendServer) pb.RegisterFrontendVirtioBlkServiceServer(s, frontendServer) pb.RegisterFrontendVirtioScsiServiceServer(s, frontendServer) diff --git a/pkg/frontend/blk.go b/pkg/frontend/blk.go index c71bbab7..9a902fdf 100644 --- a/pkg/frontend/blk.go +++ b/pkg/frontend/blk.go @@ -33,6 +33,44 @@ func sortVirtioBlks(virtioBlks []*pb.VirtioBlk) { }) } +type vhostUserBlkTransport struct{} + +// NewVhostUserBlkTransport creates objects to handle vhost user blk transport +// specifics +func NewVhostUserBlkTransport() VirtioBlkTransport { + return &vhostUserBlkTransport{} +} + +func (v vhostUserBlkTransport) CreateParams(virtioBlk *pb.VirtioBlk) (any, error) { + v.verifyTransportSpecificParams(virtioBlk) + + resourceID := path.Base(virtioBlk.Name) + return spdk.VhostCreateBlkControllerParams{ + Ctrlr: resourceID, + DevName: virtioBlk.VolumeNameRef, + }, nil +} + +func (v vhostUserBlkTransport) DeleteParams(virtioBlk *pb.VirtioBlk) (any, error) { + v.verifyTransportSpecificParams(virtioBlk) + + resourceID := path.Base(virtioBlk.Name) + return spdk.VhostDeleteControllerParams{ + Ctrlr: resourceID, + }, nil +} + +func (v vhostUserBlkTransport) verifyTransportSpecificParams(virtioBlk *pb.VirtioBlk) { + pcieID := virtioBlk.PcieId + if pcieID.PortId.Value != 0 { + log.Printf("WARNING: only port 0 is supported for vhost user. Will be replaced with an error") + } + + if pcieID.VirtualFunction.Value != 0 { + log.Println("WARNING: virtual functions are not supported for vhost user. Will be replaced with an error") + } +} + // CreateVirtioBlk creates a Virtio block device func (s *Server) CreateVirtioBlk(_ context.Context, in *pb.CreateVirtioBlkRequest) (*pb.VirtioBlk, error) { log.Printf("CreateVirtioBlk: Received from client: %v", in) @@ -66,12 +104,14 @@ func (s *Server) CreateVirtioBlk(_ context.Context, in *pb.CreateVirtioBlkReques return controller, nil } // not found, so create a new one - params := spdk.VhostCreateBlkControllerParams{ - Ctrlr: resourceID, - DevName: in.VirtioBlk.VolumeNameRef, + params, err := s.Virt.transport.CreateParams(in.VirtioBlk) + if err != nil { + log.Printf("error: failed to create params for spdk call: %v", err) + return nil, status.Error(codes.InvalidArgument, err.Error()) } + var result spdk.VhostCreateBlkControllerResult - err := s.rpc.Call("vhost_create_blk_controller", ¶ms, &result) + err = s.rpc.Call("vhost_create_blk_controller", ¶ms, &result) if err != nil { log.Printf("error: %v", err) return nil, err @@ -111,19 +151,22 @@ func (s *Server) DeleteVirtioBlk(_ context.Context, in *pb.DeleteVirtioBlkReques log.Printf("error: %v", err) return nil, err } - resourceID := path.Base(controller.Name) - params := spdk.VhostDeleteControllerParams{ - Ctrlr: resourceID, + + params, err := s.Virt.transport.DeleteParams(controller) + if err != nil { + log.Printf("error: failed to create params for spdk call: %v. Inconsistent entry in db?", err) + return nil, status.Error(codes.Internal, err.Error()) } + var result spdk.VhostDeleteControllerResult - err := s.rpc.Call("vhost_delete_controller", ¶ms, &result) + err = s.rpc.Call("vhost_delete_controller", ¶ms, &result) if err != nil { log.Printf("error: %v", err) return nil, err } log.Printf("Received from SPDK: %v", result) if !result { - msg := fmt.Sprintf("Could not delete virtio-blk: %s", resourceID) + msg := fmt.Sprintf("Could not delete virtio-blk: %s", in.Name) log.Print(msg) return nil, status.Errorf(codes.InvalidArgument, msg) } diff --git a/pkg/frontend/blk_test.go b/pkg/frontend/blk_test.go index 1b0eb35a..663cd012 100644 --- a/pkg/frontend/blk_test.go +++ b/pkg/frontend/blk_test.go @@ -99,6 +99,30 @@ func TestFrontEnd_CreateVirtioBlk(t *testing.T) { errCode: codes.Unknown, errMsg: fmt.Sprintf("segment '%s': not a valid DNS name", "-ABC-DEF"), }, + // "virtual functions are not supported for vhost user": { + // id: testVirtioCtrlID, + // in: &pb.VirtioBlk{ + // PcieId: &pb.PciEndpoint{PhysicalFunction: wrapperspb.Int32(42), VirtualFunction: wrapperspb.Int32(1), PortId: wrapperspb.Int32(0)}, + // VolumeNameRef: "Malloc42", + // MaxIoQps: 1, + // }, + // out: nil, + // spdk: []string{}, + // errCode: codes.InvalidArgument, + // errMsg: "virtual functions are not supported for vhost user", + // }, + // "only port 0 is supported for vhost user": { + // id: testVirtioCtrlID, + // in: &pb.VirtioBlk{ + // PcieId: &pb.PciEndpoint{PhysicalFunction: wrapperspb.Int32(42), VirtualFunction: wrapperspb.Int32(0), PortId: wrapperspb.Int32(1)}, + // VolumeNameRef: "Malloc42", + // MaxIoQps: 1, + // }, + // out: nil, + // spdk: []string{}, + // errCode: codes.InvalidArgument, + // errMsg: "only port 0 is supported", + // }, } for testName, tt := range tests { @@ -598,6 +622,9 @@ func TestFrontEnd_StatsVirtioBlk(t *testing.T) { } func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { + pfIndex := 0 + // vfIndex := 1 + tests := map[string]struct { in string out *emptypb.Empty @@ -605,6 +632,7 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { errCode codes.Code errMsg string missing bool + pfVf int }{ "valid request with invalid SPDK response": { testVirtioCtrlID, @@ -613,6 +641,7 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { codes.InvalidArgument, fmt.Sprintf("Could not delete virtio-blk: %s", testVirtioCtrlID), false, + pfIndex, }, "valid request with empty SPDK response": { testVirtioCtrlID, @@ -621,6 +650,7 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { codes.Unknown, fmt.Sprintf("vhost_delete_controller: %v", "EOF"), false, + pfIndex, }, "valid request with ID mismatch SPDK response": { testVirtioCtrlID, @@ -629,6 +659,7 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { codes.Unknown, fmt.Sprintf("vhost_delete_controller: %v", "json response ID mismatch"), false, + pfIndex, }, "valid request with error code from SPDK response": { testVirtioCtrlID, @@ -637,6 +668,7 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { codes.Unknown, fmt.Sprintf("vhost_delete_controller: %v", "json response error: myopierr"), false, + pfIndex, }, "valid request with valid SPDK response": { testVirtioCtrlID, @@ -645,6 +677,7 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { codes.OK, "", false, + pfIndex, }, "valid request with unknown key": { "unknown-id", @@ -653,6 +686,7 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { codes.NotFound, fmt.Sprintf("unable to find key %v", "unknown-id"), false, + pfIndex, }, "unknown key with missing allowed": { "unknown-id", @@ -661,6 +695,7 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { codes.OK, "", true, + pfIndex, }, "malformed name": { "-ABC-DEF", @@ -669,6 +704,7 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { codes.Unknown, fmt.Sprintf("segment '%s': not a valid DNS name", "-ABC-DEF"), false, + pfIndex, }, "no required field": { "", @@ -677,7 +713,17 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { codes.Unknown, "missing required field: name", false, - }, + pfIndex, + }, + // "entry with invalid address in db": { + // testVirtioCtrlID, + // &emptypb.Empty{}, + // []string{}, + // codes.Internal, + // "virtual functions are not supported for vhost user", + // false, + // vfIndex, + // }, } // run tests @@ -686,7 +732,9 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { testEnv := createTestEnvironment(tt.spdk) defer testEnv.Close() - testEnv.opiSpdkServer.Virt.BlkCtrls[testVirtioCtrlID] = &testVirtioCtrl + testEnv.opiSpdkServer.Virt.BlkCtrls[testVirtioCtrlID] = server.ProtoClone(&testVirtioCtrl) + testEnv.opiSpdkServer.Virt.BlkCtrls[testVirtioCtrlID].PcieId.VirtualFunction = + wrapperspb.Int32(int32(tt.pfVf)) request := &pb.DeleteVirtioBlkRequest{Name: tt.in, AllowMissing: tt.missing} response, err := testEnv.client.DeleteVirtioBlk(testEnv.ctx, request) diff --git a/pkg/frontend/frontend.go b/pkg/frontend/frontend.go index 8ec852dc..30bed80b 100644 --- a/pkg/frontend/frontend.go +++ b/pkg/frontend/frontend.go @@ -26,11 +26,19 @@ type NvmeParameters struct { subsysListener SubsystemListener } +// VirtioBlkTransport interface is used to provide SPDK call params to create/delete +// virtio-blk controllers depending on used transport type. +type VirtioBlkTransport interface { + CreateParams(virtioBlk *pb.VirtioBlk) (any, error) + DeleteParams(virtioBlk *pb.VirtioBlk) (any, error) +} + // VirtioParameters contains all VirtIO related structures type VirtioParameters struct { BlkCtrls map[string]*pb.VirtioBlk ScsiCtrls map[string]*pb.VirtioScsiController ScsiLuns map[string]*pb.VirtioScsiLun + transport VirtioBlkTransport } // Server contains frontend related OPI services @@ -48,6 +56,9 @@ type Server struct { // NewServer creates initialized instance of FrontEnd server communicating // with provided jsonRPC func NewServer(jsonRPC spdk.JSONRPC) *Server { + if jsonRPC == nil { + log.Panic("nil for JSONRPC is not allowed") + } return &Server{ rpc: jsonRPC, Nvme: NvmeParameters{ @@ -60,18 +71,29 @@ func NewServer(jsonRPC spdk.JSONRPC) *Server { BlkCtrls: make(map[string]*pb.VirtioBlk), ScsiCtrls: make(map[string]*pb.VirtioScsiController), ScsiLuns: make(map[string]*pb.VirtioScsiLun), + transport: NewVhostUserBlkTransport(), }, Pagination: make(map[string]int), } } -// NewServerWithSubsystemListener creates initialized instance of FrontEnd server communicating -// with provided jsonRPC and externally created SubsystemListener instead default one. -func NewServerWithSubsystemListener(jsonRPC spdk.JSONRPC, sysListener SubsystemListener) *Server { +// NewCustomizedServer creates initialized instance of FrontEnd server communicating +// with provided jsonRPC and externally created SubsystemListener and VirtioBlkTransport +func NewCustomizedServer( + jsonRPC spdk.JSONRPC, + sysListener SubsystemListener, + virtioBlkTransport VirtioBlkTransport, +) *Server { if sysListener == nil { log.Panic("nil for SubsystemListener is not allowed") } + + if virtioBlkTransport == nil { + log.Panic("nil for VirtioBlkTransport is not allowed") + } + server := NewServer(jsonRPC) server.Nvme.subsysListener = sysListener + server.Virt.transport = virtioBlkTransport return server } diff --git a/pkg/frontend/frontend_test.go b/pkg/frontend/frontend_test.go index 0946a604..5d94b3f0 100644 --- a/pkg/frontend/frontend_test.go +++ b/pkg/frontend/frontend_test.go @@ -10,6 +10,7 @@ import ( "log" "net" "os" + "testing" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" @@ -89,3 +90,57 @@ func dialer(opiSpdkServer *Server) func(context.Context, string) (net.Conn, erro return listener.Dial() } } + +func TestFrontEnd_NewCustomizedServer(t *testing.T) { + validJSONRPC := spdk.NewSpdkJSONRPC("/some/path") + validSubsyListener := NewTCPSubsystemListener("10.10.10.10:1234") + validVirtioBLkTransport := NewVhostUserBlkTransport() + + tests := map[string]struct { + jsonRPC spdk.JSONRPC + subsysListener SubsystemListener + virtioBlkTransport VirtioBlkTransport + wantPanic bool + }{ + "nil json rpc": { + jsonRPC: nil, + subsysListener: validSubsyListener, + virtioBlkTransport: validVirtioBLkTransport, + wantPanic: true, + }, + "nil subsystem listener": { + jsonRPC: validJSONRPC, + subsysListener: nil, + virtioBlkTransport: validVirtioBLkTransport, + wantPanic: true, + }, + "nil virtio blk transport": { + jsonRPC: validJSONRPC, + subsysListener: validSubsyListener, + virtioBlkTransport: nil, + wantPanic: true, + }, + "all valid arguments": { + jsonRPC: validJSONRPC, + subsysListener: validSubsyListener, + virtioBlkTransport: validVirtioBLkTransport, + wantPanic: false, + }, + } + + for testName, tt := range tests { + t.Run(testName, func(t *testing.T) { + defer func() { + r := recover() + if (r != nil) != tt.wantPanic { + t.Errorf("NewCustomizedServer() recover = %v, wantPanic = %v", r, tt.wantPanic) + } + }() + + server := NewCustomizedServer(tt.jsonRPC, tt.subsysListener, tt.virtioBlkTransport) + if server == nil && !tt.wantPanic { + t.Error("expected non nil server or panic") + } + }) + } +}