I realize that this pull request needs to be split to contribute to libcontainer separately. But I wanted to get an overall feel for the patch before I submit changes to libcontainer.

+1 from me

Thanks @rhatdan, I will test and report back.

I'd love to see a little more info, BUT

Docs LGTM - @jamtur01 @fredlf @ostezer

@rhatdan Could we something in the run docs just saying why you might do this? Or perhaps even a simple example?

+1 from me.
On Sep 24, 2014 10:23 AM, "rhatdan" notifications@github.com wrote:

I realize that this pull request needs to be split to contribute to
libcontainer separately. But I wanted to get an overall feel for the patch
before I submit changes to libcontainer.

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

@jeremyeder Could you give an example, since you were the trigger for this code.

One problem I will have with this patch is that SELinux will not allow it. I might have to change the code to say ipc=container:CONTAINERID implies that the SELinux labels of the new container has to match the label of CONTAINERID. Also ipc=host would most likely require --security label:disabled. (But that is another patch that has not been merged) :^(

Shared memory segments are used to accelerate inter-process communication at memory speed, rather than, say, through pipes or through the network stack. Shared memory is a commonly used technique used in applications such as databases and their worker threads, along with a large population of custom-built (typically C/OpenMPI, C++/using boost libraries) high performance applications that you will find in the scientific computing and financial services industries.

Without the ability to share memory between containers, this class of high-performance workloads will not be candidates for containerization. To Dan's point about security; processes that share memory are explicitly designed to cooperate, and thus should be grouped together from a security/isolation perspective at least as far as memory.

@jamtur01 Is @jeremyeder Message above enough to add?

@jamtur01 when I complete testing of this I will share the example/walkthrough. Please give me a day or 2.

+1 to adding a thoroughly explained example. Thanks.

• Using shm_server.c and shm_client.c available here: http://www.cs.cf.ac.uk/Dave/C/node27.html

host# setenforce 0 # for now...

• Testing --ipc=host mode:
• Host shows a shared memory segment with 7 pids attached, happens to be from httpd:

host# ipcs -m

------ Shared Memory Segments --------
key        shmid      owner      perms      bytes      nattch     status      
0x01128e25 0          root       600        1000       7                       

• Now run a regular container, and it correctly does NOT see the shared memory segment from the host:

host# docker run -it shm ipcs -m

------ Shared Memory Segments --------
key        shmid      owner      perms      bytes      nattch     status      

• Run a container with the new --ipc=host option, and it now sees the shared memory segment from the host httpd:

host# docker run -it --ipc=host shm ipcs -m
------ Shared Memory Segments --------
key        shmid      owner      perms      bytes      nattch     status      
0x01128e25 0          root       600        1000       7                   

• Testing --ipc=container:CONTAINERID mode:
• Start a container with a program to create a shared memory segment:

# docker run -it shm bash
root@ed735b2264ac: ~ # shm/shm_server &
[1] 21
root@ed735b2264ac: ~ # ipcs -m

------ Shared Memory Segments --------
key        shmid      owner      perms      bytes      nattch     status      
0x0000162e 0          root       666        27         1                       

• Create a 2nd container correctly shows no shared memory segment from 1st container:

# docker run shm ipcs -m

------ Shared Memory Segments --------
key        shmid      owner      perms      bytes      nattch     status      

• Create a 3rd container using the new --ipc=container:CONTAINERID option, now it shows the shared memory segment from the first:

# docker run -it --ipc=container:ed735b2264ac shm ipcs -m
root@9f4b21f2d520: ~ # ipcs -m

------ Shared Memory Segments --------
key        shmid      owner      perms      bytes      nattch     status      
0x0000162e 0          root       666        27         1

@fredlf Is this what you were thinking?

Yes, that's terrific. Examples help users so much. Thank you.

+1 to example :)

Example LGTM - add away to run.md.

+1; much desired. The code looks good at a glance.

Can you please open a PR on libcontainer for those changes?

FYI. If we want this in my 1.3 we should have it merged by first of next week. I would like it in if we finish all the changes.

FWIW Go conventions say not to capitalize error messages.
On Oct 7, 2014 6:08 AM, "O.S. Tezer" notifications@github.com wrote:

LGTM.

I think it would be better, however, if we had fmt.Errorf messages
capitalized? (@jamtur01 https://github.com/jamtur01)

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

https://code.google.com/p/go-wiki/wiki/CodeReviewComments#Error_Strings - not sure how far we've adopted those conventions though.

@thockin @jamtur01 cheers for the heads up, my bad.
I think I've seen some capitalized error messages which got me confused.

It is a very minor point in the grand scheme of things :)
On Oct 7, 2014 7:44 AM, "O.S. Tezer" notifications@github.com wrote:

@thockin https://github.com/thockin @jamtur01
https://github.com/jamtur01 cheers for the heads up, my bad.
I think I've seen some capitalized error messages which got me confused.

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

@ostezer I just copied these from the error messages for --net, so if we decide to capitilize I would probably change --network stuff also.
@SvenDowideit what do you think?

@rhatdan easy :) not in this PR - and we can leave it for the techwriter review of strings :)

mostly we need core review now.

Thanks @rhatdan ;-)

Since this is not going to make it into the docker-1.3 pull, I completed the SELinux support.

If you run a container with --ipc=host, this means you could share IPC information with any process on the host OS, so I disable SELinux label enforcement.

If you run a container with --ipc=container:ID we have to run the new container process with the same SELinux labels as the container you are sharing IPC with, or SELinux will block the interaction.

Docs LGTM once the missing word is added.

Docs are good to go, waiting on Core reviews :) @crosbymichael @tiborvass :)

Added tests to verify --ipc=host and --ipc=container:ID works properly.

Closing in favor of #8835