From 83e38c959ab7315e47ddf65e375540dad7319c8b Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Thu, 20 Dec 2018 19:13:40 -0800 Subject: [PATCH] mounts: Ignore existing mounts if they cannot be honored In case we use an hypervisor that cannot support filesystem sharing, we copy files over to the VM rootfs through the gRPC protocol. This is a nice workaround, but it only works with regular files, which means no device file, no socket file, no directory, etc... can be sent this way. This is a limitation that we accept here, by simply ignoring those non-regular files. Fixes #1068 Signed-off-by: Sebastien Boeuf Signed-off-by: Eric Ernst --- virtcontainers/container.go | 46 ++++++++++++++++++++++-------- virtcontainers/hyperstart_agent.go | 2 +- virtcontainers/kata_agent.go | 37 +++++++++++++++++++++++- virtcontainers/sandbox_test.go | 4 ++- 4 files changed, 74 insertions(+), 15 deletions(-) diff --git a/virtcontainers/container.go b/virtcontainers/container.go index fff67f4fb0..2c3f205971 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -431,10 +431,10 @@ func (c *Container) createContainersDirs() error { return nil } -func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir string) (string, error) { +func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir string) (string, bool, error) { randBytes, err := utils.GenerateRandomBytes(8) if err != nil { - return "", err + return "", false, err } filename := fmt.Sprintf("%s-%s-%s", c.id, hex.EncodeToString(randBytes), filepath.Base(m.Destination)) @@ -445,20 +445,35 @@ func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir s caps := c.sandbox.hypervisor.capabilities() if !caps.isFsSharingSupported() { c.Logger().Debug("filesystem sharing is not supported, files will be copied") + + fileInfo, err := os.Stat(m.Source) + if err != nil { + return "", false, err + } + + // Ignore the mount if this is not a regular file (excludes + // directory, socket, device, ...) as it cannot be handled by + // a simple copy. But this should not be treated as an error, + // only as a limitation. + if !fileInfo.Mode().IsRegular() { + c.Logger().WithField("ignored-file", m.Source).Debug("Ignoring non-regular file as FS sharing not supported") + return "", true, nil + } + if err := c.sandbox.agent.copyFile(m.Source, guestDest); err != nil { - return "", err + return "", false, err } } else { // These mounts are created in the shared dir mountDest := filepath.Join(hostSharedDir, c.sandbox.id, filename) if err := bindMount(c.ctx, m.Source, mountDest, false); err != nil { - return "", err + return "", false, err } // Save HostPath mount value into the mount list of the container. c.mounts[idx].HostPath = mountDest } - return guestDest, nil + return guestDest, false, nil } // mountSharedDirMounts handles bind-mounts by bindmounting to the host shared @@ -466,8 +481,9 @@ func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir s // 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(hostSharedDir, guestSharedDir string) ([]Mount, error) { +func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) ([]Mount, []Mount, error) { var sharedDirMounts []Mount + var ignoredMounts []Mount for idx, m := range c.mounts { if isSystemMount(m.Destination) || m.Type != "bind" { continue @@ -485,12 +501,12 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) ( 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(m.BlockDeviceID, c.sandbox); err != nil { - return nil, err + return nil, nil, err } if err := c.sandbox.storeSandboxDevices(); err != nil { //TODO: roll back? - return nil, err + return nil, nil, err } continue } @@ -502,9 +518,15 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) ( continue } - guestDest, err := c.shareFiles(m, idx, hostSharedDir, guestSharedDir) + guestDest, ignore, err := c.shareFiles(m, idx, hostSharedDir, guestSharedDir) if err != nil { - return nil, err + return nil, nil, err + } + + // Expand the list of mounts to ignore. + if ignore { + ignoredMounts = append(ignoredMounts, Mount{Source: m.Source}) + continue } // Check if mount is readonly, let the agent handle the readonly mount @@ -528,10 +550,10 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) ( } if err := c.storeMounts(); err != nil { - return nil, err + return nil, nil, err } - return sharedDirMounts, nil + return sharedDirMounts, ignoredMounts, nil } func (c *Container) unmountHostMounts() error { diff --git a/virtcontainers/hyperstart_agent.go b/virtcontainers/hyperstart_agent.go index f7c0697fa0..4c23b58f3f 100644 --- a/virtcontainers/hyperstart_agent.go +++ b/virtcontainers/hyperstart_agent.go @@ -549,7 +549,7 @@ func (h *hyper) startOneContainer(sandbox *Sandbox, c *Container) error { //TODO : Enter mount namespace // Handle container mounts - newMounts, err := c.mountSharedDirMounts(defaultSharedDir, "") + newMounts, _, err := c.mountSharedDirMounts(defaultSharedDir, "") if err != nil { bindUnmountAllRootfs(c.ctx, defaultSharedDir, sandbox) return err diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 6248d83210..18a508cc86 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -719,6 +719,30 @@ func (k *kataAgent) replaceOCIMountSource(spec *specs.Spec, guestMounts []Mount) return nil } +func (k *kataAgent) removeIgnoredOCIMount(spec *specs.Spec, ignoredMounts []Mount) error { + var mounts []specs.Mount + + for _, m := range spec.Mounts { + found := false + for _, ignoredMount := range ignoredMounts { + if ignoredMount.Source == m.Source { + k.Logger().WithField("removed-mount", m.Source).Debug("Removing OCI mount") + found = true + break + } + } + + if !found { + mounts = append(mounts, m) + } + } + + // Replace the OCI mounts with the updated list. + spec.Mounts = mounts + + return nil +} + func (k *kataAgent) replaceOCIMountsForStorages(spec *specs.Spec, volumeStorages []*grpc.Storage) error { ociMounts := spec.Mounts var index int @@ -981,7 +1005,7 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process, } // Handle container mounts - newMounts, err := c.mountSharedDirMounts(kataHostSharedDir, kataGuestSharedDir) + newMounts, ignoredMounts, err := c.mountSharedDirMounts(kataHostSharedDir, kataGuestSharedDir) if err != nil { return nil, err } @@ -995,6 +1019,11 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process, return nil, err } + // Remove all mounts that should be ignored from the spec + if err = k.removeIgnoredOCIMount(ociSpec, ignoredMounts); err != nil { + return nil, err + } + // Append container devices for block devices passed with --device. ctrDevices = k.appendDevices(ctrDevices, c) @@ -1769,6 +1798,12 @@ func (k *kataAgent) copyFile(src, dst string) error { Gid: int32(st.Gid), } + // Handle the special case where the file is empty + if fileSize == 0 { + _, err = k.sendReq(cpReq) + return err + } + // Copy file by parts if it's needed remainingBytes := fileSize offset := int64(0) diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index d3c30391f7..08604efb79 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -1706,10 +1706,12 @@ func TestPreAddDevice(t *testing.T) { }, } - mounts, err := container.mountSharedDirMounts("", "") + mounts, ignoreMounts, err := container.mountSharedDirMounts("", "") assert.Nil(t, err) assert.Equal(t, len(mounts), 0, "mounts should contain nothing because it only contains a block device") + assert.Equal(t, len(ignoreMounts), 0, + "ignoreMounts should contain nothing because it only contains a block device") } func TestGetNetNs(t *testing.T) {