Commit graph

247 commits

Author SHA1 Message Date
Vivek Goyal
51e059e7e9 devmapper: Provide option to enabled deferred device deletion
Provide a command line option dm.use_deferred_deletion to enable deferred
device deletion feature. By default feature will be turned off.

Not sure if there is much value in deferred deletion being turned on
without deferred removal being turned on. So for now, this feature can
be enabled only if deferred removal is on.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-10-06 17:37:21 -04:00
Vivek Goyal
6b8b4feaa1 devmapper: construct used device ID map from device Hash map
Currently during startup we walk through all the device files and read
their device ID and mark in a bitmap that device id is used.

We are anyway going through all device files. So we can as well load all
that data into device hash map. This will save us little time when
container is actually launched later.

Also this will help with later patches where cleanup deferred device
wants to go through all the devices and see which have been marked for
deletion and delete these.

So re-organize the code a bit.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-10-06 17:37:21 -04:00
Vivek Goyal
0fcd485626 devmapper: Move UUID setup and verification code in a function
Simplify setupBaseImage() even further. Move some more code in a separate
function. Pure code reorganization. No functionality change.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-10-06 08:38:03 -04:00
Vivek Goyal
69051ec0a5 devmapper: Move thin pool related checks in a function
Move thin pool related checks in a separate function. Pure code reorganization.
Makes reading code easier.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-10-06 08:38:03 -04:00
Vivek Goyal
efc1ddd7e3 devmapper: Move base device creation in separate function
This moves base device creation function in a separate function. Pure
code reorganization. Makes reading code little easier.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-10-06 08:38:03 -04:00
Vivek Goyal
482eca3099 devmapper: Few code cleanups
This patch does three things. Following are the descriptions.

===
Create a separate function for delete transactions so that parent function
is little smaller.

Also close transaction if an error happens.
===
When docker is being shutdown, save deviceset metadata first before
trying to remove the devices. Generally caller gives only 10 seconds
for shutdown to complete and then kills it after that. So if some device
is busy, we will wait 20 seconds for it removal and never be able to save
metadata. So first save metadata and then deal with device removal.
===
Move issue discard operation in a separate function. This makes reading code
little easier.

Also don't issue discards if device is still open. That means devices is
still probably being used and issuing discards is not a good idea.

This is especially true in case of deferred deletion. We want to issue
discards when device is not open. At that time device can be deleted too.
Otherwise we will issue discards and deletion will actually fail. Later
we will try deletion again and issue discards again and deletion will
fail again as device is open and busy.

So this will ensure that discards are issued once when device is not open
and it can actually be deleted.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-10-05 09:02:31 -04:00
Vivek Goyal
289145ecc6 devmapper: Get rid of metaData.devicesLock
Right now we seem to have 3 locks. 

- devinfo.lock
  This is a per device lock

- metaData.devicesLock

  This is supposedely protecting map of devices.

- Global DeviceSet lock

  This is protecting map of devices as well as serializing calls to libdevmapper.

Semantics of per devices lock and global deviceset lock seem to be very clear.
Even ordering between these two locks has been defined properly.

What is not clear is the need and ordering of metaData.devicesLock. Looks like
this lock is not necessary and global DeviceSet lock should be used to
protect map of devices as it is part of DeviceSet.

This patchset gets rid of metaData.devicesLock and instead uses DeviceSet
lock to protect map of devices.

Also at couple of places during initialization takes devices.Lock(). That
is not strictly necessary as there is supposed to be one thread of execution
during initializaiton. Still it makes the code clearer.

I think this makes code more clear and easier to understand and easier to
make further changes.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-10-01 13:02:55 -04:00
Vivek Goyal
73f8b46d84 devmapper: Get rid of unused function HasActivatedDevice()
Looks like nobody is calling HasActivatedDevice(). Get rid of it.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-10-01 07:59:12 -04:00
Vivek Goyal
94caae2477 devmapper: Move maxDeviceID check in loadMetadata
maxDeviceID is upper limit on device Id thin pool can support. Right now
we have this check only during startup. It is a good idea to move this
check in loadMetadata so that any time a device file is loaded and if it
is corrupted and device Id is more than maxDevieceID, it will be detected
right then and there.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-09-30 18:54:06 -04:00
Vivek Goyal
39081eb3aa devmapper: Use deactivateDevice() instead of removeDevice() in deleteDevice()
Use deactivateDevice() instead of removeDevice() directly. This will make
sure for device deletion, deferred removal is used if user has configured
it in. Also this makes reading code litle easier as there is single function
to remove a device and that is deactivateDevice().

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-09-30 18:54:06 -04:00
Vivek Goyal
e97e46b737 devmapper: Fail device deletion early if device is still mounted
If a device is still mounted at the time of DeleteDevice(), that means
higher layers have not called Put() properly on the device and are trying
to delete it. This is a bug in the code where Get() and Put() have not been
properly paired up. Fail device deletion if it is still mounted.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-09-30 15:21:22 -04:00
Vivek Goyal
f5c0eb9ffe devmapper: Fix comments and for HasDevice() and Exists()
Exists() and HasDevice() just check if device file exists or not. It does
not say anything about if device is mounted or not. Fix comments.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-09-30 15:21:22 -04:00
Vivek Goyal
ba02bf31cb devmapper: Do not load transaction meta file in device Hash map
device has map (device.Devices), contains valid devices and we skip all
the files which are not device files. transaction metadata file is not
device file. Skip this file when devices files are being read and loaded
into map.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-09-30 15:21:22 -04:00
Carl Henrik Lunde
9a64f2bbb3 Use pkg/mount to support more flags in dm.mountopt
The mount syscall does not handle string flags like "noatime",
we must use bitmasks like MS_NOATIME instead.

pkg/mount.Mount already handles this work.

Signed-off-by: Carl Henrik Lunde <chlunde@ping.uio.no>
2015-09-24 21:14:41 +02:00
Chun Chen
2458452a3b Try to resize data and metadata loopback file when initiating devicemapper
Signed-off-by: Chun Chen <ramichen@tencent.com>
2015-09-24 09:31:00 +08:00
Vincent Demeester
6990b76a69 Lint package pkg/devicemapper
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
2015-09-05 23:15:13 +02:00
Shishir Mahajan
4870fb36d4 Warning message for lvm devmapper running on top of loopback devices
Signed-off-by: Shishir Mahajan <shishir.mahajan@redhat.com>
2015-08-31 10:35:48 -04:00
Miguel Morales
95e3a4ca6d Typo fix then -> than
Signed-off-by: Miguel Morales <mimoralea@hp.com>
2015-08-28 10:44:39 -05:00
Laszlo Meszaros
78676f19c1 deviceset.go: fixed link in warning about udex sync is not supported
Signed-off-by: Laszlo Meszaros <lacienator@gmail.com>
2015-08-19 14:16:40 +02:00
Alexander Morozov
6f89a8ee1b Merge pull request #15404 from vbatts/vbatts-dm-zero-sized-field
devicemapper: fix zero-sized field access
2015-08-10 09:16:11 -07:00
Vincent Batts
f83d05c3be devicemapper: fix zero-sized field access
Fixes: #15279

Due to
7904946eeb
the devices field is dropped.

This solution works on go1.4 and go1.5

Signed-off-by: Vincent Batts <vbatts@redhat.com>
2015-08-10 11:11:58 -04:00
Veres Lajos
5146232723 typofix - https://github.com/vlajos/misspell_fixer
Signed-off-by: Veres Lajos <vlajos@gmail.com>
2015-08-07 23:25:49 +01:00
Kir Kolyshkin
a83a769347 Simplify and fix os.MkdirAll() usage
TL;DR: check for IsExist(err) after a failed MkdirAll() is both
redundant and wrong -- so two reasons to remove it.

Quoting MkdirAll documentation:

> MkdirAll creates a directory named path, along with any necessary
> parents, and returns nil, or else returns an error. If path
> is already a directory, MkdirAll does nothing and returns nil.

This means two things:

1. If a directory to be created already exists, no error is returned.

2. If the error returned is IsExist (EEXIST), it means there exists
a non-directory with the same name as MkdirAll need to use for
directory. Example: we want to MkdirAll("a/b"), but file "a"
(or "a/b") already exists, so MkdirAll fails.

The above is a theory, based on quoted documentation and my UNIX
knowledge.

3. In practice, though, current MkdirAll implementation [1] returns
ENOTDIR in most of cases described in #2, with the exception when
there is a race between MkdirAll and someone else creating the
last component of MkdirAll argument as a file. In this very case
MkdirAll() will indeed return EEXIST.

Because of #1, IsExist check after MkdirAll is not needed.

Because of #2 and #3, ignoring IsExist error is just plain wrong,
as directory we require is not created. It's cleaner to report
the error now.

Note this error is all over the tree, I guess due to copy-paste,
or trying to follow the same usage pattern as for Mkdir(),
or some not quite correct examples on the Internet.

[v2: a separate aufs commit is merged into this one]

[1] https://github.com/golang/go/blob/f9ed2f75/src/os/path.go

Signed-off-by: Kir Kolyshkin <kir@openvz.org>
2015-07-30 11:48:08 -07:00
Srini Brahmaroutu
972a94b449 daemon/graphdriver/devmapper/ fix lint errors/warnings
Addresses #14756
Signed-off-by: Srini Brahmaroutu <srbrahma@us.ibm.com>
2015-07-29 01:43:34 +00:00
Arnaud Porterie
98ed9a55f4 Merge pull request #14693 from LK4D4/update_libcontainer
Update libcontainer
2015-07-17 13:02:04 -07:00
Vivek Goyal
424d5e55a2 devicemapper: Change default basesize to 100G
Current default basesize is 10G. Change it to 100G. Reason being that for
some people 10G is turning out to be too small and we don't have capabilities
to grow it dyamically.

This is just overcommitting and no real space is allocated till container
actually writes data. And this is no different then fs based graphdrivers
where virtual size of a container root is unlimited.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-07-17 11:10:23 -04:00
Alexander Morozov
c86189d554 Update libcontainer
Replaced github.com/docker/libcontainer with
github.com/opencontainers/runc/libcontaier.
Also I moved AppArmor profile generation to docker.

Main idea of this update is to fix mounting cgroups inside containers.
After updating docker on CI we can even remove dind.

Signed-off-by: Alexander Morozov <lk4d4@docker.com>
2015-07-16 16:02:26 -07:00
Alexander Morozov
a5142f6ac3 Fix some formatting calls
Signed-off-by: Alexander Morozov <lk4d4@docker.com>
2015-07-15 12:25:50 -07:00
Vincent Batts
5ca3e7c54c Merge pull request #14021 from rhvgoyal/detect-pool-loopback-devices
devicemapper: Check loop devices of existing pool
2015-07-13 21:15:23 -04:00
Arnaud Porterie
f089899023 Fix panic on devicemapper initialization
The ability to save and verify base device UUID (#13896) introduced a
situation where the initialization would panic when removing the device
returns EBUSY.

Functions `verifyBaseDeviceUUID` and `saveBaseDeviceUUID` now take the
lock on the `DeviceSet`, which solves the problem as `removeDevice`
assumes it owns the lock.

Signed-off-by: Arnaud Porterie <arnaud.porterie@docker.com>
2015-07-08 09:10:20 -07:00
David Calavera
e27131519f Warn when udev_sync is not supported.
Signed-off-by: David Calavera <david.calavera@gmail.com>
2015-07-07 15:10:24 -06:00
David Calavera
9af7afb9eb Revert "Fix implicit DeviceMapper selection"
This reverts commit 0a376291b2.

Signed-off-by: David Calavera <david.calavera@gmail.com>
2015-07-07 12:27:19 -07:00
Vivek Goyal
bebf534439 devicemapper: Check loop devices of existing pool
Often it happens that docker is not able to shutdown/remove the thin
pool it created because some device has leaked into some mount name
space. That means device is in use and that means pool can't be removed.

Docker will leave pool as it is and exit. Later when user starts the
docker, it finds pool is already there and docker uses it. But docker
does not know it is same pool which is using the loop devices. Now
docker thinks loop devices are not being used. That means it does not
display the data correctly in "docker info", giving user wrong information.

This patch tries to detect if loop devices as created by docker are
being used for pool and fills in the right details in "docker info".

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-07-07 14:13:29 -04:00
David Calavera
0a376291b2 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>
2015-07-02 09:21:27 -07:00
David Calavera
85d3b75dfd Merge pull request #13896 from rhvgoyal/verify-base-uuid
devicemapper: Compare uuid of base device on startup
2015-06-17 11:30:04 -07:00
Vivek Goyal
c06b05b11e devicemapper: Compare uuid of base device on startup
It is easy for one to use docker for a while, shut it down and restart
docker with different set of storage options for device mapper driver
which will effectively change the thin pool. That means any of the
metadata stored in /var/lib/docker/devicemapper/metadata/ is not valid
for the new pool and user will run into various kind of issues like
container not found in the pool etc.

Users think that their images or containers are lost but it might just
be the case of configuration issue. People might use wrong metadata
with wrong pool.

To detect such situations, save UUID of base image and once docker
starts later, query and compare the UUID of base image with the
stored one. If they don't match, fail the initialization with the
error that UUID failed to match.

That way user will be forced to cleanup /var/lib/docker/ directory
and start docker again.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-06-16 21:12:27 -04:00
Vivek Goyal
407a626be6 docker-inspect: Extend docker inspect to export image/container metadata related to graph driver
Export image/container metadata stored in graph driver. Right now 3 fields
DeviceId, DeviceSize and DeviceName are being exported from devicemapper.
Other graph drivers can export fields as they see fit.

This data can be used to mount the thin device outside of docker and tools
can look into image/container and do some kind of inspection.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-06-15 14:05:10 -04:00
Vincent Batts
56c9917815 Merge pull request #12190 from rhvgoyal/deferred-removal
Devicemapper: Provide deferred device removal capability
2015-05-04 14:22:52 -07:00
unclejack
2e49281bd0 Merge pull request #12903 from rhvgoyal/disable-discards
devmapper: Disable mount option "discard" by default
2015-05-04 18:35:25 +03:00
Vivek Goyal
04adaaf1ee devmapper: Disable mount option "discard" by default
Right now devicemapper mounts thin device using online discards by default
and passes mount option "discard". Generally people discourage usage of
online discards as they can be a drain on performance. Instead it is 
recommended to use fstrim once in a while to reclaim the space.

In case of  containers, we recommend to keep data volumes separate. So
there might not be lot of rm, unlink operations going on and there might
not be lot of space being freed by containers. So it might not matter
much if we don't reclaim that free space in pool.

User can still pass mount option explicitly using dm.mountopt=discard to
enable discards if they would like to.

So this is more like setting the containers by default for better performance
instead of better space efficiency in pool. And user can change the behavior
if they don't like default behavior.

Reported-by: Mike Snitzer <snitzer@redhat.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-05-01 09:16:31 -04:00
Antonio Murdaca
844538142d Small if err cleaning
Signed-off-by: Antonio Murdaca <me@runcom.ninja>
2015-04-27 21:50:33 +02:00
Vivek Goyal
ddc8acebec devmapper: Cancel deferred deactivation if device is reactivated
If device is being reactivated before it could go away and deferred 
deactivation is scheduled on it, cancel it.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-04-21 18:14:59 -04:00
Vivek Goyal
66a53819ae devmapper: Export deferred removal status in status
This will help with debugging as one could just do "docker info" and figure
out of deferred removal is enabled or not.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-04-21 18:14:59 -04:00
Vivek Goyal
e37c7203bb devmapper: Use deferred removal
Make use of deferred removal of devices.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-04-21 18:14:59 -04:00
Vivek Goyal
15c158b207 devmapper: Provide a new parameter dm.deferred_device_removal
Provide a new command line knob dm.deferred_device_removal which will enable
deferred device deactivation if driver and library support it.

This patch also checks for library support and driver version.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-04-21 18:14:59 -04:00
Tibor Vass
23c12dabbd Merge pull request #11412 from vbatts/vbatts-dm_sync_is_required
devmapper: udev sync is a requirement
2015-04-10 16:52:38 -04:00
Vincent Batts
0e21782de5 devmapper: storage-opt override for udev sync
This provides an override for forcing the daemon to still attempt
running the devicemapper driver even when udev sync is not supported.

Intended to be a very clear impairment for those choosing to use it. If
udev sync is false, there will still be an error in the daemon logs,
even when the override is in place. The docs have an explicit WARNING.

Including link to the docs for users that encounter this daemon error
during an upgrade.

Signed-off-by: Vincent Batts <vbatts@redhat.com>
2015-04-10 15:43:16 -04:00
Vincent Batts
ca628c6216 devmapper: udev sync is a requirement
closes #10664
closes #4036

Signed-off-by: Vincent Batts <vbatts@redhat.com>
2015-04-06 15:17:57 -04:00
Vivek Goyal
c737800b7f devmapper: Retry device removal after 100ms instead of 10ms
Right now we try device removal at the interval of 10ms and keep on trying
till either device is removed or 10 seconds are over. That means if device
is busy, we will try 1000 times in those 10 seconds.

Sounds too high a frequency of deivce removal retrial. All the logs are
filled easily. I think it is a good idea to slow down a bit and retry at
the interval of 100ms instead of 10ms.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-04-02 16:47:14 -04:00
Vivek Goyal
f74d12012c devmapper: Remove call to waitClose()
During device removal, we are first waiting for device to close() in a tight
loop for 10 seconds. I am not sure why do we need it. First of all we come
here once the umount() is successful so device should be free. For some reason
of device is temporarily busy, then removeDevice() logic retries device removal
logic in a loop for 10 seconds and that should cover it. Can't see why one
more 10 seoncds loop is required before attempting device removal.

One loop should be able to cover all the temporary device busy conditions and
if condition is not temporary then 10 seconds loop is not going to help anyway.

So instead of two loops of 10 seconds each, I am converting it to a single
loop of 20 seconds. May be 10 second loop is good enough but for now I am
keeping it 20 seconds to avoid any regressions.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
2015-04-02 16:47:14 -04:00