From eab270395e5b47b16a41c54ec6e1427f8144bffc Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 11 Feb 2014 12:47:59 +0100 Subject: [PATCH 1/2] devmapper: Fix shutdown warnings Shutdown contains debug warnings like: [debug] deviceset.go:699 [deviceset docker-0:33-17945897] waitRemove(/dev/mapper/docker-0:33-17945897-pool) [debug] deviceset.go:380 libdevmapper(3): libdm-common.c:552 (-1) Device /dev/mapper/docker-0:33-17945897-pool not found This is because shutdown is using removeDeviceAndWait() to remove the pool device and the wait part fails because the pool is gone. We fix this by adding a pool specific removal function which avoids all the trickiness of the normal remove. Docker-DCO-1.1-Signed-off-by: Alexander Larsson (github: alexlarsson) --- graphdriver/devmapper/deviceset.go | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/graphdriver/devmapper/deviceset.go b/graphdriver/devmapper/deviceset.go index 637192dbcb..9b5ae62b67 100644 --- a/graphdriver/devmapper/deviceset.go +++ b/graphdriver/devmapper/deviceset.go @@ -639,6 +639,22 @@ func (devices *DeviceSet) DeleteDevice(hash string) error { return devices.deleteDevice(hash) } +func (devices *DeviceSet) deactivatePool() error { + utils.Debugf("[devmapper] deactivatePool()") + defer utils.Debugf("[devmapper] deactivatePool END") + devname := devices.getPoolDevName() + devinfo, err := getInfo(devname) + if err != nil { + utils.Debugf("\n--->Err: %s\n", err) + return err + } + if devinfo.Exists != 0 { + return removeDevice(devname) + } + + return nil +} + func (devices *DeviceSet) deactivateDevice(hash string) error { utils.Debugf("[devmapper] deactivateDevice(%s)", hash) defer utils.Debugf("[devmapper] deactivateDevice END") @@ -789,11 +805,8 @@ func (devices *DeviceSet) Shutdown() error { } } - pool := devices.getPoolDevName() - if devinfo, err := getInfo(pool); err == nil && devinfo.Exists != 0 { - if err := devices.deactivateDevice("pool"); err != nil { - utils.Debugf("Shutdown deactivate %s , error: %s\n", pool, err) - } + if err := devices.deactivatePool(); err != nil { + utils.Debugf("Shutdown deactivate pool , error: %s\n", err) } return nil From 6128dcea4a9bbe808baba4e18c9c4fee3a265532 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 11 Feb 2014 12:55:40 +0100 Subject: [PATCH 2/2] devmapper: Remove byHash hack We no longer pass "pool" anywhere that uses byHash() per the last commit, so we can now remove this hack. Docker-DCO-1.1-Signed-off-by: Alexander Larsson (github: alexlarsson) --- graphdriver/devmapper/deviceset.go | 40 +++++++++--------------------- 1 file changed, 12 insertions(+), 28 deletions(-) diff --git a/graphdriver/devmapper/deviceset.go b/graphdriver/devmapper/deviceset.go index 9b5ae62b67..303e363e92 100644 --- a/graphdriver/devmapper/deviceset.go +++ b/graphdriver/devmapper/deviceset.go @@ -658,19 +658,18 @@ func (devices *DeviceSet) deactivatePool() error { func (devices *DeviceSet) deactivateDevice(hash string) error { utils.Debugf("[devmapper] deactivateDevice(%s)", hash) defer utils.Debugf("[devmapper] deactivateDevice END") - var devname string - // FIXME: shouldn't we just register the pool into devices? - devname, err := devices.byHash(hash) - if err != nil { - return err + + info := devices.Devices[hash] + if info == nil { + return fmt.Errorf("Unknown device %s", hash) } - devinfo, err := getInfo(devname) + devinfo, err := getInfo(info.Name()) if err != nil { utils.Debugf("\n--->Err: %s\n", err) return err } if devinfo.Exists != 0 { - if err := devices.removeDeviceAndWait(devname); err != nil { + if err := devices.removeDeviceAndWait(info.Name()); err != nil { utils.Debugf("\n--->Err: %s\n", err) return err } @@ -741,18 +740,18 @@ func (devices *DeviceSet) waitRemove(devname string) error { // a) the device registered at - is closed, // or b) the 1 second timeout expires. func (devices *DeviceSet) waitClose(hash string) error { - devname, err := devices.byHash(hash) - if err != nil { - return err + info := devices.Devices[hash] + if info == nil { + return fmt.Errorf("Unknown device %s", hash) } i := 0 for ; i < 1000; i += 1 { - devinfo, err := getInfo(devname) + devinfo, err := getInfo(info.Name()) if err != nil { return err } if i%100 == 0 { - utils.Debugf("Waiting for unmount of %s: opencount=%d", devname, devinfo.OpenCount) + utils.Debugf("Waiting for unmount of %s: opencount=%d", hash, devinfo.OpenCount) } if devinfo.OpenCount == 0 { break @@ -760,26 +759,11 @@ func (devices *DeviceSet) waitClose(hash string) error { time.Sleep(1 * time.Millisecond) } if i == 1000 { - return fmt.Errorf("Timeout while waiting for device %s to close", devname) + return fmt.Errorf("Timeout while waiting for device %s to close", hash) } return nil } -// byHash is a hack to allow looking up the deviceset's pool by the hash "pool". -// FIXME: it seems probably cleaner to register the pool in devices.Devices, -// but I am afraid of arcane implications deep in the devicemapper code, -// so this will do. -func (devices *DeviceSet) byHash(hash string) (devname string, err error) { - if hash == "pool" { - return devices.getPoolDevName(), nil - } - info := devices.Devices[hash] - if info == nil { - return "", fmt.Errorf("hash %s doesn't exists", hash) - } - return info.Name(), nil -} - func (devices *DeviceSet) Shutdown() error { devices.Lock() defer devices.Unlock()