device-mapper udev sync #10195

vbatts · 2015-01-19T20:18:40Z

sync devicemapper and udev.
If sync is not supported, print a warning that unexpected behavior may occur.
relates to #4036
ping @unclejack

expose an api to call dm_udev_get_sync_support/dm_udev_set_sync_support Signed-off-by: Vincent Batts <vbatts@redhat.com>

unclejack · 2015-01-19T20:32:32Z

daemon/graphdriver/devmapper/deviceset.go

@@ -947,6 +947,12 @@ func (devices *DeviceSet) initDevmapper(doInit bool) error {
 		return graphdriver.ErrNotSupported
 	}

+	// https://github.com/docker/docker/issues/4036
+	if supported := devicemapper.UdevSetSyncSupport(true); !supported {
+		log.Warnf("Warning, Udev sync is not supported. Behavior may be unpredictable")


@vbatts I'd change Warning, Udev sync is not supported. Behavior may be unpredictable to something along the lines of WARNING: Udev sync is not supported. This will lead to unexpected behavior, data loss and errors.
Perhaps we should also print the kind of errors (like the ones seen on #4036) one would see when printing this warning? This should make it easier to set expectations, rather than letting users ignore it.

what about just a link to #4036?

I've thought about that, but #4036 seems to have become a catchall for devicemapper related issues. It might end up being very confusing when trying to figure out exactly what kind of errors could be attributed to this and what could be caused by something else.

when initializing the devmapper driver, attempt to sync udev and device mapper. If udev sync is not supported, print a warning. Eventually we'll likely bail here to avoid unpredictable behavior for users. Signed-off-by: Vincent Batts <vbatts@redhat.com>

vbatts · 2015-01-19T20:58:03Z

@unclejack I just updated the warning

icecrime · 2015-01-19T21:18:18Z

LGTM, and also interested to see this setting's value in docker info output!

vbatts · 2015-01-19T21:21:52Z

@icecrime oh? well that is an idea too, and easy enough to do. Though I would not print a warning then, only in the daemon log.

icecrime · 2015-01-19T21:26:28Z

It doesn't necessarily have to be in this PR. It's just that I understand from reading the patch that udev sync really matters, so if we see issues filed for device mapper problems it would probably make sense to know the value of this setting for investigation, right?

now: ``` [...] Storage Driver: devicemapper Pool Name: docker-253:2-5767172-pool [...] Udev Sync Supported: true [...] ``` Signed-off-by: Vincent Batts <vbatts@redhat.com>

vbatts · 2015-01-19T21:29:30Z

@icecrime done. PTAL.

icecrime · 2015-01-19T21:33:46Z

Perfect thanks! LGTM

unclejack · 2015-01-19T21:36:00Z

I've tested this on CentOS 7 with dynbinary and it's all working just fine.

This can be merged after the drone build is done.

docker info output on CentOS 7:

Containers: 1
Images: 5
Storage Driver: devicemapper
 Pool Name: docker-253:0-33667072-pool
 Pool Blocksize: 65.54 kB
 Backing Filesystem: <unknown>
 Data file: /dev/loop0
 Metadata file: /dev/loop1
 Data Space Used: 544.4 MB
 Data Space Total: 107.4 GB
 Metadata Space Used: 1.04 MB
 Metadata Space Total: 2.147 GB
 Udev Sync Supported: true
 Data loop file: /var/lib/docker/devicemapper/devicemapper/data
 Metadata loop file: /var/lib/docker/devicemapper/devicemapper/metadata
 Library Version: 1.02.84-RHEL7 (2014-03-26)
Execution Driver: native-0.2
Kernel Version: 3.10.0-123.13.2.el7.x86_64
Operating System: CentOS Linux 7 (Core)
CPUs: 2
Total Memory: 2.782 GiB
Name: centos7-dm

LGTM

device-mapper udev sync

rhvgoyal · 2015-01-21T17:40:11Z

I am looking at libdm code.

dm_udev_set_sync_support() does not do anything if libdm was built without UDEV_SYNC_SUPPORT. If udev sync support was enabled, dm_udev_set_sync_support() sets
variable _sync_with_udev. And default value of _sync_with_udev is 1.

IOW, if libdm is built with udev sync support, it is enabled by default. If it is not built with udev sync support then one can not enable it at run time.

So calling dm_udev_set_sync_support() from docker should not help in anyway. Am I missing something?

vbatts · 2015-01-21T18:50:43Z

@rhvgoyal while _sync_with_udev = 1 by default, the call to dm_udev_set_sync_support performs some one-time checks. Granted this is largely a no-op, because if it's compiled, it's 1, and if it's not compiled it's 0. The bigger functionality is calling of dm_udev_get_sync_support. Which does a couple of checks for support.
Since we do this on init of the driver, we can perform the checks and ensure the sync support is reported in the log and docker info

rhvgoyal · 2015-01-21T18:56:22Z

ok, so this commit will just introduce warnings if udev_sync is not supported (when docker binary is built static) and make debugging little easier. But it should not fix udev races.

So we still have the problem that how to address the issue of libdm and udev races with statically built docker.

vbatts · 2015-01-21T19:16:21Z

@rhvgoyal that is a different story. static docker, means static libdm, which would require libudev, which will not happen. 1) systemd/udev does not even allow static linking; 2) you would not want to have varying versions of udev (static in docker and whatever version on the host) because the API is not guaranteed across versions.

For static docker, you either get libdm vs. udev race or should disable the device-mapper driver entirely. Which in that case, sync_support will return 0/false.

Basically, we should get to the point of sanity checking for drivers, and if dm_udev_get_sync_support == 0 then bail on using the devicemapper driver.

davedoesdev · 2015-04-14T06:27:34Z

I'm not running udev at all - can I ignore this warning?

vbatts · 2015-04-14T13:01:29Z

David, interesting that your OS is not running udev. Can you provide more
info on your setup so that I may recreate it for myself?
On Apr 14, 2015 2:27 AM, "David Halls" notifications@github.com wrote:

I'm not running udev at all - can I ignore this warning?

—
Reply to this email directly or view it on GitHub
#10195 (comment).

vbatts · 2015-04-14T13:11:05Z

Also, if udev is not present at all then devicemapper (libdm) will
fallback to managing the devices itself, and you should be able to ignore
this warning.

Though in docker 1.7 you will need to add an additional flag to proceed
past the udev sync check.
On Apr 14, 2015 9:01 AM, "Vincent Batts" vbatts@hashbangbash.com wrote:

David, interesting that your OS is not running udev. Can you provide more
info on your setup so that I may recreate it for myself?
On Apr 14, 2015 2:27 AM, "David Halls" notifications@github.com wrote:

I'm not running udev at all - can I ignore this warning?

—
Reply to this email directly or view it on GitHub
#10195 (comment).

davedoesdev · 2015-04-14T19:21:22Z

Thanks - that'll be --storage-opt dm.override_udev_sync_check=true, right? That's not going to make it for 1.6?

I'm running a Linux OS I built myself: https://github.com/davedoesdev/heddle

vbatts · 2015-04-17T02:49:43Z

That's the right flag. Hopefully you'll trial the different drivers you can
support. Also you should ensure you are running devicemapper not on
loopfile (which is the default), but actually setting up block devices.
On Apr 14, 2015 3:21 PM, "David Halls" notifications@github.com wrote:

Thanks - that'll be --storage-opt dm.override_udev_sync_check=true,
right? That's not going to make it for 1.6?

I'm running my a Linux OS I built myself:
https://github.com/davedoesdev/heddle

—
Reply to this email directly or view it on GitHub
#10195 (comment).

davedoesdev · 2015-04-17T06:37:40Z

Thanks. I also support btrfs which I'm finding to be a great filesystem. I'll probably default to btrfs soon and leave the devicemapper backend on loopfile.

devicemapper: dm_udev_get_sync_support

9c33800

expose an api to call dm_udev_get_sync_support/dm_udev_set_sync_support Signed-off-by: Vincent Batts <vbatts@redhat.com>

unclejack reviewed Jan 19, 2015
View reviewed changes

devmapper: udev sync on init

022e123

when initializing the devmapper driver, attempt to sync udev and device mapper. If udev sync is not supported, print a warning. Eventually we'll likely bail here to avoid unpredictable behavior for users. Signed-off-by: Vincent Batts <vbatts@redhat.com>

vbatts force-pushed the vbatts-dm_udev_sync branch from 2c6fb0b to 022e123 Compare January 19, 2015 20:57

icecrime added this to the 1.5.0 milestone Jan 19, 2015

devmapper: udev sync in docker info

d259354

now: ``` [...] Storage Driver: devicemapper Pool Name: docker-253:2-5767172-pool [...] Udev Sync Supported: true [...] ``` Signed-off-by: Vincent Batts <vbatts@redhat.com>

unclejack added a commit that referenced this pull request Jan 19, 2015

Merge pull request #10195 from vbatts/vbatts-dm_udev_sync

2d61a62

device-mapper udev sync

unclejack merged commit 2d61a62 into moby:master Jan 19, 2015

This was referenced Jan 21, 2015

Try to cleanly unmount docker block devices #10229

Closed

Failed to remove instance as device is busy #10174

Closed

SvenDowideit mentioned this pull request Jan 27, 2015

Post 1.4.1 docs update 2 #9739

Merged

unclejack mentioned this pull request Feb 9, 2015

devicemapper: exclude from static Docker builds #10664

Closed

vbatts deleted the vbatts-dm_udev_sync branch April 27, 2016 18:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

device-mapper udev sync #10195

device-mapper udev sync #10195

vbatts commented Jan 19, 2015

unclejack Jan 19, 2015

vbatts Jan 19, 2015

unclejack Jan 19, 2015

vbatts commented Jan 19, 2015

icecrime commented Jan 19, 2015

vbatts commented Jan 19, 2015

icecrime commented Jan 19, 2015

vbatts commented Jan 19, 2015

icecrime commented Jan 19, 2015

unclejack commented Jan 19, 2015

rhvgoyal commented Jan 21, 2015

vbatts commented Jan 21, 2015

rhvgoyal commented Jan 21, 2015

vbatts commented Jan 21, 2015

davedoesdev commented Apr 14, 2015

vbatts commented Apr 14, 2015

vbatts commented Apr 14, 2015

davedoesdev commented Apr 14, 2015

vbatts commented Apr 17, 2015

davedoesdev commented Apr 17, 2015

device-mapper udev sync #10195

device-mapper udev sync #10195

Conversation

vbatts commented Jan 19, 2015

unclejack Jan 19, 2015

Choose a reason for hiding this comment

vbatts Jan 19, 2015

Choose a reason for hiding this comment

unclejack Jan 19, 2015

Choose a reason for hiding this comment

vbatts commented Jan 19, 2015

icecrime commented Jan 19, 2015

vbatts commented Jan 19, 2015

icecrime commented Jan 19, 2015

vbatts commented Jan 19, 2015

icecrime commented Jan 19, 2015

unclejack commented Jan 19, 2015

rhvgoyal commented Jan 21, 2015

vbatts commented Jan 21, 2015

rhvgoyal commented Jan 21, 2015

vbatts commented Jan 21, 2015

davedoesdev commented Apr 14, 2015

vbatts commented Apr 14, 2015

vbatts commented Apr 14, 2015

davedoesdev commented Apr 14, 2015

vbatts commented Apr 17, 2015

davedoesdev commented Apr 17, 2015