Feat configurable virtio blk SPDK parameters

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)

pkg/frontend/blk.go

@@ -33,6 +33,44 @@
 	})
 }
 
+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 @@
 		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", &params, &result)
+	err = s.rpc.Call("vhost_create_blk_controller", &params, &result)
 	if err != nil {
 		log.Printf("error: %v", err)
 		return nil, err
@@ -111,19 +151,22 @@
 		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", &params, &result)
+	err = s.rpc.Call("vhost_delete_controller", &params, &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)
 	}

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,13 +622,17 @@ 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
 		spdk    []string
 		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)

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
 }

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")
+			}
+		})
+	}
+}