devmapper: use 'user_xattr' as mountoption for ext4 - fixes #6189 #6190

harshavardhana · Jun 4, 2014

># docker pull centos
># docker run -i -t centos /bin/bash

bash-4.1# touch file
bash-4.1# setfattr -n user.attr -v 1 file
setfattr: file: Operation not permitted

With 'user_xattr' as mount option

bash-4.1# touch file
bash-4.1# setfattr -n user.attr -v 1 file
bash-4.1# getfattr -d -m. file
>#file: file
user.xattr="1"

@alexlarsson

ping @alexlarsson @rhatdan

Can you review this for us?

crosbymichael · Jun 4, 2014

ping @alexlarsson @rhatdan

Can you review this for us?

It needs more work, see my comments in: #6189

alexlarsson · Jun 5, 2014

It needs more work, see my comments in: #6189

Added the user xattr layer of image layer work, please review!

harshavardhana · Jun 5, 2014

Added the user xattr layer of image layer work, please review!

LGTM, pending ubuntu review

it adds the exported tar includes the xattrs as well. :-)

^@vbatts@noyee ~/src/docker/docker ((48710e5...)) $ cat D.xattr
FROM centos:latest
RUN touch file && setfattr -n user.attr -v 1 file && getfattr -d -m. file
vbatts@noyee ~/src/docker/docker ((48710e5...)) $ cat D.xattr | /home/vbatts/src/docker/docker/bundles/1.0.0-dev/dynbinary/docker-1.0.0-dev build -t x -
# ...
vbatts@noyee ~/src/docker/docker ((48710e5...)) $ /home/vbatts/src/docker/docker/bundles/1.0.0-dev/dynbinary/docker-1.0.0-dev save x | strings | grep SCHILY
28 SCHILY.xattr.user.attr=1

I'm spinning up an ubuntu box to ensure the portability of the container and process

vbatts · Jun 11, 2014

LGTM, pending ubuntu review

it adds the exported tar includes the xattrs as well. :-)

^@vbatts@noyee ~/src/docker/docker ((48710e5...)) $ cat D.xattr
FROM centos:latest
RUN touch file && setfattr -n user.attr -v 1 file && getfattr -d -m. file
vbatts@noyee ~/src/docker/docker ((48710e5...)) $ cat D.xattr | /home/vbatts/src/docker/docker/bundles/1.0.0-dev/dynbinary/docker-1.0.0-dev build -t x -
# ...
vbatts@noyee ~/src/docker/docker ((48710e5...)) $ /home/vbatts/src/docker/docker/bundles/1.0.0-dev/dynbinary/docker-1.0.0-dev save x | strings | grep SCHILY
28 SCHILY.xattr.user.attr=1

I'm spinning up an ubuntu box to ensure the portability of the container and process

I'm having issues getting my VMs set up right now,
so I'll say this LGTM

I've run it through the make test and make test-integration and it passes clean as well.

vbatts · Jun 11, 2014

I'm having issues getting my VMs set up right now,
so I'll say this LGTM

I've run it through the make test and make test-integration and it passes clean as well.

crosbymichael · Jun 12, 2014

crosbymichael reviewed Jun 12, 2014

View changes

pkg/system/xattrs_linux.go

Rebased!

harshavardhana · Jun 13, 2014

Rebased!

Any other caveats pending on this one?

harshavardhana · Jun 16, 2014

Any other caveats pending on this one?

@harshavardhana

@harshavardhana not particularly. One thing I would like to see tested is the case of different graph drivers to ensure the behaviour is consistent. Otherwise LGTM

vbatts · Jun 16, 2014

@harshavardhana not particularly. One thing I would like to see tested is the case of different graph drivers to ensure the behaviour is consistent. Otherwise LGTM

Rebased with master!

harshavardhana · Jun 18, 2014

Rebased with master!

@harshavardhana

@harshavardhana would mind providing any information on the aufs or btrfs support for user xattrs? My concern is that if a daemon with xattr support produces an image layer that stores user xattr information. Once that image is pulled down by a daemon that does not have xattr support, the image could not be pushed somewhere and be the same as the original image layers pulled down.
Make sense?

vbatts · Jun 21, 2014

@harshavardhana would mind providing any information on the aufs or btrfs support for user xattrs? My concern is that if a daemon with xattr support produces an image layer that stores user xattr information. Once that image is pulled down by a daemon that does not have xattr support, the image could not be pushed somewhere and be the same as the original image layers pulled down.
Make sense?

In-fact it is only applicable when one has user xattrs on the container image and one does a commit.

Since it does filter out only 'user xattrs' , the user xattr support is limited to devmapper driver based filesystems xfs and ext4.

Let me test out what are the results with btrfs and aufs

harshavardhana · Jun 21, 2014

In-fact it is only applicable when one has user xattrs on the container image and one does a commit.

Since it does filter out only 'user xattrs' , the user xattr support is limited to devmapper driver based filesystems xfs and ext4.

Let me test out what are the results with btrfs and aufs

For aufs there is no xattr support so - even if one invokes the code to capture xattr's noone will be captured since noone exists in the first place.

# sudo docker run -i -t ubuntu:14.04 /bin/bash

root@32e9e9018378:/# setfattr -n user.xattr -v 1 file
setfattr: file: Operation not supported
root@32e9e9018378:/#

It is quite logical from here to btrfs if an on disk FS stores xattrs, it will be captured if one commits or does a 'save/restore' .

What you are saying is in-fact true, it destroy's backward compatibility, but in such cases an import should fail of the daemon doesn't support xattrs and tries to pull in an image which contains an xattr - almost as if its the format which it doesn't understand. I guess that isn't part of previous code base is it ? ;-)

harshavardhana · Jun 21, 2014

For aufs there is no xattr support so - even if one invokes the code to capture xattr's noone will be captured since noone exists in the first place.

# sudo docker run -i -t ubuntu:14.04 /bin/bash

root@32e9e9018378:/# setfattr -n user.xattr -v 1 file
setfattr: file: Operation not supported
root@32e9e9018378:/#

It is quite logical from here to btrfs if an on disk FS stores xattrs, it will be captured if one commits or does a 'save/restore' .

What you are saying is in-fact true, it destroy's backward compatibility, but in such cases an import should fail of the daemon doesn't support xattrs and tries to pull in an image which contains an xattr - almost as if its the format which it doesn't understand. I guess that isn't part of previous code base is it ? ;-)

well apparently aufs can pass through capget(2)/capset(2) to some extent (http://permalink.gmane.org/gmane.linux.file-systems.aufs.user/2580), so the capabilities stored aren't such an issue.

But the user xattr's are completely dropped on the floor. So if we enable user xattrs in one graphdriver, then a container is built to use something stored in those user-xattrs, the container is no longer portable to docker daemon's that use the aufs graph driver.

vbatts · Jun 24, 2014

well apparently aufs can pass through capget(2)/capset(2) to some extent (http://permalink.gmane.org/gmane.linux.file-systems.aufs.user/2580), so the capabilities stored aren't such an issue.

But the user xattr's are completely dropped on the floor. So if we enable user xattrs in one graphdriver, then a container is built to use something stored in those user-xattrs, the container is no longer portable to docker daemon's that use the aufs graph driver.

@harshavardhana

I stand corrected. The AUFS driver can import a layer that has the user-xattrs included,

vbatts@ubuntu:~$ sudo /home/vbatts/docker/bundles/1.0.1-dev/dynbinary/docker-1.0.1-dev info
Containers: 0
Images: 0
Storage Driver: aufs
 Root Dir: /var/lib/docker/aufs
 Dirs: 0
Execution Driver: native-0.2
Kernel Version: 3.13.0-24-generic
Debug mode (server): true
Debug mode (client): false
Fds: 9
Goroutines: 9
EventsListeners: 0
Init SHA1: 1a075d82f3fb1f26a3b0d178e7366b719ac76dc9
Init Path: /home/vbatts/docker/bundles/1.0.1-dev/dynbinary/dockerinit
WARNING: No swap limit support
vbatts@ubuntu:~$ sudo /home/vbatts/docker/bundles/1.0.1-dev/dynbinary/docker-1.0.1-dev images
REPOSITORY          TAG                 IMAGE ID            CREATED             VIRTUAL SIZE
vbatts@ubuntu:~$ sudo /home/vbatts/docker/bundles/1.0.1-dev/dynbinary/docker-1.0.1-dev load -i ./x.tar
vbatts@ubuntu:~$ sudo /home/vbatts/docker/bundles/1.0.1-dev/dynbinary/docker-1.0.1-dev images x
REPOSITORY          TAG                 IMAGE ID            CREATED             VIRTUAL SIZE
x                   latest              b5d3da655b71        16 minutes ago      124.1 MB
vbatts@ubuntu:~$ sudo /home/vbatts/docker/bundles/1.0.1-dev/dynbinary/docker-1.0.1-dev run -it --rm x getfattr -d -m. file
vbatts@ubuntu:~$ sudo find /var/lib/docker -type f -name file -exec getfattr -d -m. "{}" \;
getfattr: Removing leading '/' from absolute path names
# file: var/lib/docker/aufs/diff/b5d3da655b71a545c950708f3644244ef1d9680262248b9d42e2ae20a508e5dc/file
user.attr="1"

vbatts@ubuntu:~$ sudo /home/vbatts/docker/bundles/1.0.1-dev/dynbinary/docker-1.0.1-dev save x | tar xO b5d3da655b71a545c950708f3644244ef1d9680262248b9d42e2ae20a508e5dc/layer.tar | strings | grep SCHILY
28 SCHILY.xattr.user.attr=1

So as long as the xattrs are supported in the host's FS, then they are preserved. AUFS will not expose them for the container.
That relieves my concern about the portability of the container.

I've tested capset/capget and user-xattrs on devmapper (fedora and slackware) and btrfs (ubuntu), they work just fine. I've tested AUFS (ubuntu) can load images with capabilites and user-xattrs, and can still save or push the image and maintain the capabilities/user-xattrs, despite not being able to let containers access the data stored there.

Now it is only a question of whether the reduced functionality of the aufs graphdriver is a concern for the functionality that a container might expect?

@harshavardhana @crosbymichael thoughts?

vbatts · Jun 24, 2014

I stand corrected. The AUFS driver can import a layer that has the user-xattrs included,

vbatts@ubuntu:~$ sudo /home/vbatts/docker/bundles/1.0.1-dev/dynbinary/docker-1.0.1-dev info
Containers: 0
Images: 0
Storage Driver: aufs
 Root Dir: /var/lib/docker/aufs
 Dirs: 0
Execution Driver: native-0.2
Kernel Version: 3.13.0-24-generic
Debug mode (server): true
Debug mode (client): false
Fds: 9
Goroutines: 9
EventsListeners: 0
Init SHA1: 1a075d82f3fb1f26a3b0d178e7366b719ac76dc9
Init Path: /home/vbatts/docker/bundles/1.0.1-dev/dynbinary/dockerinit
WARNING: No swap limit support
vbatts@ubuntu:~$ sudo /home/vbatts/docker/bundles/1.0.1-dev/dynbinary/docker-1.0.1-dev images
REPOSITORY          TAG                 IMAGE ID            CREATED             VIRTUAL SIZE
vbatts@ubuntu:~$ sudo /home/vbatts/docker/bundles/1.0.1-dev/dynbinary/docker-1.0.1-dev load -i ./x.tar
vbatts@ubuntu:~$ sudo /home/vbatts/docker/bundles/1.0.1-dev/dynbinary/docker-1.0.1-dev images x
REPOSITORY          TAG                 IMAGE ID            CREATED             VIRTUAL SIZE
x                   latest              b5d3da655b71        16 minutes ago      124.1 MB
vbatts@ubuntu:~$ sudo /home/vbatts/docker/bundles/1.0.1-dev/dynbinary/docker-1.0.1-dev run -it --rm x getfattr -d -m. file
vbatts@ubuntu:~$ sudo find /var/lib/docker -type f -name file -exec getfattr -d -m. "{}" \;
getfattr: Removing leading '/' from absolute path names
# file: var/lib/docker/aufs/diff/b5d3da655b71a545c950708f3644244ef1d9680262248b9d42e2ae20a508e5dc/file
user.attr="1"

vbatts@ubuntu:~$ sudo /home/vbatts/docker/bundles/1.0.1-dev/dynbinary/docker-1.0.1-dev save x | tar xO b5d3da655b71a545c950708f3644244ef1d9680262248b9d42e2ae20a508e5dc/layer.tar | strings | grep SCHILY
28 SCHILY.xattr.user.attr=1

So as long as the xattrs are supported in the host's FS, then they are preserved. AUFS will not expose them for the container.
That relieves my concern about the portability of the container.

I've tested capset/capget and user-xattrs on devmapper (fedora and slackware) and btrfs (ubuntu), they work just fine. I've tested AUFS (ubuntu) can load images with capabilites and user-xattrs, and can still save or push the image and maintain the capabilities/user-xattrs, despite not being able to let containers access the data stored there.

Now it is only a question of whether the reduced functionality of the aufs graphdriver is a concern for the functionality that a container might expect?

@harshavardhana @crosbymichael thoughts?

@vbatts

Thanks for testing this @vbatts - I would defer this to @crosbymichael regarding aufs. If we agree on the reduced functionality then - i could add this as a small doc explaining this.

harshavardhana · Jun 24, 2014

Thanks for testing this @vbatts - I would defer this to @crosbymichael regarding aufs. If we agree on the reduced functionality then - i could add this as a small doc explaining this.

@crosbymichael

@crosbymichael anything else do you think is needed here?

harshavardhana · Jun 27, 2014

@crosbymichael anything else do you think is needed here?

@crosbymichael

@crosbymichael @vbatts anything else pending on this one?

harshavardhana · Jun 30, 2014

@crosbymichael @vbatts anything else pending on this one?

Checking back again if anyone has any more issues here?

harshavardhana · Jul 8, 2014

Checking back again if anyone has any more issues here?

@crosbymichael

LGTM

ping @crosbymichael

vbatts · Jul 11, 2014

LGTM

ping @crosbymichael

Rebased!

harshavardhana · Jul 21, 2014

Rebased!

@harshavardhana

@harshavardhana or @vbatts could you please add some test ?

vieux · Jul 22, 2014

@harshavardhana or @vbatts could you please add some test ?

@vieux

As @vieux has stated above, the lack of tests is a concern for this PR.
How does this change things for images pulled to systems which don't support xattrs? How does this change things for images in general?

unclejack · Jul 22, 2014

As @vieux has stated above, the lack of tests is a concern for this PR.
How does this change things for images pulled to systems which don't support xattrs? How does this change things for images in general?

@unclejack

@unclejack - this was tested by @vbatts a few weeks ago - if you scroll up

@vieux - let me see where can i add the tests. Thanks for looking

harshavardhana · Jul 22, 2014

@unclejack - this was tested by @vbatts a few weeks ago - if you scroll up

@vieux - let me see where can i add the tests. Thanks for looking

@unclejack

@unclejack and @vieux having unit tests for this would be neat but would need something like vagrant so that we could test images inside hosts with particular graphdrivers. Otherwise this is a manual test.

As far as systems that do not support xattrs (AUFS), there are two considerations.

portability of the images: this feature will not break portability. You can pull an image with xattrs stored, and proceed to push or save that image, and the xattrs are preserved.
Container functionality requiring usage of xattrs: this is presently broken (for capabilities and file ACLs), and this feature does not affect that. This PR will just further allow access to user setable xattrs.

This feature LGTM

p.s. to test the presently broken xattrs, build the following Dockerfile on a fedora host

FROM fedora:20
RUN yum install -y httpd
CMD getcap /usr/sbin/suexec

Save this image, and load it to an ubuntu host (or docker push and pull through a local registry).
The ubuntu host will properly set the capabilities on the suexec file, but the container will get a permission denied on accessing them.

vbatts · Jul 22, 2014

@unclejack and @vieux having unit tests for this would be neat but would need something like vagrant so that we could test images inside hosts with particular graphdrivers. Otherwise this is a manual test.

As far as systems that do not support xattrs (AUFS), there are two considerations.

portability of the images: this feature will not break portability. You can pull an image with xattrs stored, and proceed to push or save that image, and the xattrs are preserved.
Container functionality requiring usage of xattrs: this is presently broken (for capabilities and file ACLs), and this feature does not affect that. This PR will just further allow access to user setable xattrs.

This feature LGTM

p.s. to test the presently broken xattrs, build the following Dockerfile on a fedora host

FROM fedora:20
RUN yum install -y httpd
CMD getcap /usr/sbin/suexec

Save this image, and load it to an ubuntu host (or docker push and pull through a local registry).
The ubuntu host will properly set the capabilities on the suexec file, but the container will get a permission denied on accessing them.

Anything here that needs my attention?

harshavardhana · Aug 6, 2014

Anything here that needs my attention?

@vieux

@vieux @unclejack @vbatts ping?

harshavardhana · Aug 7, 2014

@vieux @unclejack @vbatts ping?

@unclejack

ping @unclejack

vieux · Aug 11, 2014

ping @unclejack

@crosbymichael

Looks like things are building fine now, don't know what changed. In anycase the changes are now using the libcontainer xattr helpers, please review @crosbymichael @vieux @vbatts @unclejack @tiborvass

harshavardhana · Oct 7, 2014

Looks like things are building fine now, don't know what changed. In anycase the changes are now using the libcontainer xattr helpers, please review @crosbymichael @vieux @vbatts @unclejack @tiborvass

@crosbymichael

The drone builds are currently misleading, the best way would be to run the
tests on your end, most likely if they were failing before you could get to
the bottom of it that way.
On Oct 6, 2014 8:34 PM, "Harshavardhana" notifications@github.com wrote:

Looks like things are building fine now, don't know what changed. In
anycase the changes are now using the libcontainer xattr helpers, please
review @crosbymichael https://github.com/crosbymichael @vieux
https://github.com/vieux @vbatts https://github.com/vbatts @unclejack
https://github.com/unclejack @tiborvass https://github.com/tiborvass

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

jessfraz · Oct 7, 2014

The drone builds are currently misleading, the best way would be to run the
tests on your end, most likely if they were failing before you could get to
the bottom of it that way.
On Oct 6, 2014 8:34 PM, "Harshavardhana" notifications@github.com wrote:

Looks like things are building fine now, don't know what changed. In
anycase the changes are now using the libcontainer xattr helpers, please
review @crosbymichael https://github.com/crosbymichael @vieux
https://github.com/vieux @vbatts https://github.com/vbatts @unclejack
https://github.com/unclejack @tiborvass https://github.com/tiborvass

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

Info:

# ../docker/bundles/1.2.0-dev/binary/docker info
Containers: 12
Images: 73
Storage Driver: devicemapper
 Pool Name: docker-253:0-2758951-pool
 Pool Blocksize: 65.54 kB
 Data file: /var/lib/docker/devicemapper/devicemapper/data
 Metadata file: /var/lib/docker/devicemapper/devicemapper/metadata
 Data Space Used: 4.829 GB
 Data Space Total: 107.4 GB
 Metadata Space Used: 7.086 MB
 Metadata Space Total: 2.147 GB
 Library Version: 1.02.82-git (2013-10-04)
Execution Driver: native-0.2
Kernel Version: 3.15.8-200.fc20.x86_64
Operating System: Fedora 20 (Heisenbug)

Version

# ../docker/bundles/1.2.0-dev/binary/docker version
Client version: 1.2.0-dev
Client API version: 1.15
Go version (client): go1.3.3
Git commit (client): bec026a
OS/Arch (client): linux/amd64
Server version: 1.2.0-dev
Server API version: 1.15
Go version (server): go1.3.3
Git commit (server): bec026a

Running the test

# ../docker/bundles/1.2.0-dev/binary/docker run -it gh160
/usr/sbin/suexec = cap_setgid,cap_setuid+ep
getfattr: Removing leading '/' from absolute path names
# file: file
user.gh160

Dockerfile used

# cat Dockerfile.gh160 
FROM fedora:20
RUN yum install -y httpd
RUN yum install -y attr
RUN touch /file && setfattr -n user.gh160 -v test /file
CMD getcap /usr/sbin/suexec && getfattr /file

Xattrs captured

# ../docker/bundles/1.2.0-dev/binary/docker save gh160 | strings | grep SCHILY
26 SCHILY.xattr.file=test
SCHILY.xL
SCHILY.acl.access
SCHILY.acl.default
SCHILY.xattr
57 SCHILY.xattr.security.capability=
153 SCHILY.xattr.var/cache/yum/x86_64/20/fedora/repomd.xml=http://mirror.us.as6453.net/fedora/linux/releases/20/Everything/x86_64/os/repodata/repomd.xml
197 SCHILY.xattr.var/cache/yum/x86_64/20/fedora/0054652738782478eff37604abc430cdf327795d157634e1dfcca0fa18d8fedf-primary.sqlite.bz2=0054652738782478eff37604abc430cdf327795d157634e1dfcca0fa18d8fedf
195 SCHILY.xattr.var/cache/yum/x86_64/20/fedora/0da963383df1bcd1e3cd8d3403a32157b6ebb42a0790eab36896fa82bb4305e0-comps-f20.xml.gz=0da963383df1bcd1e3cd8d3403a32157b6ebb42a0790eab36896fa82bb4305e0
135 SCHILY.xattr.var/cache/yum/x86_64/20/fedora/gen/primary_db.sqlite=3f518bf1b62b32fe667635ed6c1e7b36b1bfe40d35eecdcca6915cc45e5721c3
131 SCHILY.xattr.var/cache/yum/x86_64/20/fedora/metalink.xml=https://mirrors.fedoraproject.org/metalink?repo=fedora-20&arch=x86_64
143 SCHILY.xattr.var/cache/yum/x86_64/20/updates/metalink.xml=https://mirrors.fedoraproject.org/metalink?repo=updates-released-f20&arch=x86_64
140 SCHILY.xattr.var/cache/yum/x86_64/20/updates/repomd.xml=http://mirrors.syringanetworks.net/fedora/updates/20/x86_64/repodata/repomd.xml
197 SCHILY.xattr.var/cache/yum/x86_64/20/updates/62d65adc684a8058d3781d84e6a524851c70e297803820b969226f05fbd5c3ce-updateinfo.xml.gz=62d65adc684a8058d3781d84e6a524851c70e297803820b969226f05fbd5c3ce
198 SCHILY.xattr.var/cache/yum/x86_64/20/updates/881dcfaa46650fc17cb7c81974f65a845b8a5c197b097e1a974f88e338c29f2d-primary.sqlite.bz2=881dcfaa46650fc17cb7c81974f65a845b8a5c197b097e1a974f88e338c29f2d
197 SCHILY.xattr.var/cache/yum/x86_64/20/updates/9235e2563f4c3067a4ea2e39661c23c08a19ca6586bc8e966c307156e1d36223-pkgtags.sqlite.gz=9235e2563f4c3067a4ea2e39661c23c08a19ca6586bc8e966c307156e1d36223
136 SCHILY.xattr.var/cache/yum/x86_64/20/updates/gen/primary_db.sqlite=fae251becac00c9d754faffd063543a620014aa5a7e70931f4db4cee0ff9edd2
196 SCHILY.xattr.var/cache/yum/x86_64/20/updates/8c6419ad822dfe29283fb3ac98dcc5908810cb31f4cfe690040c42c144b7492e-comps-f20.xml.gz=8c6419ad822dfe29283fb3ac98dcc5908810cb31f4cfe690040c42c144b7492e

Looks like the xattrs are being captured properly without any go runtime panic. Performing more tests.. in the meantime

harshavardhana · Oct 9, 2014

Info:

# ../docker/bundles/1.2.0-dev/binary/docker info
Containers: 12
Images: 73
Storage Driver: devicemapper
 Pool Name: docker-253:0-2758951-pool
 Pool Blocksize: 65.54 kB
 Data file: /var/lib/docker/devicemapper/devicemapper/data
 Metadata file: /var/lib/docker/devicemapper/devicemapper/metadata
 Data Space Used: 4.829 GB
 Data Space Total: 107.4 GB
 Metadata Space Used: 7.086 MB
 Metadata Space Total: 2.147 GB
 Library Version: 1.02.82-git (2013-10-04)
Execution Driver: native-0.2
Kernel Version: 3.15.8-200.fc20.x86_64
Operating System: Fedora 20 (Heisenbug)

Version

# ../docker/bundles/1.2.0-dev/binary/docker version
Client version: 1.2.0-dev
Client API version: 1.15
Go version (client): go1.3.3
Git commit (client): bec026a
OS/Arch (client): linux/amd64
Server version: 1.2.0-dev
Server API version: 1.15
Go version (server): go1.3.3
Git commit (server): bec026a

Running the test

# ../docker/bundles/1.2.0-dev/binary/docker run -it gh160
/usr/sbin/suexec = cap_setgid,cap_setuid+ep
getfattr: Removing leading '/' from absolute path names
# file: file
user.gh160

Dockerfile used

# cat Dockerfile.gh160 
FROM fedora:20
RUN yum install -y httpd
RUN yum install -y attr
RUN touch /file && setfattr -n user.gh160 -v test /file
CMD getcap /usr/sbin/suexec && getfattr /file

Xattrs captured

# ../docker/bundles/1.2.0-dev/binary/docker save gh160 | strings | grep SCHILY
26 SCHILY.xattr.file=test
SCHILY.xL
SCHILY.acl.access
SCHILY.acl.default
SCHILY.xattr
57 SCHILY.xattr.security.capability=
153 SCHILY.xattr.var/cache/yum/x86_64/20/fedora/repomd.xml=http://mirror.us.as6453.net/fedora/linux/releases/20/Everything/x86_64/os/repodata/repomd.xml
197 SCHILY.xattr.var/cache/yum/x86_64/20/fedora/0054652738782478eff37604abc430cdf327795d157634e1dfcca0fa18d8fedf-primary.sqlite.bz2=0054652738782478eff37604abc430cdf327795d157634e1dfcca0fa18d8fedf
195 SCHILY.xattr.var/cache/yum/x86_64/20/fedora/0da963383df1bcd1e3cd8d3403a32157b6ebb42a0790eab36896fa82bb4305e0-comps-f20.xml.gz=0da963383df1bcd1e3cd8d3403a32157b6ebb42a0790eab36896fa82bb4305e0
135 SCHILY.xattr.var/cache/yum/x86_64/20/fedora/gen/primary_db.sqlite=3f518bf1b62b32fe667635ed6c1e7b36b1bfe40d35eecdcca6915cc45e5721c3
131 SCHILY.xattr.var/cache/yum/x86_64/20/fedora/metalink.xml=https://mirrors.fedoraproject.org/metalink?repo=fedora-20&arch=x86_64
143 SCHILY.xattr.var/cache/yum/x86_64/20/updates/metalink.xml=https://mirrors.fedoraproject.org/metalink?repo=updates-released-f20&arch=x86_64
140 SCHILY.xattr.var/cache/yum/x86_64/20/updates/repomd.xml=http://mirrors.syringanetworks.net/fedora/updates/20/x86_64/repodata/repomd.xml
197 SCHILY.xattr.var/cache/yum/x86_64/20/updates/62d65adc684a8058d3781d84e6a524851c70e297803820b969226f05fbd5c3ce-updateinfo.xml.gz=62d65adc684a8058d3781d84e6a524851c70e297803820b969226f05fbd5c3ce
198 SCHILY.xattr.var/cache/yum/x86_64/20/updates/881dcfaa46650fc17cb7c81974f65a845b8a5c197b097e1a974f88e338c29f2d-primary.sqlite.bz2=881dcfaa46650fc17cb7c81974f65a845b8a5c197b097e1a974f88e338c29f2d
197 SCHILY.xattr.var/cache/yum/x86_64/20/updates/9235e2563f4c3067a4ea2e39661c23c08a19ca6586bc8e966c307156e1d36223-pkgtags.sqlite.gz=9235e2563f4c3067a4ea2e39661c23c08a19ca6586bc8e966c307156e1d36223
136 SCHILY.xattr.var/cache/yum/x86_64/20/updates/gen/primary_db.sqlite=fae251becac00c9d754faffd063543a620014aa5a7e70931f4db4cee0ff9edd2
196 SCHILY.xattr.var/cache/yum/x86_64/20/updates/8c6419ad822dfe29283fb3ac98dcc5908810cb31f4cfe690040c42c144b7492e-comps-f20.xml.gz=8c6419ad822dfe29283fb3ac98dcc5908810cb31f4cfe690040c42c144b7492e

Looks like the xattrs are being captured properly without any go runtime panic. Performing more tests.. in the meantime

oh! i see the issue :-
vendor/src/github.com/docker/libcontainer/xattr/xattr.go should have a corresponding noop for other platforms. So when ./pkg/archive/archive.go imports it for the cross build, it fails because there is no importable source in vendor/src/github.com/docker/libcontainer/xattr/

that xattr.go is better to be named xattr_linux.go, and also provide an xattr_unsupported.go which has // +build !linux and a bunch of noop commands. Frequently you'll see a pattern like this, and a base file, like xattr.go would look more like a header or prototype of the functions that are provided by your platform.

vbatts · Oct 9, 2014

oh! i see the issue :-
vendor/src/github.com/docker/libcontainer/xattr/xattr.go should have a corresponding noop for other platforms. So when ./pkg/archive/archive.go imports it for the cross build, it fails because there is no importable source in vendor/src/github.com/docker/libcontainer/xattr/

that xattr.go is better to be named xattr_linux.go, and also provide an xattr_unsupported.go which has // +build !linux and a bunch of noop commands. Frequently you'll see a pattern like this, and a base file, like xattr.go would look more like a header or prototype of the functions that are provided by your platform.

@vbatts

Ah okay, my bad! didn't see that with libcontainer bulld. Thanks for the heads up @vbatts , will get those changes in at libcontainer - docker/libcontainer#219

harshavardhana · Oct 9, 2014

Ah okay, my bad! didn't see that with libcontainer bulld. Thanks for the heads up @vbatts , will get those changes in at libcontainer - docker/libcontainer#219

All the necessary changes are in libcontainer, rebased with latest commits - seems like builds and tests are going through.

Please review everyone!

harshavardhana · Oct 29, 2014

All the necessary changes are in libcontainer, rebased with latest commits - seems like builds and tests are going through.

Please review everyone!

Rebased! again

harshavardhana · Oct 31, 2014

Rebased! again

@crosbymichael

@crosbymichael @vieux @vbatts @tiborvass -- any reviews please?

harshavardhana · Nov 5, 2014

@crosbymichael @vieux @vbatts @tiborvass -- any reviews please?

@harshavardhana

@harshavardhana I'll test and review ASAP.

unclejack · Nov 5, 2014

@harshavardhana I'll test and review ASAP.

will do. I've got a couple of VMs available to check the different cases too.

vbatts · Nov 5, 2014

will do. I've got a couple of VMs available to check the different cases too.

it tests out fine. Though I am trying to build the following Dockerfile

FROM centos
RUN yum install -y attr
RUN touch /file && setfattr -n user.gh6190 -v test /file
CMD getfattr /file

on this setup

vbatts@valse ~/h (master) $ /home/vbatts/src/docker/docker/bundles/1.3.1-dev/dynbinary/docker-1.3.1-dev info
Containers: 2
Images: 35
Storage Driver: devicemapper
 Pool Name: docker-253:2-5111811-pool
 Pool Blocksize: 65.54 kB
 Data file: /home/docker/devicemapper/devicemapper/data
 Metadata file: /home/docker/devicemapper/devicemapper/metadata
 Data Space Used: 2.075 GB
 Data Space Total: 107.4 GB
 Metadata Space Used: 2.896 MB
 Metadata Space Total: 2.147 GB
 Library Version: 1.02.85 (2014-04-10)
Execution Driver: native-0.2
Kernel Version: 3.16.6-203.fc20.x86_64
Operating System: Fedora 20 (Heisenbug)
CPUs: 4
Total Memory: 11.43 GiB
Debug mode (server): true
Debug mode (client): false
Fds: 11
Goroutines: 14
EventsListeners: 0
Init SHA1: 26f0b471c8396e45585b4b9f4b74923cbcd8c3bb
Init Path: /home/docker/init/dockerinit-1.3.1-dev

and it fails on the yum install step. The client shows

Sending build context to Docker daemon 3.584 kB
Sending build context to Docker daemon
Step 0 : FROM centos
 ---> ae0c2d0bdc10
Step 1 : RUN yum install -y attr
 ---> Running in 95b9b1791902
[...]
Installed:
  attr.x86_64 0:2.4.46-12.el7

Complete!
2014/11/05 17:02:07 %s operation not supported

and the daemon is http://pastebin.com/EG6dc0Pi

Thoughts?

vbatts · Nov 5, 2014

it tests out fine. Though I am trying to build the following Dockerfile

FROM centos
RUN yum install -y attr
RUN touch /file && setfattr -n user.gh6190 -v test /file
CMD getfattr /file

on this setup

vbatts@valse ~/h (master) $ /home/vbatts/src/docker/docker/bundles/1.3.1-dev/dynbinary/docker-1.3.1-dev info
Containers: 2
Images: 35
Storage Driver: devicemapper
 Pool Name: docker-253:2-5111811-pool
 Pool Blocksize: 65.54 kB
 Data file: /home/docker/devicemapper/devicemapper/data
 Metadata file: /home/docker/devicemapper/devicemapper/metadata
 Data Space Used: 2.075 GB
 Data Space Total: 107.4 GB
 Metadata Space Used: 2.896 MB
 Metadata Space Total: 2.147 GB
 Library Version: 1.02.85 (2014-04-10)
Execution Driver: native-0.2
Kernel Version: 3.16.6-203.fc20.x86_64
Operating System: Fedora 20 (Heisenbug)
CPUs: 4
Total Memory: 11.43 GiB
Debug mode (server): true
Debug mode (client): false
Fds: 11
Goroutines: 14
EventsListeners: 0
Init SHA1: 26f0b471c8396e45585b4b9f4b74923cbcd8c3bb
Init Path: /home/docker/init/dockerinit-1.3.1-dev

and it fails on the yum install step. The client shows

Sending build context to Docker daemon 3.584 kB
Sending build context to Docker daemon
Step 0 : FROM centos
 ---> ae0c2d0bdc10
Step 1 : RUN yum install -y attr
 ---> Running in 95b9b1791902
[...]
Installed:
  attr.x86_64 0:2.4.46-12.el7

Complete!
2014/11/05 17:02:07 %s operation not supported

and the daemon is http://pastebin.com/EG6dc0Pi

Thoughts?

Yep i have seen similar problems, when the host is not mounted with user_xattr.

Is your '/var/lib' on ext4? can you remount with 'user_xattr'?

harshavardhana · Nov 5, 2014

Yep i have seen similar problems, when the host is not mounted with user_xattr.

Is your '/var/lib' on ext4? can you remount with 'user_xattr'?

We need to document any new requirements around user_xattr and discuss what else needs to be changed to make this safe.

I'd rather not break this for everyone who doesn't have user_xattr on the partition where /var/lib/docker folder lives. Also, what about file systems which aren't compatbile with user_xattr?

We should probably look into not making the use of user_xattr for /var/lib/docker mandatory.

I can imagine this is probably less than ideal, but it seems we'll break Docker for a lot of users if we simply switch on user_xattr on devicemapper.

unclejack · Nov 6, 2014

We need to document any new requirements around user_xattr and discuss what else needs to be changed to make this safe.

I'd rather not break this for everyone who doesn't have user_xattr on the partition where /var/lib/docker folder lives. Also, what about file systems which aren't compatbile with user_xattr?

We should probably look into not making the use of user_xattr for /var/lib/docker mandatory.

I can imagine this is probably less than ideal, but it seems we'll break Docker for a lot of users if we simply switch on user_xattr on devicemapper.

would it be too dirty to use ./pkg/mount to checkout mountinfo for where the user_xattr is set?

vbatts · Nov 6, 2014

would it be too dirty to use ./pkg/mount to checkout mountinfo for where the user_xattr is set?

@vbatts

We need to document any new requirements around user_xattr and discuss what else needs to be changed to make this safe.

That is correct, but i wanted to see if remounting with user_xattr worked for @vbatts

I'd rather not break this for everyone who doesn't have user_xattr on the partition where /var/lib/docker folder lives. Also, what about file systems which aren't compatbile with user_xattr?

It won't break anything in general, it just adds new metadata info onto image layer - where the support doesn't exist they do not have any xattrs, so in the sense it won't be visible. But tar level packing keeps it available so that as and when the devicemapper is used a user will be able to see all his xattrs.

We should probably look into not making the use of user_xattr for /var/lib/docker mandatory.

I guess we could just document it rather than forcing this upon users, if they wish to use xattrs.

I can imagine this is probably less than ideal, but it seems we'll break Docker for a lot of users if we simply switch on user_xattr on devicemapper.

user_xattr is only for ext4, not for xfs - xfs by default enables extended attributes. There are no other filesystems supported by devicemapper plugin.

I will do some testing and documentation on how one can use extended attributes with Docker next week.

harshavardhana · Nov 7, 2014