Revert "Fix implicit DeviceMapper selection"

This reverts commit 0a376291b2213699f986a7bca1cc8c4f4ed00f8d.

Signed-off-by: David Calavera <david.calavera@gmail.com>

daemon/graphdriver/devmapper/deviceset.go

@@ -30,7 +30,8 @@ var (
 	DefaultDataLoopbackSize     int64  = 100 * 1024 * 1024 * 1024
 	DefaultMetaDataLoopbackSize int64  = 2 * 1024 * 1024 * 1024
 	DefaultBaseFsSize           uint64 = 10 * 1024 * 1024 * 1024
-	DefaultThinpBlockSize       uint32 = 128      // 64K = 128 512b sectors
+	DefaultThinpBlockSize       uint32 = 128 // 64K = 128 512b sectors
+	DefaultUdevSyncOverride     bool   = false
 	MaxDeviceId                 int    = 0xffffff // 24 bit, pool limit
 	DeviceIdMapSz               int    = (MaxDeviceId + 1) / 8
 	// We retry device removal so many a times that even error messages
@@ -89,22 +90,23 @@ type DeviceSet struct {
 	deviceIdMap   []byte
 
 	// Options
-	dataLoopbackSize     int64
-	metaDataLoopbackSize int64
-	baseFsSize           uint64
-	filesystem           string
-	mountOptions         string
-	mkfsArgs             []string
-	dataDevice           string // block or loop dev
-	dataLoopFile         string // loopback file, if used
-	metadataDevice       string // block or loop dev
-	metadataLoopFile     string // loopback file, if used
-	doBlkDiscard         bool
-	thinpBlockSize       uint32
-	thinPoolDevice       string
-	Transaction          `json:"-"`
-	deferredRemove       bool   // use deferred removal
-	BaseDeviceUUID       string //save UUID of base device
+	dataLoopbackSize      int64
+	metaDataLoopbackSize  int64
+	baseFsSize            uint64
+	filesystem            string
+	mountOptions          string
+	mkfsArgs              []string
+	dataDevice            string // block or loop dev
+	dataLoopFile          string // loopback file, if used
+	metadataDevice        string // block or loop dev
+	metadataLoopFile      string // loopback file, if used
+	doBlkDiscard          bool
+	thinpBlockSize        uint32
+	thinPoolDevice        string
+	Transaction           `json:"-"`
+	overrideUdevSyncCheck bool
+	deferredRemove        bool   // use deferred removal
+	BaseDeviceUUID        string //save UUID of base device
 }
 
 type DiskUsage struct {
@@ -1104,7 +1106,10 @@ func (devices *DeviceSet) initDevmapper(doInit bool) error {
 
 	// https://github.com/docker/docker/issues/4036
 	if supported := devicemapper.UdevSetSyncSupport(true); !supported {
-		logrus.Warn("Udev sync is not supported. This will lead to unexpected behavior, data loss and errors. For more information, see https://docs.docker.com/reference/commandline/cli/#daemon-storage-driver-option")
+		logrus.Errorf("Udev sync is not supported. This will lead to unexpected behavior, data loss and errors. For more information, see https://docs.docker.com/reference/commandline/cli/#daemon-storage-driver-option")
+		if !devices.overrideUdevSyncCheck {
+			return graphdriver.ErrNotSupported
+		}
 	}
 
 	if err := os.MkdirAll(devices.metadataDir(), 0700); err != nil && !os.IsExist(err) {
@@ -1786,15 +1791,16 @@ func NewDeviceSet(root string, doInit bool, options []string) (*DeviceSet, error
 	devicemapper.SetDevDir("/dev")
 
 	devices := &DeviceSet{
-		root:                 root,
-		MetaData:             MetaData{Devices: make(map[string]*DevInfo)},
-		dataLoopbackSize:     DefaultDataLoopbackSize,
-		metaDataLoopbackSize: DefaultMetaDataLoopbackSize,
-		baseFsSize:           DefaultBaseFsSize,
-		filesystem:           "ext4",
-		doBlkDiscard:         true,
-		thinpBlockSize:       DefaultThinpBlockSize,
-		deviceIdMap:          make([]byte, DeviceIdMapSz),
+		root:                  root,
+		MetaData:              MetaData{Devices: make(map[string]*DevInfo)},
+		dataLoopbackSize:      DefaultDataLoopbackSize,
+		metaDataLoopbackSize:  DefaultMetaDataLoopbackSize,
+		baseFsSize:            DefaultBaseFsSize,
+		overrideUdevSyncCheck: DefaultUdevSyncOverride,
+		filesystem:            "ext4",
+		doBlkDiscard:          true,
+		thinpBlockSize:        DefaultThinpBlockSize,
+		deviceIdMap:           make([]byte, DeviceIdMapSz),
 	}
 
 	foundBlkDiscard := false
@@ -1851,6 +1857,12 @@ func NewDeviceSet(root string, doInit bool, options []string) (*DeviceSet, error
 			}
 			// convert to 512b sectors
 			devices.thinpBlockSize = uint32(size) >> 9
+		case "dm.override_udev_sync_check":
+			devices.overrideUdevSyncCheck, err = strconv.ParseBool(val)
+			if err != nil {
+				return nil, err
+			}
+
 		case "dm.use_deferred_removal":
 			EnableDeferredRemoval, err = strconv.ParseBool(val)
 			if err != nil {

daemon/graphdriver/devmapper/devmapper_test.go

@@ -13,6 +13,7 @@ func init() {
 	DefaultDataLoopbackSize = 300 * 1024 * 1024
 	DefaultMetaDataLoopbackSize = 200 * 1024 * 1024
 	DefaultBaseFsSize = 300 * 1024 * 1024
+	DefaultUdevSyncOverride = true
 	if err := graphtest.InitLoopbacks(); err != nil {
 		panic(err)
 	}

daemon/graphdriver/driver.go

@@ -8,7 +8,6 @@ import (
 	"strings"
 
 	"github.com/Sirupsen/logrus"
-	"github.com/docker/docker/autogen/dockerversion"
 	"github.com/docker/docker/pkg/archive"
 )
 
@@ -23,10 +22,9 @@ var (
 	// All registred drivers
 	drivers map[string]InitFunc
 
-	ErrNotSupported                 = errors.New("driver not supported")
-	ErrPrerequisites                = errors.New("prerequisites for driver not satisfied (wrong filesystem?)")
-	ErrIncompatibleFS               = fmt.Errorf("backing file system is unsupported for this graph driver")
-	ErrDeviceMapperWithStaticDocker = fmt.Errorf("devicemapper storage driver cannot reliably be used with a statically linked docker binary: please either pick a different storage driver, install a dynamically linked docker binary, or force this unreliable setup anyway by specifying --storage-driver=devicemapper")
+	ErrNotSupported   = errors.New("driver not supported")
+	ErrPrerequisites  = errors.New("prerequisites for driver not satisfied (wrong filesystem?)")
+	ErrIncompatibleFS = fmt.Errorf("backing file system is unsupported for this graph driver")
 )
 
 type InitFunc func(root string, options []string) (Driver, error)
@@ -115,35 +113,36 @@ func New(root string, options []string) (driver Driver, err error) {
 	}
 
 	// Guess for prior driver
-	priorDriver, err := scanPriorDrivers(root)
-	if err != nil {
-		return nil, err
-	}
-
-	if len(priorDriver) != 0 {
-		// Do not allow devicemapper when it's not explicit and the Docker binary was built statically.
-		if staticWithDeviceMapper(priorDriver) {
-			return nil, ErrDeviceMapperWithStaticDocker
+	priorDrivers := scanPriorDrivers(root)
+	for _, name := range priority {
+		if name == "vfs" {
+			// don't use vfs even if there is state present.
+			continue
 		}
-
-		driver, err = GetDriver(priorDriver, root, options)
-		if err != nil {
-			// unlike below, we will return error here, because there is prior
-			// state, and now it is no longer supported/prereq/compatible, so
-			// something changed and needs attention. Otherwise the daemon's
-			// images would just "disappear".
-			logrus.Errorf("[graphdriver] prior storage driver %q failed: %s", priorDriver, err)
-			return nil, err
+		for _, prior := range priorDrivers {
+			// of the state found from prior drivers, check in order of our priority
+			// which we would prefer
+			if prior == name {
+				driver, err = GetDriver(name, root, options)
+				if err != nil {
+					// unlike below, we will return error here, because there is prior
+					// state, and now it is no longer supported/prereq/compatible, so
+					// something changed and needs attention. Otherwise the daemon's
+					// images would just "disappear".
+					logrus.Errorf("[graphdriver] prior storage driver %q failed: %s", name, err)
+					return nil, err
+				}
+				if err := checkPriorDriver(name, root); err != nil {
+					return nil, err
+				}
+				logrus.Infof("[graphdriver] using prior storage driver %q", name)
+				return driver, nil
+			}
 		}
-		logrus.Infof("[graphdriver] using prior storage driver %q", priorDriver)
-		return driver, nil
 	}
 
 	// Check for priority drivers first
 	for _, name := range priority {
-		if staticWithDeviceMapper(name) {
-			continue
-		}
 		driver, err = GetDriver(name, root, options)
 		if err != nil {
 			if err == ErrNotSupported || err == ErrPrerequisites || err == ErrIncompatibleFS {
@@ -155,10 +154,7 @@ func New(root string, options []string) (driver Driver, err error) {
 	}
 
 	// Check all registered drivers if no priority driver is found
-	for name, initFunc := range drivers {
-		if staticWithDeviceMapper(name) {
-			continue
-		}
+	for _, initFunc := range drivers {
 		if driver, err = initFunc(root, options); err != nil {
 			if err == ErrNotSupported || err == ErrPrerequisites || err == ErrIncompatibleFS {
 				continue
@@ -170,31 +166,31 @@ func New(root string, options []string) (driver Driver, err error) {
 	return nil, fmt.Errorf("No supported storage backend found")
 }
 
-// scanPriorDrivers returns a previosly used driver.
-// it returns an error when there are several drivers scanned.
-func scanPriorDrivers(root string) (string, error) {
-	var priorDrivers []string
+// scanPriorDrivers returns an un-ordered scan of directories of prior storage drivers
+func scanPriorDrivers(root string) []string {
+	priorDrivers := []string{}
 	for driver := range drivers {
 		p := filepath.Join(root, driver)
-		if _, err := os.Stat(p); err == nil && driver != "vfs" {
+		if _, err := os.Stat(p); err == nil {
 			priorDrivers = append(priorDrivers, driver)
 		}
 	}
+	return priorDrivers
+}
 
-	if len(priorDrivers) > 1 {
-		return "", multipleDriversError(root, priorDrivers)
-	}
-
-	if len(priorDrivers) == 0 {
-		return "", nil
+func checkPriorDriver(name, root string) error {
+	priorDrivers := []string{}
+	for _, prior := range scanPriorDrivers(root) {
+		if prior != name && prior != "vfs" {
+			if _, err := os.Stat(filepath.Join(root, prior)); err == nil {
+				priorDrivers = append(priorDrivers, prior)
+			}
+		}
 	}
-	return priorDrivers[0], nil
-}
 
-func multipleDriversError(root string, drivers []string) error {
-	return fmt.Errorf("%q contains several graphdrivers: %s; Please cleanup or explicitly choose storage driver (--storage-driver <DRIVER>)", root, strings.Join(drivers, ", "))
-}
+	if len(priorDrivers) > 0 {
 
-func staticWithDeviceMapper(name string) bool {
-	return name == "devicemapper" && dockerversion.IAMSTATIC == "true"
+		return errors.New(fmt.Sprintf("%q contains other graphdrivers: %s; Please cleanup or explicitly choose storage driver (-s <DRIVER>)", root, strings.Join(priorDrivers, ",")))
+	}
+	return nil
 }

docs/reference/commandline/daemon.md

@@ -323,6 +323,45 @@ options for `zfs` start with `zfs`.
 
         $ docker -d --storage-opt dm.blkdiscard=false
 
+ *  `dm.override_udev_sync_check`
+
+    Overrides the `udev` synchronization checks between `devicemapper` and `udev`.
+    `udev` is the device manager for the Linux kernel.
+
+    To view the `udev` sync support of a Docker daemon that is using the
+    `devicemapper` driver, run:
+
+        $ docker info
+        [...]
+        Udev Sync Supported: true
+        [...]
+
+    When `udev` sync support is `true`, then `devicemapper` and udev can
+    coordinate the activation and deactivation of devices for containers.
+
+    When `udev` sync support is `false`, a race condition occurs between
+    the`devicemapper` and `udev` during create and cleanup. The race condition
+    results in errors and failures. (For information on these failures, see
+    [docker#4036](https://github.com/docker/docker/issues/4036))
+
+    To allow the `docker` daemon to start, regardless of `udev` sync not being
+    supported, set `dm.override_udev_sync_check` to true:
+
+        $ docker -d --storage-opt dm.override_udev_sync_check=true
+
+    When this value is `true`, the  `devicemapper` continues and simply warns
+    you the errors are happening.
+
+    > **Note:**
+    > The ideal is to pursue a `docker` daemon and environment that does
+    > support synchronizing with `udev`. For further discussion on this
+    > topic, see [docker#4036](https://github.com/docker/docker/issues/4036).
+    > Otherwise, set this flag for migrating existing Docker daemons to
+    > a daemon with a supported environment.
+
+
+## Docker execdriver option
+
 Currently supported options of `zfs`:
 
  * `zfs.fsname`

man/docker.1.md

@@ -451,6 +451,45 @@ removed.
 
 Example use: `docker -d --storage-opt dm.blkdiscard=false`
 
+#### dm.override_udev_sync_check
+
+By default, the devicemapper backend attempts to synchronize with the
+`udev` device manager for the Linux kernel.  This option allows
+disabling that synchronization, to continue even though the
+configuration may be buggy.
+
+To view the `udev` sync support of a Docker daemon that is using the
+`devicemapper` driver, run:
+
+        $ docker info
+	[...]
+	 Udev Sync Supported: true
+	[...]
+
+When `udev` sync support is `true`, then `devicemapper` and `udev` can
+coordinate the activation and deactivation of devices for containers.
+
+When `udev` sync support is `false`, a race condition occurs between
+the`devicemapper` and `udev` during create and cleanup. The race
+condition results in errors and failures. (For information on these
+failures, see
+[docker#4036](https://github.com/docker/docker/issues/4036))
+
+To allow the `docker` daemon to start, regardless of whether `udev` sync is
+`false`, set `dm.override_udev_sync_check` to true:
+
+        $ docker -d --storage-opt dm.override_udev_sync_check=true
+
+When this value is `true`, the driver continues and simply warns you
+the errors are happening.
+
+**Note**: The ideal is to pursue a `docker` daemon and environment
+that does support synchronizing with `udev`. For further discussion on
+this topic, see
+[docker#4036](https://github.com/docker/docker/issues/4036).
+Otherwise, set this flag for migrating existing Docker daemons to a
+daemon with a supported environment.
+
 # EXEC DRIVER OPTIONS
 
 Use the **--exec-opt** flags to specify options to the exec-driver. The only