Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(lvol): add snapshot checksum APIs #169

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+152 −5
Draft

feat(lvol): add snapshot checksum APIs #169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions app/cmd/basic/bdev_lvol.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ func BdevLvolCmd() cli.Command {
BdevLvolGetXattrCmd(),
BdevLvolGetFragmapCmd(),
BdevLvolRenameCmd(),
BdevLvolRegisterSnapshotChecksumCmd(),
BdevLvolGetSnapshotChecksumCmd(),
},
}
}
Expand Down Expand Up @@ -654,3 +656,96 @@ func bdevLvolRename(c *cli.Context) error {

return util.PrintObject(renamed)
}

func BdevLvolRegisterSnapshotChecksumCmd() cli.Command {
return cli.Command{
Name: "register-snapshot-checksum",
Flags: []cli.Flag{
cli.StringFlag{
Name: "alias",
Usage: "The alias of a snapshot is <LVSTORE NAME>/<SNAPSHOT NAME>. Specify this or uuid",
},
cli.StringFlag{
Name: "uuid",
Usage: "Specify this or alias",
},
},
Usage: "compute and store checksum of snapshot's data: \"register-snapshot-checksum --alias <LVSTORE NAME>/<LVOL NAME>\"," +
" or \"register-snapshot-checksum --uuid <LVOL UUID>\"",
Action: func(c *cli.Context) {
if err := bdevLvolRegisterSnapshotChecksum(c); err != nil {
logrus.WithError(err).Fatalf("Failed to run register snapshot checksum command")
}
},
}
}

func bdevLvolRegisterSnapshotChecksum(c *cli.Context) error {
spdkCli, err := client.NewClient(context.Background())
if err != nil {
return err
}

name := c.String("alias")
if name == "" {
name = c.String("uuid")
}
if name == "" {
return fmt.Errorf("either alias or uuid must be provided")
}

registered, err := spdkCli.BdevLvolRegisterSnapshotChecksum(name)
if err != nil {
return fmt.Errorf("failed to register checksum for snapshot %q: %v", name, err)
}

return util.PrintObject(registered)
}

func BdevLvolGetSnapshotChecksumCmd() cli.Command {
return cli.Command{
Name: "get-snapshot-checksum",
Flags: []cli.Flag{
cli.StringFlag{
Name: "alias",
Usage: "The alias of a snapshot is <LVSTORE NAME>/<SNAPSHOT NAME>. Specify this or uuid",
},
cli.StringFlag{
Name: "uuid",
Usage: "Specify this or alias",
},
},
Usage: "get checksum of snapshot's data: \"get-snapshot-checksum --alias <LVSTORE NAME>/<LVOL NAME>\"," +
" or \"get-snapshot-checksum --uuid <LVOL UUID>\"",
Action: func(c *cli.Context) {
if err := bdevLvolGetSnapshotChecksum(c); err != nil {
logrus.WithError(err).Fatalf("Failed to run get snapshot checksum command")
}
},
}
}

func bdevLvolGetSnapshotChecksum(c *cli.Context) error {
spdkCli, err := client.NewClient(context.Background())
if err != nil {
return err
}

name := c.String("alias")
if name == "" {
name = c.String("uuid")
}
if name == "" {
return fmt.Errorf("either alias or uuid must be provided")
}

checksum, err := spdkCli.BdevLvolGetSnapshotChecksum(name)
if err != nil {
return fmt.Errorf("failed to get checksum for snapshot %q: %v", name, err)
}
if checksum == nil {
return fmt.Errorf("no checksum found for snapshot %q", name)
}

return util.PrintObject(*checksum)
}
43 changes: 41 additions & 2 deletions pkg/spdk/client/basic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,9 @@ func (c *Client) BdevLvolGetWithFilter(name string, timeout uint64, filter func(
// "snapshotName": Required. the logical volume name for the newly created snapshot.
func (c *Client) BdevLvolSnapshot(name, snapshotName string, xattrs []Xattr) (uuid string, err error) {
req := spdktypes.BdevLvolSnapshotRequest{
LvolName: name,
SnapshotName: snapshotName,
LvolName: name,
SnapshotName: snapshotName,
EnableAddUpdateXattrs: true,
}

req.Xattrs = make(map[string]string)
Expand Down Expand Up @@ -502,6 +503,44 @@ func (c *Client) BdevLvolGetFragmap(name string, offset, size uint64) (*spdktype
return &result, nil
}

// BdevLvolRegisterSnapshotChecksum compute and store checksum of snapshot's data. Overwrite old checksum if already registered.
//
// "name": Required. UUID or alias of the snapshot. The alias of a snapshot is <LVSTORE NAME>/<SNAPSHOT NAME>.
func (c *Client) BdevLvolRegisterSnapshotChecksum(name string) (registered bool, err error) {
req := spdktypes.BdevLvolRegisterSnapshotChecksumRequest{
Name: name,
}

cmdOutput, err := c.jsonCli.SendCommandWithLongTimeout("bdev_lvol_register_snapshot_checksum", req)
if err != nil {
return false, err
}

return registered, json.Unmarshal(cmdOutput, &registered)
}

// BdevLvolGetSnapshotChecksum gets snapshot's stored checksum. The checksum must has been previously registered.
//
// "name": Required. UUID or alias of the snapshot. The alias of a snapshot is <LVSTORE NAME>/<SNAPSHOT NAME>.
func (c *Client) BdevLvolGetSnapshotChecksum(name string) (checksum *uint64, err error) {
req := spdktypes.BdevLvolGetSnapshotChecksumRequest{
Name: name,
}

cmdOutput, err := c.jsonCli.SendCommandWithLongTimeout("bdev_lvol_get_snapshot_checksum", req)
if err != nil {
return nil, err
}

var snapshotChecksum spdktypes.BdevLvolSnapshotChecksum
err = json.Unmarshal(cmdOutput, &snapshotChecksum)
if err != nil {
return nil, err
}

return &snapshotChecksum.Checksum, nil
}

// BdevLvolRename renames a logical volume.
//
// "oldName": Required. UUID or alias of the existing logical volume.
Expand Down
19 changes: 16 additions & 3 deletions pkg/spdk/types/lvol.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,10 @@ type BdevLvolDeleteRequest struct {
}

type BdevLvolSnapshotRequest struct {
LvolName string `json:"lvol_name"`
SnapshotName string `json:"snapshot_name"`
Xattrs map[string]string `json:"xattrs,omitempty"`
LvolName string `json:"lvol_name"`
SnapshotName string `json:"snapshot_name"`
Xattrs map[string]string `json:"xattrs,omitempty"`
EnableAddUpdateXattrs bool `json:"enable_add_update_xattrs,omitempty"`
}

type BdevLvolCloneRequest struct {
Expand Down Expand Up @@ -150,6 +151,18 @@ type BdevLvolRenameRequest struct {
NewName string `json:"new_name"`
}

type BdevLvolRegisterSnapshotChecksumRequest struct {
Name string `json:"name"`
}

type BdevLvolGetSnapshotChecksumRequest struct {
Name string `json:"name"`
}

type BdevLvolSnapshotChecksum struct {
Checksum uint64 `json:"checksum"`
}
Comment on lines +154 to +164
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Let me generate the final response based on the verification results.

Implementation appears consistent but lacks test coverage

The implementation shows proper integration:

  • The new types are correctly used in the client implementation
  • RPC method names match between types and client calls
  • JSON request/response structures are properly handled

However, there are no test files found for these new snapshot checksum features.

  • pkg/spdk/client/basic.go: Add unit tests for BdevLvolRegisterSnapshotChecksum and BdevLvolGetSnapshotChecksum methods
  • pkg/spdk/types/lvol.go: Consider adding validation tests for the new types
🔗 Analysis chain

Verify integration with dependent PR.

Since this implementation depends on PR #38 from longhorn/spdk, let's verify the consistency between these changes and the dependent PR.

Let me analyze the results and generate additional verification scripts to check the implementation details.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any existing usage of the new types and verify consistency

# Check for any existing usage of the new types
echo "Checking for existing usage of new types..."
rg "BdevLvolRegisterSnapshotChecksumRequest|BdevLvolGetSnapshotChecksumRequest|BdevLvolSnapshotChecksum" --type go

# Look for any related command implementations
echo "Checking for command implementations..."
rg "RegisterSnapshotChecksum|GetSnapshotChecksum" --type go

# Look for any test files
echo "Checking for test coverage..."
fd -e go -e test

Length of output: 33142

Script:

#!/bin/bash
# Check the implementation of the snapshot checksum methods in client

# Look for the client implementation details
echo "Checking client implementation..."
rg -A 10 "BdevLvolRegisterSnapshotChecksum|BdevLvolGetSnapshotChecksum" pkg/spdk/client/basic.go

# Check for any RPC method definitions
echo "Checking RPC method definitions..."
rg "bdev_lvol_register_snapshot_checksum|bdev_lvol_get_snapshot_checksum" --type go

# Look for any test files related to snapshot checksum
echo "Checking test files..."
rg -l "TestBdevLvolRegisterSnapshotChecksum|TestBdevLvolGetSnapshotChecksum" --type go

Length of output: 1990


func GetLvolAlias(lvsName, lvolName string) string {
return fmt.Sprintf("%s/%s", lvsName, lvolName)
}
Expand Down
Loading