Skip to content

Commit

Permalink
devicemapper: add EBUSY to the retriable errors
Browse files Browse the repository at this point in the history
Testing revealed that creating device mapper targets sometimes
yields `device or resource busy` error (`EBUSY`). Add it to
the list of retriable errors and consolidate them into a single
slice.

Add unit tests for `CreateDeviceWithRetryError`.

Signed-off-by: Maksim An <maksiman@microsoft.com>
  • Loading branch information
anmaxvl committed Mar 16, 2024
1 parent 02a899c commit 67d4d90
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 9 deletions.
5 changes: 3 additions & 2 deletions internal/guest/storage/devicemapper/devicemapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const (
var (
removeDeviceWrapper = removeDevice
openMapperWrapper = openMapper
_createDevice = CreateDevice
)

//nolint:stylecheck // ST1003: ALL_CAPS
Expand Down Expand Up @@ -229,7 +230,7 @@ func CreateDeviceWithRetryErrors(
) (string, error) {
retry:
for {
dmPath, err := CreateDevice(name, flags, targets)
dmPath, err := _createDevice(name, flags, targets)
if err == nil {
return dmPath, nil
}
Expand All @@ -244,7 +245,7 @@ retry:
if errors.Is(dmErr.Err, e) {
select {
case <-ctx.Done():
log.G(ctx).WithError(err).Warning("CreateDeviceWithRetryErrors failed, context timeout")
log.G(ctx).WithError(err).Error("CreateDeviceWithRetryErrors failed, context timeout")
return "", err
default:
time.Sleep(100 * time.Millisecond)
Expand Down
39 changes: 38 additions & 1 deletion internal/guest/storage/devicemapper/devicemapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
package devicemapper

import (
"context"
"errors"
"flag"
"fmt"
"os"
"syscall"
"testing"
Expand All @@ -19,6 +21,7 @@ var (
)

func clearTestDependencies() {
_createDevice = CreateDevice
removeDeviceWrapper = removeDevice
openMapperWrapper = openMapper
}
Expand Down Expand Up @@ -73,7 +76,7 @@ func (d *device) Close() (err error) {
}

func createDevice(name string, flags CreateFlags, targets []Target) (*device, error) {
p, err := CreateDevice(name, flags, targets)
p, err := _createDevice(name, flags, targets)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -188,6 +191,40 @@ func TestRemoveDeviceRetriesOnSyscallEBUSY(t *testing.T) {
}
}

func TestCreateDeviceWithRetryError(t *testing.T) {
for _, rerr := range retryErrors {
t.Run(fmt.Sprintf("error-%s", rerr), func(t *testing.T) {
clearTestDependencies()
createDeviceCalled := false
retryDone := false
createDeviceStub := func(name string, flags CreateFlags, targets []Target) (string, error) {
if !createDeviceCalled {
createDeviceCalled = true
return "", &dmError{
Op: 0,
Err: rerr,
}
}
retryDone = true
return fmt.Sprintf("/dev/mapper/%s", name), nil
}
_createDevice = createDeviceStub

ctx := context.Background()
_, err := CreateDeviceWithRetryErrors(ctx, "test-device", 0, []Target{}, retryErrors...)
if err != nil {
t.Fatalf("unexpected error: %s\n", err)
}
if !createDeviceCalled {
t.Fatal("CreateDevice was not called")
}
if !retryDone {
t.Fatalf("error %s was not retried\n", rerr)
}
})
}
}

func TestRemoveDeviceFailsOnNonSyscallEBUSY(t *testing.T) {
clearTestDependencies()

Expand Down
19 changes: 13 additions & 6 deletions internal/guest/storage/devicemapper/targets.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@ import (
"github.com/Microsoft/hcsshim/internal/protocol/guestresource"
)

// retryErrors are treated as transient errors and device-mapper operations will be tried again.
//
// These errors are occasionally returned by the kernel when a device-mapper operation is attempted. These errors
// are transient and can occur when the kernel is busy or the device is not ready yet.
var retryErrors = []error{
unix.EBUSY,
unix.ENOENT,
unix.ENXIO,
unix.ENODEV,
}

// CreateZeroSectorLinearTarget creates dm-linear target for a device at `devPath` and `mappingInfo`, returns
// virtual block device path.
func CreateZeroSectorLinearTarget(ctx context.Context, devPath, devName string, mappingInfo *guestresource.LCOWVPMemMappingInfo) (_ string, err error) {
Expand All @@ -37,9 +48,7 @@ func CreateZeroSectorLinearTarget(ctx context.Context, devPath, devName string,
devName,
CreateReadOnly,
[]Target{linearTarget},
unix.ENOENT,
unix.ENXIO,
unix.ENODEV,
retryErrors...,
)
if err != nil {
return "", fmt.Errorf("failed to create dm-linear target, device=%s, offset=%d: %w", devPath,
Expand Down Expand Up @@ -94,9 +103,7 @@ func CreateVerityTarget(ctx context.Context, devPath, devName string, verityInfo
devName,
CreateReadOnly,
[]Target{verityTarget},
unix.ENOENT,
unix.ENXIO,
unix.ENODEV,
retryErrors...,
)
if err != nil {
return "", fmt.Errorf("frailed to create dm-verity target for device=%s: %w", devPath, err)
Expand Down

0 comments on commit 67d4d90

Please sign in to comment.