clh: Consolidate the code path for device unplug

In cloud-hypervisor, it provides a single unified way of unplugging
devices, e.g. the `/vm.RemoveDevice` HTTP API. Taking advantage of this
API, we can simplify our implementation of `hotplugRemoveDevice` in
`clh.go`, where we can consolidate similar code paths for different
device unplug (e.g. no need to implement `hotplugRemoveBlockDevice` and
`hotplugRemoveVfioDevice` separately). We will only need to retrieve the
right `deviceID` based on the type of devices, and use the single
unified HTTP API for device unplug.

Fixes: #3001

Signed-off-by: Bo Chen <[email protected]>

virtcontainers/clh.go

@@ -473,55 +473,35 @@ func (clh *cloudHypervisor) hotplugAddDevice(devInfo interface{}, devType device
 
 }
 
-func (clh *cloudHypervisor) hotplugRemoveBlockDevice(drive *config.BlockDrive) error {
-	cl := clh.client()
-	ctx, cancel := context.WithTimeout(context.Background(), clhHotPlugAPITimeout*time.Second)
-	defer cancel()
-
-	driveID := clhDriveIndexToID(drive.Index)
-
-	if drive.Pmem {
-		return fmt.Errorf("pmem device hotplug remove not supported")
-	}
-
-	_, err := cl.VmRemoveDevicePut(ctx, chclient.VmRemoveDevice{Id: driveID})
-
-	if err != nil {
-		err = fmt.Errorf("failed to hotplug remove block device %+v %s", drive, openAPIClientError(err))
-	}
-
-	return err
-}
-
-func (clh *cloudHypervisor) hotplugRemoveVfioDevice(device *config.VFIODev) error {
-	cl := clh.client()
-	ctx, cancel := context.WithTimeout(context.Background(), clhHotPlugAPITimeout*time.Second)
-	defer cancel()
-
-	_, err := cl.VmRemoveDevicePut(ctx, chclient.VmRemoveDevice{Id: device.ID})
-
-	if err != nil {
-		err = fmt.Errorf("failed to hotplug remove vfio device %+v %s", device, openAPIClientError(err))
-	}
-
-	return err
-}
-
 func (clh *cloudHypervisor) hotplugRemoveDevice(devInfo interface{}, devType deviceType) (interface{}, error) {
 	span, _ := clh.trace("hotplugRemoveDevice")
 	defer span.Finish()
 
+	var deviceID string
+
 	switch devType {
 	case blockDev:
-		return nil, clh.hotplugRemoveBlockDevice(devInfo.(*config.BlockDrive))
+		deviceID = clhDriveIndexToID(devInfo.(*config.BlockDrive).Index)
 	case vfioDev:
-		return nil, clh.hotplugRemoveVfioDevice(devInfo.(*config.VFIODev))
+		deviceID = devInfo.(*config.VFIODev).ID
 	default:
 		clh.Logger().WithFields(log.Fields{"devInfo": devInfo,
 			"deviceType": devType}).Error("hotplugRemoveDevice: unsupported device")
 		return nil, fmt.Errorf("Could not hot remove device: unsupported device: %v, type: %v",
 			devInfo, devType)
 	}
+
+	cl := clh.client()
+	ctx, cancel := context.WithTimeout(context.Background(), clhHotPlugAPITimeout*time.Second)
+	defer cancel()
+
+	_, err := cl.VmRemoveDevicePut(ctx, chclient.VmRemoveDevice{Id: deviceID})
+
+	if err != nil {
+		err = fmt.Errorf("failed to hotplug remove (unplug) device %+v: %s", devInfo, openAPIClientError(err))
+	}
+
+	return nil, err
 }
 
 func (clh *cloudHypervisor) hypervisorConfig() HypervisorConfig {

virtcontainers/clh_test.go

@@ -390,7 +390,7 @@ func TestCloudHypervisorHotplugAddBlockDevice(t *testing.T) {
 	assert.Error(err, "Hotplug block device not using 'virtio-blk' expected error")
 }
 
-func TestCloudHypervisorHotplugRemoveBlockDevice(t *testing.T) {
+func TestCloudHypervisorHotplugRemoveDevice(t *testing.T) {
 	assert := assert.New(t)
 
 	clhConfig, err := newClhConfig()
@@ -400,10 +400,12 @@ func TestCloudHypervisorHotplugRemoveBlockDevice(t *testing.T) {
 	clh.config = clhConfig
 	clh.APIClient = &clhClientMock{}
 
-	clh.config.BlockDeviceDriver = config.VirtioBlock
-	err = clh.hotplugRemoveBlockDevice(&config.BlockDrive{Pmem: false})
-	assert.NoError(err, "Hotplug remove disk block device expected no error")
+	_, err = clh.hotplugRemoveDevice(&config.BlockDrive{}, blockDev)
+	assert.NoError(err, "Hotplug remove block device expected no error")
+
+	_, err = clh.hotplugRemoveDevice(&config.VFIODev{}, vfioDev)
+	assert.NoError(err, "Hotplug remove vfio block device expected no error")
 
-	err = clh.hotplugRemoveBlockDevice(&config.BlockDrive{Pmem: true})
+	_, err = clh.hotplugRemoveDevice(nil, netDev)
 	assert.Error(err, "Hotplug remove pmem block device expected error")
 }