Merge pull request containerd#3567 from renzhengeek/renzhen/devmapper

crosbymichael · web-flow · commit 95301fee3a39 · 2019-08-30T14:47:45.000-04:00
devmapper: activate dm device if snap device marked as activated
diff --git a/snapshots/devmapper/pool_device.go b/snapshots/devmapper/pool_device.go
@@ -73,23 +73,41 @@ func NewPoolDevice(ctx context.Context, config *Config) (*PoolDevice, error) {
 	return poolDevice, nil
 }
 
-// ensureDeviceStates marks devices with incomplete states (after crash) as 'Faulty'
+// ensureDeviceStates updates devices to their real state:
+//   - marks devices with incomplete states (after crash) as 'Faulty'
+//   - activates devices if they are marked as 'Activated' but the dm
+//     device is not active, which can happen to a stopped container
+//     after a reboot
 func (p *PoolDevice) ensureDeviceStates(ctx context.Context) error {
-	var devices []*DeviceInfo
+	var faultyDevices []*DeviceInfo
+	var activatedDevices []*DeviceInfo
 
 	if err := p.metadata.WalkDevices(ctx, func(info *DeviceInfo) error {
 		switch info.State {
-		case Activated, Suspended, Resumed, Deactivated, Removed, Faulty:
-			return nil
+		case Suspended, Resumed, Deactivated, Removed, Faulty:
+		case Activated:
+			activatedDevices = append(activatedDevices, info)
+		default:
+			faultyDevices = append(faultyDevices, info)
 		}
-		devices = append(devices, info)
 		return nil
 	}); err != nil {
 		return errors.Wrap(err, "failed to query devices from metastore")
 	}
 
 	var result *multierror.Error
-	for _, dev := range devices {
+	for _, dev := range activatedDevices {
+		if p.IsActivated(dev.Name) {
+			continue
+		}
+
+		log.G(ctx).Warnf("devmapper device %q marked as %q but not active, activating it", dev.Name, dev.State)
+		if err := p.activateDevice(ctx, dev); err != nil {
+			result = multierror.Append(result, err)
+		}
+	}
+
+	for _, dev := range faultyDevices {
 		log.G(ctx).
 			WithField("dev_id", dev.DeviceID).
 			WithField("parent", dev.ParentName).
@@ -350,19 +368,19 @@ func (p *PoolDevice) DeactivateDevice(ctx context.Context, deviceName string, de
 	return nil
 }
 
-// IsActivated returns true if thin-device is activated and not suspended
+// IsActivated returns true if thin-device is activated
 func (p *PoolDevice) IsActivated(deviceName string) bool {
 	infos, err := dmsetup.Info(deviceName)
 	if err != nil || len(infos) != 1 {
 		// Couldn't query device info, device not active
 		return false
 	}
 
-	if devInfo := infos[0]; devInfo.Suspended {
-		return false
+	if devInfo := infos[0]; devInfo.TableLive {
+		return true
 	}
 
-	return true
+	return false
 }
 
 // IsLoaded returns true if thin-device is visible for dmsetup
diff --git a/snapshots/devmapper/pool_device_test.go b/snapshots/devmapper/pool_device_test.go
@@ -161,7 +161,9 @@ func TestPoolDeviceMarkFaulty(t *testing.T) {
 	err := store.AddDevice(testCtx, &DeviceInfo{Name: "1", State: Unknown})
 	assert.NilError(t, err)
 
-	err = store.AddDevice(testCtx, &DeviceInfo{Name: "2", State: Activated})
+	// Note: do not use 'Activated' here because pool.ensureDeviceStates() will
+	// try to activate the real dm device, which will fail on a faked device.
+	err = store.AddDevice(testCtx, &DeviceInfo{Name: "2", State: Deactivated})
 	assert.NilError(t, err)
 
 	pool := &PoolDevice{metadata: store}
@@ -177,7 +179,7 @@ func TestPoolDeviceMarkFaulty(t *testing.T) {
 			assert.Equal(t, Faulty, info.State)
 			assert.Equal(t, "1", info.Name)
 		case 2:
-			assert.Equal(t, Activated, info.State)
+			assert.Equal(t, Deactivated, info.State)
 			assert.Equal(t, "2", info.Name)
 		default:
 			t.Error("unexpected walk call")
