shm: Create shared /dev/shm by amshinde · Pull Request #357 · kata-containers/runtime

amshinde · 2018-05-31T22:54:30Z

This commit checks the size of "/dev/shm" for the sandbox container
which is then used to create the shared memory inside the guest.
kata agent then uses this size to set up a sandbox level shm
storage. The containers then simply bind mount this sandbox level
shm.

With this, we will now be able to support docker --shm-size option
as well have a shared shm within containers in a pod, since they are
supposed to be in the same IPC namespace.

Fixed #356

Signed-off-by: Archana Shinde archana.m.shinde@intel.com

amshinde · 2018-06-01T00:51:48Z

@bergwolf PTAL. Let me know what you think about the approach. I am planning to add a new Storage in the kata agent with driver "shm" that will create a tmpfs based storage.

jodh-intel · 2018-06-01T09:34:04Z

virtcontainers/kata_agent.go

 			grpcSpec.Mounts[idx].Type = "tmpfs"
 			grpcSpec.Mounts[idx].Source = "shm"
-			grpcSpec.Mounts[idx].Options = []string{"noexec", "nosuid", "nodev", "mode=1777", "size=65536k"}
+			grpcSpec.Mounts[idx].Options = []string{"noexec", "nosuid", "nodev", "mode=1777", sizeOption}


It might be worth adding a log call to show the requested size for debugging purposes.

I have added log calls for the shm size.

jodh-intel · 2018-06-01T09:34:07Z

virtcontainers/pkg/oci/utils.go

+			continue
+		}
+
+		shmSize = vc.DefaultShmSize


Why is this set inside the loop? Can't you just initialize shmSize to this value as currently if there is no host mount this function will return 0?

@jodh-intel I have added a break here. If no shm mount is found, then the shmSize will be zero, hence it is initialized to zero. In case host shm is to be used, this will be set to DefaultShmSize.

bergwolf · 2018-06-04T01:55:31Z

@amshinde Can you reuse kata-containers/agent/pull/236 and base your change on top of #307? shm is just a special case of tmpfs and tmpfs is handled as ephemeral storage in #307.

WeiZhang555 · 2018-06-04T03:52:47Z

@bergwolf Maybe we can create a new general storage type in kata-agent, for supporting the case that sharing a volume among containers in pod, but not share it with host?

bergwolf · 2018-06-04T04:03:19Z

@WeiZhang555 Sharing a volume within the guest is already supported, by letting different containers bind mount the same volume source at the sandbox level. Each container can mount the referenced volume to its own namespace.

amshinde · 2018-06-04T19:47:13Z

@bergwolf I have changed this PR to use ehemeral storage now,, instead of introducing a new storage specific to just shm. PTAL.

amshinde · 2018-06-05T00:37:02Z

virtcontainers/kata_agent.go

+		shmStorage := &grpc.Storage{
+			Driver:     kataEphemeralDevType,
+			MountPoint: path,
+			Source:     "shm",


@bergwolf I just realised that the source in case of /dev/shm is set to shm and not tmpfs, various for ephemeral volumes it is tmpfs .

$ sudo docker run -it --runtime=runc debian sh -c "mount | grep shm" shm on /dev/shm type tmpfs (rw,nosuid,nodev,noexec,relatime,size=65536k)

This source field would need to be changed to tmpfs to use the ephemeral storage. I know shm is a special case of tmpfs, but I havent looked into the implications of doing this, do you think it is ok to use tmpfs instead of shm.

I would prefer to introduce a new storage type for shm to be consistent with runc behaviour.

@amshinde for ephemeral storage, the source actually does not matter. You can set it as shm or tmpfs or whatever, and it will be shown as such inside the container. Do you mean that kata-agent would only accept tmpfs source for ephemeral storage? If so, we need to fix it in kata-agent side.

@bergwolf Yes, you are right. I set this to shm and tested it.

bergwolf · 2018-06-05T02:14:33Z

LTGM! Thanks @amshinde !

My only concern is that we are mounting all these volumes under kataGuestSharedDir that is actually the mount point of 9pfs. This might confuse the 9pfs, e.g., these internal volume mountpoints directories are also visible to host side via 9pfs.

jodh-intel · 2018-06-05T07:34:53Z

/cc @grahamwhaley for the 9p comment.

lgtm although the CI doesn't look very happy.

katabuilder · 2018-06-05T18:06:23Z

PSS Measurement:
Qemu: 159671 KB
Proxy: 4689 KB
Shim: 11070 KB

Memory inside container:
Total Memory: 2045972 KB
Free Memory: 1996756 KB

katabuilder · 2018-06-06T20:11:40Z

PSS Measurement:
Qemu: 161784 KB
Proxy: 4783 KB
Shim: 10822 KB

Memory inside container:
Total Memory: 2045972 KB
Free Memory: 1995756 KB

katabuilder · 2018-06-06T23:51:28Z

PSS Measurement:
Qemu: 161773 KB
Proxy: 6814 KB
Shim: 10823 KB

Memory inside container:
Total Memory: 2045972 KB
Free Memory: 1996384 KB

katabuilder · 2018-06-07T01:15:45Z

PSS Measurement:
Qemu: 159750 KB
Proxy: 6744 KB
Shim: 8975 KB

Memory inside container:
Total Memory: 2045972 KB
Free Memory: 1997128 KB

codecov · 2018-06-07T01:16:17Z

Codecov Report

Merging #357 into master will increase coverage by <.01%.
The diff coverage is 67.39%.

@@            Coverage Diff            @@
##           master    #357      +/-   ##
=========================================
+ Coverage   63.79%   63.8%   +<.01%     
=========================================
  Files          87      87              
  Lines        8765    8807      +42     
=========================================
+ Hits         5592    5619      +27     
- Misses       2574    2586      +12     
- Partials      599     602       +3

Impacted Files	Coverage Δ
virtcontainers/mount.go	`80.17% <ø> (ø)`	⬆️
virtcontainers/sandbox.go	`67.17% <100%> (+0.05%)`	⬆️
virtcontainers/kata_agent.go	`30.25% <51.85%> (+0.46%)`	⬆️
virtcontainers/pkg/oci/utils.go	`78.4% <88.88%> (+0.56%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd2bf15...4d470e5. Read the comment docs.

amshinde · 2018-06-07T16:26:57Z

@jodh-intel I am no longer using kataGuestSharedDir , so the shm mount will be not be visible to the host side via 9pfs. CI has been fixed as well.

jodh-intel · 2018-06-07T16:39:16Z

Hi @amshinde - ok. codecov is complaining, but I think if you create a unit test for getShmSize() it'll be within thresholds.

This commit checks the size of "/dev/shm" for the sandbox container which is then used to create the shared memory inside the guest. kata agent then uses this size to set up a sandbox level ephemeral storage for shm. The containers then simply bind mount this sandbox level shm. With this, we will now be able to support docker --shm-size option as well have a shared shm within containers in a pod, since they are supposed to be in the same IPC namespace. Fixes kata-containers#356 Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>

katabuilder · 2018-06-10T09:23:00Z

PSS Measurement:
Qemu: 162138 KB
Proxy: 6723 KB
Shim: 10733 KB

Memory inside container:
Total Memory: 2045972 KB
Free Memory: 1998492 KB

amshinde · 2018-06-11T21:05:05Z

@egernst @sboeuf Can you merge this one? Code coverage needed to be fixed which has been done now.

amshinde added review labels May 31, 2018

amshinde force-pushed the handle-shared-shm branch 2 times, most recently from cda7583 to ed1746d Compare June 1, 2018 00:51

jodh-intel reviewed Jun 1, 2018

View reviewed changes

amshinde force-pushed the handle-shared-shm branch from ed1746d to e5e498e Compare June 4, 2018 19:40

amshinde force-pushed the handle-shared-shm branch from e5e498e to 233f964 Compare June 5, 2018 00:26

amshinde commented Jun 5, 2018

View reviewed changes

amshinde force-pushed the handle-shared-shm branch from 233f964 to 105dd1e Compare June 5, 2018 01:33

amshinde changed the title ~~wip: shm: Create shared /dev/shm~~ shm: Create shared /dev/shm Jun 5, 2018

amshinde removed the do-not-merge label Jun 5, 2018

jodh-intel mentioned this pull request Jun 5, 2018

refactor guest storage mountpoint structure #372

Closed

amshinde force-pushed the handle-shared-shm branch from 105dd1e to 6dd6c10 Compare June 5, 2018 17:32

amshinde force-pushed the handle-shared-shm branch from 6dd6c10 to 4e8ec27 Compare June 6, 2018 19:44

amshinde force-pushed the handle-shared-shm branch from 4e8ec27 to ddd6bf8 Compare June 6, 2018 23:24

amshinde force-pushed the handle-shared-shm branch from ddd6bf8 to 9d793f6 Compare June 7, 2018 00:02

amshinde force-pushed the handle-shared-shm branch 2 times, most recently from ccc752e to abf82b4 Compare June 10, 2018 08:34

amshinde force-pushed the handle-shared-shm branch from abf82b4 to 4d470e5 Compare June 10, 2018 08:55

egernst merged commit ca9f7ab into kata-containers:master Jun 11, 2018

jodh-intel mentioned this pull request Jun 22, 2018

Version the gRPC protocol kata-containers/agent#272

Closed

amshinde deleted the handle-shared-shm branch July 11, 2019 22:26

amshinde mentioned this pull request Apr 20, 2020

/dev/shm mount based on empty-dir volumes are not handled correctly. #2631

Closed

Conversation

amshinde commented May 31, 2018

Uh oh!

amshinde commented Jun 1, 2018

Uh oh!

jodh-intel Jun 1, 2018

Choose a reason for hiding this comment

Uh oh!

amshinde Jun 4, 2018

Choose a reason for hiding this comment

Uh oh!

jodh-intel Jun 1, 2018

Choose a reason for hiding this comment

Uh oh!

amshinde Jun 4, 2018

Choose a reason for hiding this comment

Uh oh!

bergwolf commented Jun 4, 2018

Uh oh!

WeiZhang555 commented Jun 4, 2018

Uh oh!

bergwolf commented Jun 4, 2018

Uh oh!

amshinde commented Jun 4, 2018

Uh oh!

amshinde Jun 5, 2018

Choose a reason for hiding this comment

Uh oh!

bergwolf Jun 5, 2018

Choose a reason for hiding this comment

Uh oh!

amshinde Jun 5, 2018

Choose a reason for hiding this comment

Uh oh!

bergwolf commented Jun 5, 2018

Uh oh!

jodh-intel commented Jun 5, 2018 • edited by amshinde Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

katabuilder commented Jun 5, 2018

Uh oh!

katabuilder commented Jun 6, 2018

Uh oh!

katabuilder commented Jun 6, 2018

Uh oh!

katabuilder commented Jun 7, 2018

Uh oh!

codecov bot commented Jun 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

amshinde commented Jun 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jodh-intel commented Jun 7, 2018

Uh oh!

katabuilder commented Jun 10, 2018

Uh oh!

amshinde commented Jun 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jodh-intel commented Jun 5, 2018 •

edited by amshinde

Loading

codecov bot commented Jun 7, 2018 •

edited

Loading

amshinde commented Jun 7, 2018 •

edited

Loading