From 188424a18a7800a051547b2493ff65e71eeeb3d3 Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Mon, 11 Jan 2021 14:55:45 -0800 Subject: [PATCH 1/4] vendor: update govmm from intel to kata-containers Let's keep aligned with Kata 2.0, and utilize the kata-container organizations' govmm. Fixes: #3000 Signed-off-by: Eric Ernst --- Gopkg.lock | 16 ++++++++-------- Gopkg.toml | 2 +- pkg/katautils/config.go | 2 +- .../{intel => kata-containers}/govmm/COPYING | 0 .../govmm/qemu/image.go | 0 .../govmm/qemu/qemu.go | 0 .../{intel => kata-containers}/govmm/qemu/qmp.go | 0 virtcontainers/qemu.go | 2 +- virtcontainers/qemu_amd64.go | 2 +- virtcontainers/qemu_amd64_test.go | 2 +- virtcontainers/qemu_arch_base.go | 2 +- virtcontainers/qemu_arch_base_test.go | 2 +- virtcontainers/qemu_arm64.go | 2 +- virtcontainers/qemu_arm64_test.go | 2 +- virtcontainers/qemu_ppc64le.go | 2 +- virtcontainers/qemu_ppc64le_test.go | 2 +- virtcontainers/qemu_s390x.go | 2 +- virtcontainers/qemu_s390x_test.go | 2 +- virtcontainers/qemu_test.go | 2 +- 19 files changed, 22 insertions(+), 22 deletions(-) rename vendor/github.com/{intel => kata-containers}/govmm/COPYING (100%) rename vendor/github.com/{intel => kata-containers}/govmm/qemu/image.go (100%) rename vendor/github.com/{intel => kata-containers}/govmm/qemu/qemu.go (100%) rename vendor/github.com/{intel => kata-containers}/govmm/qemu/qmp.go (100%) diff --git a/Gopkg.lock b/Gopkg.lock index 1ed2ef2673..5ce8a52921 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -419,13 +419,6 @@ pruneopts = "NUT" revision = "2f1d1f20f75d5404f53b9edf6b53ed5505508675" -[[projects]] - digest = "1:647e03799be4f4878fa6deb1656f1c172c13d1e31b3aaf96384755006c94ded9" - name = "github.com/intel/govmm" - packages = ["qemu"] - pruneopts = "NUT" - revision = "5e9aa08c4fd1201d65c6dd1136c8ab70ff17ee82" - [[projects]] digest = "1:ee3d719407ec4bd877eaa4da37e2935298c3d9029ec3c20e502c0e14768b754c" name = "github.com/kata-containers/agent" @@ -437,6 +430,13 @@ pruneopts = "NUT" revision = "f9eab0fe9adb34e4f9f4a11f42a3eff983fd0659" +[[projects]] + digest = "1:647e03799be4f4878fa6deb1656f1c172c13d1e31b3aaf96384755006c94ded9" + name = "github.com/kata-containers/govmm" + packages = ["qemu"] + pruneopts = "NUT" + revision = "5e9aa08c4fd1201d65c6dd1136c8ab70ff17ee82" + [[projects]] digest = "1:58999a98719fddbac6303cb17e8d85b945f60b72f48e3a2df6b950b97fa926f1" name = "github.com/konsorten/go-windows-terminal-sequences" @@ -821,10 +821,10 @@ "github.com/gogo/protobuf/proto", "github.com/gogo/protobuf/types", "github.com/hashicorp/go-multierror", - "github.com/intel/govmm/qemu", "github.com/kata-containers/agent/pkg/types", "github.com/kata-containers/agent/protocols/client", "github.com/kata-containers/agent/protocols/grpc", + "github.com/kata-containers/govmm/qemu", "github.com/mitchellh/mapstructure", "github.com/opencontainers/runc/libcontainer/cgroups", "github.com/opencontainers/runc/libcontainer/cgroups/fs", diff --git a/Gopkg.toml b/Gopkg.toml index 879effcaf8..e74f0f5807 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -47,7 +47,7 @@ revision = "v1.4.2" [[constraint]] - name = "github.com/intel/govmm" + name = "github.com/kata-containers/govmm" revision = "5e9aa08c4fd1201d65c6dd1136c8ab70ff17ee82" [[constraint]] diff --git a/pkg/katautils/config.go b/pkg/katautils/config.go index 28f973d7eb..af0fe82b1e 100644 --- a/pkg/katautils/config.go +++ b/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" diff --git a/vendor/github.com/intel/govmm/COPYING b/vendor/github.com/kata-containers/govmm/COPYING similarity index 100% rename from vendor/github.com/intel/govmm/COPYING rename to vendor/github.com/kata-containers/govmm/COPYING diff --git a/vendor/github.com/intel/govmm/qemu/image.go b/vendor/github.com/kata-containers/govmm/qemu/image.go similarity index 100% rename from vendor/github.com/intel/govmm/qemu/image.go rename to vendor/github.com/kata-containers/govmm/qemu/image.go diff --git a/vendor/github.com/intel/govmm/qemu/qemu.go b/vendor/github.com/kata-containers/govmm/qemu/qemu.go similarity index 100% rename from vendor/github.com/intel/govmm/qemu/qemu.go rename to vendor/github.com/kata-containers/govmm/qemu/qemu.go diff --git a/vendor/github.com/intel/govmm/qemu/qmp.go b/vendor/github.com/kata-containers/govmm/qemu/qmp.go similarity index 100% rename from vendor/github.com/intel/govmm/qemu/qmp.go rename to vendor/github.com/kata-containers/govmm/qemu/qmp.go diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 9ceb5473ea..baf47e1a29 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -24,7 +24,7 @@ import ( "time" "unsafe" - govmmQemu "github.com/intel/govmm/qemu" + govmmQemu "github.com/kata-containers/govmm/qemu" "github.com/opencontainers/selinux/go-selinux/label" "github.com/opentracing/opentracing-go" "github.com/pkg/errors" diff --git a/virtcontainers/qemu_amd64.go b/virtcontainers/qemu_amd64.go index ac7bc693de..cf00da70b1 100644 --- a/virtcontainers/qemu_amd64.go +++ b/virtcontainers/qemu_amd64.go @@ -10,7 +10,7 @@ import ( "github.com/kata-containers/runtime/virtcontainers/types" - govmmQemu "github.com/intel/govmm/qemu" + govmmQemu "github.com/kata-containers/govmm/qemu" ) type qemuAmd64 struct { diff --git a/virtcontainers/qemu_amd64_test.go b/virtcontainers/qemu_amd64_test.go index 4524788faf..50e9fdc8ed 100644 --- a/virtcontainers/qemu_amd64_test.go +++ b/virtcontainers/qemu_amd64_test.go @@ -11,7 +11,7 @@ import ( "os" "testing" - govmmQemu "github.com/intel/govmm/qemu" + govmmQemu "github.com/kata-containers/govmm/qemu" "github.com/kata-containers/runtime/virtcontainers/types" "github.com/stretchr/testify/assert" ) diff --git a/virtcontainers/qemu_arch_base.go b/virtcontainers/qemu_arch_base.go index 0abb9d14ff..36ac64b812 100644 --- a/virtcontainers/qemu_arch_base.go +++ b/virtcontainers/qemu_arch_base.go @@ -14,7 +14,7 @@ import ( "strconv" "strings" - govmmQemu "github.com/intel/govmm/qemu" + govmmQemu "github.com/kata-containers/govmm/qemu" "github.com/kata-containers/runtime/virtcontainers/device/config" "github.com/kata-containers/runtime/virtcontainers/types" diff --git a/virtcontainers/qemu_arch_base_test.go b/virtcontainers/qemu_arch_base_test.go index 06c38294ef..cbf6f3b51d 100644 --- a/virtcontainers/qemu_arch_base_test.go +++ b/virtcontainers/qemu_arch_base_test.go @@ -13,7 +13,7 @@ import ( "path/filepath" "testing" - govmmQemu "github.com/intel/govmm/qemu" + govmmQemu "github.com/kata-containers/govmm/qemu" "github.com/stretchr/testify/assert" "github.com/kata-containers/runtime/virtcontainers/device/config" diff --git a/virtcontainers/qemu_arm64.go b/virtcontainers/qemu_arm64.go index aa6634de3b..aea67f323f 100644 --- a/virtcontainers/qemu_arm64.go +++ b/virtcontainers/qemu_arm64.go @@ -10,7 +10,7 @@ import ( "fmt" "time" - govmmQemu "github.com/intel/govmm/qemu" + govmmQemu "github.com/kata-containers/govmm/qemu" "github.com/kata-containers/runtime/virtcontainers/types" ) diff --git a/virtcontainers/qemu_arm64_test.go b/virtcontainers/qemu_arm64_test.go index c53c2e3b11..7027538952 100644 --- a/virtcontainers/qemu_arm64_test.go +++ b/virtcontainers/qemu_arm64_test.go @@ -11,7 +11,7 @@ import ( "os" "testing" - govmmQemu "github.com/intel/govmm/qemu" + govmmQemu "github.com/kata-containers/govmm/qemu" "github.com/stretchr/testify/assert" ) diff --git a/virtcontainers/qemu_ppc64le.go b/virtcontainers/qemu_ppc64le.go index ea9a6f58af..0ebc086e92 100644 --- a/virtcontainers/qemu_ppc64le.go +++ b/virtcontainers/qemu_ppc64le.go @@ -9,7 +9,7 @@ import ( "fmt" "time" - govmmQemu "github.com/intel/govmm/qemu" + govmmQemu "github.com/kata-containers/govmm/qemu" "github.com/kata-containers/runtime/virtcontainers/types" "github.com/sirupsen/logrus" ) diff --git a/virtcontainers/qemu_ppc64le_test.go b/virtcontainers/qemu_ppc64le_test.go index 9a4783e041..b1cb4e2593 100644 --- a/virtcontainers/qemu_ppc64le_test.go +++ b/virtcontainers/qemu_ppc64le_test.go @@ -12,7 +12,7 @@ import ( "strconv" "testing" - govmmQemu "github.com/intel/govmm/qemu" + govmmQemu "github.com/kata-containers/govmm/qemu" "github.com/stretchr/testify/assert" ) diff --git a/virtcontainers/qemu_s390x.go b/virtcontainers/qemu_s390x.go index 155a3b6b34..8cbceef0f6 100644 --- a/virtcontainers/qemu_s390x.go +++ b/virtcontainers/qemu_s390x.go @@ -9,7 +9,7 @@ import ( "fmt" "time" - govmmQemu "github.com/intel/govmm/qemu" + govmmQemu "github.com/kata-containers/govmm/qemu" "github.com/kata-containers/runtime/virtcontainers/device/config" "github.com/kata-containers/runtime/virtcontainers/types" ) diff --git a/virtcontainers/qemu_s390x_test.go b/virtcontainers/qemu_s390x_test.go index fb491ce146..0649598025 100644 --- a/virtcontainers/qemu_s390x_test.go +++ b/virtcontainers/qemu_s390x_test.go @@ -9,7 +9,7 @@ import ( "fmt" "testing" - govmmQemu "github.com/intel/govmm/qemu" + govmmQemu "github.com/kata-containers/govmm/qemu" "github.com/kata-containers/runtime/virtcontainers/device/config" "github.com/stretchr/testify/assert" ) diff --git a/virtcontainers/qemu_test.go b/virtcontainers/qemu_test.go index cc03d81dbe..7a39fc5a8b 100644 --- a/virtcontainers/qemu_test.go +++ b/virtcontainers/qemu_test.go @@ -14,7 +14,7 @@ import ( "strings" "testing" - govmmQemu "github.com/intel/govmm/qemu" + govmmQemu "github.com/kata-containers/govmm/qemu" "github.com/kata-containers/runtime/virtcontainers/device/config" "github.com/kata-containers/runtime/virtcontainers/persist" "github.com/kata-containers/runtime/virtcontainers/types" From cf32518ee320a9148a7c8325edb59b4980fab7f3 Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Mon, 11 Jan 2021 17:40:55 -0800 Subject: [PATCH 2/4] govmm: revendor to get latest changes We are adding access-mode for hotplugged blk devices. Let's revendor govmm so we can apply this for QEMU. Signed-off-by: Eric Ernst --- Gopkg.lock | 4 ++-- Gopkg.toml | 2 +- .../github.com/kata-containers/govmm/qemu/qemu.go | 2 +- vendor/github.com/kata-containers/govmm/qemu/qmp.go | 13 +++++++------ 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 5ce8a52921..4262f79adb 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -431,11 +431,11 @@ revision = "f9eab0fe9adb34e4f9f4a11f42a3eff983fd0659" [[projects]] - digest = "1:647e03799be4f4878fa6deb1656f1c172c13d1e31b3aaf96384755006c94ded9" + digest = "1:168ed421445ef3bf865d9ea9c2d87aa6b5114f01c885c4c33f0d2d12a9dffa4a" name = "github.com/kata-containers/govmm" packages = ["qemu"] pruneopts = "NUT" - revision = "5e9aa08c4fd1201d65c6dd1136c8ab70ff17ee82" + revision = "7d320e8f5dcad260c1723bda3bff21539750d51f" [[projects]] digest = "1:58999a98719fddbac6303cb17e8d85b945f60b72f48e3a2df6b950b97fa926f1" diff --git a/Gopkg.toml b/Gopkg.toml index e74f0f5807..bb8f3194d3 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -48,7 +48,7 @@ [[constraint]] name = "github.com/kata-containers/govmm" - revision = "5e9aa08c4fd1201d65c6dd1136c8ab70ff17ee82" + revision = "7d320e8f5dcad260c1723bda3bff21539750d51f" [[constraint]] name = "github.com/kata-containers/agent" diff --git a/vendor/github.com/kata-containers/govmm/qemu/qemu.go b/vendor/github.com/kata-containers/govmm/qemu/qemu.go index 567a316f40..9cfe39cbd1 100644 --- a/vendor/github.com/kata-containers/govmm/qemu/qemu.go +++ b/vendor/github.com/kata-containers/govmm/qemu/qemu.go @@ -135,7 +135,7 @@ const ( func isDimmSupported(config *Config) bool { switch runtime.GOARCH { - case "amd64", "386", "ppc64le": + case "amd64", "386", "ppc64le", "arm64": if config != nil && config.Machine.Type == MachineTypeMicrovm { // microvm does not support NUMA return false diff --git a/vendor/github.com/kata-containers/govmm/qemu/qmp.go b/vendor/github.com/kata-containers/govmm/qemu/qmp.go index 53ba105a8e..73d2ec4cff 100644 --- a/vendor/github.com/kata-containers/govmm/qemu/qmp.go +++ b/vendor/github.com/kata-containers/govmm/qemu/qmp.go @@ -773,11 +773,12 @@ func (q *QMP) ExecuteQuit(ctx context.Context) error { return q.executeCommand(ctx, "quit", nil, nil) } -func (q *QMP) blockdevAddBaseArgs(device, blockdevID string) (map[string]interface{}, map[string]interface{}) { +func (q *QMP) blockdevAddBaseArgs(device, blockdevID string, ro bool) (map[string]interface{}, map[string]interface{}) { var args map[string]interface{} blockdevArgs := map[string]interface{}{ - "driver": "raw", + "driver": "raw", + "read-only": ro, "file": map[string]interface{}{ "driver": "file", "filename": device, @@ -801,8 +802,8 @@ func (q *QMP) blockdevAddBaseArgs(device, blockdevID string) (map[string]interfa // path of the device to add, e.g., /dev/rdb0, and blockdevID is an identifier // used to name the device. As this identifier will be passed directly to QMP, // it must obey QMP's naming rules, e,g., it must start with a letter. -func (q *QMP) ExecuteBlockdevAdd(ctx context.Context, device, blockdevID string) error { - args, _ := q.blockdevAddBaseArgs(device, blockdevID) +func (q *QMP) ExecuteBlockdevAdd(ctx context.Context, device, blockdevID string, ro bool) error { + args, _ := q.blockdevAddBaseArgs(device, blockdevID, ro) return q.executeCommand(ctx, "blockdev-add", args, nil) } @@ -814,8 +815,8 @@ func (q *QMP) ExecuteBlockdevAdd(ctx context.Context, device, blockdevID string) // direct denotes whether use of O_DIRECT (bypass the host page cache) // is enabled. noFlush denotes whether flush requests for the device are // ignored. -func (q *QMP) ExecuteBlockdevAddWithCache(ctx context.Context, device, blockdevID string, direct, noFlush bool) error { - args, blockdevArgs := q.blockdevAddBaseArgs(device, blockdevID) +func (q *QMP) ExecuteBlockdevAddWithCache(ctx context.Context, device, blockdevID string, direct, noFlush, ro bool) error { + args, blockdevArgs := q.blockdevAddBaseArgs(device, blockdevID, ro) if q.version.Major < 2 || (q.version.Major == 2 && q.version.Minor < 9) { return fmt.Errorf("versions of qemu (%d.%d) older than 2.9 do not support set cache-related options for block devices", From 8b740662cee36e7d40e65797efa8bdf96b07a5d2 Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Thu, 7 Jan 2021 16:31:08 -0800 Subject: [PATCH 3/4] volumes: cleanup, minimal refactoring Update some headers, very minor refactoring Signed-off-by: Eric Ernst --- virtcontainers/container.go | 52 ++++++++++++++++++------------ virtcontainers/kata_agent.go | 53 +++++++++++++++++-------------- virtcontainers/kata_agent_test.go | 43 +++++++++++++++++++++---- virtcontainers/qemu.go | 4 +-- virtcontainers/sandbox.go | 5 +-- 5 files changed, 103 insertions(+), 54 deletions(-) diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 37117d7e38..4dad304171 100644 --- a/virtcontainers/container.go +++ b/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) @@ -681,7 +690,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 +758,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 +801,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 +883,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 +1321,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 +1390,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 { diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 934b1bee82..0aef5c7ab4 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -1333,7 +1333,6 @@ func (k *kataAgent) buildContainerRootfs(sandbox *Sandbox, c *Container, rootPat } case sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioSCSI: - rootfs.Driver = kataSCSIDevType rootfs.Source = blockDrive.SCSIAddr default: @@ -1348,8 +1347,8 @@ func (k *kataAgent) buildContainerRootfs(sandbox *Sandbox, c *Container, rootPat } // Ensure container mount destination exists - // TODO: remove dependency on shared fs path. shared fs is just one kind of storage sources. - // we should not always use shared fs path for all kinds of storage. Stead, all storage + // TODO: remove dependency on shared fs path. shared fs is just one kind of storage source. + // we should not always use shared fs path for all kinds of storage. Instead, all storage // should be bind mounted to a tmpfs path for containers to use. if err := os.MkdirAll(filepath.Join(getMountPath(c.sandbox.id), c.id, c.rootfsSuffix), DirMode); err != nil { return nil, err @@ -1357,13 +1356,10 @@ func (k *kataAgent) buildContainerRootfs(sandbox *Sandbox, c *Container, rootPat return rootfs, nil } - // This is not a block based device rootfs. - // We are going to bind mount it into the 9pfs - // shared drive between the host and the guest. - // With 9pfs we don't need to ask the agent to - // mount the rootfs as the shared directory - // (kataGuestSharedDir) is already mounted in the - // guest. We only need to mount the rootfs from + // This is not a block based device rootfs. We are going to bind mount it into the shared drive + // between the host and the guest. + // With virtiofs/9pfs we don't need to ask the agent to mount the rootfs as the shared directory + // (kataGuestSharedDir) is already mounted in the guest. We only need to mount the rootfs from // the host and it will show up in the guest. if err := bindMountContainerRootfs(k.ctx, getMountPath(sandbox.id), c.id, c.rootFs.Target, false); err != nil { return nil, err @@ -1403,9 +1399,14 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process, } }() + // setup rootfs -- if its block based, we'll receive a non-nil storage object representing + // the block device for the rootfs, which us utilized for mounting in the guest. This'll be handled + // already for non-block based rootfs if rootfs, err = k.buildContainerRootfs(sandbox, c, rootPathParent); err != nil { return nil, err - } else if rootfs != nil { + } + + if rootfs != nil { // Add rootfs to the list of container storage. // We only need to do this for block based rootfs, as we // want the agent to mount it into the right location @@ -1454,6 +1455,7 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process, if err != nil { return nil, err } + if err := k.replaceOCIMountsForStorages(ociSpec, volumeStorages); err != nil { return nil, err } @@ -1580,7 +1582,7 @@ func (k *kataAgent) handleLocalStorage(mounts []specs.Mount, sandboxID string, r // handleDeviceBlockVolume handles volume that is block device file // and DeviceBlock type. -func (k *kataAgent) handleDeviceBlockVolume(c *Container, device api.Device) (*grpc.Storage, error) { +func (k *kataAgent) handleDeviceBlockVolume(c *Container, m Mount, device api.Device) (*grpc.Storage, error) { vol := &grpc.Storage{} blockDrive, ok := device.GetDeviceInfo().(*config.BlockDrive) @@ -1615,12 +1617,22 @@ func (k *kataAgent) handleDeviceBlockVolume(c *Container, device api.Device) (*g return nil, fmt.Errorf("Unknown block device driver: %s", c.sandbox.config.HypervisorConfig.BlockDeviceDriver) } + vol.MountPoint = m.Destination + + // If no explicit FS Type or Options are being set, then let's use what is provided for the particular mount: + if vol.Fstype == "" { + vol.Fstype = m.Type + } + if len(vol.Options) == 0 { + vol.Options = m.Options + } + return vol, nil } // handleVhostUserBlkVolume handles volume that is block device file // and VhostUserBlk type. -func (k *kataAgent) handleVhostUserBlkVolume(c *Container, device api.Device) (*grpc.Storage, error) { +func (k *kataAgent) handleVhostUserBlkVolume(c *Container, m Mount, device api.Device) (*grpc.Storage, error) { vol := &grpc.Storage{} d, ok := device.GetDeviceInfo().(*config.VhostUserDeviceAttrs) @@ -1631,6 +1643,9 @@ func (k *kataAgent) handleVhostUserBlkVolume(c *Container, device api.Device) (* vol.Driver = kataBlkDevType vol.Source = d.PCIPath.String() + vol.Fstype = "bind" + vol.Options = []string{"bind"} + vol.MountPoint = m.Destination return vol, nil } @@ -1663,9 +1678,9 @@ func (k *kataAgent) handleBlockVolumes(c *Container) ([]*grpc.Storage, error) { var err error switch device.DeviceType() { case config.DeviceBlock: - vol, err = k.handleDeviceBlockVolume(c, device) + vol, err = k.handleDeviceBlockVolume(c, m, device) case config.VhostUserBlk: - vol, err = k.handleVhostUserBlkVolume(c, device) + vol, err = k.handleVhostUserBlkVolume(c, m, device) default: k.Logger().Error("Unknown device type") continue @@ -1675,14 +1690,6 @@ func (k *kataAgent) handleBlockVolumes(c *Container) ([]*grpc.Storage, error) { return nil, err } - vol.MountPoint = m.Destination - if vol.Fstype == "" { - vol.Fstype = "bind" - } - if len(vol.Options) == 0 { - vol.Options = []string{"bind"} - } - volumeStorages = append(volumeStorages, vol) } diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index 0041184fa0..e5c5bc12d4 100644 --- a/virtcontainers/kata_agent_test.go +++ b/virtcontainers/kata_agent_test.go @@ -413,6 +413,7 @@ func TestHandleDeviceBlockVolume(t *testing.T) { tests := []struct { BlockDeviceDriver string + inputMount Mount inputDev *drivers.BlockDevice resultVol *pb.Storage }{ @@ -424,6 +425,7 @@ func TestHandleDeviceBlockVolume(t *testing.T) { Format: testBlkDriveFormat, }, }, + inputMount: Mount{}, resultVol: &pb.Storage{ Driver: kataNvdimmDevType, Source: fmt.Sprintf("/dev/pmem%s", testNvdimmID), @@ -433,18 +435,25 @@ func TestHandleDeviceBlockVolume(t *testing.T) { }, { BlockDeviceDriver: config.VirtioBlockCCW, + inputMount: Mount{ + Type: "bind", + Options: []string{"ro"}, + }, inputDev: &drivers.BlockDevice{ BlockDrive: &config.BlockDrive{ DevNo: testDevNo, }, }, resultVol: &pb.Storage{ - Driver: kataBlkCCWDevType, - Source: testDevNo, + Driver: kataBlkCCWDevType, + Source: testDevNo, + Fstype: "bind", + Options: []string{"ro"}, }, }, { BlockDeviceDriver: config.VirtioBlock, + inputMount: Mount{}, inputDev: &drivers.BlockDevice{ BlockDrive: &config.BlockDrive{ PCIPath: testPCIPath, @@ -505,7 +514,7 @@ func TestHandleDeviceBlockVolume(t *testing.T) { }, } - vol, _ := k.handleDeviceBlockVolume(c, test.inputDev) + vol, _ := k.handleDeviceBlockVolume(c, test.inputMount, test.inputDev) assert.True(t, reflect.DeepEqual(vol, test.resultVol), "Volume didn't match: got %+v, expecting %+v", vol, test.resultVol) @@ -521,24 +530,30 @@ func TestHandleBlockVolume(t *testing.T) { containers := map[string]*Container{} containers[c.id] = c - // Create a VhostUserBlk device and a DeviceBlock device + // Create a devices for VhostUserBlk, standard DeviceBlock and direct assigned Block device vDevID := "MockVhostUserBlk" bDevID := "MockDeviceBlock" + dDevID := "MockDeviceBlockDirect" vDestination := "/VhostUserBlk/destination" bDestination := "/DeviceBlock/destination" + dDestination := "/DeviceDirectBlock/destination" vPCIPath, err := vcTypes.PciPathFromString("01/02") assert.NoError(t, err) bPCIPath, err := vcTypes.PciPathFromString("03/04") assert.NoError(t, err) + dPCIPath, err := vcTypes.PciPathFromString("05/06") + assert.NoError(t, err) vDev := drivers.NewVhostUserBlkDevice(&config.DeviceInfo{ID: vDevID}) bDev := drivers.NewBlockDevice(&config.DeviceInfo{ID: bDevID}) + dDev := drivers.NewBlockDevice(&config.DeviceInfo{ID: dDevID}) vDev.VhostUserDeviceAttrs = &config.VhostUserDeviceAttrs{PCIPath: vPCIPath} bDev.BlockDrive = &config.BlockDrive{PCIPath: bPCIPath} + dDev.BlockDrive = &config.BlockDrive{PCIPath: dPCIPath} var devices []api.Device - devices = append(devices, vDev, bDev) + devices = append(devices, vDev, bDev, dDev) // Create a VhostUserBlk mount and a DeviceBlock mount var mounts []Mount @@ -549,8 +564,16 @@ func TestHandleBlockVolume(t *testing.T) { bMount := Mount{ BlockDeviceID: bDevID, Destination: bDestination, + Type: "bind", + Options: []string{"bind"}, + } + dMount := Mount{ + BlockDeviceID: dDevID, + Destination: dDestination, + Type: "ext4", + Options: []string{"ro"}, } - mounts = append(mounts, vMount, bMount) + mounts = append(mounts, vMount, bMount, dMount) tmpDir := "/vhost/user/dir" dm := manager.NewDeviceManager(manager.VirtioBlock, true, tmpDir, devices) @@ -585,9 +608,17 @@ func TestHandleBlockVolume(t *testing.T) { Driver: kataBlkDevType, Source: bPCIPath.String(), } + dStorage := &pb.Storage{ + MountPoint: dDestination, + Fstype: "ext4", + Options: []string{"ro"}, + Driver: kataBlkDevType, + Source: dPCIPath.String(), + } assert.Equal(t, vStorage, volumeStorages[0], "Error while handle VhostUserBlk type block volume") assert.Equal(t, bStorage, volumeStorages[1], "Error while handle BlockDevice type block volume") + assert.Equal(t, dStorage, volumeStorages[2], "Error while handle direct BlockDevice type block volume") } func TestAppendDevicesEmptyContainerDeviceList(t *testing.T) { diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index baf47e1a29..4328a39441 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -1113,9 +1113,9 @@ func (q *qemu) hotplugAddBlockDevice(drive *config.BlockDrive, op operation, dev } if q.config.BlockDeviceCacheSet { - err = q.qmpMonitorCh.qmp.ExecuteBlockdevAddWithCache(q.qmpMonitorCh.ctx, drive.File, drive.ID, q.config.BlockDeviceCacheDirect, q.config.BlockDeviceCacheNoflush) + err = q.qmpMonitorCh.qmp.ExecuteBlockdevAddWithCache(q.qmpMonitorCh.ctx, drive.File, drive.ID, q.config.BlockDeviceCacheDirect, q.config.BlockDeviceCacheNoflush, false) } else { - err = q.qmpMonitorCh.qmp.ExecuteBlockdevAdd(q.qmpMonitorCh.ctx, drive.File, drive.ID) + err = q.qmpMonitorCh.qmp.ExecuteBlockdevAdd(q.qmpMonitorCh.ctx, drive.File, drive.ID, false) } if err != nil { return err diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 5613e35db7..2b9ca578e5 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -1148,13 +1148,13 @@ func (s *Sandbox) fetchContainers() error { // This should be called only when the sandbox is already created. // It will add new container config to sandbox.config.Containers func (s *Sandbox) CreateContainer(contConfig ContainerConfig) (VCContainer, error) { - // Create the container. + // Create the container object, add devices to the sandbox's device-manager: c, err := newContainer(s, &contConfig) if err != nil { return nil, err } - // Update sandbox config. + // Update sandbox config to include the new container's config s.config.Containers = append(s.config.Containers, contConfig) defer func() { @@ -1170,6 +1170,7 @@ func (s *Sandbox) CreateContainer(contConfig ContainerConfig) (VCContainer, erro } }() + // create and start the container err = c.create() if err != nil { return nil, err From b2956f39154722957977cbef3c39302ef44b3892 Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Thu, 7 Jan 2021 16:31:08 -0800 Subject: [PATCH 4/4] blk-dev: hotplug read only if applicable If a block based volume is read only, let's make sure we add as a RO device Fixes: #3114 Signed-off-by: Eric Ernst --- virtcontainers/container.go | 1 + virtcontainers/device/config/config.go | 3 +++ virtcontainers/device/drivers/block.go | 11 ++++++----- virtcontainers/qemu.go | 4 ++-- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 4dad304171..55440e75ca 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -680,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 { diff --git a/virtcontainers/device/config/config.go b/virtcontainers/device/config/config.go index bcd0d62695..ef3a9f523d 100644 --- a/virtcontainers/device/config/config.go +++ b/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 diff --git a/virtcontainers/device/drivers/block.go b/virtcontainers/device/drivers/block.go index 31010a4068..6bb7f4c6db 100644 --- a/virtcontainers/device/drivers/block.go +++ b/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 { diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 4328a39441..95afc59b66 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -1113,9 +1113,9 @@ func (q *qemu) hotplugAddBlockDevice(drive *config.BlockDrive, op operation, dev } if q.config.BlockDeviceCacheSet { - err = q.qmpMonitorCh.qmp.ExecuteBlockdevAddWithCache(q.qmpMonitorCh.ctx, drive.File, drive.ID, q.config.BlockDeviceCacheDirect, q.config.BlockDeviceCacheNoflush, false) + err = q.qmpMonitorCh.qmp.ExecuteBlockdevAddWithCache(q.qmpMonitorCh.ctx, drive.File, drive.ID, q.config.BlockDeviceCacheDirect, q.config.BlockDeviceCacheNoflush, drive.ReadOnly) } else { - err = q.qmpMonitorCh.qmp.ExecuteBlockdevAdd(q.qmpMonitorCh.ctx, drive.File, drive.ID, false) + err = q.qmpMonitorCh.qmp.ExecuteBlockdevAdd(q.qmpMonitorCh.ctx, drive.File, drive.ID, drive.ReadOnly) } if err != nil { return err