Skip to content

Commit 94c8bd9

Browse files
estespestesp
authored
Merge pull request #4496 from kzys/backport-1.4-4437
[release/1.4 backport] snapshots/devmapper: fix rollback
2 parents 09814d4 + 23e0ea2 commit 94c8bd9

File tree

3 files changed

+67
-33
lines changed

3 files changed

+67
-33
lines changed

‎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: 42 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,22 @@ func (p *PoolDevice) transition(ctx context.Context, deviceName string, tryingSt
162162
result = multierror.Append(result, uerr)
163163
}
164164

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

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

@@ -230,6 +231,23 @@ func (p *PoolDevice) CreateThinDevice(ctx context.Context, deviceName string, vi
230231
return nil
231232
}
232233

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

@@ -490,7 +494,13 @@ func (p *PoolDevice) RemoveDevice(ctx context.Context, deviceName string) error
490494
func (p *PoolDevice) deleteDevice(ctx context.Context, info *DeviceInfo) error {
491495
if err := p.transition(ctx, info.Name, Removing, Removed, func() error {
492496
// Send 'delete' message to thin-pool
493-
return dmsetup.DeleteDevice(p.poolName, info.DeviceID)
497+
e := dmsetup.DeleteDevice(p.poolName, info.DeviceID)
498+
499+
// Ignores the error if the device has been deleted already.
500+
if e != nil && !errors.Is(e, unix.ENODATA) {
501+
return e
502+
}
503+
return nil
494504
}); err != nil {
495505
return errors.Wrapf(err, "failed to delete device %q (dev id: %d)", info.Name, info.DeviceID)
496506
}

‎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)