Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

Commit f1ef63e

Browse files
author
Sebastien Boeuf
authored
Merge pull request #1394 from WeiZhang555/improve-readability
refactor: improve readability of `bumpAttachCount`
2 parents 8e72cf1 + 26a9b72 commit f1ef63e

File tree

7 files changed

+53
-72
lines changed

7 files changed

+53
-72
lines changed

‎virtcontainers/device/drivers/block.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ func (device *BlockDevice) Attach(devReceiver api.DeviceReceiver) (err error) {
5151
defer func() {
5252
if err != nil {
5353
devReceiver.DecrementSandboxBlockIndex()
54-
} else {
55-
device.AttachCount = 1
54+
device.bumpAttachCount(false)
5655
}
5756
}()
5857

@@ -122,13 +121,18 @@ func (device *BlockDevice) Detach(devReceiver api.DeviceReceiver) error {
122121
return nil
123122
}
124123

124+
defer func() {
125+
if err != nil {
126+
device.bumpAttachCount(true)
127+
}
128+
}()
129+
125130
deviceLogger().WithField("device", device.DeviceInfo.HostPath).Info("Unplugging block device")
126131

127-
if err := devReceiver.HotplugRemoveDevice(device, config.DeviceBlock); err != nil {
132+
if err = devReceiver.HotplugRemoveDevice(device, config.DeviceBlock); err != nil {
128133
deviceLogger().WithError(err).Error("Failed to unplug block device")
129134
return err
130135
}
131-
device.AttachCount = 0
132136
return nil
133137
}
134138

‎virtcontainers/device/drivers/generic.go‎

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -32,28 +32,14 @@ func NewGenericDevice(devInfo *config.DeviceInfo) *GenericDevice {
3232

3333
// Attach is standard interface of api.Device
3434
func (device *GenericDevice) Attach(devReceiver api.DeviceReceiver) error {
35-
skip, err := device.bumpAttachCount(true)
36-
if err != nil {
37-
return err
38-
}
39-
if skip {
40-
return nil
41-
}
42-
device.AttachCount = 1
43-
return nil
35+
_, err := device.bumpAttachCount(true)
36+
return err
4437
}
4538

4639
// Detach is standard interface of api.Device
4740
func (device *GenericDevice) Detach(devReceiver api.DeviceReceiver) error {
48-
skip, err := device.bumpAttachCount(false)
49-
if err != nil {
50-
return err
51-
}
52-
if skip {
53-
return nil
54-
}
55-
device.AttachCount = 0
56-
return nil
41+
_, err := device.bumpAttachCount(false)
42+
return err
5743
}
5844

5945
// DeviceType is standard interface of api.Device, it returns device type
@@ -107,6 +93,7 @@ func (device *GenericDevice) bumpAttachCount(attach bool) (skip bool, err error)
10793
switch device.AttachCount {
10894
case 0:
10995
// do real attach
96+
device.AttachCount++
11097
return false, nil
11198
case intMax:
11299
return true, fmt.Errorf("device was attached too many times")
@@ -120,6 +107,7 @@ func (device *GenericDevice) bumpAttachCount(attach bool) (skip bool, err error)
120107
return true, fmt.Errorf("detaching a device that wasn't attached")
121108
case 1:
122109
// do real work
110+
device.AttachCount--
123111
return false, nil
124112
default:
125113
device.AttachCount--

‎virtcontainers/device/drivers/generic_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ func TestBumpAttachCount(t *testing.T) {
2121
}
2222

2323
data := []testData{
24-
{true, 0, 0, false, false},
24+
{true, 0, 1, false, false},
2525
{true, 1, 2, true, false},
2626
{true, intMax, intMax, true, true},
2727
{false, 0, 0, true, true},
28-
{false, 1, 1, false, false},
28+
{false, 1, 0, false, false},
2929
{false, intMax, intMax - 1, true, false},
3030
}
3131

‎virtcontainers/device/drivers/vfio.go‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func NewVFIODevice(devInfo *config.DeviceInfo) *VFIODevice {
4747

4848
// Attach is standard interface of api.Device, it's used to add device to some
4949
// DeviceReceiver
50-
func (device *VFIODevice) Attach(devReceiver api.DeviceReceiver) error {
50+
func (device *VFIODevice) Attach(devReceiver api.DeviceReceiver) (retErr error) {
5151
skip, err := device.bumpAttachCount(true)
5252
if err != nil {
5353
return err
@@ -56,6 +56,12 @@ func (device *VFIODevice) Attach(devReceiver api.DeviceReceiver) error {
5656
return nil
5757
}
5858

59+
defer func() {
60+
if retErr != nil {
61+
device.bumpAttachCount(false)
62+
}
63+
}()
64+
5965
vfioGroup := filepath.Base(device.DeviceInfo.HostPath)
6066
iommuDevicesPath := filepath.Join(config.SysIOMMUPath, vfioGroup, "devices")
6167

@@ -90,13 +96,12 @@ func (device *VFIODevice) Attach(devReceiver api.DeviceReceiver) error {
9096
"device-group": device.DeviceInfo.HostPath,
9197
"device-type": "vfio-passthrough",
9298
}).Info("Device group attached")
93-
device.AttachCount = 1
9499
return nil
95100
}
96101

97102
// Detach is standard interface of api.Device, it's used to remove device from some
98103
// DeviceReceiver
99-
func (device *VFIODevice) Detach(devReceiver api.DeviceReceiver) error {
104+
func (device *VFIODevice) Detach(devReceiver api.DeviceReceiver) (retErr error) {
100105
skip, err := device.bumpAttachCount(false)
101106
if err != nil {
102107
return err
@@ -105,6 +110,12 @@ func (device *VFIODevice) Detach(devReceiver api.DeviceReceiver) error {
105110
return nil
106111
}
107112

113+
defer func() {
114+
if retErr != nil {
115+
device.bumpAttachCount(true)
116+
}
117+
}()
118+
108119
// hotplug a VFIO device is actually hotplugging a group of iommu devices
109120
if err := devReceiver.HotplugRemoveDevice(device, config.DeviceVFIO); err != nil {
110121
deviceLogger().WithError(err).Error("Failed to remove device")
@@ -115,7 +126,6 @@ func (device *VFIODevice) Detach(devReceiver api.DeviceReceiver) error {
115126
"device-group": device.DeviceInfo.HostPath,
116127
"device-type": "vfio-passthrough",
117128
}).Info("Device group detached")
118-
device.AttachCount = 0
119129
return nil
120130
}
121131

‎virtcontainers/device/drivers/vhost_user_blk.go‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ func (device *VhostUserBlkDevice) Attach(devReceiver api.DeviceReceiver) (err er
3434
if skip {
3535
return nil
3636
}
37+
defer func() {
38+
if err != nil {
39+
device.bumpAttachCount(false)
40+
}
41+
}()
3742

3843
// generate a unique ID to be used for hypervisor commandline fields
3944
randBytes, err := utils.GenerateRandomBytes(8)
@@ -45,27 +50,14 @@ func (device *VhostUserBlkDevice) Attach(devReceiver api.DeviceReceiver) (err er
4550
device.DevID = id
4651
device.Type = device.DeviceType()
4752

48-
defer func() {
49-
if err == nil {
50-
device.AttachCount = 1
51-
}
52-
}()
5353
return devReceiver.AppendDevice(device)
5454
}
5555

5656
// Detach is standard interface of api.Device, it's used to remove device from some
5757
// DeviceReceiver
5858
func (device *VhostUserBlkDevice) Detach(devReceiver api.DeviceReceiver) error {
59-
skip, err := device.bumpAttachCount(false)
60-
if err != nil {
61-
return err
62-
}
63-
if skip {
64-
return nil
65-
}
66-
67-
device.AttachCount = 0
68-
return nil
59+
_, err := device.bumpAttachCount(false)
60+
return err
6961
}
7062

7163
// DeviceType is standard interface of api.Device, it returns device type

‎virtcontainers/device/drivers/vhost_user_net.go‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,12 @@ func (device *VhostUserNetDevice) Attach(devReceiver api.DeviceReceiver) (err er
3535
return nil
3636
}
3737

38+
defer func() {
39+
if err != nil {
40+
device.bumpAttachCount(false)
41+
}
42+
}()
43+
3844
// generate a unique ID to be used for hypervisor commandline fields
3945
randBytes, err := utils.GenerateRandomBytes(8)
4046
if err != nil {
@@ -45,27 +51,14 @@ func (device *VhostUserNetDevice) Attach(devReceiver api.DeviceReceiver) (err er
4551
device.DevID = id
4652
device.Type = device.DeviceType()
4753

48-
defer func() {
49-
if err == nil {
50-
device.AttachCount = 1
51-
}
52-
}()
5354
return devReceiver.AppendDevice(device)
5455
}
5556

5657
// Detach is standard interface of api.Device, it's used to remove device from some
5758
// DeviceReceiver
5859
func (device *VhostUserNetDevice) Detach(devReceiver api.DeviceReceiver) error {
59-
skip, err := device.bumpAttachCount(false)
60-
if err != nil {
61-
return err
62-
}
63-
if skip {
64-
return nil
65-
}
66-
67-
device.AttachCount = 0
68-
return nil
60+
_, err := device.bumpAttachCount(false)
61+
return err
6962
}
7063

7164
// DeviceType is standard interface of api.Device, it returns device type

‎virtcontainers/device/drivers/vhost_user_scsi.go‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,12 @@ func (device *VhostUserSCSIDevice) Attach(devReceiver api.DeviceReceiver) (err e
3535
return nil
3636
}
3737

38+
defer func() {
39+
if err != nil {
40+
device.bumpAttachCount(false)
41+
}
42+
}()
43+
3844
// generate a unique ID to be used for hypervisor commandline fields
3945
randBytes, err := utils.GenerateRandomBytes(8)
4046
if err != nil {
@@ -45,26 +51,14 @@ func (device *VhostUserSCSIDevice) Attach(devReceiver api.DeviceReceiver) (err e
4551
device.DevID = id
4652
device.Type = device.DeviceType()
4753

48-
defer func() {
49-
if err == nil {
50-
device.AttachCount = 1
51-
}
52-
}()
5354
return devReceiver.AppendDevice(device)
5455
}
5556

5657
// Detach is standard interface of api.Device, it's used to remove device from some
5758
// DeviceReceiver
5859
func (device *VhostUserSCSIDevice) Detach(devReceiver api.DeviceReceiver) error {
59-
skip, err := device.bumpAttachCount(false)
60-
if err != nil {
61-
return err
62-
}
63-
if skip {
64-
return nil
65-
}
66-
device.AttachCount = 0
67-
return nil
60+
_, err := device.bumpAttachCount(false)
61+
return err
6862
}
6963

7064
// DeviceType is standard interface of api.Device, it returns device type

0 commit comments

Comments
 (0)