From adba4532a402c5b3d5894baee75156a4891d3d15 Mon Sep 17 00:00:00 2001 From: Jakob Naucke Date: Mon, 26 Apr 2021 17:52:01 +0200 Subject: [PATCH 1/3] virtcontainers: Revert "virtcontainers: Allow s390x appendVhostUserDevice" This reverts commit 7f6091133302f3038361b37bfdb3cc39f057154e. Patch allowed other vhost user devices besides FS not supported on s390x and failed to attach a CCW device number, which results in the inavailability to use more devices after vhost-user-fs-ccw. Signed-off-by: Jakob Naucke --- src/runtime/virtcontainers/qemu.go | 4 ++-- src/runtime/virtcontainers/qemu_arch_base.go | 6 +++--- src/runtime/virtcontainers/qemu_arch_base_test.go | 2 +- src/runtime/virtcontainers/qemu_s390x.go | 6 ++++++ src/runtime/virtcontainers/qemu_s390x_test.go | 14 ++++++++++++++ 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index d127c1273..5e2bb8593 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -1924,7 +1924,7 @@ func (q *qemu) addDevice(ctx context.Context, devInfo interface{}, devType devic vhostDev.SocketPath = sockPath vhostDev.DevID = id - q.qemuConfig.Devices = q.arch.appendVhostUserDevice(q.qemuConfig.Devices, vhostDev) + q.qemuConfig.Devices, err = q.arch.appendVhostUserDevice(q.qemuConfig.Devices, vhostDev) } else { q.Logger().WithField("volume-type", "virtio-9p").Info("adding volume") q.qemuConfig.Devices, err = q.arch.append9PVolume(ctx, q.qemuConfig.Devices, v) @@ -1939,7 +1939,7 @@ func (q *qemu) addDevice(ctx context.Context, devInfo interface{}, devType devic case config.BlockDrive: q.qemuConfig.Devices, err = q.arch.appendBlockDevice(ctx, q.qemuConfig.Devices, v) case config.VhostUserDeviceAttrs: - q.qemuConfig.Devices = q.arch.appendVhostUserDevice(q.qemuConfig.Devices, v) + q.qemuConfig.Devices, err = q.arch.appendVhostUserDevice(q.qemuConfig.Devices, v) case config.VFIODev: q.qemuConfig.Devices = q.arch.appendVFIODevice(q.qemuConfig.Devices, v) default: diff --git a/src/runtime/virtcontainers/qemu_arch_base.go b/src/runtime/virtcontainers/qemu_arch_base.go index aa8e4d59f..ae0ce3a13 100644 --- a/src/runtime/virtcontainers/qemu_arch_base.go +++ b/src/runtime/virtcontainers/qemu_arch_base.go @@ -96,7 +96,7 @@ type qemuArch interface { appendBlockDevice(ctx context.Context, devices []govmmQemu.Device, drive config.BlockDrive) ([]govmmQemu.Device, error) // appendVhostUserDevice appends a vhost user device to devices - appendVhostUserDevice(devices []govmmQemu.Device, drive config.VhostUserDeviceAttrs) []govmmQemu.Device + appendVhostUserDevice(devices []govmmQemu.Device, drive config.VhostUserDeviceAttrs) ([]govmmQemu.Device, error) // appendVFIODevice appends a VFIO device to devices appendVFIODevice(devices []govmmQemu.Device, vfioDevice config.VFIODev) []govmmQemu.Device @@ -633,7 +633,7 @@ func (q *qemuArchBase) appendBlockDevice(_ context.Context, devices []govmmQemu. return devices, nil } -func (q *qemuArchBase) appendVhostUserDevice(devices []govmmQemu.Device, attr config.VhostUserDeviceAttrs) []govmmQemu.Device { +func (q *qemuArchBase) appendVhostUserDevice(devices []govmmQemu.Device, attr config.VhostUserDeviceAttrs) ([]govmmQemu.Device, error) { qemuVhostUserDevice := govmmQemu.VhostUserDevice{} switch attr.Type { @@ -658,7 +658,7 @@ func (q *qemuArchBase) appendVhostUserDevice(devices []govmmQemu.Device, attr co devices = append(devices, qemuVhostUserDevice) - return devices + return devices, nil } func (q *qemuArchBase) appendVFIODevice(devices []govmmQemu.Device, vfioDev config.VFIODev) []govmmQemu.Device { diff --git a/src/runtime/virtcontainers/qemu_arch_base_test.go b/src/runtime/virtcontainers/qemu_arch_base_test.go index f134a15ae..d4aba54fa 100644 --- a/src/runtime/virtcontainers/qemu_arch_base_test.go +++ b/src/runtime/virtcontainers/qemu_arch_base_test.go @@ -224,7 +224,7 @@ func testQemuArchBaseAppend(t *testing.T, structure interface{}, expected []govm case config.VFIODev: devices = qemuArchBase.appendVFIODevice(devices, s) case config.VhostUserDeviceAttrs: - devices = qemuArchBase.appendVhostUserDevice(devices, s) + devices, err = qemuArchBase.appendVhostUserDevice(devices, s) } assert.NoError(err) diff --git a/src/runtime/virtcontainers/qemu_s390x.go b/src/runtime/virtcontainers/qemu_s390x.go index 6a4c78264..01968b6b1 100644 --- a/src/runtime/virtcontainers/qemu_s390x.go +++ b/src/runtime/virtcontainers/qemu_s390x.go @@ -152,6 +152,12 @@ func (q *qemuS390x) appendCCWBlockDevice(ctx context.Context, devices []govmmQem return devices, nil } +// appendVhostUserDevice throws an error if vhost devices are tried to be used. +// See issue https://github.com/kata-containers/runtime/issues/659 +func (q *qemuS390x) appendVhostUserDevice(devices []govmmQemu.Device, attr config.VhostUserDeviceAttrs) ([]govmmQemu.Device, error) { + return nil, fmt.Errorf("No vhost-user devices supported on s390x") +} + // supportGuestMemoryHotplug return false for s390x architecture. The pc-dimm backend device for s390x // is not support. PC-DIMM is not listed in the devices supported by qemu-system-s390x -device help func (q *qemuS390x) supportGuestMemoryHotplug() bool { diff --git a/src/runtime/virtcontainers/qemu_s390x_test.go b/src/runtime/virtcontainers/qemu_s390x_test.go index 3355680bb..70ddf3ff2 100644 --- a/src/runtime/virtcontainers/qemu_s390x_test.go +++ b/src/runtime/virtcontainers/qemu_s390x_test.go @@ -52,3 +52,17 @@ func TestQemuS390xMemoryTopology(t *testing.T) { m := s390x.memoryTopology(mem, hostMem, slots) assert.Equal(expectedMemory, m) } + +func TestQemuS390xAppendVhostUserDevice(t *testing.T) { + macAddress := "00:11:22:33:44:55:66" + qemu := qemuS390x{} + assert := assert.New(t) + + vhostUserDevice := config.VhostUserDeviceAttrs{ + Type: config.VhostUserNet, + MacAddress: macAddress, + } + + _, err := qemu.appendVhostUserDevice(nil, vhostUserDevice) + assert.Error(err) +} From 8385ff9554188d798fba07297240b7cb0feec251 Mon Sep 17 00:00:00 2001 From: Jakob Naucke Date: Mon, 26 Apr 2021 18:05:10 +0200 Subject: [PATCH 2/3] runtime: Re-vendor GoVMM for vhost-user-fs-ccw devno support shortlog: f0e9a35 Merge pull request #171 from Jakob-Naucke/fix-virtiofs-s390x abd3c7e qemu: VhostUserDevice CCW device numbers 3eaeda7 qemu: Refactor vhostuserDev.QemuParams 7183b12 Merge pull request #166 from kata-containers/egernst-patch-1 092293f Merge pull request #169 from QiuMike/master 511cf58 Fix qemu commandline issue with empty romfile 8ba62b0 Merge pull request #164 from devimc/2021-03-30/tdxSupport b3eac95 qmp: remove frequent, chatty log 3141894 qemu: add support for tdx-guest object Signed-off-by: Jakob Naucke --- src/runtime/go.mod | 2 +- src/runtime/go.sum | 2 + .../kata-containers/govmm/qemu/qemu.go | 232 ++++++++++++------ .../kata-containers/govmm/qemu/qmp.go | 5 - src/runtime/vendor/modules.txt | 2 +- 5 files changed, 164 insertions(+), 79 deletions(-) diff --git a/src/runtime/go.mod b/src/runtime/go.mod index df9c2d222..272cd837b 100644 --- a/src/runtime/go.mod +++ b/src/runtime/go.mod @@ -31,7 +31,7 @@ require ( github.com/gogo/googleapis v1.4.0 // indirect github.com/gogo/protobuf v1.3.1 github.com/hashicorp/go-multierror v1.0.0 - github.com/kata-containers/govmm v0.0.0-20210329205824-7fbc685865d2 + github.com/kata-containers/govmm v0.0.0-20210428163604-f0e9a35308ee github.com/mdlayher/vsock v0.0.0-20191108225356-d9c65923cb8f github.com/opencontainers/image-spec v1.0.1 // indirect github.com/opencontainers/runc v1.0.0-rc9.0.20200102164712-2b52db75279c diff --git a/src/runtime/go.sum b/src/runtime/go.sum index 46e7d2b00..9d495d531 100644 --- a/src/runtime/go.sum +++ b/src/runtime/go.sum @@ -278,6 +278,8 @@ github.com/kata-containers/govmm v0.0.0-20210112013750-7d320e8f5dca h1:UdXFthwas github.com/kata-containers/govmm v0.0.0-20210112013750-7d320e8f5dca/go.mod h1:VmAHbsL5lLfzHW/MNL96NVLF840DNEV5i683kISgFKk= github.com/kata-containers/govmm v0.0.0-20210329205824-7fbc685865d2 h1:oDJsQ5avmW8PFUkSJdsuaGL3SR4/YQRno51GNGl+IDU= github.com/kata-containers/govmm v0.0.0-20210329205824-7fbc685865d2/go.mod h1:VmAHbsL5lLfzHW/MNL96NVLF840DNEV5i683kISgFKk= +github.com/kata-containers/govmm v0.0.0-20210428163604-f0e9a35308ee h1:M4N7AdSHgWz/ubV5AZQdeqmK+9Ztpea6oqeXgk8GCHk= +github.com/kata-containers/govmm v0.0.0-20210428163604-f0e9a35308ee/go.mod h1:VmAHbsL5lLfzHW/MNL96NVLF840DNEV5i683kISgFKk= github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/konsorten/go-windows-terminal-sequences v1.0.1 h1:mweAR1A6xJ3oS2pRaGiHgQ4OO8tzTaLawm8vnODuwDk= diff --git a/src/runtime/vendor/github.com/kata-containers/govmm/qemu/qemu.go b/src/runtime/vendor/github.com/kata-containers/govmm/qemu/qemu.go index 53ac89840..61ab2cc20 100644 --- a/src/runtime/vendor/github.com/kata-containers/govmm/qemu/qemu.go +++ b/src/runtime/vendor/github.com/kata-containers/govmm/qemu/qemu.go @@ -227,6 +227,9 @@ type ObjectType string const ( // MemoryBackendFile represents a guest memory mapped file. MemoryBackendFile ObjectType = "memory-backend-file" + + // TDXGuest represents a TDX object + TDXGuest ObjectType = "tdx-guest" ) // Object is a qemu object representation. @@ -249,6 +252,12 @@ type Object struct { // Size is the object size in bytes Size uint64 + + // Debug this is a debug object + Debug bool + + // File is the device file + File string } // Valid returns true if the Object structure is valid and complete. @@ -259,6 +268,11 @@ func (object Object) Valid() bool { return false } + case TDXGuest: + if object.ID == "" || object.File == "" || object.DeviceID == "" { + return false + } + default: return false } @@ -283,6 +297,13 @@ func (object Object) QemuParams(config *Config) []string { objectParams = append(objectParams, fmt.Sprintf(",size=%d", object.Size)) deviceParams = append(deviceParams, fmt.Sprintf(",memdev=%s", object.ID)) + case TDXGuest: + objectParams = append(objectParams, string(object.Type)) + objectParams = append(objectParams, fmt.Sprintf(",id=%s", object.ID)) + if object.Debug { + objectParams = append(objectParams, ",debug=on") + } + deviceParams = append(deviceParams, fmt.Sprintf(",file=%s", object.File)) } qemuParams = append(qemuParams, "-device") @@ -408,7 +429,7 @@ func (fsdev FSDevice) QemuParams(config *Config) []string { } deviceParams = append(deviceParams, fmt.Sprintf(",fsdev=%s", fsdev.ID)) deviceParams = append(deviceParams, fmt.Sprintf(",mount_tag=%s", fsdev.MountTag)) - if fsdev.Transport.isVirtioPCI(config) { + if fsdev.Transport.isVirtioPCI(config) && fsdev.ROMFile != "" { deviceParams = append(deviceParams, fmt.Sprintf(",romfile=%s", fsdev.ROMFile)) } if fsdev.Transport.isVirtioCCW(config) { @@ -541,7 +562,7 @@ func (cdev CharDevice) QemuParams(config *Config) []string { if cdev.Name != "" { deviceParams = append(deviceParams, fmt.Sprintf(",name=%s", cdev.Name)) } - if cdev.Driver == VirtioSerial && cdev.Transport.isVirtioPCI(config) { + if cdev.Driver == VirtioSerial && cdev.Transport.isVirtioPCI(config) && cdev.ROMFile != "" { deviceParams = append(deviceParams, fmt.Sprintf(",romfile=%s", cdev.ROMFile)) } @@ -811,7 +832,7 @@ func (netdev NetDevice) QemuDeviceParams(config *Config) []string { deviceParams = append(deviceParams, netdev.mqParameter(config)) } - if netdev.Transport.isVirtioPCI(config) { + if netdev.Transport.isVirtioPCI(config) && netdev.ROMFile != "" { deviceParams = append(deviceParams, fmt.Sprintf(",romfile=%s", netdev.ROMFile)) } @@ -944,7 +965,7 @@ func (dev SerialDevice) QemuParams(config *Config) []string { deviceParams = append(deviceParams, fmt.Sprintf(",%s", s)) } deviceParams = append(deviceParams, fmt.Sprintf(",id=%s", dev.ID)) - if dev.Transport.isVirtioPCI(config) { + if dev.Transport.isVirtioPCI(config) && dev.ROMFile != "" { deviceParams = append(deviceParams, fmt.Sprintf(",romfile=%s", dev.ROMFile)) if dev.Driver == VirtioSerial && dev.MaxPorts != 0 { deviceParams = append(deviceParams, fmt.Sprintf(",max_ports=%d", dev.MaxPorts)) @@ -1075,7 +1096,7 @@ func (blkdev BlockDevice) QemuParams(config *Config) []string { deviceParams = append(deviceParams, ",config-wce=off") } - if blkdev.Transport.isVirtioPCI(config) { + if blkdev.Transport.isVirtioPCI(config) && blkdev.ROMFile != "" { deviceParams = append(deviceParams, fmt.Sprintf(",romfile=%s", blkdev.ROMFile)) } @@ -1190,6 +1211,9 @@ type VhostUserDevice struct { // ROMFile specifies the ROM file being used for this device. ROMFile string + // DevNo identifies the CCW device for s390x. + DevNo string + // Transport is the virtio transport for this device. Transport VirtioTransport } @@ -1254,88 +1278,150 @@ func (vhostuserDev VhostUserDevice) Valid() bool { return true } +// QemuNetParams builds QEMU netdev and device parameters for a VhostUserNet device +func (vhostuserDev VhostUserDevice) QemuNetParams(config *Config) []string { + var qemuParams []string + var netParams []string + var devParams []string + + driver := vhostuserDev.deviceName(config) + if driver == "" { + return nil + } + + netParams = append(netParams, "type=vhost-user") + netParams = append(netParams, fmt.Sprintf("id=%s", vhostuserDev.TypeDevID)) + netParams = append(netParams, fmt.Sprintf("chardev=%s", vhostuserDev.CharDevID)) + netParams = append(netParams, "vhostforce") + + devParams = append(devParams, driver) + devParams = append(devParams, fmt.Sprintf("netdev=%s", vhostuserDev.TypeDevID)) + devParams = append(devParams, fmt.Sprintf("mac=%s", vhostuserDev.Address)) + + if vhostuserDev.Transport.isVirtioPCI(config) && vhostuserDev.ROMFile != "" { + devParams = append(devParams, fmt.Sprintf("romfile=%s", vhostuserDev.ROMFile)) + } + + qemuParams = append(qemuParams, "-netdev") + qemuParams = append(qemuParams, strings.Join(netParams, ",")) + qemuParams = append(qemuParams, "-device") + qemuParams = append(qemuParams, strings.Join(devParams, ",")) + + return qemuParams +} + +// QemuSCSIParams builds QEMU device parameters for a VhostUserSCSI device +func (vhostuserDev VhostUserDevice) QemuSCSIParams(config *Config) []string { + var qemuParams []string + var devParams []string + + driver := vhostuserDev.deviceName(config) + if driver == "" { + return nil + } + + devParams = append(devParams, driver) + devParams = append(devParams, fmt.Sprintf("id=%s", vhostuserDev.TypeDevID)) + devParams = append(devParams, fmt.Sprintf("chardev=%s", vhostuserDev.CharDevID)) + + if vhostuserDev.Transport.isVirtioPCI(config) && vhostuserDev.ROMFile != "" { + devParams = append(devParams, fmt.Sprintf("romfile=%s", vhostuserDev.ROMFile)) + } + + qemuParams = append(qemuParams, "-device") + qemuParams = append(qemuParams, strings.Join(devParams, ",")) + + return qemuParams +} + +// QemuBlkParams builds QEMU device parameters for a VhostUserBlk device +func (vhostuserDev VhostUserDevice) QemuBlkParams(config *Config) []string { + var qemuParams []string + var devParams []string + + driver := vhostuserDev.deviceName(config) + if driver == "" { + return nil + } + + devParams = append(devParams, driver) + devParams = append(devParams, "logical_block_size=4096") + devParams = append(devParams, "size=512M") + devParams = append(devParams, fmt.Sprintf("chardev=%s", vhostuserDev.CharDevID)) + + if vhostuserDev.Transport.isVirtioPCI(config) && vhostuserDev.ROMFile != "" { + devParams = append(devParams, fmt.Sprintf("romfile=%s", vhostuserDev.ROMFile)) + } + + qemuParams = append(qemuParams, "-device") + qemuParams = append(qemuParams, strings.Join(devParams, ",")) + + return qemuParams +} + +// QemuFSParams builds QEMU device parameters for a VhostUserFS device +func (vhostuserDev VhostUserDevice) QemuFSParams(config *Config) []string { + var qemuParams []string + var devParams []string + + driver := vhostuserDev.deviceName(config) + if driver == "" { + return nil + } + + devParams = append(devParams, driver) + devParams = append(devParams, fmt.Sprintf("chardev=%s", vhostuserDev.CharDevID)) + devParams = append(devParams, fmt.Sprintf("tag=%s", vhostuserDev.Tag)) + if vhostuserDev.CacheSize != 0 { + devParams = append(devParams, fmt.Sprintf("cache-size=%dM", vhostuserDev.CacheSize)) + } + if vhostuserDev.SharedVersions { + devParams = append(devParams, "versiontable=/dev/shm/fuse_shared_versions") + } + if vhostuserDev.Transport.isVirtioCCW(config) { + devParams = append(devParams, fmt.Sprintf("devno=%s", vhostuserDev.DevNo)) + } + if vhostuserDev.Transport.isVirtioPCI(config) && vhostuserDev.ROMFile != "" { + devParams = append(devParams, fmt.Sprintf("romfile=%s", vhostuserDev.ROMFile)) + } + + qemuParams = append(qemuParams, "-device") + qemuParams = append(qemuParams, strings.Join(devParams, ",")) + + return qemuParams +} + // QemuParams returns the qemu parameters built out of this vhostuser device. func (vhostuserDev VhostUserDevice) QemuParams(config *Config) []string { var qemuParams []string var charParams []string - var netParams []string var devParams []string - var driver string charParams = append(charParams, "socket") charParams = append(charParams, fmt.Sprintf("id=%s", vhostuserDev.CharDevID)) charParams = append(charParams, fmt.Sprintf("path=%s", vhostuserDev.SocketPath)) + qemuParams = append(qemuParams, "-chardev") + qemuParams = append(qemuParams, strings.Join(charParams, ",")) + switch vhostuserDev.VhostUserType { - // if network based vhost device: case VhostUserNet: - driver = vhostuserDev.deviceName(config) - if driver == "" { - return nil - } - - netParams = append(netParams, "type=vhost-user") - netParams = append(netParams, fmt.Sprintf("id=%s", vhostuserDev.TypeDevID)) - netParams = append(netParams, fmt.Sprintf("chardev=%s", vhostuserDev.CharDevID)) - netParams = append(netParams, "vhostforce") - - devParams = append(devParams, driver) - devParams = append(devParams, fmt.Sprintf("netdev=%s", vhostuserDev.TypeDevID)) - devParams = append(devParams, fmt.Sprintf("mac=%s", vhostuserDev.Address)) + devParams = vhostuserDev.QemuNetParams(config) case VhostUserSCSI: - driver = vhostuserDev.deviceName(config) - if driver == "" { - return nil - } - - devParams = append(devParams, driver) - devParams = append(devParams, fmt.Sprintf("id=%s", vhostuserDev.TypeDevID)) - devParams = append(devParams, fmt.Sprintf("chardev=%s", vhostuserDev.CharDevID)) + devParams = vhostuserDev.QemuSCSIParams(config) case VhostUserBlk: - driver = vhostuserDev.deviceName(config) - if driver == "" { - return nil - } - - devParams = append(devParams, driver) - devParams = append(devParams, "logical_block_size=4096") - devParams = append(devParams, "size=512M") - devParams = append(devParams, fmt.Sprintf("chardev=%s", vhostuserDev.CharDevID)) + devParams = vhostuserDev.QemuBlkParams(config) case VhostUserFS: - driver = vhostuserDev.deviceName(config) - if driver == "" { - return nil - } - - devParams = append(devParams, driver) - devParams = append(devParams, fmt.Sprintf("chardev=%s", vhostuserDev.CharDevID)) - devParams = append(devParams, fmt.Sprintf("tag=%s", vhostuserDev.Tag)) - if vhostuserDev.CacheSize != 0 { - devParams = append(devParams, fmt.Sprintf("cache-size=%dM", vhostuserDev.CacheSize)) - } - if vhostuserDev.SharedVersions { - devParams = append(devParams, "versiontable=/dev/shm/fuse_shared_versions") - } + devParams = vhostuserDev.QemuFSParams(config) default: return nil } - if vhostuserDev.Transport.isVirtioPCI(config) { - devParams = append(devParams, fmt.Sprintf("romfile=%s", vhostuserDev.ROMFile)) + if devParams != nil { + return append(qemuParams, devParams...) } - qemuParams = append(qemuParams, "-chardev") - qemuParams = append(qemuParams, strings.Join(charParams, ",")) - - // if network based vhost device: - if vhostuserDev.VhostUserType == VhostUserNet { - qemuParams = append(qemuParams, "-netdev") - qemuParams = append(qemuParams, strings.Join(netParams, ",")) - } - qemuParams = append(qemuParams, "-device") - qemuParams = append(qemuParams, strings.Join(devParams, ",")) - - return qemuParams + return nil } // deviceName returns the QEMU device name for the current combination of @@ -1510,7 +1596,9 @@ func (vfioDev VFIODevice) QemuParams(config *Config) []string { if vfioDev.DeviceID != "" { deviceParams = append(deviceParams, fmt.Sprintf(",x-pci-device-id=%s", vfioDev.DeviceID)) } - deviceParams = append(deviceParams, fmt.Sprintf(",romfile=%s", vfioDev.ROMFile)) + if vfioDev.ROMFile != "" { + deviceParams = append(deviceParams, fmt.Sprintf(",romfile=%s", vfioDev.ROMFile)) + } } if vfioDev.Bus != "" { @@ -1595,7 +1683,7 @@ func (scsiCon SCSIController) QemuParams(config *Config) []string { if scsiCon.IOThread != "" { devParams = append(devParams, fmt.Sprintf("iothread=%s", scsiCon.IOThread)) } - if scsiCon.Transport.isVirtioPCI(config) { + if scsiCon.Transport.isVirtioPCI(config) && scsiCon.ROMFile != "" { devParams = append(devParams, fmt.Sprintf("romfile=%s", scsiCon.ROMFile)) } @@ -1701,7 +1789,7 @@ func (bridgeDev BridgeDevice) QemuParams(config *Config) []string { } var transport VirtioTransport - if transport.isVirtioPCI(config) { + if transport.isVirtioPCI(config) && bridgeDev.ROMFile != "" { deviceParam = append(deviceParam, fmt.Sprintf(",romfile=%s", bridgeDev.ROMFile)) } @@ -1780,7 +1868,7 @@ func (vsock VSOCKDevice) QemuParams(config *Config) []string { deviceParams = append(deviceParams, fmt.Sprintf(",id=%s", vsock.ID)) deviceParams = append(deviceParams, fmt.Sprintf(",%s=%d", VSOCKGuestCID, vsock.ContextID)) - if vsock.Transport.isVirtioPCI(config) { + if vsock.Transport.isVirtioPCI(config) && vsock.ROMFile != "" { deviceParams = append(deviceParams, fmt.Sprintf(",romfile=%s", vsock.ROMFile)) } @@ -1853,7 +1941,7 @@ func (v RngDevice) QemuParams(config *Config) []string { deviceParams = append(deviceParams, v.deviceName(config)) deviceParams = append(deviceParams, "rng="+v.ID) - if v.Transport.isVirtioPCI(config) { + if v.Transport.isVirtioPCI(config) && v.ROMFile != "" { deviceParams = append(deviceParams, fmt.Sprintf("romfile=%s", v.ROMFile)) } @@ -1930,7 +2018,7 @@ func (b BalloonDevice) QemuParams(config *Config) []string { deviceParams = append(deviceParams, "id="+b.ID) } - if b.Transport.isVirtioPCI(config) { + if b.Transport.isVirtioPCI(config) && b.ROMFile != "" { deviceParams = append(deviceParams, fmt.Sprintf("romfile=%s", b.ROMFile)) } diff --git a/src/runtime/vendor/github.com/kata-containers/govmm/qemu/qmp.go b/src/runtime/vendor/github.com/kata-containers/govmm/qemu/qmp.go index 73d2ec4cf..97e924559 100644 --- a/src/runtime/vendor/github.com/kata-containers/govmm/qemu/qmp.go +++ b/src/runtime/vendor/github.com/kata-containers/govmm/qemu/qmp.go @@ -267,10 +267,6 @@ func (q *QMP) readLoop(fromVMCh chan<- []byte) { for scanner.Scan() { line := scanner.Bytes() - if q.cfg.Logger.V(1) { - q.cfg.Logger.Infof("read from QMP: %s", string(line)) - } - // Since []byte channel type transfer slice info(include slice underlying array pointer, len, cap) // between channel sender and receiver. scanner.Bytes() returned slice's underlying array // may point to data that will be overwritten by a subsequent call to Scan(reference from: @@ -442,7 +438,6 @@ func (q *QMP) writeNextQMPCommand(cmdQueue *list.List) { } cmdQueue.Remove(cmdEl) } - q.cfg.Logger.Infof("%s", string(encodedCmd)) encodedCmd = append(encodedCmd, '\n') if unixConn, ok := q.conn.(*net.UnixConn); ok && len(cmd.oob) > 0 { _, _, err = unixConn.WriteMsgUnix(encodedCmd, cmd.oob, nil) diff --git a/src/runtime/vendor/modules.txt b/src/runtime/vendor/modules.txt index aceaac2a9..fb825ff84 100644 --- a/src/runtime/vendor/modules.txt +++ b/src/runtime/vendor/modules.txt @@ -222,7 +222,7 @@ github.com/hashicorp/errwrap # github.com/hashicorp/go-multierror v1.0.0 ## explicit github.com/hashicorp/go-multierror -# github.com/kata-containers/govmm v0.0.0-20210329205824-7fbc685865d2 +# github.com/kata-containers/govmm v0.0.0-20210428163604-f0e9a35308ee ## explicit github.com/kata-containers/govmm/qemu # github.com/konsorten/go-windows-terminal-sequences v1.0.1 From 3ee61776d60d843c699d473cf1b6180fc29d0408 Mon Sep 17 00:00:00 2001 From: Jakob Naucke Date: Mon, 26 Apr 2021 18:06:54 +0200 Subject: [PATCH 3/3] virtcontainers: Enable virtio-fs on s390x Allow and configure vhost-user-fs devices (virtio-fs) on s390x. As a consequence, appendVhostUserDevice now takes a context, which affects its signature for other architectures. Fixes: #1753 Signed-off-by: Jakob Naucke --- src/runtime/virtcontainers/qemu.go | 4 +- src/runtime/virtcontainers/qemu_arch_base.go | 4 +- .../virtcontainers/qemu_arch_base_test.go | 2 +- src/runtime/virtcontainers/qemu_s390x.go | 32 ++++++++++-- src/runtime/virtcontainers/qemu_s390x_test.go | 49 ++++++++++++++++--- 5 files changed, 75 insertions(+), 16 deletions(-) diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index 5e2bb8593..9f00a12a4 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -1924,7 +1924,7 @@ func (q *qemu) addDevice(ctx context.Context, devInfo interface{}, devType devic vhostDev.SocketPath = sockPath vhostDev.DevID = id - q.qemuConfig.Devices, err = q.arch.appendVhostUserDevice(q.qemuConfig.Devices, vhostDev) + q.qemuConfig.Devices, err = q.arch.appendVhostUserDevice(ctx, q.qemuConfig.Devices, vhostDev) } else { q.Logger().WithField("volume-type", "virtio-9p").Info("adding volume") q.qemuConfig.Devices, err = q.arch.append9PVolume(ctx, q.qemuConfig.Devices, v) @@ -1939,7 +1939,7 @@ func (q *qemu) addDevice(ctx context.Context, devInfo interface{}, devType devic case config.BlockDrive: q.qemuConfig.Devices, err = q.arch.appendBlockDevice(ctx, q.qemuConfig.Devices, v) case config.VhostUserDeviceAttrs: - q.qemuConfig.Devices, err = q.arch.appendVhostUserDevice(q.qemuConfig.Devices, v) + q.qemuConfig.Devices, err = q.arch.appendVhostUserDevice(ctx, q.qemuConfig.Devices, v) case config.VFIODev: q.qemuConfig.Devices = q.arch.appendVFIODevice(q.qemuConfig.Devices, v) default: diff --git a/src/runtime/virtcontainers/qemu_arch_base.go b/src/runtime/virtcontainers/qemu_arch_base.go index ae0ce3a13..d2ffac4a1 100644 --- a/src/runtime/virtcontainers/qemu_arch_base.go +++ b/src/runtime/virtcontainers/qemu_arch_base.go @@ -96,7 +96,7 @@ type qemuArch interface { appendBlockDevice(ctx context.Context, devices []govmmQemu.Device, drive config.BlockDrive) ([]govmmQemu.Device, error) // appendVhostUserDevice appends a vhost user device to devices - appendVhostUserDevice(devices []govmmQemu.Device, drive config.VhostUserDeviceAttrs) ([]govmmQemu.Device, error) + appendVhostUserDevice(ctx context.Context, devices []govmmQemu.Device, drive config.VhostUserDeviceAttrs) ([]govmmQemu.Device, error) // appendVFIODevice appends a VFIO device to devices appendVFIODevice(devices []govmmQemu.Device, vfioDevice config.VFIODev) []govmmQemu.Device @@ -633,7 +633,7 @@ func (q *qemuArchBase) appendBlockDevice(_ context.Context, devices []govmmQemu. return devices, nil } -func (q *qemuArchBase) appendVhostUserDevice(devices []govmmQemu.Device, attr config.VhostUserDeviceAttrs) ([]govmmQemu.Device, error) { +func (q *qemuArchBase) appendVhostUserDevice(ctx context.Context, devices []govmmQemu.Device, attr config.VhostUserDeviceAttrs) ([]govmmQemu.Device, error) { qemuVhostUserDevice := govmmQemu.VhostUserDevice{} switch attr.Type { diff --git a/src/runtime/virtcontainers/qemu_arch_base_test.go b/src/runtime/virtcontainers/qemu_arch_base_test.go index d4aba54fa..3db561c9f 100644 --- a/src/runtime/virtcontainers/qemu_arch_base_test.go +++ b/src/runtime/virtcontainers/qemu_arch_base_test.go @@ -224,7 +224,7 @@ func testQemuArchBaseAppend(t *testing.T, structure interface{}, expected []govm case config.VFIODev: devices = qemuArchBase.appendVFIODevice(devices, s) case config.VhostUserDeviceAttrs: - devices, err = qemuArchBase.appendVhostUserDevice(devices, s) + devices, err = qemuArchBase.appendVhostUserDevice(context.Background(), devices, s) } assert.NoError(err) diff --git a/src/runtime/virtcontainers/qemu_s390x.go b/src/runtime/virtcontainers/qemu_s390x.go index 01968b6b1..effced44b 100644 --- a/src/runtime/virtcontainers/qemu_s390x.go +++ b/src/runtime/virtcontainers/qemu_s390x.go @@ -13,6 +13,7 @@ import ( govmmQemu "github.com/kata-containers/govmm/qemu" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/config" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types" + "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils" ) type qemuS390x struct { @@ -152,10 +153,33 @@ func (q *qemuS390x) appendCCWBlockDevice(ctx context.Context, devices []govmmQem return devices, nil } -// appendVhostUserDevice throws an error if vhost devices are tried to be used. -// See issue https://github.com/kata-containers/runtime/issues/659 -func (q *qemuS390x) appendVhostUserDevice(devices []govmmQemu.Device, attr config.VhostUserDeviceAttrs) ([]govmmQemu.Device, error) { - return nil, fmt.Errorf("No vhost-user devices supported on s390x") +func (q *qemuS390x) appendVhostUserDevice(ctx context.Context, devices []govmmQemu.Device, attr config.VhostUserDeviceAttrs) ([]govmmQemu.Device, error) { + if attr.Type != config.VhostUserFS { + return devices, fmt.Errorf("vhost-user device of type %s not supported on s390x, only vhost-user-fs-ccw is supported", attr.Type) + } + + addr, b, err := q.addDeviceToBridge(ctx, attr.DevID, types.CCW) + if err != nil { + return devices, fmt.Errorf("Failed to append vhost user device: %s", err) + } + var devno string + devno, err = b.AddressFormatCCW(addr) + if err != nil { + return devices, fmt.Errorf("Failed to append vhost user device: %s", err) + } + + qemuVhostUserDevice := govmmQemu.VhostUserDevice{ + SocketPath: attr.SocketPath, + CharDevID: utils.MakeNameID("char", attr.DevID, maxDevIDSize), + TypeDevID: utils.MakeNameID("fs", attr.DevID, maxDevIDSize), + Tag: attr.Tag, + CacheSize: attr.CacheSize, + VhostUserType: govmmQemu.VhostUserFS, + DevNo: devno, + } + + devices = append(devices, qemuVhostUserDevice) + return devices, nil } // supportGuestMemoryHotplug return false for s390x architecture. The pc-dimm backend device for s390x diff --git a/src/runtime/virtcontainers/qemu_s390x_test.go b/src/runtime/virtcontainers/qemu_s390x_test.go index 70ddf3ff2..7b8067bee 100644 --- a/src/runtime/virtcontainers/qemu_s390x_test.go +++ b/src/runtime/virtcontainers/qemu_s390x_test.go @@ -6,11 +6,14 @@ package virtcontainers import ( + "context" "fmt" "testing" govmmQemu "github.com/kata-containers/govmm/qemu" "github.com/stretchr/testify/assert" + + "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/config" ) func newTestQemu(assert *assert.Assertions, machineType string) qemuArch { @@ -54,15 +57,47 @@ func TestQemuS390xMemoryTopology(t *testing.T) { } func TestQemuS390xAppendVhostUserDevice(t *testing.T) { - macAddress := "00:11:22:33:44:55:66" - qemu := qemuS390x{} assert := assert.New(t) + qemu := newTestQemu(assert, QemuCCWVirtio) - vhostUserDevice := config.VhostUserDeviceAttrs{ - Type: config.VhostUserNet, - MacAddress: macAddress, + // test devices that should not work + for _, deviceType := range []config.DeviceType{config.VhostUserSCSI, config.VhostUserNet, config.VhostUserBlk} { + vhostUserDevice := config.VhostUserDeviceAttrs{ + Type: deviceType, + } + _, err := qemu.appendVhostUserDevice(context.Background(), nil, vhostUserDevice) + assert.Error(err) } - _, err := qemu.appendVhostUserDevice(nil, vhostUserDevice) - assert.Error(err) + // test vhost user fs (virtio-fs) + socketPath := "nonexistentpath.sock" + id := "deadbeef" + tag := "shared" + var cacheSize uint32 = 0 + + expected := []govmmQemu.Device{ + govmmQemu.VhostUserDevice{ + SocketPath: socketPath, + CharDevID: fmt.Sprintf("char-%s", id), + TypeDevID: fmt.Sprintf("fs-%s", id), + Tag: tag, + CacheSize: cacheSize, + VhostUserType: govmmQemu.VhostUserFS, + DevNo: "fe.0.0001", + }, + } + + vhostUserDevice := config.VhostUserDeviceAttrs{ + DevID: id, + SocketPath: socketPath, + Type: config.VhostUserFS, + Tag: tag, + CacheSize: cacheSize, + } + + var devices []govmmQemu.Device + devices, err := qemu.appendVhostUserDevice(context.Background(), devices, vhostUserDevice) + + assert.NoError(err) + assert.Equal(devices, expected) }