Hi,

The best would be to not delete anything and to return an error to the client.
Something like, "xxx volumes are still in use, can't remove them"

Thanks

Let me explain clearer.

Suppose we launch a container by:
sudo docker run -lxc-conf=lxc.aa_profile=unconfined -privileged -v pwd:/go/src/github.com/dotcloud/docker docker

And pwd is '/home/user/docker' here.

The behavior of original codes would try to trim the right of volumeId:

   // volumeId is '/home/user/docker' at first

   volumeId = strings.TrimRight(volumeId, "/layer")  // volumeId now becomes '/home/user/dock'
   volumeId = filepath.Base(volumeId)                        // volumeId now becomes 'dock'

Then the later code in same function:
err := srv.runtime.volumes.Delete(volumeId); // error happens because the volume 'dock' doesn't exist

Because of returing the error, the other volumes of the container are not deleted. But the container is deleted already. So those volumes would be left in disk.

Since the volume is mounted from external of the container (i.e. host), it would be best not to delete it (otherwise '/home/user/docker' would be removed). So this PR tries to skip those mounted volumes.

The usage of TrimRight in original codes has problem. It will remove the 'er' from 'docker', for example. But I do not think that is what we want here. So this PR uses TrimSuffix instead. If there is no the suffix '/layer' in volumeId, we think it is a mounted volume and skip it in later deleting.

ping @vieux

Hi @viirya , sorry for the late reply.

I understand your point.
The only thing I'm not fond of is that you use /layer to detect if the volume in mounted on the host or not.

docker run -i -t -v /tmp/test/layer:/tmp/test ubuntu bash
docker rm -v `docker ps -l -q`

The best practice would be to use look as the Binds key in the hostConfig.

Could you update your PR ?

Thanks

Hi @vieux, modified codes use Binds key to detect mounted volumes. Thanks.

LGTM, ping @shykes @crosbymichael

LGTM