From 57c0cee0a516399dac355172d074e1c8c30450fc Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Tue, 1 Jun 2021 20:30:22 -0700 Subject: [PATCH] runtime: Cleanup mountSharedDirMounts, shareFile parameters There's no reason to pass the paths; they can be determined when they are actually used. Let's make the return values more comparable to the other mount handling functions (we'll add storage object in future commit), and pass the mount maps as function parameters. ...No functional changes here... Signed-off-by: Eric Ernst --- src/runtime/virtcontainers/container.go | 19 +++++++++---------- src/runtime/virtcontainers/kata_agent.go | 7 +++++-- src/runtime/virtcontainers/sandbox_test.go | 5 ++++- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/runtime/virtcontainers/container.go b/src/runtime/virtcontainers/container.go index d2ce6021c..68a48bdf2 100644 --- a/src/runtime/virtcontainers/container.go +++ b/src/runtime/virtcontainers/container.go @@ -436,14 +436,14 @@ func (c *Container) setContainerState(state types.StateString) error { return nil } -func (c *Container) shareFiles(ctx context.Context, m Mount, idx int, hostSharedDir, hostMountDir, guestSharedDir string) (string, bool, error) { +func (c *Container) shareFiles(ctx context.Context, m Mount, idx int) (string, bool, error) { randBytes, err := utils.GenerateRandomBytes(8) if err != nil { return "", false, err } filename := fmt.Sprintf("%s-%s-%s", c.id, hex.EncodeToString(randBytes), filepath.Base(m.Destination)) - guestDest := filepath.Join(guestSharedDir, filename) + guestDest := filepath.Join(kataGuestSharedDir(), filename) // copy file to contaier's rootfs if filesystem sharing is not supported, otherwise // bind mount it in the shared directory. @@ -470,7 +470,7 @@ func (c *Container) shareFiles(ctx context.Context, m Mount, idx int, hostShared } } else { // These mounts are created in the shared dir - mountDest := filepath.Join(hostMountDir, filename) + mountDest := filepath.Join(getMountPath(c.sandboxID), filename) if !m.ReadOnly { if err := bindMount(c.ctx, m.Source, mountDest, false, "private"); err != nil { return "", false, err @@ -511,9 +511,8 @@ func (c *Container) shareFiles(ctx context.Context, m Mount, idx int, hostShared // It also updates the container mount list with the HostPath info, and store // container mounts to the storage. This way, we will have the HostPath info // available when we will need to unmount those mounts. -func (c *Container) mountSharedDirMounts(ctx context.Context, hostSharedDir, hostMountDir, guestSharedDir string) (sharedDirMounts map[string]Mount, ignoredMounts map[string]Mount, err error) { - sharedDirMounts = make(map[string]Mount) - ignoredMounts = make(map[string]Mount) +func (c *Container) mountSharedDirMounts(ctx context.Context, sharedDirMounts, ignoredMounts map[string]Mount) (err error) { + var devicesToDetach []string defer func() { if err != nil { @@ -535,7 +534,7 @@ func (c *Container) mountSharedDirMounts(ctx context.Context, hostSharedDir, hos if len(m.BlockDeviceID) > 0 { // Attach this block device, all other devices passed in the config have been attached at this point if err = c.sandbox.devManager.AttachDevice(ctx, m.BlockDeviceID, c.sandbox); err != nil { - return nil, nil, err + return err } devicesToDetach = append(devicesToDetach, m.BlockDeviceID) continue @@ -562,9 +561,9 @@ func (c *Container) mountSharedDirMounts(ctx context.Context, hostSharedDir, hos var ignore bool var guestDest string - guestDest, ignore, err = c.shareFiles(ctx, m, idx, hostSharedDir, hostMountDir, guestSharedDir) + guestDest, ignore, err = c.shareFiles(ctx, m, idx) if err != nil { - return nil, nil, err + return err } // Expand the list of mounts to ignore. @@ -584,7 +583,7 @@ func (c *Container) mountSharedDirMounts(ctx context.Context, hostSharedDir, hos sharedDirMounts[sharedDirMount.Destination] = sharedDirMount } - return sharedDirMounts, ignoredMounts, nil + return nil } func (c *Container) unmountHostMounts(ctx context.Context) error { diff --git a/src/runtime/virtcontainers/kata_agent.go b/src/runtime/virtcontainers/kata_agent.go index 3607e5279..fcf671186 100644 --- a/src/runtime/virtcontainers/kata_agent.go +++ b/src/runtime/virtcontainers/kata_agent.go @@ -1340,7 +1340,10 @@ func (k *kataAgent) createContainer(ctx context.Context, sandbox *Sandbox, c *Co } // Handle container mounts - newMounts, ignoredMounts, err := c.mountSharedDirMounts(ctx, getSharePath(sandbox.id), getMountPath(sandbox.id), kataGuestSharedDir()) + sharedDirMounts := make(map[string]Mount) + ignoredMounts := make(map[string]Mount) + + err = c.mountSharedDirMounts(ctx, sharedDirMounts, ignoredMounts) if err != nil { return nil, err } @@ -1363,7 +1366,7 @@ func (k *kataAgent) createContainer(ctx context.Context, sandbox *Sandbox, c *Co // We replace all OCI mount sources that match our container mount // with the right source path (The guest one). - if err = k.replaceOCIMountSource(ociSpec, newMounts); err != nil { + if err = k.replaceOCIMountSource(ociSpec, sharedDirMounts); err != nil { return nil, err } diff --git a/src/runtime/virtcontainers/sandbox_test.go b/src/runtime/virtcontainers/sandbox_test.go index a9627e157..ae89b04b2 100644 --- a/src/runtime/virtcontainers/sandbox_test.go +++ b/src/runtime/virtcontainers/sandbox_test.go @@ -1297,7 +1297,10 @@ func TestPreAddDevice(t *testing.T) { }, } - mounts, ignoreMounts, err := container.mountSharedDirMounts(context.Background(), "", "", "") + mounts := make(map[string]Mount) + ignoreMounts := make(map[string]Mount) + err = container.mountSharedDirMounts(context.Background(), mounts, ignoreMounts) + assert.Nil(t, err) assert.Equal(t, len(mounts), 0, "mounts should contain nothing because it only contains a block device")