From 44814dce19e88c2426bd9ada2c015ed073946c24 Mon Sep 17 00:00:00 2001 From: Snir Sheriber Date: Tue, 10 May 2022 19:36:25 +0300 Subject: [PATCH 1/4] qemu: treat console kernel params within appendConsole as it is tightly coupled with the appended console device additionally have it tested Signed-off-by: Snir Sheriber --- src/runtime/virtcontainers/qemu.go | 15 ++++++++------- src/runtime/virtcontainers/qemu_amd64.go | 2 -- src/runtime/virtcontainers/qemu_arch_base.go | 6 ++++++ src/runtime/virtcontainers/qemu_arch_base_test.go | 2 ++ src/runtime/virtcontainers/qemu_arm64.go | 2 -- src/runtime/virtcontainers/qemu_ppc64le.go | 2 -- src/runtime/virtcontainers/qemu_s390x.go | 6 +++--- 7 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index a910c580f..070ace1e7 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -552,13 +552,6 @@ func (q *qemu) CreateVM(ctx context.Context, id string, network Network, hypervi return err } - // Breaks hypervisor abstration Has Kata Specific logic - kernel := govmmQemu.Kernel{ - Path: kernelPath, - InitrdPath: initrdPath, - Params: q.kernelParameters(), - } - incoming := q.setupTemplate(&knobs, &memory) // With the current implementations, VM templating will not work with file @@ -630,6 +623,14 @@ func (q *qemu) CreateVM(ctx context.Context, id string, network Network, hypervi return err } + // Breaks hypervisor abstraction has Kata Specific logic + kernel := govmmQemu.Kernel{ + Path: kernelPath, + InitrdPath: initrdPath, + // some devices configuration may also change kernel params, make sure this is called afterwards + Params: q.kernelParameters(), + } + qemuConfig := govmmQemu.Config{ Name: fmt.Sprintf("sandbox-%s", q.id), UUID: q.state.UUID, diff --git a/src/runtime/virtcontainers/qemu_amd64.go b/src/runtime/virtcontainers/qemu_amd64.go index 3f52ab756..773af95e8 100644 --- a/src/runtime/virtcontainers/qemu_amd64.go +++ b/src/runtime/virtcontainers/qemu_amd64.go @@ -56,8 +56,6 @@ var kernelParams = []Param{ {"i8042.noaux", "1"}, {"noreplace-smp", ""}, {"reboot", "k"}, - {"console", "hvc0"}, - {"console", "hvc1"}, {"cryptomgr.notests", ""}, {"net.ifnames", "0"}, {"pci", "lastbus=0"}, diff --git a/src/runtime/virtcontainers/qemu_arch_base.go b/src/runtime/virtcontainers/qemu_arch_base.go index 62fec60a7..0b5d50520 100644 --- a/src/runtime/virtcontainers/qemu_arch_base.go +++ b/src/runtime/virtcontainers/qemu_arch_base.go @@ -337,6 +337,12 @@ func (q *qemuArchBase) appendConsole(_ context.Context, devices []govmmQemu.Devi devices = append(devices, console) + consoleKernelParams := []Param{ + {"console", "hvc0"}, + {"console", "hvc1"}, + } + q.kernelParams = append(q.kernelParams, consoleKernelParams...) + return devices, nil } diff --git a/src/runtime/virtcontainers/qemu_arch_base_test.go b/src/runtime/virtcontainers/qemu_arch_base_test.go index ff20ba447..2625a8a99 100644 --- a/src/runtime/virtcontainers/qemu_arch_base_test.go +++ b/src/runtime/virtcontainers/qemu_arch_base_test.go @@ -259,6 +259,8 @@ func TestQemuArchBaseAppendConsoles(t *testing.T) { devices, err = qemuArchBase.appendConsole(context.Background(), devices, path) assert.NoError(err) assert.Equal(expectedOut, devices) + assert.Contains(qemuArchBase.kernelParams, Param{"console", "hvc0"}) + assert.Contains(qemuArchBase.kernelParams, Param{"console", "hvc1"}) } func TestQemuArchBaseAppendImage(t *testing.T) { diff --git a/src/runtime/virtcontainers/qemu_arm64.go b/src/runtime/virtcontainers/qemu_arm64.go index f5af19e21..f3a557660 100644 --- a/src/runtime/virtcontainers/qemu_arm64.go +++ b/src/runtime/virtcontainers/qemu_arm64.go @@ -33,8 +33,6 @@ const qmpMigrationWaitTimeout = 10 * time.Second const defaultQemuMachineOptions = "usb=off,accel=kvm,gic-version=host" var kernelParams = []Param{ - {"console", "hvc0"}, - {"console", "hvc1"}, {"iommu.passthrough", "0"}, } diff --git a/src/runtime/virtcontainers/qemu_ppc64le.go b/src/runtime/virtcontainers/qemu_ppc64le.go index e18f2264b..cfff5329c 100644 --- a/src/runtime/virtcontainers/qemu_ppc64le.go +++ b/src/runtime/virtcontainers/qemu_ppc64le.go @@ -41,8 +41,6 @@ const tpmHostPath = "/dev/tpmrm0" var kernelParams = []Param{ {"rcupdate.rcu_expedited", "1"}, {"reboot", "k"}, - {"console", "hvc0"}, - {"console", "hvc1"}, {"cryptomgr.notests", ""}, {"net.ifnames", "0"}, } diff --git a/src/runtime/virtcontainers/qemu_s390x.go b/src/runtime/virtcontainers/qemu_s390x.go index 32716e776..210a34176 100644 --- a/src/runtime/virtcontainers/qemu_s390x.go +++ b/src/runtime/virtcontainers/qemu_s390x.go @@ -39,9 +39,7 @@ const ( ) // Verify needed parameters -var kernelParams = []Param{ - {"console", "ttysclp0"}, -} +var kernelParams = []Param{} var ccwbridge = types.NewBridge(types.CCW, "", make(map[uint32]string, types.CCWBridgeMaxCapacity), 0) @@ -112,6 +110,8 @@ func (q *qemuS390x) appendConsole(ctx context.Context, devices []govmmQemu.Devic return devices, fmt.Errorf("Failed to append console %v", err) } + q.kernelParams = append(q.kernelParams, Param{"console", "ttysclp0"}) + serial := govmmQemu.SerialDevice{ Driver: virtioSerialCCW, ID: id, From c67b9d2975f6c24a63a9ae3d11bcb1380dfde8b2 Mon Sep 17 00:00:00 2001 From: Snir Sheriber Date: Thu, 12 May 2022 14:26:51 +0300 Subject: [PATCH 2/4] qemu: allow using legacy serial device for the console This allows to get guest early boot logs which are usually missed when virtconsole is used. - It utilizes previous work on the govmm side: https://github.com/kata-containers/govmm/pull/203 - unit test added Fixes: #4237 Signed-off-by: Snir Sheriber --- src/runtime/config/configuration-qemu.toml.in | 3 + .../pkg/katautils/config-settings.go.in | 1 + src/runtime/pkg/katautils/config.go | 3 + src/runtime/virtcontainers/hypervisor.go | 3 + src/runtime/virtcontainers/qemu_amd64.go | 1 + src/runtime/virtcontainers/qemu_arch_base.go | 59 +++++++++++++------ .../virtcontainers/qemu_arch_base_test.go | 28 +++++++++ src/runtime/virtcontainers/qemu_arm64.go | 1 + src/runtime/virtcontainers/qemu_ppc64le.go | 1 + src/runtime/virtcontainers/qemu_s390x.go | 1 + 10 files changed, 82 insertions(+), 19 deletions(-) diff --git a/src/runtime/config/configuration-qemu.toml.in b/src/runtime/config/configuration-qemu.toml.in index e0dd09cf6..09c219545 100644 --- a/src/runtime/config/configuration-qemu.toml.in +++ b/src/runtime/config/configuration-qemu.toml.in @@ -389,6 +389,9 @@ valid_entropy_sources = @DEFVALIDENTROPYSOURCES@ # be default_memory. #enable_guest_swap = true +# use legacy serial for guest console if available and implemented for architecture. Default false +#use_legacy_serial = true + [factory] # VM templating support. Once enabled, new VMs are created from template # using vm cloning. They will share the same initial kernel, initramfs and diff --git a/src/runtime/pkg/katautils/config-settings.go.in b/src/runtime/pkg/katautils/config-settings.go.in index 09d6b5f30..2aad22bd8 100644 --- a/src/runtime/pkg/katautils/config-settings.go.in +++ b/src/runtime/pkg/katautils/config-settings.go.in @@ -89,6 +89,7 @@ const defaultGuestSwap = false const defaultRootlessHypervisor = false const defaultDisableSeccomp = false const defaultVfioMode = "guest-kernel" +const defaultLegacySerial = false var defaultSGXEPCSize = int64(0) diff --git a/src/runtime/pkg/katautils/config.go b/src/runtime/pkg/katautils/config.go index ec02b4bc2..2a62c3d6c 100644 --- a/src/runtime/pkg/katautils/config.go +++ b/src/runtime/pkg/katautils/config.go @@ -146,6 +146,7 @@ type hypervisor struct { Rootless bool `toml:"rootless"` DisableSeccomp bool `toml:"disable_seccomp"` DisableSeLinux bool `toml:"disable_selinux"` + LegacySerial bool `toml:"use_legacy_serial"` } type runtime struct { @@ -775,6 +776,7 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { ConfidentialGuest: h.ConfidentialGuest, GuestSwap: h.GuestSwap, Rootless: h.Rootless, + LegacySerial: h.LegacySerial, }, nil } @@ -1132,6 +1134,7 @@ func GetDefaultHypervisorConfig() vc.HypervisorConfig { GuestSwap: defaultGuestSwap, Rootless: defaultRootlessHypervisor, DisableSeccomp: defaultDisableSeccomp, + LegacySerial: defaultLegacySerial, } } diff --git a/src/runtime/virtcontainers/hypervisor.go b/src/runtime/virtcontainers/hypervisor.go index c689eae3c..9de4dc0d0 100644 --- a/src/runtime/virtcontainers/hypervisor.go +++ b/src/runtime/virtcontainers/hypervisor.go @@ -537,6 +537,9 @@ type HypervisorConfig struct { // Disable selinux from the hypervisor process DisableSeLinux bool + + // Use legacy serial for the guest console + LegacySerial bool } // vcpu mapping from vcpu number to thread number diff --git a/src/runtime/virtcontainers/qemu_amd64.go b/src/runtime/virtcontainers/qemu_amd64.go index 773af95e8..61c18c68e 100644 --- a/src/runtime/virtcontainers/qemu_amd64.go +++ b/src/runtime/virtcontainers/qemu_amd64.go @@ -122,6 +122,7 @@ func newQemuArch(config HypervisorConfig) (qemuArch, error) { disableNvdimm: config.DisableImageNvdimm, dax: true, protection: noneProtection, + legacySerial: config.LegacySerial, }, vmFactory: factory, } diff --git a/src/runtime/virtcontainers/qemu_arch_base.go b/src/runtime/virtcontainers/qemu_arch_base.go index 0b5d50520..a2873a216 100644 --- a/src/runtime/virtcontainers/qemu_arch_base.go +++ b/src/runtime/virtcontainers/qemu_arch_base.go @@ -169,6 +169,7 @@ type qemuArchBase struct { vhost bool disableNvdimm bool dax bool + legacySerial bool } const ( @@ -318,29 +319,49 @@ func (q *qemuArchBase) memoryTopology(memoryMb, hostMemoryMb uint64, slots uint8 } func (q *qemuArchBase) appendConsole(_ context.Context, devices []govmmQemu.Device, path string) ([]govmmQemu.Device, error) { - serial := govmmQemu.SerialDevice{ - Driver: govmmQemu.VirtioSerial, - ID: "serial0", - DisableModern: q.nestedRun, - MaxPorts: uint(2), + var serial, console govmmQemu.Device + var consoleKernelParams []Param + + if q.legacySerial { + serial = govmmQemu.LegacySerialDevice{ + Chardev: "charconsole0", + } + + console = govmmQemu.CharDevice{ + Driver: govmmQemu.LegacySerial, + Backend: govmmQemu.Socket, + DeviceID: "console0", + ID: "charconsole0", + Path: path, + } + + consoleKernelParams = []Param{ + {"console", "ttyS0"}, + } + } else { + serial = govmmQemu.SerialDevice{ + Driver: govmmQemu.VirtioSerial, + ID: "serial0", + DisableModern: q.nestedRun, + MaxPorts: uint(2), + } + + console = govmmQemu.CharDevice{ + Driver: govmmQemu.Console, + Backend: govmmQemu.Socket, + DeviceID: "console0", + ID: "charconsole0", + Path: path, + } + + consoleKernelParams = []Param{ + {"console", "hvc0"}, + {"console", "hvc1"}, + } } devices = append(devices, serial) - - console := govmmQemu.CharDevice{ - Driver: govmmQemu.Console, - Backend: govmmQemu.Socket, - DeviceID: "console0", - ID: "charconsole0", - Path: path, - } - devices = append(devices, console) - - consoleKernelParams := []Param{ - {"console", "hvc0"}, - {"console", "hvc1"}, - } q.kernelParams = append(q.kernelParams, consoleKernelParams...) return devices, nil diff --git a/src/runtime/virtcontainers/qemu_arch_base_test.go b/src/runtime/virtcontainers/qemu_arch_base_test.go index 2625a8a99..57c97f773 100644 --- a/src/runtime/virtcontainers/qemu_arch_base_test.go +++ b/src/runtime/virtcontainers/qemu_arch_base_test.go @@ -263,6 +263,34 @@ func TestQemuArchBaseAppendConsoles(t *testing.T) { assert.Contains(qemuArchBase.kernelParams, Param{"console", "hvc1"}) } +func TestQemuArchBaseAppendConsolesLegacy(t *testing.T) { + var devices []govmmQemu.Device + var err error + assert := assert.New(t) + qemuArchBase := newQemuArchBase() + qemuArchBase.legacySerial = true + + path := filepath.Join(filepath.Join(fs.MockRunStoragePath(), "test"), consoleSocket) + + expectedOut := []govmmQemu.Device{ + govmmQemu.LegacySerialDevice{ + Chardev: "charconsole0", + }, + govmmQemu.CharDevice{ + Driver: govmmQemu.LegacySerial, + Backend: govmmQemu.Socket, + DeviceID: "console0", + ID: "charconsole0", + Path: path, + }, + } + + devices, err = qemuArchBase.appendConsole(context.Background(), devices, path) + assert.NoError(err) + assert.Equal(expectedOut, devices) + assert.Contains(qemuArchBase.kernelParams, Param{"console", "ttyS0"}) +} + func TestQemuArchBaseAppendImage(t *testing.T) { var devices []govmmQemu.Device assert := assert.New(t) diff --git a/src/runtime/virtcontainers/qemu_arm64.go b/src/runtime/virtcontainers/qemu_arm64.go index f3a557660..378b89c6d 100644 --- a/src/runtime/virtcontainers/qemu_arm64.go +++ b/src/runtime/virtcontainers/qemu_arm64.go @@ -62,6 +62,7 @@ func newQemuArch(config HypervisorConfig) (qemuArch, error) { disableNvdimm: config.DisableImageNvdimm, dax: true, protection: noneProtection, + legacySerial: config.LegacySerial, }, } diff --git a/src/runtime/virtcontainers/qemu_ppc64le.go b/src/runtime/virtcontainers/qemu_ppc64le.go index cfff5329c..27bac3581 100644 --- a/src/runtime/virtcontainers/qemu_ppc64le.go +++ b/src/runtime/virtcontainers/qemu_ppc64le.go @@ -74,6 +74,7 @@ func newQemuArch(config HypervisorConfig) (qemuArch, error) { kernelParamsDebug: kernelParamsDebug, kernelParams: kernelParams, protection: noneProtection, + legacySerial: config.LegacySerial, }, } diff --git a/src/runtime/virtcontainers/qemu_s390x.go b/src/runtime/virtcontainers/qemu_s390x.go index 210a34176..f0946698f 100644 --- a/src/runtime/virtcontainers/qemu_s390x.go +++ b/src/runtime/virtcontainers/qemu_s390x.go @@ -66,6 +66,7 @@ func newQemuArch(config HypervisorConfig) (qemuArch, error) { kernelParamsNonDebug: kernelParamsNonDebug, kernelParamsDebug: kernelParamsDebug, kernelParams: kernelParams, + legacySerial: false, }, } // Set first bridge type to CCW From f4994e486bc7118cb0fe498cf48f88b3f0300d68 Mon Sep 17 00:00:00 2001 From: Snir Sheriber Date: Wed, 18 May 2022 14:18:31 +0300 Subject: [PATCH 3/4] runtime: allow annotation configuration to use_legacy_serial and update the docs and test Signed-off-by: Snir Sheriber --- docs/how-to/how-to-set-sandbox-config-kata.md | 1 + src/runtime/pkg/oci/utils.go | 6 ++++++ src/runtime/pkg/oci/utils_test.go | 2 ++ src/runtime/virtcontainers/pkg/annotations/annotations.go | 3 +++ 4 files changed, 12 insertions(+) diff --git a/docs/how-to/how-to-set-sandbox-config-kata.md b/docs/how-to/how-to-set-sandbox-config-kata.md index 28d5ea788..343b7fe2e 100644 --- a/docs/how-to/how-to-set-sandbox-config-kata.md +++ b/docs/how-to/how-to-set-sandbox-config-kata.md @@ -91,6 +91,7 @@ There are several kinds of Kata configurations and they are listed below. | `io.katacontainers.config.hypervisor.virtio_fs_daemon` | string | virtio-fs `vhost-user` daemon path | | `io.katacontainers.config.hypervisor.virtio_fs_extra_args` | string | extra options passed to `virtiofs` daemon | | `io.katacontainers.config.hypervisor.enable_guest_swap` | `boolean` | enable swap in the guest | +| `io.katacontainers.config.hypervisor.use_legacy_serial` | `boolean` | uses legacy serial device for guest's console (QEMU) | ## Container Options | Key | Value Type | Comments | diff --git a/src/runtime/pkg/oci/utils.go b/src/runtime/pkg/oci/utils.go index f72f1147e..e6e4dcfe6 100644 --- a/src/runtime/pkg/oci/utils.go +++ b/src/runtime/pkg/oci/utils.go @@ -474,6 +474,12 @@ func addHypervisorConfigOverrides(ocispec specs.Spec, config *vc.SandboxConfig, return err } + if err := newAnnotationConfiguration(ocispec, vcAnnotations.UseLegacySerial).setBool(func(useLegacySerial bool) { + config.HypervisorConfig.LegacySerial = useLegacySerial + }); err != nil { + return err + } + if err := newAnnotationConfiguration(ocispec, vcAnnotations.PCIeRootPort).setUint(func(pcieRootPort uint64) { config.HypervisorConfig.PCIeRootPort = uint32(pcieRootPort) }); err != nil { diff --git a/src/runtime/pkg/oci/utils_test.go b/src/runtime/pkg/oci/utils_test.go index bfa71801d..e6158a96e 100644 --- a/src/runtime/pkg/oci/utils_test.go +++ b/src/runtime/pkg/oci/utils_test.go @@ -670,6 +670,7 @@ func TestAddHypervisorAnnotations(t *testing.T) { ocispec.Annotations[vcAnnotations.PCIeRootPort] = "2" ocispec.Annotations[vcAnnotations.IOMMUPlatform] = "true" ocispec.Annotations[vcAnnotations.SGXEPC] = "64Mi" + ocispec.Annotations[vcAnnotations.UseLegacySerial] = "true" // 10Mbit ocispec.Annotations[vcAnnotations.RxRateLimiterMaxRate] = "10000000" ocispec.Annotations[vcAnnotations.TxRateLimiterMaxRate] = "10000000" @@ -706,6 +707,7 @@ func TestAddHypervisorAnnotations(t *testing.T) { assert.Equal(config.HypervisorConfig.PCIeRootPort, uint32(2)) assert.Equal(config.HypervisorConfig.IOMMUPlatform, true) assert.Equal(config.HypervisorConfig.SGXEPCSize, int64(67108864)) + assert.Equal(config.HypervisorConfig.LegacySerial, true) assert.Equal(config.HypervisorConfig.RxRateLimiterMaxRate, uint64(10000000)) assert.Equal(config.HypervisorConfig.TxRateLimiterMaxRate, uint64(10000000)) diff --git a/src/runtime/virtcontainers/pkg/annotations/annotations.go b/src/runtime/virtcontainers/pkg/annotations/annotations.go index 5bd9b26db..6618d0acd 100644 --- a/src/runtime/virtcontainers/pkg/annotations/annotations.go +++ b/src/runtime/virtcontainers/pkg/annotations/annotations.go @@ -130,6 +130,9 @@ const ( // entropy (/dev/random, /dev/urandom or real hardware RNG device) EntropySource = kataAnnotHypervisorPrefix + "entropy_source" + // UseLegacySerial sets legacy serial device for guest console if available and implemented for architecture + UseLegacySerial = kataAnnotHypervisorPrefix + "use_legacy_serial" + // // CPU Annotations // From 834f93ce8a11bf6fca5500e6a8f5cecf71e4f6f8 Mon Sep 17 00:00:00 2001 From: Snir Sheriber Date: Thu, 19 May 2022 09:47:32 +0300 Subject: [PATCH 4/4] docs: fix annotations example annotation value should always be quoted, regardless to its type Signed-off-by: Snir Sheriber --- docs/how-to/how-to-set-sandbox-config-kata.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/how-to/how-to-set-sandbox-config-kata.md b/docs/how-to/how-to-set-sandbox-config-kata.md index 343b7fe2e..9f612831c 100644 --- a/docs/how-to/how-to-set-sandbox-config-kata.md +++ b/docs/how-to/how-to-set-sandbox-config-kata.md @@ -173,7 +173,7 @@ kind: Pod metadata: name: pod2 annotations: - io.katacontainers.config.runtime.disable_guest_seccomp: false + io.katacontainers.config.runtime.disable_guest_seccomp: "false" spec: runtimeClassName: kata containers: