From 9af7afb9eb138bdba33c22445f36f41e5aa26bd1 Mon Sep 17 00:00:00 2001 From: David Calavera Date: Tue, 7 Jul 2015 12:27:19 -0700 Subject: [PATCH] Revert "Fix implicit DeviceMapper selection" This reverts commit 0a376291b2213699f986a7bca1cc8c4f4ed00f8d. 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 | 39 ++++++++ man/docker.1.md | 39 ++++++++ 5 files changed, 164 insertions(+), 77 deletions(-) diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go index 2060be06e2..ffba32b916 100644 --- a/daemon/graphdriver/devmapper/deviceset.go +++ b/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 { diff --git a/daemon/graphdriver/devmapper/devmapper_test.go b/daemon/graphdriver/devmapper/devmapper_test.go index 6cb7572384..60006af5a5 100644 --- a/daemon/graphdriver/devmapper/devmapper_test.go +++ b/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) } diff --git a/daemon/graphdriver/driver.go b/daemon/graphdriver/driver.go index e02dafa8fb..68e1383b15 100644 --- a/daemon/graphdriver/driver.go +++ b/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) +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) == 0 { - return "", nil + 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, ","))) } - 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" + return nil } diff --git a/docs/reference/commandline/daemon.md b/docs/reference/commandline/daemon.md index 07acb65e10..4f623239f2 100644 --- a/docs/reference/commandline/daemon.md +++ b/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` diff --git a/man/docker.1.md b/man/docker.1.md index ee0344a14f..98135e50ff 100644 --- a/man/docker.1.md +++ b/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