Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

Commit f779d5c

Browse files
authored
Merge pull request #3051 from amshinde/my-stable-1.11
Backports: Read-only mount fixes for 1.11
2 parents eb34521 + 3f0e61c commit f779d5c

5 files changed

Lines changed: 28 additions & 18 deletions

File tree

virtcontainers/container.go

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ func (c *Container) setContainerState(state types.StateString) error {
446446
return nil
447447
}
448448

449-
func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir string) (string, bool, error) {
449+
func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, hostMountDir, guestSharedDir string) (string, bool, error) {
450450
randBytes, err := utils.GenerateRandomBytes(8)
451451
if err != nil {
452452
return "", false, err
@@ -480,12 +480,19 @@ func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir s
480480
}
481481
} else {
482482
// These mounts are created in the shared dir
483-
mountDest := filepath.Join(hostSharedDir, filename)
484-
if err := bindMount(c.ctx, m.Source, mountDest, false, "private"); err != nil {
483+
mountDest := filepath.Join(hostMountDir, filename)
484+
if err := bindMount(c.ctx, m.Source, mountDest, m.ReadOnly, "private"); err != nil {
485485
return "", false, err
486486
}
487487
// Save HostPath mount value into the mount list of the container.
488488
c.mounts[idx].HostPath = mountDest
489+
// bindmount remount event is not propagated to mount subtrees, so we have to remount the shared dir mountpoint directly.
490+
if m.ReadOnly {
491+
mountDest = filepath.Join(hostSharedDir, filename)
492+
if err := remountRo(c.ctx, mountDest); err != nil {
493+
return "", false, err
494+
}
495+
}
489496
}
490497

491498
return guestDest, false, nil
@@ -496,7 +503,7 @@ func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir s
496503
// It also updates the container mount list with the HostPath info, and store
497504
// container mounts to the storage. This way, we will have the HostPath info
498505
// available when we will need to unmount those mounts.
499-
func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) (sharedDirMounts map[string]Mount, ignoredMounts map[string]Mount, err error) {
506+
func (c *Container) mountSharedDirMounts(hostSharedDir, hostMountDir, guestSharedDir string) (sharedDirMounts map[string]Mount, ignoredMounts map[string]Mount, err error) {
500507
sharedDirMounts = make(map[string]Mount)
501508
ignoredMounts = make(map[string]Mount)
502509
var devicesToDetach []string
@@ -546,7 +553,7 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) (
546553

547554
var ignore bool
548555
var guestDest string
549-
guestDest, ignore, err = c.shareFiles(m, idx, hostSharedDir, guestSharedDir)
556+
guestDest, ignore, err = c.shareFiles(m, idx, hostSharedDir, hostMountDir, guestSharedDir)
550557
if err != nil {
551558
return nil, nil, err
552559
}
@@ -557,22 +564,12 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) (
557564
continue
558565
}
559566

560-
// Check if mount is readonly, let the agent handle the readonly mount
561-
// within the VM.
562-
readonly := false
563-
for _, flag := range m.Options {
564-
if flag == "ro" {
565-
readonly = true
566-
break
567-
}
568-
}
569-
570567
sharedDirMount := Mount{
571568
Source: guestDest,
572569
Destination: m.Destination,
573570
Type: m.Type,
574571
Options: m.Options,
575-
ReadOnly: readonly,
572+
ReadOnly: m.ReadOnly,
576573
}
577574

578575
sharedDirMounts[sharedDirMount.Destination] = sharedDirMount

virtcontainers/kata_agent.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1387,7 +1387,7 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process,
13871387
}
13881388

13891389
// Handle container mounts
1390-
newMounts, ignoredMounts, err := c.mountSharedDirMounts(getMountPath(sandbox.id), kataGuestSharedDir())
1390+
newMounts, ignoredMounts, err := c.mountSharedDirMounts(getSharePath(sandbox.id), getMountPath(sandbox.id), kataGuestSharedDir())
13911391
if err != nil {
13921392
return nil, err
13931393
}

virtcontainers/mount.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,11 @@ func remount(ctx context.Context, mountflags uintptr, src string) error {
273273
return nil
274274
}
275275

276+
// remount a mount point as readonly
277+
func remountRo(ctx context.Context, src string) error {
278+
return remount(ctx, syscall.MS_BIND|syscall.MS_RDONLY, src)
279+
}
280+
276281
// bindMountContainerRootfs bind mounts a container rootfs into a 9pfs shared
277282
// directory between the guest and the host.
278283
func bindMountContainerRootfs(ctx context.Context, shareDir, cid, cRootFs string, readonly bool) error {

virtcontainers/pkg/oci/utils.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,11 +162,19 @@ func cmdEnvs(spec specs.Spec, envs []types.EnvVar) []types.EnvVar {
162162
}
163163

164164
func newMount(m specs.Mount) vc.Mount {
165+
readonly := false
166+
for _, flag := range m.Options {
167+
if flag == "ro" {
168+
readonly = true
169+
break
170+
}
171+
}
165172
return vc.Mount{
166173
Source: m.Source,
167174
Destination: m.Destination,
168175
Type: m.Type,
169176
Options: m.Options,
177+
ReadOnly: readonly,
170178
}
171179
}
172180

virtcontainers/sandbox_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1377,7 +1377,7 @@ func TestPreAddDevice(t *testing.T) {
13771377
},
13781378
}
13791379

1380-
mounts, ignoreMounts, err := container.mountSharedDirMounts("", "")
1380+
mounts, ignoreMounts, err := container.mountSharedDirMounts("", "", "")
13811381
assert.Nil(t, err)
13821382
assert.Equal(t, len(mounts), 0,
13831383
"mounts should contain nothing because it only contains a block device")

0 commit comments

Comments
 (0)