From 77c7244cbc9f4af094daeb8080c399fdfc62d9cd Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Wed, 31 May 2023 17:20:54 +0200 Subject: [PATCH 1/7] Fix delete virtio-blk tests. Signed-off-by: Artsiom Koltun --- pkg/frontend/blk.go | 1 + pkg/frontend/blk_test.go | 18 ++++++++---------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/pkg/frontend/blk.go b/pkg/frontend/blk.go index 9afaa04c..d71750cc 100644 --- a/pkg/frontend/blk.go +++ b/pkg/frontend/blk.go @@ -91,6 +91,7 @@ func (s *Server) DeleteVirtioBlk(_ context.Context, in *pb.DeleteVirtioBlkReques log.Printf("Received from SPDK: %v", result) if !result { log.Printf("Could not delete: %v", in) + return nil, spdk.ErrUnexpectedSpdkCallResult } delete(s.Virt.BlkCtrls, controller.Name) return &emptypb.Empty{}, nil diff --git a/pkg/frontend/blk_test.go b/pkg/frontend/blk_test.go index da2cfb5e..86d4bfa9 100644 --- a/pkg/frontend/blk_test.go +++ b/pkg/frontend/blk_test.go @@ -516,8 +516,8 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { testVirtioCtrlID, nil, []string{`{"id":%d,"error":{"code":0,"message":""},"result":false}`}, - codes.InvalidArgument, - fmt.Sprintf("Could not delete NQN:ID %v", "nqn.2022-09.io.spdk:opi3:17"), + status.Convert(spdk.ErrUnexpectedSpdkCallResult).Code(), + status.Convert(spdk.ErrUnexpectedSpdkCallResult).Message(), true, false, }, @@ -587,14 +587,12 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { request := &pb.DeleteVirtioBlkRequest{Name: tt.in, AllowMissing: tt.missing} response, err := testEnv.client.DeleteVirtioBlk(testEnv.ctx, request) - if err != nil { - if er, ok := status.FromError(err); ok { - if er.Code() != tt.errCode { - t.Error("error code: expected", tt.errCode, "received", er.Code()) - } - if er.Message() != tt.errMsg { - t.Error("error message: expected", tt.errMsg, "received", er.Message()) - } + if er, ok := status.FromError(err); ok { + if er.Code() != tt.errCode { + t.Error("error code: expected", tt.errCode, "received", er.Code()) + } + if er.Message() != tt.errMsg { + t.Error("error message: expected", tt.errMsg, "received", er.Message()) } } if reflect.TypeOf(response) != reflect.TypeOf(tt.out) { From 62248c43db6fb8d8f71550019342bc6d4a5c29b0 Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Thu, 1 Jun 2023 09:40:40 +0200 Subject: [PATCH 2/7] Add interface to provide virtio-blk QoS capabilities. Signed-off-by: Artsiom Koltun --- cmd/main.go | 14 ++++++++++---- pkg/frontend/blk.go | 10 ++++++++++ pkg/frontend/blk_test.go | 13 +++++++++++++ pkg/frontend/frontend.go | 22 +++++++++++++++++++--- pkg/frontend/frontend_test.go | 9 ++++++++- pkg/kvm/blk_test.go | 4 ++-- pkg/kvm/kvm_test.go | 6 ++++++ pkg/kvm/nvme_test.go | 4 ++-- 8 files changed, 70 insertions(+), 12 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 7409fc49..421ae007 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -66,16 +66,22 @@ func main() { if useKvm { log.Println("Creating KVM server.") - frontendServer := frontend.NewServerWithSubsystemListener(jsonRPC, - kvm.NewVfiouserSubsystemListener(ctrlrDir)) + frontendServer := frontend.NewServerWithSubsystemListener( + jsonRPC, + frontend.VirtioBlkQosProviderFromMiddleendQosService(middleendServer), + kvm.NewVfiouserSubsystemListener(ctrlrDir), + ) 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.NewServerWithSubsystemListener( + jsonRPC, + frontend.VirtioBlkQosProviderFromMiddleendQosService(middleendServer), + frontend.NewTCPSubsystemListener(tcpTransportListenAddr), + ) 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 d71750cc..bcc88d5f 100644 --- a/pkg/frontend/blk.go +++ b/pkg/frontend/blk.go @@ -23,6 +23,16 @@ import ( "google.golang.org/protobuf/types/known/emptypb" ) +// VirtioBlkQosProvider provides QoS capabilities for virtio-blk +// At the moment it is just a couple of methods from middleend QoS service, but +// it can be changed to less verbose later. +// If client uses VirtioBlkQosProviderFromMiddleendQosService to create an instance, +// the interface can be changed without affecting the client code. +type VirtioBlkQosProvider interface { + CreateQosVolume(context.Context, *pb.CreateQosVolumeRequest) (*pb.QosVolume, error) + DeleteQosVolume(context.Context, *pb.DeleteQosVolumeRequest) (*emptypb.Empty, error) +} + func sortVirtioBlks(virtioBlks []*pb.VirtioBlk) { sort.Slice(virtioBlks, func(i int, j int) bool { return virtioBlks[i].Name < virtioBlks[j].Name diff --git a/pkg/frontend/blk_test.go b/pkg/frontend/blk_test.go index 86d4bfa9..d96da26f 100644 --- a/pkg/frontend/blk_test.go +++ b/pkg/frontend/blk_test.go @@ -7,6 +7,7 @@ package frontend import ( "bytes" + "context" "fmt" "reflect" "strings" @@ -22,6 +23,18 @@ import ( pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" ) +type stubQosProvider struct { + err error +} + +func (p stubQosProvider) CreateQosVolume(context.Context, *pb.CreateQosVolumeRequest) (*pb.QosVolume, error) { + return &pb.QosVolume{}, p.err +} + +func (p stubQosProvider) DeleteQosVolume(context.Context, *pb.DeleteQosVolumeRequest) (*emptypb.Empty, error) { + return &emptypb.Empty{}, p.err +} + var ( testVirtioCtrlID = "virtio-blk-42" testVirtioCtrName = fmt.Sprintf("//storage.opiproject.org/volumes/%s", testVirtioCtrlID) diff --git a/pkg/frontend/frontend.go b/pkg/frontend/frontend.go index 8ec852dc..64e560e7 100644 --- a/pkg/frontend/frontend.go +++ b/pkg/frontend/frontend.go @@ -31,6 +31,8 @@ type VirtioParameters struct { BlkCtrls map[string]*pb.VirtioBlk ScsiCtrls map[string]*pb.VirtioScsiController ScsiLuns map[string]*pb.VirtioScsiLun + + qosProvider VirtioBlkQosProvider } // Server contains frontend related OPI services @@ -47,7 +49,7 @@ type Server struct { // NewServer creates initialized instance of FrontEnd server communicating // with provided jsonRPC -func NewServer(jsonRPC spdk.JSONRPC) *Server { +func NewServer(jsonRPC spdk.JSONRPC, qosProvider VirtioBlkQosProvider) *Server { return &Server{ rpc: jsonRPC, Nvme: NvmeParameters{ @@ -60,6 +62,8 @@ func NewServer(jsonRPC spdk.JSONRPC) *Server { BlkCtrls: make(map[string]*pb.VirtioBlk), ScsiCtrls: make(map[string]*pb.VirtioScsiController), ScsiLuns: make(map[string]*pb.VirtioScsiLun), + + qosProvider: qosProvider, }, Pagination: make(map[string]int), } @@ -67,11 +71,23 @@ func NewServer(jsonRPC spdk.JSONRPC) *Server { // 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 { +func NewServerWithSubsystemListener( + jsonRPC spdk.JSONRPC, qosProvider VirtioBlkQosProvider, sysListener SubsystemListener, +) *Server { if sysListener == nil { log.Panic("nil for SubsystemListener is not allowed") } - server := NewServer(jsonRPC) + server := NewServer(jsonRPC, qosProvider) server.Nvme.subsysListener = sysListener return server } + +// VirtioBlkQosProviderFromMiddleendQosService provides QoS provider based on middleend QoS service +func VirtioBlkQosProviderFromMiddleendQosService( + s pb.MiddleendQosVolumeServiceServer, +) VirtioBlkQosProvider { + if s == nil { + log.Panic("nil middleend QoS service is not allowed") + } + return s +} diff --git a/pkg/frontend/frontend_test.go b/pkg/frontend/frontend_test.go index 2ed80845..2defdc2c 100644 --- a/pkg/frontend/frontend_test.go +++ b/pkg/frontend/frontend_test.go @@ -47,10 +47,17 @@ func (e *testEnv) Close() { } func createTestEnvironment(startSpdkServer bool, spdkResponses []string) *testEnv { + return createTestEnvironmentWithVirtioBlkQosProvider( + startSpdkServer, spdkResponses, stubQosProvider{}) +} + +func createTestEnvironmentWithVirtioBlkQosProvider( + startSpdkServer bool, spdkResponses []string, qosProvider VirtioBlkQosProvider, +) *testEnv { env := &testEnv{} env.testSocket = server.GenerateSocketName("frontend") env.ln, env.jsonRPC = server.CreateTestSpdkServer(env.testSocket, startSpdkServer, spdkResponses) - env.opiSpdkServer = NewServer(env.jsonRPC) + env.opiSpdkServer = NewServer(env.jsonRPC, qosProvider) ctx := context.Background() conn, err := grpc.DialContext(ctx, diff --git a/pkg/kvm/blk_test.go b/pkg/kvm/blk_test.go index 3ec6abe1..722fac8a 100644 --- a/pkg/kvm/blk_test.go +++ b/pkg/kvm/blk_test.go @@ -141,7 +141,7 @@ func TestCreateVirtioBlk(t *testing.T) { for testName, test := range tests { t.Run(testName, func(t *testing.T) { - opiSpdkServer := frontend.NewServer(test.jsonRPC) + opiSpdkServer := frontend.NewServer(test.jsonRPC, stubQosProvider) qmpServer := startMockQmpServer(t, test.mockQmpCalls) defer qmpServer.Stop() qmpAddress := qmpServer.socketPath @@ -226,7 +226,7 @@ func TestDeleteVirtioBlk(t *testing.T) { for testName, test := range tests { t.Run(testName, func(t *testing.T) { - opiSpdkServer := frontend.NewServer(test.jsonRPC) + opiSpdkServer := frontend.NewServer(test.jsonRPC, stubQosProvider) opiSpdkServer.Virt.BlkCtrls[testVirtioBlkID] = proto.Clone(testCreateVirtioBlkRequest.VirtioBlk).(*pb.VirtioBlk) opiSpdkServer.Virt.BlkCtrls[testVirtioBlkID].Name = testVirtioBlkID diff --git a/pkg/kvm/kvm_test.go b/pkg/kvm/kvm_test.go index 039ea97d..72846c84 100644 --- a/pkg/kvm/kvm_test.go +++ b/pkg/kvm/kvm_test.go @@ -18,6 +18,8 @@ import ( "time" "github.com/opiproject/gospdk/spdk" + pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" + "github.com/opiproject/opi-spdk-bridge/pkg/frontend" ) var ( @@ -32,6 +34,10 @@ var ( qmplibTimeout = 250 * time.Millisecond pathRegexpStr = `\/[a-zA-Z\/\-\_0-9]*\/` + + stubQosProvider = frontend.VirtioBlkQosProviderFromMiddleendQosService( + &pb.UnimplementedMiddleendQosVolumeServiceServer{}, + ) ) type stubJSONRRPC struct { diff --git a/pkg/kvm/nvme_test.go b/pkg/kvm/nvme_test.go index 520a6c6b..d50804d1 100644 --- a/pkg/kvm/nvme_test.go +++ b/pkg/kvm/nvme_test.go @@ -274,7 +274,7 @@ func TestCreateNvmeController(t *testing.T) { for testName, test := range tests { t.Run(testName, func(t *testing.T) { - opiSpdkServer := frontend.NewServer(test.jsonRPC) + opiSpdkServer := frontend.NewServer(test.jsonRPC, stubQosProvider) opiSpdkServer.Nvme.Subsystems[testSubsystemID] = &testSubsystem qmpServer := startMockQmpServer(t, test.mockQmpCalls) defer qmpServer.Stop() @@ -402,7 +402,7 @@ func TestDeleteNvmeController(t *testing.T) { for testName, test := range tests { t.Run(testName, func(t *testing.T) { - opiSpdkServer := frontend.NewServer(test.jsonRPC) + opiSpdkServer := frontend.NewServer(test.jsonRPC, stubQosProvider) opiSpdkServer.Nvme.Subsystems[testSubsystemID] = &testSubsystem if !test.noController { opiSpdkServer.Nvme.Controllers[testNvmeControllerID] = From 554aeae25e4b46883b8b4df82319dec9f32c4b08 Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Thu, 1 Jun 2023 09:49:36 +0200 Subject: [PATCH 3/7] Set QoS on virtio-blk creation. Signed-off-by: Artsiom Koltun --- pkg/frontend/blk.go | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/pkg/frontend/blk.go b/pkg/frontend/blk.go index bcc88d5f..5dcd9334 100644 --- a/pkg/frontend/blk.go +++ b/pkg/frontend/blk.go @@ -20,6 +20,7 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/emptypb" ) @@ -40,7 +41,7 @@ func sortVirtioBlks(virtioBlks []*pb.VirtioBlk) { } // CreateVirtioBlk creates a Virtio block device -func (s *Server) CreateVirtioBlk(_ context.Context, in *pb.CreateVirtioBlkRequest) (*pb.VirtioBlk, error) { +func (s *Server) CreateVirtioBlk(ctx context.Context, in *pb.CreateVirtioBlkRequest) (*pb.VirtioBlk, error) { log.Printf("CreateVirtioBlk: Received from client: %v", in) // see https://google.aip.dev/133#user-specified-ids resourceID := uuid.New().String() @@ -56,6 +57,20 @@ func (s *Server) CreateVirtioBlk(_ context.Context, in *pb.CreateVirtioBlkReques return controller, nil } // not found, so create a new one + if s.needToSetVirtioBlkQos(in.VirtioBlk) { + if _, err := s.Virt.qosProvider.CreateQosVolume(ctx, &pb.CreateQosVolumeRequest{ + QosVolumeId: s.qosVolumeIDFromVirtioBlkName(name), + QosVolume: &pb.QosVolume{ + VolumeId: in.VirtioBlk.VolumeId, + MaxLimit: in.VirtioBlk.MaxLimit, + MinLimit: in.VirtioBlk.MinLimit, + }, + }); err != nil { + log.Printf("error: %v", err) + return nil, err + } + } + params := spdk.VhostCreateBlkControllerParams{ Ctrlr: resourceID, DevName: in.VirtioBlk.VolumeId.Value, @@ -63,11 +78,13 @@ func (s *Server) CreateVirtioBlk(_ context.Context, in *pb.CreateVirtioBlkReques var result spdk.VhostCreateBlkControllerResult err := s.rpc.Call("vhost_create_blk_controller", ¶ms, &result) if err != nil { + // TODO: cleanup QoS if needed log.Printf("error: %v", err) return nil, fmt.Errorf("%w for %v", spdk.ErrFailedSpdkCall, in) } log.Printf("Received from SPDK: %v", result) if !result { + // TODO: cleanup QoS if needed log.Printf("Could not create: %v", in) return nil, fmt.Errorf("%w for %v", spdk.ErrUnexpectedSpdkCallResult, in) } @@ -203,3 +220,13 @@ func (s *Server) VirtioBlkStats(_ context.Context, in *pb.VirtioBlkStatsRequest) log.Printf("TODO: send anme to SPDK and get back stats: %v", resourceID) return nil, status.Errorf(codes.Unimplemented, "VirtioBlkStats method is not implemented") } + +func (s *Server) needToSetVirtioBlkQos(virtioBlk *pb.VirtioBlk) bool { + return (virtioBlk.MaxLimit != nil && !proto.Equal(virtioBlk.MaxLimit, &pb.QosLimit{})) || + (virtioBlk.MinLimit != nil && !proto.Equal(virtioBlk.MinLimit, &pb.QosLimit{})) +} + +func (s *Server) qosVolumeIDFromVirtioBlkName(name string) string { + const virtioBlkRelatedQosVolumeNames = "__opi-internal-" + return virtioBlkRelatedQosVolumeNames + name +} From aac53fad56a753fa21920ba5859755cd3f8c3bef Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Thu, 1 Jun 2023 09:50:43 +0200 Subject: [PATCH 4/7] Test QoS on virtio-blk creation. Signed-off-by: Artsiom Koltun --- pkg/frontend/blk_test.go | 66 +++++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 17 deletions(-) diff --git a/pkg/frontend/blk_test.go b/pkg/frontend/blk_test.go index d96da26f..ab06d0c5 100644 --- a/pkg/frontend/blk_test.go +++ b/pkg/frontend/blk_test.go @@ -21,6 +21,7 @@ import ( "github.com/opiproject/gospdk/spdk" pc "github.com/opiproject/opi-api/common/v1/gen/go" pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" + "github.com/opiproject/opi-spdk-bridge/pkg/server" ) type stubQosProvider struct { @@ -43,38 +44,69 @@ var ( VolumeId: &pc.ObjectKey{Value: "Malloc42"}, MaxIoQps: 1, } + testVirtioCtrlWithQos = pb.VirtioBlk{ + PcieId: &pb.PciEndpoint{PhysicalFunction: 42}, + VolumeId: &pc.ObjectKey{Value: "Malloc42"}, + MaxIoQps: 1, + MaxLimit: &pb.QosLimit{ + RwBandwidthMbs: 1, + }, + } ) func TestFrontEnd_CreateVirtioBlk(t *testing.T) { tests := map[string]struct { - in *pb.VirtioBlk - out *pb.VirtioBlk - spdk []string - expectedErr error + in *pb.VirtioBlk + out *pb.VirtioBlk + spdk []string + expectedErr error + qosCreateErr error }{ "valid virtio-blk creation": { - in: &testVirtioCtrl, - out: &testVirtioCtrl, - spdk: []string{`{"id":%d,"error":{"code":0,"message":""},"result":true}`}, - expectedErr: status.Error(codes.OK, ""), + in: &testVirtioCtrl, + out: &testVirtioCtrl, + spdk: []string{`{"id":%d,"error":{"code":0,"message":""},"result":true}`}, + expectedErr: status.Error(codes.OK, ""), + qosCreateErr: nil, }, "spdk virtio-blk creation error": { - in: &testVirtioCtrl, - out: nil, - spdk: []string{`{"id":%d,"error":{"code":1,"message":"some internal error"},"result":false}`}, - expectedErr: spdk.ErrFailedSpdkCall, + in: &testVirtioCtrl, + out: nil, + spdk: []string{`{"id":%d,"error":{"code":1,"message":"some internal error"},"result":false}`}, + expectedErr: spdk.ErrFailedSpdkCall, + qosCreateErr: nil, }, "spdk virtio-blk creation returned false response with no error": { - in: &testVirtioCtrl, - out: nil, - spdk: []string{`{"id":%d,"error":{"code":0,"message":""},"result":false}`}, - expectedErr: spdk.ErrUnexpectedSpdkCallResult, + in: &testVirtioCtrl, + out: nil, + spdk: []string{`{"id":%d,"error":{"code":0,"message":""},"result":false}`}, + expectedErr: spdk.ErrUnexpectedSpdkCallResult, + qosCreateErr: nil, + }, + "valid virtio-blk creation with qos limits": { + in: &testVirtioCtrlWithQos, + out: &testVirtioCtrlWithQos, + spdk: []string{`{"id":%d,"error":{"code":0,"message":""},"result":true}`}, + expectedErr: status.Error(codes.OK, ""), + qosCreateErr: nil, + }, + "valid virtio-blk creation with qos limits failure": { + in: &testVirtioCtrlWithQos, + out: nil, + spdk: []string{}, + expectedErr: status.Error(codes.InvalidArgument, "invalid argument"), + qosCreateErr: status.Error(codes.InvalidArgument, "invalid argument"), }, } for testName, test := range tests { t.Run(testName, func(t *testing.T) { - testEnv := createTestEnvironment(true, test.spdk) + test.in = server.ProtoClone(test.in) + test.out = server.ProtoClone(test.out) + + testEnv := createTestEnvironmentWithVirtioBlkQosProvider( + true, test.spdk, stubQosProvider{test.qosCreateErr}, + ) defer testEnv.Close() if test.out != nil { From 725f03d2df8af3d459f7e7645accb85882fd7f43 Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Thu, 1 Jun 2023 09:52:16 +0200 Subject: [PATCH 5/7] Clean QoS on virtio-blk deletion. Signed-off-by: Artsiom Koltun --- pkg/frontend/blk.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pkg/frontend/blk.go b/pkg/frontend/blk.go index 5dcd9334..837fc439 100644 --- a/pkg/frontend/blk.go +++ b/pkg/frontend/blk.go @@ -95,7 +95,7 @@ func (s *Server) CreateVirtioBlk(ctx context.Context, in *pb.CreateVirtioBlkRequ } // DeleteVirtioBlk deletes a Virtio block device -func (s *Server) DeleteVirtioBlk(_ context.Context, in *pb.DeleteVirtioBlkRequest) (*emptypb.Empty, error) { +func (s *Server) DeleteVirtioBlk(ctx context.Context, in *pb.DeleteVirtioBlkRequest) (*emptypb.Empty, error) { log.Printf("DeleteVirtioBlk: Received from client: %v", in) controller, ok := s.Virt.BlkCtrls[in.Name] if !ok { @@ -120,6 +120,16 @@ func (s *Server) DeleteVirtioBlk(_ context.Context, in *pb.DeleteVirtioBlkReques log.Printf("Could not delete: %v", in) return nil, spdk.ErrUnexpectedSpdkCallResult } + + if s.needToSetVirtioBlkQos(controller) { + if _, err := s.Virt.qosProvider.DeleteQosVolume(ctx, + &pb.DeleteQosVolumeRequest{ + Name: s.qosVolumeIDFromVirtioBlkName(controller.Name), + }); err != nil { + log.Printf("error: %v", err) + return nil, err + } + } delete(s.Virt.BlkCtrls, controller.Name) return &emptypb.Empty{}, nil } From a3077746744f81d7e89fef4b3294a2787134a80b Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Thu, 1 Jun 2023 09:52:43 +0200 Subject: [PATCH 6/7] Test QoS on virtio-blk deletion. Signed-off-by: Artsiom Koltun --- pkg/frontend/blk_test.go | 60 ++++++++++++++++++++++++++++++++++------ 1 file changed, 51 insertions(+), 9 deletions(-) diff --git a/pkg/frontend/blk_test.go b/pkg/frontend/blk_test.go index ab06d0c5..b15854b5 100644 --- a/pkg/frontend/blk_test.go +++ b/pkg/frontend/blk_test.go @@ -549,13 +549,15 @@ func TestFrontEnd_VirtioBlkStats(t *testing.T) { func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { tests := map[string]struct { - in string - out *emptypb.Empty - spdk []string - errCode codes.Code - errMsg string - start bool - missing bool + in string + out *emptypb.Empty + spdk []string + errCode codes.Code + errMsg string + start bool + missing bool + existingController *pb.VirtioBlk + qosDeleteErr error }{ "valid request with invalid SPDK response": { testVirtioCtrlID, @@ -565,6 +567,8 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { status.Convert(spdk.ErrUnexpectedSpdkCallResult).Message(), true, false, + &testVirtioCtrl, + nil, }, "valid request with empty SPDK response": { testVirtioCtrlID, @@ -574,6 +578,8 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { fmt.Sprintf("vhost_delete_controller: %v", "EOF"), true, false, + &testVirtioCtrl, + nil, }, "valid request with ID mismatch SPDK response": { testVirtioCtrlID, @@ -583,6 +589,8 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { fmt.Sprintf("vhost_delete_controller: %v", "json response ID mismatch"), true, false, + &testVirtioCtrl, + nil, }, "valid request with error code from SPDK response": { testVirtioCtrlID, @@ -592,6 +600,8 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { fmt.Sprintf("vhost_delete_controller: %v", "json response error: myopierr"), true, false, + &testVirtioCtrl, + nil, }, "valid request with valid SPDK response": { testVirtioCtrlID, @@ -601,6 +611,8 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { "", true, false, + &testVirtioCtrl, + nil, }, "valid request with unknown key": { "unknown-id", @@ -610,6 +622,8 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { fmt.Sprintf("unable to find key %v", "unknown-id"), false, false, + &testVirtioCtrl, + nil, }, "unknown key with missing allowed": { "unknown-id", @@ -619,16 +633,44 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { "", false, true, + &testVirtioCtrl, + nil, + }, + "valid request with valid SPDK response and max QoS limit virtio-blk": { + testVirtioCtrlID, + &emptypb.Empty{}, + []string{`{"id":%d,"error":{"code":0,"message":""},"result":true}`}, + codes.OK, + "", + true, + false, + &testVirtioCtrlWithQos, + nil, + }, + "valid request with valid SPDK response and max QoS limit error": { + testVirtioCtrlID, + &emptypb.Empty{}, + []string{`{"id":%d,"error":{"code":0,"message":""},"result":true}`}, + status.Convert(spdk.ErrFailedSpdkCall).Code(), + status.Convert(spdk.ErrFailedSpdkCall).Message(), + true, + false, + &testVirtioCtrlWithQos, + spdk.ErrFailedSpdkCall, }, } // run tests for name, tt := range tests { t.Run(name, func(t *testing.T) { - testEnv := createTestEnvironment(tt.start, tt.spdk) + tt.existingController = server.ProtoClone(tt.existingController) + + testEnv := createTestEnvironmentWithVirtioBlkQosProvider( + tt.start, tt.spdk, stubQosProvider{tt.qosDeleteErr}, + ) defer testEnv.Close() - testEnv.opiSpdkServer.Virt.BlkCtrls[testVirtioCtrlID] = &testVirtioCtrl + testEnv.opiSpdkServer.Virt.BlkCtrls[testVirtioCtrlID] = tt.existingController request := &pb.DeleteVirtioBlkRequest{Name: tt.in, AllowMissing: tt.missing} response, err := testEnv.client.DeleteVirtioBlk(testEnv.ctx, request) From 751137e42fbb240799313c8267de31931b9507d6 Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Mon, 5 Jun 2023 10:37:01 +0200 Subject: [PATCH 7/7] Use QoS volume full names for virtio-blk. Signed-off-by: Artsiom Koltun --- pkg/frontend/blk.go | 20 +++++++++++++------- pkg/frontend/blk_test.go | 7 ++++++- pkg/frontend/frontend.go | 6 ++++-- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/pkg/frontend/blk.go b/pkg/frontend/blk.go index 837fc439..579b98fd 100644 --- a/pkg/frontend/blk.go +++ b/pkg/frontend/blk.go @@ -58,17 +58,19 @@ func (s *Server) CreateVirtioBlk(ctx context.Context, in *pb.CreateVirtioBlkRequ } // not found, so create a new one if s.needToSetVirtioBlkQos(in.VirtioBlk) { - if _, err := s.Virt.qosProvider.CreateQosVolume(ctx, &pb.CreateQosVolumeRequest{ - QosVolumeId: s.qosVolumeIDFromVirtioBlkName(name), + out, err := s.Virt.qosProvider.CreateQosVolume(ctx, &pb.CreateQosVolumeRequest{ + QosVolumeId: s.qosVolumeIDFromVirtioBlkResourceID(resourceID), QosVolume: &pb.QosVolume{ VolumeId: in.VirtioBlk.VolumeId, MaxLimit: in.VirtioBlk.MaxLimit, MinLimit: in.VirtioBlk.MinLimit, }, - }); err != nil { + }) + if err != nil { log.Printf("error: %v", err) return nil, err } + s.Virt.qosVolumeNames[in.VirtioBlk.Name] = out.Name } params := spdk.VhostCreateBlkControllerParams{ @@ -122,9 +124,13 @@ func (s *Server) DeleteVirtioBlk(ctx context.Context, in *pb.DeleteVirtioBlkRequ } if s.needToSetVirtioBlkQos(controller) { + qosVolumeName, ok := s.Virt.qosVolumeNames[controller.Name] + if !ok { + return nil, status.Errorf(codes.Internal, "Underlying QosVolume name is not found") + } if _, err := s.Virt.qosProvider.DeleteQosVolume(ctx, &pb.DeleteQosVolumeRequest{ - Name: s.qosVolumeIDFromVirtioBlkName(controller.Name), + Name: qosVolumeName, }); err != nil { log.Printf("error: %v", err) return nil, err @@ -236,7 +242,7 @@ func (s *Server) needToSetVirtioBlkQos(virtioBlk *pb.VirtioBlk) bool { (virtioBlk.MinLimit != nil && !proto.Equal(virtioBlk.MinLimit, &pb.QosLimit{})) } -func (s *Server) qosVolumeIDFromVirtioBlkName(name string) string { - const virtioBlkRelatedQosVolumeNames = "__opi-internal-" - return virtioBlkRelatedQosVolumeNames + name +func (s *Server) qosVolumeIDFromVirtioBlkResourceID(id string) string { + const virtioBlkRelatedQosVolumePrefix = "__opi-internal-" + return virtioBlkRelatedQosVolumePrefix + id } diff --git a/pkg/frontend/blk_test.go b/pkg/frontend/blk_test.go index b15854b5..33075548 100644 --- a/pkg/frontend/blk_test.go +++ b/pkg/frontend/blk_test.go @@ -29,7 +29,7 @@ type stubQosProvider struct { } func (p stubQosProvider) CreateQosVolume(context.Context, *pb.CreateQosVolumeRequest) (*pb.QosVolume, error) { - return &pb.QosVolume{}, p.err + return &pb.QosVolume{Name: "//storage.opiproject.org/volumes/id"}, p.err } func (p stubQosProvider) DeleteQosVolume(context.Context, *pb.DeleteQosVolumeRequest) (*emptypb.Empty, error) { @@ -670,7 +670,12 @@ func TestFrontEnd_DeleteVirtioBlk(t *testing.T) { ) defer testEnv.Close() + tt.existingController.Name = testVirtioCtrlID testEnv.opiSpdkServer.Virt.BlkCtrls[testVirtioCtrlID] = tt.existingController + if tt.existingController.MaxLimit != nil { + qosName := fmt.Sprintf("//storage.opiproject.org/volumes/%s", testVirtioCtrlID) + testEnv.opiSpdkServer.Virt.qosVolumeNames[testVirtioCtrlID] = qosName + } 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 64e560e7..dffe05c8 100644 --- a/pkg/frontend/frontend.go +++ b/pkg/frontend/frontend.go @@ -32,7 +32,8 @@ type VirtioParameters struct { ScsiCtrls map[string]*pb.VirtioScsiController ScsiLuns map[string]*pb.VirtioScsiLun - qosProvider VirtioBlkQosProvider + qosProvider VirtioBlkQosProvider + qosVolumeNames map[string]string } // Server contains frontend related OPI services @@ -63,7 +64,8 @@ func NewServer(jsonRPC spdk.JSONRPC, qosProvider VirtioBlkQosProvider) *Server { ScsiCtrls: make(map[string]*pb.VirtioScsiController), ScsiLuns: make(map[string]*pb.VirtioScsiLun), - qosProvider: qosProvider, + qosProvider: qosProvider, + qosVolumeNames: make(map[string]string), }, Pagination: make(map[string]int), }