@rhatdan @vbatts Thoughts?

I agree that long term, docker volumes or libgraphdriver are the right solution, but we need something now.

I just mounted an image and then tried deleting it using docker. Deletion succeeded. Actual thin device and associated metadata file deletion failed in graph driver but top layers do not care for the error code.
func (graph *Graph) Delete(name string) error {
// Remove rootfs data from the driver
graph.driver.Remove(id)
}

Side affect of this is that when you try to pull delete image again, it fails as graph driver metadata file with same id still exists. And one needs to first delete that file.

I am thinking that how about extending this to containers and allow directly mounting container roots. That way we could quickly do following.

• docker create <image_of_interest>
• mount container root.

Now one should not be able to delete an image as container is using it. And container deletion will fail as device is already mounted. I have already cleaned up the container deletion path so that we will end up with a "Dead" container if container deletion fails.

At least for this use case, this should be relatively race free path.

Another advantage of first creating a container and mounting it is that it can be mounted read/write without any risk of any accidental changes ever showing up in image. So it is safer that way too.

func (graph *Graph) Delete(name string) error {
// Remove rootfs data from the driver
graph.driver.Remove(id)
}

Isn't this a bug?

return graph.driver.Remove(id)

Or doesn't this return an error?

graph.driver.Remove() does return an error.

Yes it is a bug. Just that I am not sure how would you fix this bug. In an image deletion there are so many things which can go wrong. As we don't have the notion of rollback, if error happens on any step, we are in a bad state. We need something like an image state, like containers where we can mark image as "Dead". So that one can try to clean it up later but nobody can launch a container using that image.

Refreshed the patch. This time also exported graphdriver data for containers. Now same graphdriver data should be available both for images and containers using docker inspect.

For the curious, I am writing some poof of concept scripts here to mount/unmount docker images.

https://github.com/rhvgoyal/docker-mount

@crosbymichael ping.

How do I make progress on this pull request. Can we reach some sort of conclusion on this?

not sure what to say about the windows CI. It's failed 3 different ways for 3 rebuilds.

docker folks please have a look at this pull request. I think it is a very reasonable addition and allows us to take this metadata information about images/containers and build more tools on top. (mount/unmount).

I don't want it to miss 1.7 release window.

We really want to move forward on this, since we need an easy way to examine containers for vulnerabilities.

I am working on a vulnerability scanner for containers ... I have been testing this patch with the git repo and it works as expected.

Refreshed the patch one more time. This time made use of interface{} for graph driver data. That way I can easily access data using docker templates.

Ex.

docker inspect --format='{{.GraphDriver.Name}}' fedora
docker inspect --format='{{.GraphDriver.Data.DeviceId}}' fedora
docker inspect --format='{{printf "%0.f" .GraphDriver.Data.DeviceSize}}' fedora

@tiborvass @icecrime can we please review this? I really thought there would be a feedback for the 1.7 release :-\

To speed up the process; IIUC, this adds extra information to the output of docker inspect; please update the example response in the documentation to match the new output.

also ping @jfrazelle; she likes cherry-picking :-)

@thaJeztah updated the patch and updated the docker-inspect man page.

@jfrazelle can you have a look at this one.

@rhvgoyal In general, it's a bad practice to use interface{}. You should declare a type interface that other drivers can implement and return that interface.

This is not going into 1.7. It's passed the feature freeze, sorry about that.

@calavera

Different backend can return different type of information. So may be instead of interface{} I can force them to return a map[string]string. And then graph drivers can decide what strings they want to return. (like docker info). Will that work?

I wished this had got some attention earlier and had got into 1.7.

SGTM

@calavera

Refreshed the patch. This time using map[string]string instead of interface{}.

One can still acess the fields using docker template as follows.

docker inspect --format='{{.GraphDriver.Name}}' fedora
docker inspect --format='{{.GraphDriver.Data.DeviceId}}' fedora
docker inspect --format='{{.GraphDriver.Data.DeviceSize}}' fedora

This is how the additional exported information look like in docker-inspect.

"GraphDriver": {
    "Name": "devicemapper",
    "Data": {
        "DeviceId": "3",
        "DeviceSize": "10737418240"
    }
}

Having examples from overlayfs and other back ends might help to move this along? What do you think @jfrazelle @tianon @crosbymichael

I looked into this for btrfs, but none of stats and usage is plumbed yet in
the docker driver, and not sure that we could give clear container specific
info. At best it just be the path on disk that the container is set up at.
On Jun 2, 2015 12:10, "Daniel J Walsh" notifications@github.com wrote:

Having examples from overlayfs and other back ends might help to move this
along? What do you think @jfrazelle https://github.com/jfrazelle @tianon
https://github.com/tianon @crosbymichael
https://github.com/crosbymichael

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

For vfs we might be done with simply exporting directory of container.

For things like overlayfs, I think just exporting container directory might not be sufficient. Somebody will have to setup container mount point with proper lower and upper directories and that should happen in caller. So we might have to export this upper and lower dir information too.

A map looks more appropriate, thanks for changing that.

Having some tests would be great!

@calavera

Ok, I will look into adding test.

@calavera

Pushed patch one more time. This time added 2 new tests and extended 1 existing one. Also got rid of devices.lock.

my test have run against "vfs" locally. Not sure How to force running tests against "devicemapper" graphdriver. I tried "export DOCKER_GRAPHDRIVER=devicemapper; make test-integration-cli" and that fails very early.

Added this to the 1.8 milestone; let's work on getting it into that release! (And the experimental builds?)

@thaJeztah Given that it can't make 1.7, I can live with 1.8.

What's experimental builds?

@rhvgoyal not sure if this applies to this PR, but starting with the 1.7 release, Docker will have three "release channels";

• Official Releases (1.7.x, 1.8.x) each two months (as it is now), and patch releases (x.x.1)
• Master builds (built directly of the current master), via https://master.dockerproject.org
• Experimental release (nightly or weekly, t.b.d.), via https://experimental.dockerproject.org

The experimental release includes new features that are being worked on and hidden behind an EXPERIMENTAL build-flag. (For example, the network API, #13441). Basically an "incubation" period for testing new features, giving them more exposure and testers, and allows further tweaking.

Features in experimental builds are not guaranteed to end up in an official release; this allows the team to "pull the plug" on them (hopefully not, of course), without the headaches of breaking production setups.

Refreshed patch one more time. This time added another field "DeviceName". This exports the name of the dm device docker will use for image/container.

LGTM

Refreshed the patch on top of latest code.

LGTM

WooHoo