From 0a376291b2213699f986a7bca1cc8c4f4ed00f8d Mon Sep 17 00:00:00 2001 From: David Calavera Date: Tue, 30 Jun 2015 09:00:02 -0700 Subject: [PATCH] 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 --- daemon/graphdriver/devmapper/deviceset.go | 66 ++++++------- .../graphdriver/devmapper/devmapper_test.go | 1 - daemon/graphdriver/driver.go | 96 ++++++++++--------- docs/reference/commandline/daemon.md | 36 ------- man/docker.1.md | 39 -------- 5 files changed, 77 insertions(+), 161 deletions(-) diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go index ffba32b916..2060be06e2 100644 --- a/daemon/graphdriver/devmapper/deviceset.go +++ b/daemon/graphdriver/devmapper/deviceset.go @@ -30,8 +30,7 @@ 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 - DefaultUdevSyncOverride bool = false + DefaultThinpBlockSize uint32 = 128 // 64K = 128 512b sectors MaxDeviceId int = 0xffffff // 24 bit, pool limit DeviceIdMapSz int = (MaxDeviceId + 1) / 8 // We retry device removal so many a times that even error messages @@ -90,23 +89,22 @@ 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:"-"` - overrideUdevSyncCheck bool - 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:"-"` + deferredRemove bool // use deferred removal + BaseDeviceUUID string //save UUID of base device } type DiskUsage struct { @@ -1106,10 +1104,7 @@ func (devices *DeviceSet) initDevmapper(doInit bool) error { // https://github.com/docker/docker/issues/4036 if supported := devicemapper.UdevSetSyncSupport(true); !supported { - 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 - } + 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") } 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 devicemapper.SetDevDir("/dev") devices := &DeviceSet{ - 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), + 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), } foundBlkDiscard := false @@ -1857,12 +1851,6 @@ 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 { diff --git a/daemon/graphdriver/devmapper/devmapper_test.go b/daemon/graphdriver/devmapper/devmapper_test.go index 60006af5a5..6cb7572384 100644 --- a/daemon/graphdriver/devmapper/devmapper_test.go +++ b/daemon/graphdriver/devmapper/devmapper_test.go @@ -13,7 +13,6 @@ func init() { DefaultDataLoopbackSize = 300 * 1024 * 1024 DefaultMetaDataLoopbackSize = 200 * 1024 * 1024 DefaultBaseFsSize = 300 * 1024 * 1024 - DefaultUdevSyncOverride = true if err := graphtest.InitLoopbacks(); err != nil { panic(err) } diff --git a/daemon/graphdriver/driver.go b/daemon/graphdriver/driver.go index 68e1383b15..e02dafa8fb 100644 --- a/daemon/graphdriver/driver.go +++ b/daemon/graphdriver/driver.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/Sirupsen/logrus" + "github.com/docker/docker/autogen/dockerversion" "github.com/docker/docker/pkg/archive" ) @@ -22,9 +23,10 @@ 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") + 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") ) type InitFunc func(root string, options []string) (Driver, error) @@ -113,36 +115,35 @@ func New(root string, options []string) (driver Driver, err error) { } // Guess for prior driver - priorDrivers := scanPriorDrivers(root) - for _, name := range priority { - if name == "vfs" { - // don't use vfs even if there is state present. - continue + 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 } - 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 - } + + 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 } + 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 { @@ -154,7 +155,10 @@ func New(root string, options []string) (driver Driver, err error) { } // Check all registered drivers if no priority driver is found - for _, initFunc := range drivers { + for name, initFunc := range drivers { + if staticWithDeviceMapper(name) { + continue + } if driver, err = initFunc(root, options); err != nil { if err == ErrNotSupported || err == ErrPrerequisites || err == ErrIncompatibleFS { continue @@ -166,31 +170,31 @@ func New(root string, options []string) (driver Driver, err error) { return nil, fmt.Errorf("No supported storage backend found") } -// scanPriorDrivers returns an un-ordered scan of directories of prior storage drivers -func scanPriorDrivers(root string) []string { - priorDrivers := []string{} +// 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 for driver := range drivers { p := filepath.Join(root, driver) - if _, err := os.Stat(p); err == nil { + if _, err := os.Stat(p); err == nil && driver != "vfs" { priorDrivers = append(priorDrivers, driver) } } - return priorDrivers -} -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) - } - } + if len(priorDrivers) > 1 { + return "", multipleDriversError(root, priorDrivers) } - if len(priorDrivers) > 0 { - - return errors.New(fmt.Sprintf("%q contains other graphdrivers: %s; Please cleanup or explicitly choose storage driver (-s )", root, strings.Join(priorDrivers, ","))) + if len(priorDrivers) == 0 { + return "", nil } - return nil + 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 )", root, strings.Join(drivers, ", ")) +} + +func staticWithDeviceMapper(name string) bool { + return name == "devicemapper" && dockerversion.IAMSTATIC == "true" } diff --git a/docs/reference/commandline/daemon.md b/docs/reference/commandline/daemon.md index 0cd9f734e0..cb56ab5454 100644 --- a/docs/reference/commandline/daemon.md +++ b/docs/reference/commandline/daemon.md @@ -323,42 +323,6 @@ 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 diff --git a/man/docker.1.md b/man/docker.1.md index 98135e50ff..ee0344a14f 100644 --- a/man/docker.1.md +++ b/man/docker.1.md @@ -451,45 +451,6 @@ 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