From 67e696bf624582342ee796d604aad4d1390b46a2 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Fri, 11 Jan 2019 16:46:48 +0100 Subject: [PATCH 1/3] virtcontainers: Add Asset to the types package In order to move the hypervisor implementations into their own package, we need to put the asset type into the types package and break the hypervisor->asset->virtcontainers->hypervisor cyclic dependency. Fixes: #1119 Signed-off-by: Samuel Ortiz --- virtcontainers/hypervisor.go | 53 +++++++------- virtcontainers/sandbox.go | 10 +-- virtcontainers/sandbox_test.go | 8 ++- virtcontainers/{ => types}/asset.go | 88 +++++++++++++++--------- virtcontainers/{ => types}/asset_test.go | 33 ++++----- 5 files changed, 106 insertions(+), 86 deletions(-) rename virtcontainers/{ => types}/asset.go (58%) rename virtcontainers/{ => types}/asset_test.go (77%) diff --git a/virtcontainers/hypervisor.go b/virtcontainers/hypervisor.go index 6d5335917c..4df16803a4 100644 --- a/virtcontainers/hypervisor.go +++ b/virtcontainers/hypervisor.go @@ -15,6 +15,7 @@ import ( "strings" "github.com/kata-containers/runtime/virtcontainers/device/config" + "github.com/kata-containers/runtime/virtcontainers/types" ) // HypervisorType describes an hypervisor type. @@ -221,7 +222,7 @@ type HypervisorConfig struct { // Each value in that map takes precedence over the configured assets. // For example, if there is a value for the "kernel" key in this map, // it will be used for the sandbox's kernel path instead of KernelPath. - customAssets map[assetType]*asset + customAssets map[types.AssetType]*types.Asset // BlockDeviceCacheSet specifies cache-related options will be set to block devices or not. BlockDeviceCacheSet bool @@ -357,53 +358,53 @@ func (conf *HypervisorConfig) AddKernelParam(p Param) error { return nil } -func (conf *HypervisorConfig) addCustomAsset(a *asset) error { - if a == nil || a.path == "" { +func (conf *HypervisorConfig) addCustomAsset(a *types.Asset) error { + if a == nil || a.Path() == "" { // We did not get a custom asset, we will use the default one. return nil } - if !a.valid() { - return fmt.Errorf("Invalid %s at %s", a.kind, a.path) + if !a.Valid() { + return fmt.Errorf("Invalid %s at %s", a.Type(), a.Path()) } - virtLog.Debugf("Using custom %v asset %s", a.kind, a.path) + virtLog.Debugf("Using custom %v asset %s", a.Type(), a.Path()) if conf.customAssets == nil { - conf.customAssets = make(map[assetType]*asset) + conf.customAssets = make(map[types.AssetType]*types.Asset) } - conf.customAssets[a.kind] = a + conf.customAssets[a.Type()] = a return nil } -func (conf *HypervisorConfig) assetPath(t assetType) (string, error) { +func (conf *HypervisorConfig) assetPath(t types.AssetType) (string, error) { // Custom assets take precedence over the configured ones a, ok := conf.customAssets[t] if ok { - return a.path, nil + return a.Path(), nil } // We could not find a custom asset for the given type, let's // fall back to the configured ones. switch t { - case kernelAsset: + case types.KernelAsset: return conf.KernelPath, nil - case imageAsset: + case types.ImageAsset: return conf.ImagePath, nil - case initrdAsset: + case types.InitrdAsset: return conf.InitrdPath, nil - case hypervisorAsset: + case types.HypervisorAsset: return conf.HypervisorPath, nil - case firmwareAsset: + case types.FirmwareAsset: return conf.FirmwarePath, nil default: return "", fmt.Errorf("Unknown asset type %v", t) } } -func (conf *HypervisorConfig) isCustomAsset(t assetType) bool { +func (conf *HypervisorConfig) isCustomAsset(t types.AssetType) bool { _, ok := conf.customAssets[t] if ok { return true @@ -414,52 +415,52 @@ func (conf *HypervisorConfig) isCustomAsset(t assetType) bool { // KernelAssetPath returns the guest kernel path func (conf *HypervisorConfig) KernelAssetPath() (string, error) { - return conf.assetPath(kernelAsset) + return conf.assetPath(types.KernelAsset) } // CustomKernelAsset returns true if the kernel asset is a custom one, false otherwise. func (conf *HypervisorConfig) CustomKernelAsset() bool { - return conf.isCustomAsset(kernelAsset) + return conf.isCustomAsset(types.KernelAsset) } // ImageAssetPath returns the guest image path func (conf *HypervisorConfig) ImageAssetPath() (string, error) { - return conf.assetPath(imageAsset) + return conf.assetPath(types.ImageAsset) } // CustomImageAsset returns true if the image asset is a custom one, false otherwise. func (conf *HypervisorConfig) CustomImageAsset() bool { - return conf.isCustomAsset(imageAsset) + return conf.isCustomAsset(types.ImageAsset) } // InitrdAssetPath returns the guest initrd path func (conf *HypervisorConfig) InitrdAssetPath() (string, error) { - return conf.assetPath(initrdAsset) + return conf.assetPath(types.InitrdAsset) } // CustomInitrdAsset returns true if the initrd asset is a custom one, false otherwise. func (conf *HypervisorConfig) CustomInitrdAsset() bool { - return conf.isCustomAsset(initrdAsset) + return conf.isCustomAsset(types.InitrdAsset) } // HypervisorAssetPath returns the VM hypervisor path func (conf *HypervisorConfig) HypervisorAssetPath() (string, error) { - return conf.assetPath(hypervisorAsset) + return conf.assetPath(types.HypervisorAsset) } // CustomHypervisorAsset returns true if the hypervisor asset is a custom one, false otherwise. func (conf *HypervisorConfig) CustomHypervisorAsset() bool { - return conf.isCustomAsset(hypervisorAsset) + return conf.isCustomAsset(types.HypervisorAsset) } // FirmwareAssetPath returns the guest firmware path func (conf *HypervisorConfig) FirmwareAssetPath() (string, error) { - return conf.assetPath(firmwareAsset) + return conf.assetPath(types.FirmwareAsset) } // CustomFirmwareAsset returns true if the firmware asset is a custom one, false otherwise. func (conf *HypervisorConfig) CustomFirmwareAsset() bool { - return conf.isCustomAsset(firmwareAsset) + return conf.isCustomAsset(types.FirmwareAsset) } func appendParam(params []Param, parameter string, value string) []Param { diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 51a3a7f745..a3794ab90f 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -435,26 +435,26 @@ func createAssets(ctx context.Context, sandboxConfig *SandboxConfig) error { span, _ := trace(ctx, "createAssets") defer span.Finish() - kernel, err := newAsset(sandboxConfig, kernelAsset) + kernel, err := types.NewAsset(sandboxConfig.Annotations, types.KernelAsset) if err != nil { return err } - image, err := newAsset(sandboxConfig, imageAsset) + image, err := types.NewAsset(sandboxConfig.Annotations, types.ImageAsset) if err != nil { return err } - initrd, err := newAsset(sandboxConfig, initrdAsset) + initrd, err := types.NewAsset(sandboxConfig.Annotations, types.InitrdAsset) if err != nil { return err } if image != nil && initrd != nil { - return fmt.Errorf("%s and %s cannot be both set", imageAsset, initrdAsset) + return fmt.Errorf("%s and %s cannot be both set", types.ImageAsset, types.InitrdAsset) } - for _, a := range []*asset{kernel, image, initrd} { + for _, a := range []*types.Asset{kernel, image, initrd} { if err := sandboxConfig.HypervisorConfig.addCustomAsset(a); err != nil { return err } diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index 125e354220..84ee50ebb0 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -1201,6 +1201,10 @@ func TestSandboxAttachDevicesVFIO(t *testing.T) { assert.Nil(t, err, "Error while detaching devices %s", err) } +var assetContent = []byte("FakeAsset fake asset FAKE ASSET") +var assetContentHash = "92549f8d2018a95a294d28a65e795ed7d1a9d150009a28cea108ae10101178676f04ab82a6950d0099e4924f9c5e41dcba8ece56b75fc8b4e0a7492cb2a8c880" +var assetContentWrongHash = "92549f8d2018a95a294d28a65e795ed7d1a9d150009a28cea108ae10101178676f04ab82a6950d0099e4924f9c5e41dcba8ece56b75fc8b4e0a7492cb2a8c881" + func TestSandboxCreateAssets(t *testing.T) { assert := assert.New(t) @@ -1234,9 +1238,9 @@ func TestSandboxCreateAssets(t *testing.T) { err = createAssets(context.Background(), p) assert.Nil(err) - a, ok := p.HypervisorConfig.customAssets[kernelAsset] + a, ok := p.HypervisorConfig.customAssets[types.KernelAsset] assert.True(ok) - assert.Equal(a.path, tmpfile.Name()) + assert.Equal(a.Path(), tmpfile.Name()) p = &SandboxConfig{ Annotations: map[string]string{ diff --git a/virtcontainers/asset.go b/virtcontainers/types/asset.go similarity index 58% rename from virtcontainers/asset.go rename to virtcontainers/types/asset.go index 8294fc5f23..dd9bab3b7c 100644 --- a/virtcontainers/asset.go +++ b/virtcontainers/types/asset.go @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // -package virtcontainers +package types import ( "crypto/sha512" @@ -15,19 +15,21 @@ import ( "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" ) -type assetType string +// AssetType describe a type of assets. +type AssetType string -func (t assetType) annotations() (string, string, error) { +// Annotations returns the path and hash annotations for a given Asset type. +func (t AssetType) Annotations() (string, string, error) { switch t { - case kernelAsset: + case KernelAsset: return annotations.KernelPath, annotations.KernelHash, nil - case imageAsset: + case ImageAsset: return annotations.ImagePath, annotations.ImageHash, nil - case initrdAsset: + case InitrdAsset: return annotations.InitrdPath, annotations.InitrdHash, nil - case hypervisorAsset: + case HypervisorAsset: return annotations.HypervisorPath, annotations.HypervisorHash, nil - case firmwareAsset: + case FirmwareAsset: return annotations.FirmwarePath, annotations.FirmwareHash, nil } @@ -35,42 +37,63 @@ func (t assetType) annotations() (string, string, error) { } const ( - kernelAsset assetType = "kernel" - imageAsset assetType = "image" - initrdAsset assetType = "initrd" - hypervisorAsset assetType = "hypervisor" - firmwareAsset assetType = "firmware" + // KernelAsset is a kernel asset. + KernelAsset AssetType = "kernel" + + // ImageAsset is an image asset. + ImageAsset AssetType = "image" + + // InitrdAsset is an intird asset. + InitrdAsset AssetType = "initrd" + + // HypervisorAsset is an hypervisor asset. + HypervisorAsset AssetType = "hypervisor" + + // FirmwareAsset is a firmware asset. + FirmwareAsset AssetType = "firmware" ) -type asset struct { +// Asset represents a virtcontainers asset. +type Asset struct { path string computedHash string - kind assetType + kind AssetType } -func (a *asset) valid() bool { +// Path returns an asset path. +func (a Asset) Path() string { + return a.path +} + +// Type returns an asset type. +func (a Asset) Type() AssetType { + return a.kind +} + +// Valid checks if an asset is valid or not. +func (a *Asset) Valid() bool { if !filepath.IsAbs(a.path) { return false } switch a.kind { - case kernelAsset: + case KernelAsset: return true - case imageAsset: + case ImageAsset: return true - case initrdAsset: + case InitrdAsset: return true - case hypervisorAsset: + case HypervisorAsset: return true - case firmwareAsset: + case FirmwareAsset: return true } return false } -// hash returns the hex encoded string for the asset hash -func (a *asset) hash(hashType string) (string, error) { +// Hash returns the hex encoded string for the asset hash +func (a *Asset) Hash(hashType string) (string, error) { var hashEncodedLen int var hash string @@ -88,13 +111,11 @@ func (a *asset) hash(hashType string) (string, error) { // We only support SHA512 for now. switch hashType { case annotations.SHA512: - virtLog.Debugf("Computing %v hash", a.path) hashComputed := sha512.Sum512(bytes) hashEncodedLen = hex.EncodedLen(len(hashComputed)) hashEncoded := make([]byte, hashEncodedLen) hex.Encode(hashEncoded, hashComputed[:]) hash = string(hashEncoded[:]) - virtLog.Debugf("%v hash: %s", a.path, hash) default: return "", fmt.Errorf("Invalid hash type %s", hashType) } @@ -104,9 +125,9 @@ func (a *asset) hash(hashType string) (string, error) { return hash, nil } -// newAsset returns a new asset from the sandbox annotations. -func newAsset(sandboxConfig *SandboxConfig, t assetType) (*asset, error) { - pathAnnotation, hashAnnotation, err := t.annotations() +// NewAsset returns a new asset from a slice of annotations. +func NewAsset(anno map[string]string, t AssetType) (*Asset, error) { + pathAnnotation, hashAnnotation, err := t.Annotations() if err != nil { return nil, err } @@ -115,7 +136,7 @@ func newAsset(sandboxConfig *SandboxConfig, t assetType) (*asset, error) { return nil, fmt.Errorf("Missing annotation paths for %s", t) } - path, ok := sandboxConfig.Annotations[pathAnnotation] + path, ok := anno[pathAnnotation] if !ok || path == "" { return nil, nil } @@ -124,21 +145,20 @@ func newAsset(sandboxConfig *SandboxConfig, t assetType) (*asset, error) { return nil, fmt.Errorf("%s is not an absolute path", path) } - a := &asset{path: path, kind: t} + a := &Asset{path: path, kind: t} - hash, ok := sandboxConfig.Annotations[hashAnnotation] + hash, ok := anno[hashAnnotation] if !ok || hash == "" { return a, nil } // We have a hash annotation, we need to verify the asset against it. - hashType, ok := sandboxConfig.Annotations[annotations.AssetHashType] + hashType, ok := anno[annotations.AssetHashType] if !ok { - virtLog.Warningf("Unrecognized hash type: %s, switching to %s", hashType, annotations.SHA512) hashType = annotations.SHA512 } - hashComputed, err := a.hash(hashType) + hashComputed, err := a.Hash(hashType) if err != nil { return a, err } diff --git a/virtcontainers/asset_test.go b/virtcontainers/types/asset_test.go similarity index 77% rename from virtcontainers/asset_test.go rename to virtcontainers/types/asset_test.go index dd414fec2d..b025fe0e03 100644 --- a/virtcontainers/asset_test.go +++ b/virtcontainers/types/asset_test.go @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // -package virtcontainers +package types import ( "io/ioutil" @@ -32,11 +32,11 @@ func TestAssetWrongHashType(t *testing.T) { _, err = tmpfile.Write(assetContent) assert.Nil(err) - a := &asset{ + a := &Asset{ path: tmpfile.Name(), } - h, err := a.hash("shafoo") + h, err := a.Hash("shafoo") assert.Equal(h, "") assert.NotNil(err) } @@ -55,11 +55,11 @@ func TestAssetHash(t *testing.T) { _, err = tmpfile.Write(assetContent) assert.Nil(err) - a := &asset{ + a := &Asset{ path: tmpfile.Name(), } - hash, err := a.hash(annotations.SHA512) + hash, err := a.Hash(annotations.SHA512) assert.Nil(err) assert.Equal(assetContentHash, hash) assert.Equal(assetContentHash, a.computedHash) @@ -79,28 +79,23 @@ func TestAssetNew(t *testing.T) { _, err = tmpfile.Write(assetContent) assert.Nil(err) - p := &SandboxConfig{ - Annotations: map[string]string{ - annotations.KernelPath: tmpfile.Name(), - annotations.KernelHash: assetContentHash, - }, + anno := map[string]string{ + annotations.KernelPath: tmpfile.Name(), + annotations.KernelHash: assetContentHash, } - - a, err := newAsset(p, imageAsset) + a, err := NewAsset(anno, ImageAsset) assert.Nil(err) assert.Nil(a) - a, err = newAsset(p, kernelAsset) + a, err = NewAsset(anno, KernelAsset) assert.Nil(err) assert.Equal(assetContentHash, a.computedHash) - p = &SandboxConfig{ - Annotations: map[string]string{ - annotations.KernelPath: tmpfile.Name(), - annotations.KernelHash: assetContentWrongHash, - }, + anno = map[string]string{ + annotations.KernelPath: tmpfile.Name(), + annotations.KernelHash: assetContentWrongHash, } - _, err = newAsset(p, kernelAsset) + _, err = NewAsset(anno, KernelAsset) assert.NotNil(err) } From b25f43e8659de4de0b5505df10478ab5e796044b Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Fri, 11 Jan 2019 20:15:21 +0100 Subject: [PATCH 2/3] virtcontainers: Add Capabilities to the types package In order to move the hypervisor implementations into their own package, we need to put the capabilities type into the types package. Fixes: #1119 Signed-off-by: Samuel Ortiz --- virtcontainers/agent.go | 2 +- virtcontainers/capabilities.go | 58 ------------------- virtcontainers/capabilities_test.go | 50 ----------------- virtcontainers/container.go | 4 +- virtcontainers/fc.go | 9 +-- virtcontainers/hyperstart_agent.go | 6 +- virtcontainers/hypervisor.go | 2 +- virtcontainers/kata_agent.go | 10 ++-- virtcontainers/mock_hypervisor.go | 6 +- virtcontainers/network.go | 2 +- virtcontainers/noop_agent.go | 4 +- virtcontainers/qemu.go | 2 +- virtcontainers/qemu_amd64.go | 10 ++-- virtcontainers/qemu_amd64_test.go | 4 +- virtcontainers/qemu_arch_base.go | 10 ++-- virtcontainers/qemu_arch_base_test.go | 2 +- virtcontainers/qemu_ppc64le.go | 9 +-- virtcontainers/qemu_test.go | 2 +- virtcontainers/types/capabilities.go | 68 +++++++++++++++++++++++ virtcontainers/types/capabilities_test.go | 50 +++++++++++++++++ 20 files changed, 163 insertions(+), 147 deletions(-) delete mode 100644 virtcontainers/capabilities.go delete mode 100644 virtcontainers/capabilities_test.go create mode 100644 virtcontainers/types/capabilities.go create mode 100644 virtcontainers/types/capabilities_test.go diff --git a/virtcontainers/agent.go b/virtcontainers/agent.go index 8774d49ceb..cf0c71c504 100644 --- a/virtcontainers/agent.go +++ b/virtcontainers/agent.go @@ -137,7 +137,7 @@ type agent interface { // capabilities should return a structure that specifies the capabilities // supported by the agent. - capabilities() capabilities + capabilities() types.Capabilities // check will check the agent liveness check() error diff --git a/virtcontainers/capabilities.go b/virtcontainers/capabilities.go deleted file mode 100644 index 72716ddb24..0000000000 --- a/virtcontainers/capabilities.go +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright (c) 2017 Intel Corporation -// -// SPDX-License-Identifier: Apache-2.0 -// - -package virtcontainers - -const ( - blockDeviceSupport = 1 << iota - blockDeviceHotplugSupport - multiQueueSupport - fsSharingUnsupported -) - -type capabilities struct { - flags uint -} - -func (caps *capabilities) isBlockDeviceSupported() bool { - if caps.flags&blockDeviceSupport != 0 { - return true - } - return false -} - -func (caps *capabilities) setBlockDeviceSupport() { - caps.flags = caps.flags | blockDeviceSupport -} - -func (caps *capabilities) isBlockDeviceHotplugSupported() bool { - if caps.flags&blockDeviceHotplugSupport != 0 { - return true - } - return false -} - -func (caps *capabilities) setBlockDeviceHotplugSupport() { - caps.flags |= blockDeviceHotplugSupport -} - -func (caps *capabilities) isMultiQueueSupported() bool { - if caps.flags&multiQueueSupport != 0 { - return true - } - return false -} - -func (caps *capabilities) setMultiQueueSupport() { - caps.flags |= multiQueueSupport -} - -func (caps *capabilities) isFsSharingSupported() bool { - return caps.flags&fsSharingUnsupported == 0 -} - -func (caps *capabilities) setFsSharingUnsupported() { - caps.flags |= fsSharingUnsupported -} diff --git a/virtcontainers/capabilities_test.go b/virtcontainers/capabilities_test.go deleted file mode 100644 index 3396989a85..0000000000 --- a/virtcontainers/capabilities_test.go +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright (c) 2017 Intel Corporation -// -// SPDX-License-Identifier: Apache-2.0 -// - -package virtcontainers - -import "testing" - -func TestBlockDeviceCapability(t *testing.T) { - var caps capabilities - - if caps.isBlockDeviceSupported() { - t.Fatal() - } - - caps.setBlockDeviceSupport() - - if !caps.isBlockDeviceSupported() { - t.Fatal() - } -} - -func TestBlockDeviceHotplugCapability(t *testing.T) { - var caps capabilities - - if caps.isBlockDeviceHotplugSupported() { - t.Fatal() - } - - caps.setBlockDeviceHotplugSupport() - - if !caps.isBlockDeviceHotplugSupported() { - t.Fatal() - } -} - -func TestFsSharingCapability(t *testing.T) { - var caps capabilities - - if !caps.isFsSharingSupported() { - t.Fatal() - } - - caps.setFsSharingUnsupported() - - if caps.isFsSharingSupported() { - t.Fatal() - } -} diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 545700b77f..db9715e97d 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -445,7 +445,7 @@ func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir s // copy file to contaier's rootfs if filesystem sharing is not supported, otherwise // bind mount it in the shared directory. caps := c.sandbox.hypervisor.capabilities() - if !caps.isFsSharingSupported() { + if !caps.IsFsSharingSupported() { c.Logger().Debug("filesystem sharing is not supported, files will be copied") fileInfo, err := os.Stat(m.Source) @@ -713,7 +713,7 @@ func (c *Container) checkBlockDeviceSupport() bool { agentCaps := c.sandbox.agent.capabilities() hypervisorCaps := c.sandbox.hypervisor.capabilities() - if agentCaps.isBlockDeviceSupported() && hypervisorCaps.isBlockDeviceHotplugSupported() { + if agentCaps.IsBlockDeviceSupported() && hypervisorCaps.IsBlockDeviceHotplugSupported() { return true } } diff --git a/virtcontainers/fc.go b/virtcontainers/fc.go index 7942da9582..203add79de 100644 --- a/virtcontainers/fc.go +++ b/virtcontainers/fc.go @@ -21,6 +21,7 @@ import ( "github.com/sirupsen/logrus" "github.com/kata-containers/runtime/virtcontainers/device/config" + "github.com/kata-containers/runtime/virtcontainers/types" "net" "net/http" @@ -656,12 +657,12 @@ func (fc *firecracker) disconnect() { } // Adds all capabilities supported by firecracker implementation of hypervisor interface -func (fc *firecracker) capabilities() capabilities { +func (fc *firecracker) capabilities() types.Capabilities { span, _ := fc.trace("capabilities") defer span.Finish() - var caps capabilities - caps.setFsSharingUnsupported() - caps.setBlockDeviceHotplugSupport() + var caps types.Capabilities + caps.SetFsSharingUnsupported() + caps.SetBlockDeviceHotplugSupport() return caps } diff --git a/virtcontainers/hyperstart_agent.go b/virtcontainers/hyperstart_agent.go index 31d774ac5e..feabc1f5a2 100644 --- a/virtcontainers/hyperstart_agent.go +++ b/virtcontainers/hyperstart_agent.go @@ -330,11 +330,11 @@ func (h *hyper) createSandbox(sandbox *Sandbox) (err error) { return h.configure(sandbox.hypervisor, "", h.getSharePath(sandbox.id), false, nil) } -func (h *hyper) capabilities() capabilities { - var caps capabilities +func (h *hyper) capabilities() types.Capabilities { + var caps types.Capabilities // add all capabilities supported by agent - caps.setBlockDeviceSupport() + caps.SetBlockDeviceSupport() return caps } diff --git a/virtcontainers/hypervisor.go b/virtcontainers/hypervisor.go index 4df16803a4..23cf82343d 100644 --- a/virtcontainers/hypervisor.go +++ b/virtcontainers/hypervisor.go @@ -604,7 +604,7 @@ type hypervisor interface { resizeVCPUs(vcpus uint32) (uint32, uint32, error) getSandboxConsole(sandboxID string) (string, error) disconnect() - capabilities() capabilities + capabilities() types.Capabilities hypervisorConfig() HypervisorConfig getThreadIDs() (*threadIDs, error) cleanup() error diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 64b40a789c..3f0abc423f 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -211,11 +211,11 @@ func (k *kataAgent) agentURL() (string, error) { } } -func (k *kataAgent) capabilities() capabilities { - var caps capabilities +func (k *kataAgent) capabilities() types.Capabilities { + var caps types.Capabilities // add all capabilities supported by agent - caps.setBlockDeviceSupport() + caps.SetBlockDeviceSupport() return caps } @@ -261,7 +261,7 @@ func (k *kataAgent) configure(h hypervisor, id, sharePath string, builtin bool, // Neither create shared directory nor add 9p device if hypervisor // doesn't support filesystem sharing. caps := h.capabilities() - if !caps.isFsSharingSupported() { + if !caps.IsFsSharingSupported() { return nil } @@ -629,7 +629,7 @@ func (k *kataAgent) startSandbox(sandbox *Sandbox) error { caps := sandbox.hypervisor.capabilities() // append 9p shared volume to storages only if filesystem sharing is supported - if caps.isFsSharingSupported() { + if caps.IsFsSharingSupported() { sharedDir9pOptions = append(sharedDir9pOptions, fmt.Sprintf("msize=%d", sandbox.config.HypervisorConfig.Msize9p)) // We mount the shared directory in a predefined location diff --git a/virtcontainers/mock_hypervisor.go b/virtcontainers/mock_hypervisor.go index f23b3b8762..1146c6f746 100644 --- a/virtcontainers/mock_hypervisor.go +++ b/virtcontainers/mock_hypervisor.go @@ -8,13 +8,15 @@ package virtcontainers import ( "context" "os" + + "github.com/kata-containers/runtime/virtcontainers/types" ) type mockHypervisor struct { } -func (m *mockHypervisor) capabilities() capabilities { - return capabilities{} +func (m *mockHypervisor) capabilities() types.Capabilities { + return types.Capabilities{} } func (m *mockHypervisor) hypervisorConfig() HypervisorConfig { diff --git a/virtcontainers/network.go b/virtcontainers/network.go index bfa087f961..777d12fec2 100644 --- a/virtcontainers/network.go +++ b/virtcontainers/network.go @@ -511,7 +511,7 @@ func xConnectVMNetwork(endpoint Endpoint, h hypervisor) error { queues := 0 caps := h.capabilities() - if caps.isMultiQueueSupported() { + if caps.IsMultiQueueSupported() { queues = int(h.hypervisorConfig().NumVCPUs) } diff --git a/virtcontainers/noop_agent.go b/virtcontainers/noop_agent.go index f0e38bcc97..1783454b93 100644 --- a/virtcontainers/noop_agent.go +++ b/virtcontainers/noop_agent.go @@ -37,8 +37,8 @@ func (n *noopAgent) createSandbox(sandbox *Sandbox) error { } // capabilities returns empty capabilities, i.e no capabilties are supported. -func (n *noopAgent) capabilities() capabilities { - return capabilities{} +func (n *noopAgent) capabilities() types.Capabilities { + return types.Capabilities{} } // disconnect is the Noop agent connection closer. It does nothing. diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 59d9dbbb17..ee6298203b 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -162,7 +162,7 @@ func (q *qemu) kernelParameters() string { } // Adds all capabilities supported by qemu implementation of hypervisor interface -func (q *qemu) capabilities() capabilities { +func (q *qemu) capabilities() types.Capabilities { span, _ := q.trace("capabilities") defer span.Finish() diff --git a/virtcontainers/qemu_amd64.go b/virtcontainers/qemu_amd64.go index 7e3fc9d3de..dec7ec9259 100644 --- a/virtcontainers/qemu_amd64.go +++ b/virtcontainers/qemu_amd64.go @@ -8,6 +8,8 @@ package virtcontainers import ( "os" + "github.com/kata-containers/runtime/virtcontainers/types" + govmmQemu "github.com/intel/govmm/qemu" ) @@ -101,16 +103,16 @@ func newQemuArch(config HypervisorConfig) qemuArch { return q } -func (q *qemuAmd64) capabilities() capabilities { - var caps capabilities +func (q *qemuAmd64) capabilities() types.Capabilities { + var caps types.Capabilities if q.machineType == QemuPC || q.machineType == QemuQ35 || q.machineType == QemuVirt { - caps.setBlockDeviceHotplugSupport() + caps.SetBlockDeviceHotplugSupport() } - caps.setMultiQueueSupport() + caps.SetMultiQueueSupport() return caps } diff --git a/virtcontainers/qemu_amd64_test.go b/virtcontainers/qemu_amd64_test.go index c10d126653..14bcb6e832 100644 --- a/virtcontainers/qemu_amd64_test.go +++ b/virtcontainers/qemu_amd64_test.go @@ -27,11 +27,11 @@ func TestQemuAmd64Capabilities(t *testing.T) { amd64 := newTestQemu(QemuPC) caps := amd64.capabilities() - assert.True(caps.isBlockDeviceHotplugSupported()) + assert.True(caps.IsBlockDeviceHotplugSupported()) amd64 = newTestQemu(QemuQ35) caps = amd64.capabilities() - assert.True(caps.isBlockDeviceHotplugSupported()) + assert.True(caps.IsBlockDeviceHotplugSupported()) } func TestQemuAmd64Bridges(t *testing.T) { diff --git a/virtcontainers/qemu_arch_base.go b/virtcontainers/qemu_arch_base.go index aa852353d8..fa8bd49be2 100644 --- a/virtcontainers/qemu_arch_base.go +++ b/virtcontainers/qemu_arch_base.go @@ -45,7 +45,7 @@ type qemuArch interface { kernelParameters(debug bool) []Param //capabilities returns the capabilities supported by QEMU - capabilities() capabilities + capabilities() types.Capabilities // bridges returns the number bridges for the machine type bridges(number uint32) []Bridge @@ -228,10 +228,10 @@ func (q *qemuArchBase) kernelParameters(debug bool) []Param { return params } -func (q *qemuArchBase) capabilities() capabilities { - var caps capabilities - caps.setBlockDeviceHotplugSupport() - caps.setMultiQueueSupport() +func (q *qemuArchBase) capabilities() types.Capabilities { + var caps types.Capabilities + caps.SetBlockDeviceHotplugSupport() + caps.SetMultiQueueSupport() return caps } diff --git a/virtcontainers/qemu_arch_base_test.go b/virtcontainers/qemu_arch_base_test.go index 711d585f4a..c6ae625282 100644 --- a/virtcontainers/qemu_arch_base_test.go +++ b/virtcontainers/qemu_arch_base_test.go @@ -138,7 +138,7 @@ func TestQemuArchBaseCapabilities(t *testing.T) { qemuArchBase := newQemuArchBase() c := qemuArchBase.capabilities() - assert.True(c.isBlockDeviceHotplugSupported()) + assert.True(c.IsBlockDeviceHotplugSupported()) } func TestQemuArchBaseBridges(t *testing.T) { diff --git a/virtcontainers/qemu_ppc64le.go b/virtcontainers/qemu_ppc64le.go index ca69e2d831..f6151f5f47 100644 --- a/virtcontainers/qemu_ppc64le.go +++ b/virtcontainers/qemu_ppc64le.go @@ -11,6 +11,7 @@ import ( govmmQemu "github.com/intel/govmm/qemu" deviceConfig "github.com/kata-containers/runtime/virtcontainers/device/config" + "github.com/kata-containers/runtime/virtcontainers/types" "github.com/kata-containers/runtime/virtcontainers/utils" "github.com/sirupsen/logrus" ) @@ -90,15 +91,15 @@ func newQemuArch(config HypervisorConfig) qemuArch { return q } -func (q *qemuPPC64le) capabilities() capabilities { - var caps capabilities +func (q *qemuPPC64le) capabilities() types.Capabilities { + var caps types.Capabilities // pseries machine type supports hotplugging drives if q.machineType == QemuPseries { - caps.setBlockDeviceHotplugSupport() + caps.SetBlockDeviceHotplugSupport() } - caps.setMultiQueueSupport() + caps.SetMultiQueueSupport() return caps } diff --git a/virtcontainers/qemu_test.go b/virtcontainers/qemu_test.go index 73ad6adfc9..1a01868167 100644 --- a/virtcontainers/qemu_test.go +++ b/virtcontainers/qemu_test.go @@ -306,7 +306,7 @@ func TestQemuCapabilities(t *testing.T) { } caps := q.capabilities() - if !caps.isBlockDeviceHotplugSupported() { + if !caps.IsBlockDeviceHotplugSupported() { t.Fatal("Block device hotplug should be supported") } } diff --git a/virtcontainers/types/capabilities.go b/virtcontainers/types/capabilities.go new file mode 100644 index 0000000000..c361601ef5 --- /dev/null +++ b/virtcontainers/types/capabilities.go @@ -0,0 +1,68 @@ +// Copyright (c) 2017 Intel Corporation +// +// SPDX-License-Identifier: Apache-2.0 +// + +package types + +const ( + blockDeviceSupport = 1 << iota + blockDeviceHotplugSupport + multiQueueSupport + fsSharingUnsupported +) + +// Capabilities describe a virtcontainers hypervisor capabilities +// through a bit mask. +type Capabilities struct { + flags uint +} + +// IsBlockDeviceSupported tells if an hypervisor supports block devices. +func (caps *Capabilities) IsBlockDeviceSupported() bool { + if caps.flags&blockDeviceSupport != 0 { + return true + } + return false +} + +// SetBlockDeviceSupport sets the block device support capability to true. +func (caps *Capabilities) SetBlockDeviceSupport() { + caps.flags = caps.flags | blockDeviceSupport +} + +// IsBlockDeviceHotplugSupported tells if an hypervisor supports hotplugging block devices. +func (caps *Capabilities) IsBlockDeviceHotplugSupported() bool { + if caps.flags&blockDeviceHotplugSupport != 0 { + return true + } + return false +} + +// SetBlockDeviceHotplugSupport sets the block device hotplugging capability to true. +func (caps *Capabilities) SetBlockDeviceHotplugSupport() { + caps.flags |= blockDeviceHotplugSupport +} + +// IsMultiQueueSupported tells if an hypervisor supports device multi queue support. +func (caps *Capabilities) IsMultiQueueSupported() bool { + if caps.flags&multiQueueSupport != 0 { + return true + } + return false +} + +// SetMultiQueueSupport sets the device multi queue capability to true. +func (caps *Capabilities) SetMultiQueueSupport() { + caps.flags |= multiQueueSupport +} + +// IsFsSharingSupported tells if an hypervisor supports host filesystem sharing. +func (caps *Capabilities) IsFsSharingSupported() bool { + return caps.flags&fsSharingUnsupported == 0 +} + +// SetFsSharingUnsupported sets the host filesystem sharing capability to true. +func (caps *Capabilities) SetFsSharingUnsupported() { + caps.flags |= fsSharingUnsupported +} diff --git a/virtcontainers/types/capabilities_test.go b/virtcontainers/types/capabilities_test.go new file mode 100644 index 0000000000..c5b0bcd068 --- /dev/null +++ b/virtcontainers/types/capabilities_test.go @@ -0,0 +1,50 @@ +// Copyright (c) 2017 Intel Corporation +// +// SPDX-License-Identifier: Apache-2.0 +// + +package types + +import "testing" + +func TestBlockDeviceCapability(t *testing.T) { + var caps Capabilities + + if caps.IsBlockDeviceSupported() { + t.Fatal() + } + + caps.SetBlockDeviceSupport() + + if !caps.IsBlockDeviceSupported() { + t.Fatal() + } +} + +func TestBlockDeviceHotplugCapability(t *testing.T) { + var caps Capabilities + + if caps.IsBlockDeviceHotplugSupported() { + t.Fatal() + } + + caps.SetBlockDeviceHotplugSupport() + + if !caps.IsBlockDeviceHotplugSupported() { + t.Fatal() + } +} + +func TestFsSharingCapability(t *testing.T) { + var caps Capabilities + + if !caps.IsFsSharingSupported() { + t.Fatal() + } + + caps.SetFsSharingUnsupported() + + if caps.IsFsSharingSupported() { + t.Fatal() + } +} From 2e1ddbc7255b93c240f584cdf3493ebb2dc203e3 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Wed, 16 Jan 2019 15:45:08 +0100 Subject: [PATCH 3/3] virtcontainers: Add Bridge to the types package Bridge is representing a PCI/E bridge, so we're moving the bridge*.go to types/pci*.go. Fixes: #1119 Signed-off-by: Samuel Ortiz --- virtcontainers/qemu.go | 30 ++++++++--------- virtcontainers/qemu_amd64.go | 4 +-- virtcontainers/qemu_amd64_test.go | 9 ++--- virtcontainers/qemu_arch_base.go | 18 +++++----- virtcontainers/qemu_arch_base_test.go | 4 +-- virtcontainers/qemu_ppc64le.go | 4 +-- virtcontainers/{bridge.go => types/pci.go} | 33 ++++++++++--------- .../{bridge_test.go => types/pci_test.go} | 12 +++---- 8 files changed, 59 insertions(+), 55 deletions(-) rename virtcontainers/{bridge.go => types/pci.go} (57%) rename virtcontainers/{bridge_test.go => types/pci_test.go} (76%) diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index ee6298203b..65419a83ed 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -47,7 +47,7 @@ type CPUDevice struct { // QemuState keeps Qemu's state type QemuState struct { - Bridges []Bridge + Bridges []types.PCIBridge // HotpluggedCPUs is the list of CPUs that were hot-added HotpluggedVCPUs []CPUDevice HotpluggedMemory int @@ -722,25 +722,25 @@ func (q *qemu) qmpShutdown() { } } -func (q *qemu) addDeviceToBridge(ID string) (string, Bridge, error) { +func (q *qemu) addDeviceToBridge(ID string) (string, types.PCIBridge, error) { var err error var addr uint32 // looking for an empty address in the bridges for _, b := range q.state.Bridges { - addr, err = b.addDevice(ID) + addr, err = b.AddDevice(ID) if err == nil { return fmt.Sprintf("%02x", addr), b, nil } } - return "", Bridge{}, err + return "", types.PCIBridge{}, err } func (q *qemu) removeDeviceFromBridge(ID string) error { var err error for _, b := range q.state.Bridges { - err = b.removeDevice(ID) + err = b.RemoveDevice(ID) if err == nil { // device was removed correctly return nil @@ -1377,7 +1377,7 @@ func (q *qemu) resizeMemory(reqMemMB uint32, memoryBlockSizeMB uint32) (uint32, } // genericAppendBridges appends to devices the given bridges -func genericAppendBridges(devices []govmmQemu.Device, bridges []Bridge, machineType string) []govmmQemu.Device { +func genericAppendBridges(devices []govmmQemu.Device, bridges []types.PCIBridge, machineType string) []govmmQemu.Device { bus := defaultPCBridgeBus switch machineType { case QemuQ35, QemuVirt: @@ -1386,7 +1386,7 @@ func genericAppendBridges(devices []govmmQemu.Device, bridges []Bridge, machineT for idx, b := range bridges { t := govmmQemu.PCIBridge - if b.Type == pcieBridge { + if b.Type == types.PCIE { t = govmmQemu.PCIEBridge } @@ -1408,9 +1408,9 @@ func genericAppendBridges(devices []govmmQemu.Device, bridges []Bridge, machineT return devices } -func genericBridges(number uint32, machineType string) []Bridge { - var bridges []Bridge - var bt bridgeType +func genericBridges(number uint32, machineType string) []types.PCIBridge { + var bridges []types.PCIBridge + var bt types.PCIType switch machineType { case QemuQ35: @@ -1418,19 +1418,19 @@ func genericBridges(number uint32, machineType string) []Bridge { // qemu-2.10 will introduce pcie bridges fallthrough case QemuPC: - bt = pciBridge + bt = types.PCI case QemuVirt: - bt = pcieBridge + bt = types.PCIE case QemuPseries: - bt = pciBridge + bt = types.PCI case QemuCCWVirtio: - bt = pciBridge + bt = types.PCI default: return nil } for i := uint32(0); i < number; i++ { - bridges = append(bridges, Bridge{ + bridges = append(bridges, types.PCIBridge{ Type: bt, ID: fmt.Sprintf("%s-bridge-%d", bt, i), Address: make(map[uint32]string), diff --git a/virtcontainers/qemu_amd64.go b/virtcontainers/qemu_amd64.go index dec7ec9259..65d9838e44 100644 --- a/virtcontainers/qemu_amd64.go +++ b/virtcontainers/qemu_amd64.go @@ -117,7 +117,7 @@ func (q *qemuAmd64) capabilities() types.Capabilities { return caps } -func (q *qemuAmd64) bridges(number uint32) []Bridge { +func (q *qemuAmd64) bridges(number uint32) []types.PCIBridge { return genericBridges(number, q.machineType) } @@ -160,6 +160,6 @@ func (q *qemuAmd64) appendImage(devices []govmmQemu.Device, path string) ([]govm } // appendBridges appends to devices the given bridges -func (q *qemuAmd64) appendBridges(devices []govmmQemu.Device, bridges []Bridge) []govmmQemu.Device { +func (q *qemuAmd64) appendBridges(devices []govmmQemu.Device, bridges []types.PCIBridge) []govmmQemu.Device { return genericAppendBridges(devices, bridges, q.machineType) } diff --git a/virtcontainers/qemu_amd64_test.go b/virtcontainers/qemu_amd64_test.go index 14bcb6e832..31e42341ad 100644 --- a/virtcontainers/qemu_amd64_test.go +++ b/virtcontainers/qemu_amd64_test.go @@ -12,6 +12,7 @@ import ( "testing" govmmQemu "github.com/intel/govmm/qemu" + "github.com/kata-containers/runtime/virtcontainers/types" "github.com/stretchr/testify/assert" ) @@ -43,8 +44,8 @@ func TestQemuAmd64Bridges(t *testing.T) { assert.Len(bridges, len) for i, b := range bridges { - id := fmt.Sprintf("%s-bridge-%d", pciBridge, i) - assert.Equal(pciBridge, b.Type) + id := fmt.Sprintf("%s-bridge-%d", types.PCI, i) + assert.Equal(types.PCI, b.Type) assert.Equal(id, b.ID) assert.NotNil(b.Address) } @@ -54,8 +55,8 @@ func TestQemuAmd64Bridges(t *testing.T) { assert.Len(bridges, len) for i, b := range bridges { - id := fmt.Sprintf("%s-bridge-%d", pciBridge, i) - assert.Equal(pciBridge, b.Type) + id := fmt.Sprintf("%s-bridge-%d", types.PCI, i) + assert.Equal(types.PCI, b.Type) assert.Equal(id, b.ID) assert.NotNil(b.Address) } diff --git a/virtcontainers/qemu_arch_base.go b/virtcontainers/qemu_arch_base.go index fa8bd49be2..5e68bfd292 100644 --- a/virtcontainers/qemu_arch_base.go +++ b/virtcontainers/qemu_arch_base.go @@ -48,7 +48,7 @@ type qemuArch interface { capabilities() types.Capabilities // bridges returns the number bridges for the machine type - bridges(number uint32) []Bridge + bridges(number uint32) []types.PCIBridge // cpuTopology returns the CPU topology for the given amount of vcpus cpuTopology(vcpus, maxvcpus uint32) govmmQemu.SMP @@ -69,7 +69,7 @@ type qemuArch interface { appendSCSIController(devices []govmmQemu.Device, enableIOThreads bool) ([]govmmQemu.Device, *govmmQemu.IOThread) // appendBridges appends bridges to devices - appendBridges(devices []govmmQemu.Device, bridges []Bridge) []govmmQemu.Device + appendBridges(devices []govmmQemu.Device, bridges []types.PCIBridge) []govmmQemu.Device // append9PVolume appends a 9P volume to devices append9PVolume(devices []govmmQemu.Device, volume types.Volume) []govmmQemu.Device @@ -235,13 +235,13 @@ func (q *qemuArchBase) capabilities() types.Capabilities { return caps } -func (q *qemuArchBase) bridges(number uint32) []Bridge { - var bridges []Bridge +func (q *qemuArchBase) bridges(number uint32) []types.PCIBridge { + var bridges []types.PCIBridge for i := uint32(0); i < number; i++ { - bridges = append(bridges, Bridge{ - Type: pciBridge, - ID: fmt.Sprintf("%s-bridge-%d", pciBridge, i), + bridges = append(bridges, types.PCIBridge{ + Type: types.PCI, + ID: fmt.Sprintf("%s-bridge-%d", types.PCI, i), Address: make(map[uint32]string), }) } @@ -346,10 +346,10 @@ func (q *qemuArchBase) appendSCSIController(devices []govmmQemu.Device, enableIO } // appendBridges appends to devices the given bridges -func (q *qemuArchBase) appendBridges(devices []govmmQemu.Device, bridges []Bridge) []govmmQemu.Device { +func (q *qemuArchBase) appendBridges(devices []govmmQemu.Device, bridges []types.PCIBridge) []govmmQemu.Device { for idx, b := range bridges { t := govmmQemu.PCIBridge - if b.Type == pcieBridge { + if b.Type == types.PCIE { t = govmmQemu.PCIEBridge } diff --git a/virtcontainers/qemu_arch_base_test.go b/virtcontainers/qemu_arch_base_test.go index c6ae625282..7dac7dc59f 100644 --- a/virtcontainers/qemu_arch_base_test.go +++ b/virtcontainers/qemu_arch_base_test.go @@ -150,8 +150,8 @@ func TestQemuArchBaseBridges(t *testing.T) { assert.Len(bridges, len) for i, b := range bridges { - id := fmt.Sprintf("%s-bridge-%d", pciBridge, i) - assert.Equal(pciBridge, b.Type) + id := fmt.Sprintf("%s-bridge-%d", types.PCI, i) + assert.Equal(types.PCI, b.Type) assert.Equal(id, b.ID) assert.NotNil(b.Address) } diff --git a/virtcontainers/qemu_ppc64le.go b/virtcontainers/qemu_ppc64le.go index f6151f5f47..e9be4b485d 100644 --- a/virtcontainers/qemu_ppc64le.go +++ b/virtcontainers/qemu_ppc64le.go @@ -104,7 +104,7 @@ func (q *qemuPPC64le) capabilities() types.Capabilities { return caps } -func (q *qemuPPC64le) bridges(number uint32) []Bridge { +func (q *qemuPPC64le) bridges(number uint32) []types.PCIBridge { return genericBridges(number, q.machineType) } @@ -151,6 +151,6 @@ func (q *qemuPPC64le) appendImage(devices []govmmQemu.Device, path string) ([]go } // appendBridges appends to devices the given bridges -func (q *qemuPPC64le) appendBridges(devices []govmmQemu.Device, bridges []Bridge) []govmmQemu.Device { +func (q *qemuPPC64le) appendBridges(devices []govmmQemu.Device, bridges []types.PCIBridge) []govmmQemu.Device { return genericAppendBridges(devices, bridges, q.machineType) } diff --git a/virtcontainers/bridge.go b/virtcontainers/types/pci.go similarity index 57% rename from virtcontainers/bridge.go rename to virtcontainers/types/pci.go index 4e8b43853a..78942600d7 100644 --- a/virtcontainers/bridge.go +++ b/virtcontainers/types/pci.go @@ -3,37 +3,41 @@ // SPDX-License-Identifier: Apache-2.0 // -package virtcontainers +package types import "fmt" -type bridgeType string +// PCIType represents a type of PCI bus and bridge. +type PCIType string const ( - pciBridge bridgeType = "pci" - pcieBridge bridgeType = "pcie" + // PCI represents a PCI bus and bridge + PCI PCIType = "pci" + + // PCIE represents a PCIe bus and bridge + PCIE PCIType = "pcie" ) const pciBridgeMaxCapacity = 30 -// Bridge is a bridge where devices can be hot plugged -type Bridge struct { +// PCIBridge is a PCI or PCIe bridge where devices can be hot plugged +type PCIBridge struct { // Address contains information about devices plugged and its address in the bridge Address map[uint32]string - // Type is the type of the bridge (pci, pcie, etc) - Type bridgeType + // Type is the PCI type of the bridge (pci, pcie, etc) + Type PCIType - //ID is used to identify the bridge in the hypervisor + // ID is used to identify the bridge in the hypervisor ID string // Addr is the PCI/e slot of the bridge Addr int } -// addDevice on success adds the device ID to the bridge and return the address -// where the device was added, otherwise an error is returned -func (b *Bridge) addDevice(ID string) (uint32, error) { +// AddDevice on success adds the device ID to the PCI bridge and returns +// the address where the device was added. +func (b *PCIBridge) AddDevice(ID string) (uint32, error) { var addr uint32 // looking for the first available address @@ -53,9 +57,8 @@ func (b *Bridge) addDevice(ID string) (uint32, error) { return addr, nil } -// removeDevice on success removes the device ID from the bridge and return nil, -// otherwise an error is returned -func (b *Bridge) removeDevice(ID string) error { +// RemoveDevice removes the device ID from the PCI bridge. +func (b *PCIBridge) RemoveDevice(ID string) error { // check if the device was hot plugged in the bridge for addr, devID := range b.Address { if devID == ID { diff --git a/virtcontainers/bridge_test.go b/virtcontainers/types/pci_test.go similarity index 76% rename from virtcontainers/bridge_test.go rename to virtcontainers/types/pci_test.go index 097662a5c4..3a90cfed20 100644 --- a/virtcontainers/bridge_test.go +++ b/virtcontainers/types/pci_test.go @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // -package virtcontainers +package types import ( "fmt" @@ -16,22 +16,22 @@ func TestAddRemoveDevice(t *testing.T) { assert := assert.New(t) // create a bridge - bridges := []*Bridge{{make(map[uint32]string), pciBridge, "rgb123", 5}} + bridges := []*PCIBridge{{make(map[uint32]string), PCI, "rgb123", 5}} // add device devID := "abc123" b := bridges[0] - addr, err := b.addDevice(devID) + addr, err := b.AddDevice(devID) assert.NoError(err) if addr < 1 { assert.Fail("address cannot be less than 1") } // remove device - err = b.removeDevice("") + err = b.RemoveDevice("") assert.Error(err) - err = b.removeDevice(devID) + err = b.RemoveDevice(devID) assert.NoError(err) // add device when the bridge is full @@ -39,7 +39,7 @@ func TestAddRemoveDevice(t *testing.T) { for i := uint32(1); i <= pciBridgeMaxCapacity; i++ { bridges[0].Address[i] = fmt.Sprintf("%d", i) } - addr, err = b.addDevice(devID) + addr, err = b.AddDevice(devID) assert.Error(err) if addr != 0 { assert.Fail("address should be 0")