Skip to content

Commit fd6c915

Browse files
author
Kazuyoshi Kato
committed
snapshots/devmapper: fix rollback
The rollback mechanism is implemented by calling deleteDevice() and RemoveDevice(). But RemoveDevice() is internally calling deleteDevice() as well. Since a device will be deleted by first deleteDevice(), RemoveDevice() always will see ENODATA. The specific error must be ignored to remove the device's metadata correctly. Signed-off-by: Kazuyoshi Kato <[email protected]>
1 parent 6c71fe1 commit fd6c915

3 files changed

Lines changed: 68 additions & 33 deletions

File tree

‎snapshots/devmapper/metadata.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func (m *PoolMetadata) AddDevice(ctx context.Context, info *DeviceInfo) error {
101101
// See https://github.com/containerd/containerd/pull/3436 for more context.
102102
var existing DeviceInfo
103103
if err := getObject(devicesBucket, info.Name, &existing); err == nil && existing.State != Faulty {
104-
return ErrAlreadyExists
104+
return errors.Wrapf(ErrAlreadyExists, "device %q is already there %+v", info.Name, existing)
105105
}
106106

107107
// Find next available device ID

‎snapshots/devmapper/pool_device.go‎

Lines changed: 43 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525

2626
"github.com/hashicorp/go-multierror"
2727
"github.com/pkg/errors"
28+
"golang.org/x/sys/unix"
2829

2930
"github.com/containerd/containerd/log"
3031
"github.com/containerd/containerd/snapshots/devmapper/dmsetup"
@@ -160,7 +161,22 @@ func (p *PoolDevice) transition(ctx context.Context, deviceName string, tryingSt
160161
result = multierror.Append(result, uerr)
161162
}
162163

163-
return result.ErrorOrNil()
164+
return unwrapError(result)
165+
}
166+
167+
// unwrapError converts multierror.Error to the original error when it is possible.
168+
// multierror 1.1.0 has the similar function named Unwrap, but it requires Go 1.14.
169+
func unwrapError(e *multierror.Error) error {
170+
if e == nil {
171+
return nil
172+
}
173+
174+
// If the error can be expressed without multierror, return the original error.
175+
if len(e.Errors) == 1 {
176+
return e.Errors[0]
177+
}
178+
179+
return e.ErrorOrNil()
164180
}
165181

166182
// CreateThinDevice creates new devmapper thin-device with given name and size.
@@ -182,21 +198,7 @@ func (p *PoolDevice) CreateThinDevice(ctx context.Context, deviceName string, vi
182198
defer func() {
183199
// We've created a devmapper device, but failed to activate it, try rollback everything
184200
if activeErr != nil {
185-
// Delete the device first.
186-
delErr := p.deleteDevice(ctx, info)
187-
if delErr != nil {
188-
// Failed to rollback, mark the device as faulty and keep metadata in order to
189-
// preserve the faulty device ID
190-
retErr = multierror.Append(retErr, delErr, p.metadata.MarkFaulty(ctx, info.Name))
191-
return
192-
}
193-
194-
// The devmapper device has been successfully deleted, deallocate device ID
195-
if err := p.RemoveDevice(ctx, info.Name); err != nil {
196-
retErr = multierror.Append(retErr, err)
197-
return
198-
}
199-
201+
retErr = p.rollbackActivate(ctx, info, activeErr)
200202
return
201203
}
202204

@@ -228,6 +230,23 @@ func (p *PoolDevice) CreateThinDevice(ctx context.Context, deviceName string, vi
228230
return nil
229231
}
230232

233+
func (p *PoolDevice) rollbackActivate(ctx context.Context, info *DeviceInfo, activateErr error) error {
234+
// Delete the device first.
235+
delErr := p.deleteDevice(ctx, info)
236+
if delErr != nil {
237+
// Failed to rollback, mark the device as faulty and keep metadata in order to
238+
// preserve the faulty device ID
239+
return multierror.Append(activateErr, delErr, p.metadata.MarkFaulty(ctx, info.Name))
240+
}
241+
242+
// The devmapper device has been successfully deleted, deallocate device ID
243+
if err := p.RemoveDevice(ctx, info.Name); err != nil {
244+
return multierror.Append(activateErr, err)
245+
}
246+
247+
return activateErr
248+
}
249+
231250
// createDevice creates thin device
232251
func (p *PoolDevice) createDevice(ctx context.Context, info *DeviceInfo) error {
233252
if err := p.transition(ctx, info.Name, Creating, Created, func() error {
@@ -273,21 +292,7 @@ func (p *PoolDevice) CreateSnapshotDevice(ctx context.Context, deviceName string
273292
defer func() {
274293
// We've created a devmapper device, but failed to activate it, try rollback everything
275294
if activeErr != nil {
276-
// Delete the device first.
277-
delErr := p.deleteDevice(ctx, snapInfo)
278-
if delErr != nil {
279-
// Failed to rollback, mark the device as faulty and keep metadata in order to
280-
// preserve the faulty device ID
281-
retErr = multierror.Append(retErr, delErr, p.metadata.MarkFaulty(ctx, snapInfo.Name))
282-
return
283-
}
284-
285-
// The devmapper device has been successfully deleted, deallocate device ID
286-
if err := p.RemoveDevice(ctx, snapInfo.Name); err != nil {
287-
retErr = multierror.Append(retErr, err)
288-
return
289-
}
290-
295+
retErr = p.rollbackActivate(ctx, snapInfo, activeErr)
291296
return
292297
}
293298

@@ -465,7 +470,13 @@ func (p *PoolDevice) RemoveDevice(ctx context.Context, deviceName string) error
465470
func (p *PoolDevice) deleteDevice(ctx context.Context, info *DeviceInfo) error {
466471
if err := p.transition(ctx, info.Name, Removing, Removed, func() error {
467472
// Send 'delete' message to thin-pool
468-
return dmsetup.DeleteDevice(p.poolName, info.DeviceID)
473+
e := dmsetup.DeleteDevice(p.poolName, info.DeviceID)
474+
475+
// Ignores the error if the device has been deleted already.
476+
if e != nil && errors.Cause(e) != unix.ENODATA {
477+
return e
478+
}
479+
return nil
469480
}); err != nil {
470481
return errors.Wrapf(err, "failed to delete device %q (dev id: %d)", info.Name, info.DeviceID)
471482
}

‎snapshots/devmapper/pool_device_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,27 @@ func TestPoolDevice(t *testing.T) {
152152
t.Run("RemoveDevice", func(t *testing.T) {
153153
testRemoveThinDevice(t, pool)
154154
})
155+
156+
t.Run("rollbackActivate", func(t *testing.T) {
157+
testCreateThinDevice(t, pool)
158+
159+
ctx := context.Background()
160+
161+
snapDevice := "snap2"
162+
163+
err := pool.CreateSnapshotDevice(ctx, thinDevice1, snapDevice, device1Size)
164+
assert.NilError(t, err)
165+
166+
info, err := pool.metadata.GetDevice(ctx, snapDevice)
167+
assert.NilError(t, err)
168+
169+
// Simulate a case that the device cannot be activated.
170+
err = pool.DeactivateDevice(ctx, info.Name, false, false)
171+
assert.NilError(t, err)
172+
173+
err = pool.rollbackActivate(ctx, info, err)
174+
assert.NilError(t, err)
175+
})
155176
}
156177

157178
func TestPoolDeviceMarkFaulty(t *testing.T) {
@@ -256,6 +277,9 @@ func testDeactivateThinDevice(t *testing.T, pool *PoolDevice) {
256277
func testRemoveThinDevice(t *testing.T, pool *PoolDevice) {
257278
err := pool.RemoveDevice(testCtx, thinDevice1)
258279
assert.NilError(t, err, "should delete thin device from pool")
280+
281+
err = pool.RemoveDevice(testCtx, thinDevice2)
282+
assert.NilError(t, err, "should delete thin device from pool")
259283
}
260284

261285
func getMounts(thinDeviceName string) []mount.Mount {

0 commit comments

Comments
 (0)