Skip to content

Commit

Permalink
rbd: rename setImageOptions() to constructImageOptions()
Browse files Browse the repository at this point in the history
A function called `setImageOptions()` is expected to set the passed
options on the volume. However, the passed options parameter is only
filled with the options that should get set on the RBD-image at the time
of creation.

The naming of the function, and it's parameter is confusing. Rename the
function to `constructImageOptions()` and return the ImageOptions to
make it easier to understand.

Signed-off-by: Niels de Vos <ndevos@ibm.com>
  • Loading branch information
nixpanic authored and mergify[bot] committed Sep 12, 2024
1 parent 61c23dd commit 42fc0b6
Showing 1 changed file with 19 additions and 15 deletions.
34 changes: 19 additions & 15 deletions internal/rbd/rbd_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,13 +431,11 @@ func createImage(ctx context.Context, pOpts *rbdVolume, cr *util.Credentials) er
log.DebugLog(ctx, "rbd: create %s size %s (features: %s) using mon %s",
pOpts, volSzMiB, pOpts.ImageFeatureSet.Names(), pOpts.Monitors)

options := librbd.NewRbdImageOptions()
defer options.Destroy()

err := pOpts.setImageOptions(ctx, options)
options, err := pOpts.constructImageOptions(ctx)
if err != nil {
return err
}
defer options.Destroy()

err = pOpts.Connect(cr)
if err != nil {
Expand Down Expand Up @@ -1494,12 +1492,11 @@ func (rv *rbdVolume) cloneRbdImageFromSnapshot(
parentVol.ioctx = nil
}()

options := librbd.NewRbdImageOptions()
defer options.Destroy()
err = rv.setImageOptions(ctx, options)
options, err := rv.constructImageOptions(ctx)
if err != nil {
return err
}
defer options.Destroy()

err = options.SetUint64(librbd.ImageOptionCloneFormat, 2)
if err != nil {
Expand Down Expand Up @@ -1545,35 +1542,42 @@ func (rv *rbdVolume) cloneRbdImageFromSnapshot(
return nil
}

// setImageOptions sets the image options.
func (rv *rbdVolume) setImageOptions(ctx context.Context, options *librbd.ImageOptions) error {
// constructImageOptions constructs the ImageOptions that should get set on the
// RBD-image at the time of its creation/cloning.
func (rv *rbdVolume) constructImageOptions(ctx context.Context) (*librbd.ImageOptions, error) {
var err error
options := librbd.NewRbdImageOptions()
defer func() {
if err != nil {
options.Destroy()
}
}()

logMsg := fmt.Sprintf("setting image options on %s", rv)
if rv.DataPool != "" {
logMsg += ", data pool %s" + rv.DataPool
err = options.SetString(librbd.RbdImageOptionDataPool, rv.DataPool)
if err != nil {
return fmt.Errorf("failed to set data pool: %w", err)
return nil, fmt.Errorf("failed to set data pool: %w", err)
}
}

if rv.ImageFeatureSet != 0 {
err = options.SetUint64(librbd.RbdImageOptionFeatures, uint64(rv.ImageFeatureSet))
if err != nil {
return fmt.Errorf("failed to set image features: %w", err)
return nil, fmt.Errorf("failed to set image features: %w", err)
}
}

if rv.StripeCount != 0 {
logMsg += fmt.Sprintf(", stripe count %d, stripe unit %d", rv.StripeCount, rv.StripeUnit)
err = options.SetUint64(librbd.RbdImageOptionStripeCount, rv.StripeCount)
if err != nil {
return fmt.Errorf("failed to set stripe count: %w", err)
return nil, fmt.Errorf("failed to set stripe count: %w", err)
}
err = options.SetUint64(librbd.RbdImageOptionStripeUnit, rv.StripeUnit)
if err != nil {
return fmt.Errorf("failed to set stripe unit: %w", err)
return nil, fmt.Errorf("failed to set stripe unit: %w", err)
}
}

Expand All @@ -1582,13 +1586,13 @@ func (rv *rbdVolume) setImageOptions(ctx context.Context, options *librbd.ImageO
logMsg += fmt.Sprintf(", object size %d, order %d", rv.ObjectSize, order)
err = options.SetUint64(librbd.RbdImageOptionOrder, order)
if err != nil {
return fmt.Errorf("failed to set object size: %w", err)
return nil, fmt.Errorf("failed to set object size: %w", err)
}
}

log.DebugLog(ctx, logMsg)

return nil
return options, nil
}

// GetCreationTime returns the creation time of the image. if the image
Expand Down

0 comments on commit 42fc0b6

Please sign in to comment.