devices: add reference count for devices.

Fixes kata-containers#635

Remove `Hotplugged bool` field from device and add two new fields
instead:
* `RefCount`: how many references to this device. One device can be
referenced(`NewDevice()`) many times by same/different container(s),
two devices are regarded identical if they have same hostPath
* `AttachCount`: how many times this device has been attached. A device
can only be hotplugged once to the qemu, every new Attach command will
add the AttachCount, and real `Detach` will be done only when
`AttachCount == 0`

Signed-off-by: Wei Zhang <[email protected]>

virtcontainers/container.go

@@ -457,8 +457,7 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) (
 		// instead of passing this as a shared mount.
 		if len(m.BlockDeviceID) > 0 {
 			// Attach this block device, all other devices passed in the config have been attached at this point
-			if err := c.sandbox.devManager.AttachDevice(m.BlockDeviceID, c.sandbox); err != nil &&
-				err != manager.ErrDeviceAttached {
+			if err := c.sandbox.devManager.AttachDevice(m.BlockDeviceID, c.sandbox); err != nil {
 				return nil, err
 			}
 
@@ -1153,10 +1152,6 @@ func (c *Container) attachDevices() error {
 	// and rollbackFailingContainerCreation could do all the rollbacks
 	for _, dev := range c.devices {
 		if err := c.sandbox.devManager.AttachDevice(dev.ID, c.sandbox); err != nil {
-			if err == manager.ErrDeviceAttached {
-				// skip if device is already attached before
-				continue
-			}
 			return err
 		}
 	}

virtcontainers/device/api/interface.go

@@ -49,13 +49,20 @@ type Device interface {
 	// DeviceType indicates which kind of device it is
 	// e.g. block, vfio or vhost user
 	DeviceType() config.DeviceType
+	// GetMajorMinor returns major and minor numbers
+	GetMajorMinor() (int64, int64)
 	// GetDeviceInfo returns device specific data used for hotplugging by hypervisor
 	// Caller could cast the return value to device specific struct
 	// e.g. Block device returns *config.BlockDrive and
 	// vfio device returns []*config.VFIODev
 	GetDeviceInfo() interface{}
-	// IsAttached checks if the device is attached
-	IsAttached() bool
+	// GetAttachCount returns how many times the device has been attached
+	GetAttachCount() uint
+
+	// Reference adds one reference to device then returns final ref count
+	Reference() uint
+	// Dereference removes one reference to device then returns final ref count
+	Dereference() uint
 }
 
 // DeviceManager can be used to create a new device, this can be used as single

virtcontainers/device/config/config.go

@@ -75,10 +75,6 @@ type DeviceInfo struct {
 	// id of the device group.
 	GID uint32
 
-	// Hotplugged is used to store device state indicating if the
-	// device was hotplugged.
-	Hotplugged bool
-
 	// ID for the device that is passed to the hypervisor.
 	ID string
 

virtcontainers/device/drivers/block.go

@@ -35,7 +35,11 @@ func NewBlockDevice(devInfo *config.DeviceInfo) *BlockDevice {
 // Attach is standard interface of api.Device, it's used to add device to some
 // DeviceReceiver
 func (device *BlockDevice) Attach(devReceiver api.DeviceReceiver) (err error) {
-	if device.DeviceInfo.Hotplugged {
+	skip, err := device.bumpAttachCount(true)
+	if err != nil {
+		return err
+	}
+	if skip {
 		return nil
 	}
 
@@ -47,6 +51,8 @@ func (device *BlockDevice) Attach(devReceiver api.DeviceReceiver) (err error) {
 	defer func() {
 		if err != nil {
 			devReceiver.DecrementSandboxBlockIndex()
+		} else {
+			device.AttachCount = 1
 		}
 	}()
 
@@ -84,15 +90,17 @@ func (device *BlockDevice) Attach(devReceiver api.DeviceReceiver) (err error) {
 		return err
 	}
 
-	device.DeviceInfo.Hotplugged = true
-
 	return nil
 }
 
 // Detach is standard interface of api.Device, it's used to remove device from some
 // DeviceReceiver
 func (device *BlockDevice) Detach(devReceiver api.DeviceReceiver) error {
-	if !device.DeviceInfo.Hotplugged {
+	skip, err := device.bumpAttachCount(false)
+	if err != nil {
+		return err
+	}
+	if skip {
 		return nil
 	}
 
@@ -102,7 +110,7 @@ func (device *BlockDevice) Detach(devReceiver api.DeviceReceiver) error {
 		deviceLogger().WithError(err).Error("Failed to unplug block device")
 		return err
 	}
-	device.DeviceInfo.Hotplugged = false
+	device.AttachCount = 0
 	return nil
 }
 
@@ -116,5 +124,5 @@ func (device *BlockDevice) GetDeviceInfo() interface{} {
 	return device.BlockDrive
 }
 
-// It should implement IsAttached() and DeviceID() as api.Device implementation
+// It should implement GetAttachCount() and DeviceID() as api.Device implementation
 // here it shares function from *GenericDevice so we don't need duplicate codes

virtcontainers/device/drivers/generic.go

@@ -7,6 +7,8 @@
 package drivers
 
 import (
+	"fmt"
+
 	"github.com/kata-containers/runtime/virtcontainers/device/api"
 	"github.com/kata-containers/runtime/virtcontainers/device/config"
 )
@@ -15,6 +17,9 @@ import (
 type GenericDevice struct {
 	ID         string
 	DeviceInfo *config.DeviceInfo
+
+	RefCount    uint
+	AttachCount uint
 }
 
 // NewGenericDevice creates a new GenericDevice
@@ -27,19 +32,27 @@ func NewGenericDevice(devInfo *config.DeviceInfo) *GenericDevice {
 
 // Attach is standard interface of api.Device
 func (device *GenericDevice) Attach(devReceiver api.DeviceReceiver) error {
-	if device.DeviceInfo.Hotplugged {
+	skip, err := device.bumpAttachCount(true)
+	if err != nil {
+		return err
+	}
+	if skip {
 		return nil
 	}
-
+	device.AttachCount = 1
 	return nil
 }
 
 // Detach is standard interface of api.Device
 func (device *GenericDevice) Detach(devReceiver api.DeviceReceiver) error {
-	if !device.DeviceInfo.Hotplugged {
+	skip, err := device.bumpAttachCount(false)
+	if err != nil {
+		return err
+	}
+	if skip {
 		return nil
 	}
-
+	device.AttachCount = 0
 	return nil
 }
 
@@ -53,12 +66,64 @@ func (device *GenericDevice) GetDeviceInfo() interface{} {
 	return device.DeviceInfo
 }
 
-// IsAttached checks if the device is attached
-func (device *GenericDevice) IsAttached() bool {
-	return device.DeviceInfo.Hotplugged
+// GetAttachCount returns how many times the device has been attached
+func (device *GenericDevice) GetAttachCount() uint {
+	return device.AttachCount
 }
 
 // DeviceID returns device ID
 func (device *GenericDevice) DeviceID() string {
 	return device.ID
 }
+
+// GetMajorMinor returns device major and minor numbers
+func (device *GenericDevice) GetMajorMinor() (int64, int64) {
+	return device.DeviceInfo.Major, device.DeviceInfo.Minor
+}
+
+// Reference adds one reference to device
+func (device *GenericDevice) Reference() uint {
+	if device.RefCount != intMax {
+		device.RefCount++
+	}
+	return device.RefCount
+}
+
+// Dereference remove one reference from device
+func (device *GenericDevice) Dereference() uint {
+	if device.RefCount != 0 {
+		device.RefCount--
+	}
+	return device.RefCount
+}
+
+// bumpAttachCount is used to add/minus attach count for a device
+// * attach bool: true means attach, false means detach
+// return values:
+// * skip bool: no need to do real attach/detach, skip following actions.
+// * err error: error while do attach count bump
+func (device *GenericDevice) bumpAttachCount(attach bool) (skip bool, err error) {
+	if attach { // attach use case
+		switch device.AttachCount {
+		case 0:
+			// do real attach
+			return false, nil
+		case intMax:
+			return true, fmt.Errorf("device was attached too many times")
+		default:
+			device.AttachCount++
+			return true, nil
+		}
+	} else { // detach use case
+		switch device.AttachCount {
+		case 0:
+			return true, fmt.Errorf("detaching a device that wasn't attached")
+		case 1:
+			// do real work
+			return false, nil
+		default:
+			device.AttachCount--
+			return true, nil
+		}
+	}
+}

virtcontainers/device/drivers/generic_test.go

@@ -0,0 +1,44 @@
+// Copyright (c) 2018 Huawei Corporation
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+package drivers
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestBumpAttachCount(t *testing.T) {
+	type testData struct {
+		attach      bool
+		attachCount uint
+		expectedAC  uint
+		expectSkip  bool
+		expectErr   bool
+	}
+
+	data := []testData{
+		{true, 0, 0, false, false},
+		{true, 1, 2, true, false},
+		{true, intMax, intMax, true, true},
+		{false, 0, 0, true, true},
+		{false, 1, 1, false, false},
+		{false, intMax, intMax - 1, true, false},
+	}
+
+	dev := &GenericDevice{}
+	for _, d := range data {
+		dev.AttachCount = d.attachCount
+		skip, err := dev.bumpAttachCount(d.attach)
+		assert.Equal(t, skip, d.expectSkip, "")
+		assert.Equal(t, dev.GetAttachCount(), d.expectedAC, "")
+		if d.expectErr {
+			assert.NotNil(t, err)
+		} else {
+			assert.Nil(t, err)
+		}
+	}
+}

virtcontainers/device/drivers/utils.go

@@ -12,6 +12,8 @@ import (
 	"github.com/kata-containers/runtime/virtcontainers/device/api"
 )
 
+const intMax uint = ^uint(0)
+
 func deviceLogger() *logrus.Entry {
 	return api.DeviceLogger()
 }

virtcontainers/device/drivers/vfio.go

@@ -47,7 +47,11 @@ func NewVFIODevice(devInfo *config.DeviceInfo) *VFIODevice {
 // Attach is standard interface of api.Device, it's used to add device to some
 // DeviceReceiver
 func (device *VFIODevice) Attach(devReceiver api.DeviceReceiver) error {
-	if device.DeviceInfo.Hotplugged {
+	skip, err := device.bumpAttachCount(true)
+	if err != nil {
+		return err
+	}
+	if skip {
 		return nil
 	}
 
@@ -83,14 +87,18 @@ func (device *VFIODevice) Attach(devReceiver api.DeviceReceiver) error {
 		"device-group": device.DeviceInfo.HostPath,
 		"device-type":  "vfio-passthrough",
 	}).Info("Device group attached")
-	device.DeviceInfo.Hotplugged = true
+	device.AttachCount = 1
 	return nil
 }
 
 // Detach is standard interface of api.Device, it's used to remove device from some
 // DeviceReceiver
 func (device *VFIODevice) Detach(devReceiver api.DeviceReceiver) error {
-	if !device.DeviceInfo.Hotplugged {
+	skip, err := device.bumpAttachCount(false)
+	if err != nil {
+		return err
+	}
+	if skip {
 		return nil
 	}
 
@@ -104,7 +112,7 @@ func (device *VFIODevice) Detach(devReceiver api.DeviceReceiver) error {
 		"device-group": device.DeviceInfo.HostPath,
 		"device-type":  "vfio-passthrough",
 	}).Info("Device group detached")
-	device.DeviceInfo.Hotplugged = false
+	device.AttachCount = 0
 	return nil
 }
 
@@ -118,7 +126,7 @@ func (device *VFIODevice) GetDeviceInfo() interface{} {
 	return device.vfioDevs
 }
 
-// It should implement IsAttached() and DeviceID() as api.Device implementation
+// It should implement GetAttachCount() and DeviceID() as api.Device implementation
 // here it shares function from *GenericDevice so we don't need duplicate codes
 
 // getBDF returns the BDF of pci device

virtcontainers/device/drivers/vhost_user_blk.go

@@ -27,7 +27,11 @@ type VhostUserBlkDevice struct {
 // Attach is standard interface of api.Device, it's used to add device to some
 // DeviceReceiver
 func (device *VhostUserBlkDevice) Attach(devReceiver api.DeviceReceiver) (err error) {
-	if device.DeviceInfo.Hotplugged {
+	skip, err := device.bumpAttachCount(true)
+	if err != nil {
+		return err
+	}
+	if skip {
 		return nil
 	}
 
@@ -43,7 +47,7 @@ func (device *VhostUserBlkDevice) Attach(devReceiver api.DeviceReceiver) (err er
 
 	defer func() {
 		if err == nil {
-			device.DeviceInfo.Hotplugged = true
+			device.AttachCount = 1
 		}
 	}()
 	return devReceiver.AppendDevice(device)
@@ -52,11 +56,15 @@ func (device *VhostUserBlkDevice) Attach(devReceiver api.DeviceReceiver) (err er
 // Detach is standard interface of api.Device, it's used to remove device from some
 // DeviceReceiver
 func (device *VhostUserBlkDevice) Detach(devReceiver api.DeviceReceiver) error {
-	if !device.DeviceInfo.Hotplugged {
+	skip, err := device.bumpAttachCount(true)
+	if err != nil {
+		return err
+	}
+	if skip {
 		return nil
 	}
 
-	device.DeviceInfo.Hotplugged = false
+	device.AttachCount = 0
 	return nil
 }
 
@@ -71,5 +79,5 @@ func (device *VhostUserBlkDevice) GetDeviceInfo() interface{} {
 	return &device.VhostUserDeviceAttrs
 }
 
-// It should implement IsAttached() and DeviceID() as api.Device implementation
+// It should implement GetAttachCount() and DeviceID() as api.Device implementation
 // here it shares function from *GenericDevice so we don't need duplicate codes