Merge pull request #3115 from egernst/storage-cleanup

blk-dev: hotplug read only if applicable

Gopkg.toml

@@ -47,8 +47,8 @@
   revision = "v1.4.2"
 
 [[constraint]]
-  name = "github.com/intel/govmm"
-  revision = "5e9aa08c4fd1201d65c6dd1136c8ab70ff17ee82"
+  name = "github.com/kata-containers/govmm"
+  revision = "7d320e8f5dcad260c1723bda3bff21539750d51f"
 
 [[constraint]]
   name = "github.com/kata-containers/agent"

pkg/katautils/config.go

@@ -14,7 +14,7 @@ import (
 	"strings"
 
 	"github.com/BurntSushi/toml"
-	govmmQemu "github.com/intel/govmm/qemu"
+	govmmQemu "github.com/kata-containers/govmm/qemu"
 	vc "github.com/kata-containers/runtime/virtcontainers"
 	"github.com/kata-containers/runtime/virtcontainers/device/config"
 	exp "github.com/kata-containers/runtime/virtcontainers/experimental"

virtcontainers/container.go

@@ -499,7 +499,7 @@ func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, hostMountDir, gu
 }
 
 // mountSharedDirMounts handles bind-mounts by bindmounting to the host shared
-// directory which is mounted through 9pfs in the VM.
+// directory which is mounted through virtiofs/9pfs in the VM.
 // 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.
@@ -522,6 +522,18 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, hostMountDir, guestShare
 			continue
 		}
 
+		// Check if mount is a block device file. If it is, the block device will be attached to the host
+		// instead of passing this as a shared mount:
+		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, nil, err
+			}
+			devicesToDetach = append(devicesToDetach, m.BlockDeviceID)
+			continue
+		}
+
+		// For non-block based mounts, we are only interested in bind mounts
 		if m.Type != "bind" {
 			continue
 		}
@@ -533,17 +545,6 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, hostMountDir, guestShare
 			continue
 		}
 
-		// Check if mount is a block device file. If it is, the block device will be attached to the host
-		// instead of passing this as a shared mount.
-		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, nil, err
-			}
-			devicesToDetach = append(devicesToDetach, m.BlockDeviceID)
-			continue
-		}
-
 		// Ignore /dev, directories and all other device files. We handle
 		// only regular files in /dev. It does not make sense to pass the host
 		// device nodes to the guest.
@@ -639,6 +640,9 @@ func filterDevices(c *Container, devices []ContainerDevice) (ret []ContainerDevi
 	return
 }
 
+// Add any mount based block devices to the device manager and save the
+// device ID for the particular mount. This'll occur when the mountpoint source
+// is a block device.
 func (c *Container) createBlockDevices() error {
 	if !c.checkBlockDeviceSupport() {
 		c.Logger().Warn("Block device not supported")
@@ -647,13 +651,18 @@ func (c *Container) createBlockDevices() error {
 
 	// iterate all mounts and create block device if it's block based.
 	for i, m := range c.mounts {
-		if len(m.BlockDeviceID) > 0 || m.Type != "bind" {
+		if len(m.BlockDeviceID) > 0 {
 			// Non-empty m.BlockDeviceID indicates there's already one device
 			// associated with the mount,so no need to create a new device for it
 			// and we only create block device for bind mount
 			continue
 		}
 
+		if m.Type != "bind" {
+			// We only handle for bind-mounts
+			continue
+		}
+
 		var stat unix.Stat_t
 		if err := unix.Stat(m.Source, &stat); err != nil {
 			return fmt.Errorf("stat %q failed: %v", m.Source, err)
@@ -671,6 +680,7 @@ func (c *Container) createBlockDevices() error {
 				DevType:       "b",
 				Major:         int64(unix.Major(stat.Rdev)),
 				Minor:         int64(unix.Minor(stat.Rdev)),
+				ReadOnly:      m.ReadOnly,
 			}
 			// check whether source can be used as a pmem device
 		} else if di, err = config.PmemDeviceInfo(m.Source, m.Destination); err != nil {
@@ -681,7 +691,6 @@ func (c *Container) createBlockDevices() error {
 
 		if err == nil && di != nil {
 			b, err := c.sandbox.devManager.NewDevice(*di)
-
 			if err != nil {
 				// Do not return an error, try to create
 				// devices for other mounts
@@ -750,11 +759,12 @@ func newContainer(sandbox *Sandbox, contConfig *ContainerConfig) (*Container, er
 		}
 	}
 
-	// Go to next step for first created container
+	// If mounts are block devices, add to devmanager
 	if err := c.createMounts(); err != nil {
 		return nil, err
 	}
 
+	// Add container's devices to sandbox's device-manager
 	if err := c.createDevices(contConfig); err != nil {
 		return nil, err
 	}
@@ -792,11 +802,7 @@ func (c *Container) createMounts() error {
 	}
 
 	// Create block devices for newly created container
-	if err := c.createBlockDevices(); err != nil {
-		return err
-	}
-
-	return nil
+	return c.createBlockDevices()
 }
 
 func (c *Container) createDevices(contConfig *ContainerConfig) error {
@@ -878,6 +884,7 @@ func (c *Container) create() (err error) {
 	}()
 
 	if c.checkBlockDeviceSupport() {
+		// If the rootfs is backed by a block device, go ahead and hotplug it to the guest
 		if err = c.hotplugDrive(); err != nil {
 			return
 		}
@@ -1315,11 +1322,14 @@ func (c *Container) resume() error {
 	return c.setContainerState(types.StateRunning)
 }
 
+// hotplugDrive will attempt to hotplug the container rootfs if it is backed by a
+// block device
 func (c *Container) hotplugDrive() error {
 	var dev device
 	var err error
 
-	// container rootfs is blockdevice backed and isn't mounted
+	// Check to see if the rootfs is an umounted block device (source) or if the
+	// mount (target) is backed by a block device:
 	if !c.rootFs.Mounted {
 		dev, err = getDeviceForPath(c.rootFs.Source)
 		// there is no "rootfs" dir on block device backed rootfs
@@ -1381,6 +1391,7 @@ func (c *Container) hotplugDrive() error {
 	return c.setStateFstype(fsType)
 }
 
+// plugDevice will attach the rootfs if blockdevice is supported (this is rootfs specific)
 func (c *Container) plugDevice(devicePath string) error {
 	var stat unix.Stat_t
 	if err := unix.Stat(devicePath, &stat); err != nil {

virtcontainers/device/config/config.go

@@ -115,6 +115,9 @@ type DeviceInfo struct {
 	// for a nvdimm device in the guest.
 	Pmem bool
 
+	// If applicable, should this device be considered RO
+	ReadOnly bool
+
 	// ColdPlug specifies whether the device must be cold plugged (true)
 	// or hot plugged (false).
 	ColdPlug bool

virtcontainers/device/drivers/block.go

@@ -61,11 +61,12 @@ func (device *BlockDevice) Attach(devReceiver api.DeviceReceiver) (err error) {
 	}
 
 	drive := &config.BlockDrive{
-		File:   device.DeviceInfo.HostPath,
-		Format: "raw",
-		ID:     utils.MakeNameID("drive", device.DeviceInfo.ID, maxDevIDSize),
-		Index:  index,
-		Pmem:   device.DeviceInfo.Pmem,
+		File:     device.DeviceInfo.HostPath,
+		Format:   "raw",
+		ID:       utils.MakeNameID("drive", device.DeviceInfo.ID, maxDevIDSize),
+		Index:    index,
+		Pmem:     device.DeviceInfo.Pmem,
+		ReadOnly: device.DeviceInfo.ReadOnly,
 	}
 
 	if fs, ok := device.DeviceInfo.DriverOptions["fstype"]; ok {