Browse code

Fix implicit DeviceMapper selection

DeviceMapper must be explicitly selected because the Docker binary might not be linked to the right devmapper library.

With this change, Docker fails fast if the driver detection finds the devicemapper directory but the driver is not the default option.
The option `override_udev_sync_check` doesn't make sense anymore, since the user must be explicit to select devicemapper, so it's being removed.
Docker fails to use devicemapper only if Docker has been built statically unless the option was explicit.

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

David Calavera authored on 2015/07/01 01:00:02
Showing 5 changed files
... ...
@@ -30,8 +30,7 @@ var (
30 30
 	DefaultDataLoopbackSize     int64  = 100 * 1024 * 1024 * 1024
31 31
 	DefaultMetaDataLoopbackSize int64  = 2 * 1024 * 1024 * 1024
32 32
 	DefaultBaseFsSize           uint64 = 10 * 1024 * 1024 * 1024
33
-	DefaultThinpBlockSize       uint32 = 128 // 64K = 128 512b sectors
34
-	DefaultUdevSyncOverride     bool   = false
33
+	DefaultThinpBlockSize       uint32 = 128      // 64K = 128 512b sectors
35 34
 	MaxDeviceId                 int    = 0xffffff // 24 bit, pool limit
36 35
 	DeviceIdMapSz               int    = (MaxDeviceId + 1) / 8
37 36
 	// We retry device removal so many a times that even error messages
... ...
@@ -90,23 +89,22 @@ type DeviceSet struct {
90 90
 	deviceIdMap   []byte
91 91
 
92 92
 	// Options
93
-	dataLoopbackSize      int64
94
-	metaDataLoopbackSize  int64
95
-	baseFsSize            uint64
96
-	filesystem            string
97
-	mountOptions          string
98
-	mkfsArgs              []string
99
-	dataDevice            string // block or loop dev
100
-	dataLoopFile          string // loopback file, if used
101
-	metadataDevice        string // block or loop dev
102
-	metadataLoopFile      string // loopback file, if used
103
-	doBlkDiscard          bool
104
-	thinpBlockSize        uint32
105
-	thinPoolDevice        string
106
-	Transaction           `json:"-"`
107
-	overrideUdevSyncCheck bool
108
-	deferredRemove        bool   // use deferred removal
109
-	BaseDeviceUUID        string //save UUID of base device
93
+	dataLoopbackSize     int64
94
+	metaDataLoopbackSize int64
95
+	baseFsSize           uint64
96
+	filesystem           string
97
+	mountOptions         string
98
+	mkfsArgs             []string
99
+	dataDevice           string // block or loop dev
100
+	dataLoopFile         string // loopback file, if used
101
+	metadataDevice       string // block or loop dev
102
+	metadataLoopFile     string // loopback file, if used
103
+	doBlkDiscard         bool
104
+	thinpBlockSize       uint32
105
+	thinPoolDevice       string
106
+	Transaction          `json:"-"`
107
+	deferredRemove       bool   // use deferred removal
108
+	BaseDeviceUUID       string //save UUID of base device
110 109
 }
111 110
 
112 111
 type DiskUsage struct {
... ...
@@ -1106,10 +1104,7 @@ func (devices *DeviceSet) initDevmapper(doInit bool) error {
1106 1106
 
1107 1107
 	// https://github.com/docker/docker/issues/4036
1108 1108
 	if supported := devicemapper.UdevSetSyncSupport(true); !supported {
1109
-		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")
1110
-		if !devices.overrideUdevSyncCheck {
1111
-			return graphdriver.ErrNotSupported
1112
-		}
1109
+		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")
1113 1110
 	}
1114 1111
 
1115 1112
 	if err := os.MkdirAll(devices.metadataDir(), 0700); err != nil && !os.IsExist(err) {
... ...
@@ -1791,16 +1786,15 @@ func NewDeviceSet(root string, doInit bool, options []string) (*DeviceSet, error
1791 1791
 	devicemapper.SetDevDir("/dev")
1792 1792
 
1793 1793
 	devices := &DeviceSet{
1794
-		root:                  root,
1795
-		MetaData:              MetaData{Devices: make(map[string]*DevInfo)},
1796
-		dataLoopbackSize:      DefaultDataLoopbackSize,
1797
-		metaDataLoopbackSize:  DefaultMetaDataLoopbackSize,
1798
-		baseFsSize:            DefaultBaseFsSize,
1799
-		overrideUdevSyncCheck: DefaultUdevSyncOverride,
1800
-		filesystem:            "ext4",
1801
-		doBlkDiscard:          true,
1802
-		thinpBlockSize:        DefaultThinpBlockSize,
1803
-		deviceIdMap:           make([]byte, DeviceIdMapSz),
1794
+		root:                 root,
1795
+		MetaData:             MetaData{Devices: make(map[string]*DevInfo)},
1796
+		dataLoopbackSize:     DefaultDataLoopbackSize,
1797
+		metaDataLoopbackSize: DefaultMetaDataLoopbackSize,
1798
+		baseFsSize:           DefaultBaseFsSize,
1799
+		filesystem:           "ext4",
1800
+		doBlkDiscard:         true,
1801
+		thinpBlockSize:       DefaultThinpBlockSize,
1802
+		deviceIdMap:          make([]byte, DeviceIdMapSz),
1804 1803
 	}
1805 1804
 
1806 1805
 	foundBlkDiscard := false
... ...
@@ -1857,12 +1851,6 @@ func NewDeviceSet(root string, doInit bool, options []string) (*DeviceSet, error
1857 1857
 			}
1858 1858
 			// convert to 512b sectors
1859 1859
 			devices.thinpBlockSize = uint32(size) >> 9
1860
-		case "dm.override_udev_sync_check":
1861
-			devices.overrideUdevSyncCheck, err = strconv.ParseBool(val)
1862
-			if err != nil {
1863
-				return nil, err
1864
-			}
1865
-
1866 1860
 		case "dm.use_deferred_removal":
1867 1861
 			EnableDeferredRemoval, err = strconv.ParseBool(val)
1868 1862
 			if err != nil {
... ...
@@ -13,7 +13,6 @@ func init() {
13 13
 	DefaultDataLoopbackSize = 300 * 1024 * 1024
14 14
 	DefaultMetaDataLoopbackSize = 200 * 1024 * 1024
15 15
 	DefaultBaseFsSize = 300 * 1024 * 1024
16
-	DefaultUdevSyncOverride = true
17 16
 	if err := graphtest.InitLoopbacks(); err != nil {
18 17
 		panic(err)
19 18
 	}
... ...
@@ -8,6 +8,7 @@ import (
8 8
 	"strings"
9 9
 
10 10
 	"github.com/Sirupsen/logrus"
11
+	"github.com/docker/docker/autogen/dockerversion"
11 12
 	"github.com/docker/docker/pkg/archive"
12 13
 )
13 14
 
... ...
@@ -22,9 +23,10 @@ var (
22 22
 	// All registred drivers
23 23
 	drivers map[string]InitFunc
24 24
 
25
-	ErrNotSupported   = errors.New("driver not supported")
26
-	ErrPrerequisites  = errors.New("prerequisites for driver not satisfied (wrong filesystem?)")
27
-	ErrIncompatibleFS = fmt.Errorf("backing file system is unsupported for this graph driver")
25
+	ErrNotSupported                 = errors.New("driver not supported")
26
+	ErrPrerequisites                = errors.New("prerequisites for driver not satisfied (wrong filesystem?)")
27
+	ErrIncompatibleFS               = fmt.Errorf("backing file system is unsupported for this graph driver")
28
+	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")
28 29
 )
29 30
 
30 31
 type InitFunc func(root string, options []string) (Driver, error)
... ...
@@ -113,36 +115,35 @@ func New(root string, options []string) (driver Driver, err error) {
113 113
 	}
114 114
 
115 115
 	// Guess for prior driver
116
-	priorDrivers := scanPriorDrivers(root)
117
-	for _, name := range priority {
118
-		if name == "vfs" {
119
-			// don't use vfs even if there is state present.
120
-			continue
116
+	priorDriver, err := scanPriorDrivers(root)
117
+	if err != nil {
118
+		return nil, err
119
+	}
120
+
121
+	if len(priorDriver) != 0 {
122
+		// Do not allow devicemapper when it's not explicit and the Docker binary was built statically.
123
+		if staticWithDeviceMapper(priorDriver) {
124
+			return nil, ErrDeviceMapperWithStaticDocker
121 125
 		}
122
-		for _, prior := range priorDrivers {
123
-			// of the state found from prior drivers, check in order of our priority
124
-			// which we would prefer
125
-			if prior == name {
126
-				driver, err = GetDriver(name, root, options)
127
-				if err != nil {
128
-					// unlike below, we will return error here, because there is prior
129
-					// state, and now it is no longer supported/prereq/compatible, so
130
-					// something changed and needs attention. Otherwise the daemon's
131
-					// images would just "disappear".
132
-					logrus.Errorf("[graphdriver] prior storage driver %q failed: %s", name, err)
133
-					return nil, err
134
-				}
135
-				if err := checkPriorDriver(name, root); err != nil {
136
-					return nil, err
137
-				}
138
-				logrus.Infof("[graphdriver] using prior storage driver %q", name)
139
-				return driver, nil
140
-			}
126
+
127
+		driver, err = GetDriver(priorDriver, root, options)
128
+		if err != nil {
129
+			// unlike below, we will return error here, because there is prior
130
+			// state, and now it is no longer supported/prereq/compatible, so
131
+			// something changed and needs attention. Otherwise the daemon's
132
+			// images would just "disappear".
133
+			logrus.Errorf("[graphdriver] prior storage driver %q failed: %s", priorDriver, err)
134
+			return nil, err
141 135
 		}
136
+		logrus.Infof("[graphdriver] using prior storage driver %q", priorDriver)
137
+		return driver, nil
142 138
 	}
143 139
 
144 140
 	// Check for priority drivers first
145 141
 	for _, name := range priority {
142
+		if staticWithDeviceMapper(name) {
143
+			continue
144
+		}
146 145
 		driver, err = GetDriver(name, root, options)
147 146
 		if err != nil {
148 147
 			if err == ErrNotSupported || err == ErrPrerequisites || err == ErrIncompatibleFS {
... ...
@@ -154,7 +155,10 @@ func New(root string, options []string) (driver Driver, err error) {
154 154
 	}
155 155
 
156 156
 	// Check all registered drivers if no priority driver is found
157
-	for _, initFunc := range drivers {
157
+	for name, initFunc := range drivers {
158
+		if staticWithDeviceMapper(name) {
159
+			continue
160
+		}
158 161
 		if driver, err = initFunc(root, options); err != nil {
159 162
 			if err == ErrNotSupported || err == ErrPrerequisites || err == ErrIncompatibleFS {
160 163
 				continue
... ...
@@ -166,31 +170,31 @@ func New(root string, options []string) (driver Driver, err error) {
166 166
 	return nil, fmt.Errorf("No supported storage backend found")
167 167
 }
168 168
 
169
-// scanPriorDrivers returns an un-ordered scan of directories of prior storage drivers
170
-func scanPriorDrivers(root string) []string {
171
-	priorDrivers := []string{}
169
+// scanPriorDrivers returns a previosly used driver.
170
+// it returns an error when there are several drivers scanned.
171
+func scanPriorDrivers(root string) (string, error) {
172
+	var priorDrivers []string
172 173
 	for driver := range drivers {
173 174
 		p := filepath.Join(root, driver)
174
-		if _, err := os.Stat(p); err == nil {
175
+		if _, err := os.Stat(p); err == nil && driver != "vfs" {
175 176
 			priorDrivers = append(priorDrivers, driver)
176 177
 		}
177 178
 	}
178
-	return priorDrivers
179
-}
180 179
 
181
-func checkPriorDriver(name, root string) error {
182
-	priorDrivers := []string{}
183
-	for _, prior := range scanPriorDrivers(root) {
184
-		if prior != name && prior != "vfs" {
185
-			if _, err := os.Stat(filepath.Join(root, prior)); err == nil {
186
-				priorDrivers = append(priorDrivers, prior)
187
-			}
188
-		}
180
+	if len(priorDrivers) > 1 {
181
+		return "", multipleDriversError(root, priorDrivers)
189 182
 	}
190 183
 
191
-	if len(priorDrivers) > 0 {
192
-
193
-		return errors.New(fmt.Sprintf("%q contains other graphdrivers: %s; Please cleanup or explicitly choose storage driver (-s <DRIVER>)", root, strings.Join(priorDrivers, ",")))
184
+	if len(priorDrivers) == 0 {
185
+		return "", nil
194 186
 	}
195
-	return nil
187
+	return priorDrivers[0], nil
188
+}
189
+
190
+func multipleDriversError(root string, drivers []string) error {
191
+	return fmt.Errorf("%q contains several graphdrivers: %s; Please cleanup or explicitly choose storage driver (--storage-driver <DRIVER>)", root, strings.Join(drivers, ", "))
192
+}
193
+
194
+func staticWithDeviceMapper(name string) bool {
195
+	return name == "devicemapper" && dockerversion.IAMSTATIC == "true"
196 196
 }
... ...
@@ -323,42 +323,6 @@ options for `zfs` start with `zfs`.
323 323
 
324 324
         $ docker -d --storage-opt dm.blkdiscard=false
325 325
 
326
- *  `dm.override_udev_sync_check`
327
-
328
-    Overrides the `udev` synchronization checks between `devicemapper` and `udev`.
329
-    `udev` is the device manager for the Linux kernel.
330
-
331
-    To view the `udev` sync support of a Docker daemon that is using the
332
-    `devicemapper` driver, run:
333
-
334
-        $ docker info
335
-        [...]
336
-        Udev Sync Supported: true
337
-        [...]
338
-
339
-    When `udev` sync support is `true`, then `devicemapper` and udev can
340
-    coordinate the activation and deactivation of devices for containers.
341
-
342
-    When `udev` sync support is `false`, a race condition occurs between
343
-    the`devicemapper` and `udev` during create and cleanup. The race condition
344
-    results in errors and failures. (For information on these failures, see
345
-    [docker#4036](https://github.com/docker/docker/issues/4036))
346
-
347
-    To allow the `docker` daemon to start, regardless of `udev` sync not being
348
-    supported, set `dm.override_udev_sync_check` to true:
349
-
350
-	$ docker -d --storage-opt dm.override_udev_sync_check=true
351
-
352
-    When this value is `true`, the  `devicemapper` continues and simply warns
353
-    you the errors are happening.
354
-
355
-    > **Note:**
356
-    > The ideal is to pursue a `docker` daemon and environment that does
357
-    > support synchronizing with `udev`. For further discussion on this
358
-    > topic, see [docker#4036](https://github.com/docker/docker/issues/4036).
359
-    > Otherwise, set this flag for migrating existing Docker daemons to
360
-    > a daemon with a supported environment.
361
-
362 326
 
363 327
 ## Docker execdriver option
364 328
 
... ...
@@ -451,45 +451,6 @@ removed.
451 451
 
452 452
 Example use: `docker -d --storage-opt dm.blkdiscard=false`
453 453
 
454
-#### dm.override_udev_sync_check
455
-
456
-By default, the devicemapper backend attempts to synchronize with the
457
-`udev` device manager for the Linux kernel.  This option allows
458
-disabling that synchronization, to continue even though the
459
-configuration may be buggy.
460
-
461
-To view the `udev` sync support of a Docker daemon that is using the
462
-`devicemapper` driver, run:
463
-
464
-        $ docker info
465
-	[...]
466
-	 Udev Sync Supported: true
467
-	[...]
468
-
469
-When `udev` sync support is `true`, then `devicemapper` and `udev` can
470
-coordinate the activation and deactivation of devices for containers.
471
-
472
-When `udev` sync support is `false`, a race condition occurs between
473
-the`devicemapper` and `udev` during create and cleanup. The race
474
-condition results in errors and failures. (For information on these
475
-failures, see
476
-[docker#4036](https://github.com/docker/docker/issues/4036))
477
-
478
-To allow the `docker` daemon to start, regardless of whether `udev` sync is
479
-`false`, set `dm.override_udev_sync_check` to true:
480
-
481
-        $ docker -d --storage-opt dm.override_udev_sync_check=true
482
-
483
-When this value is `true`, the driver continues and simply warns you
484
-the errors are happening.
485
-
486
-**Note**: The ideal is to pursue a `docker` daemon and environment
487
-that does support synchronizing with `udev`. For further discussion on
488
-this topic, see
489
-[docker#4036](https://github.com/docker/docker/issues/4036).
490
-Otherwise, set this flag for migrating existing Docker daemons to a
491
-daemon with a supported environment.
492
-
493 454
 # EXEC DRIVER OPTIONS
494 455
 
495 456
 Use the **--exec-opt** flags to specify options to the exec-driver. The only