Merge pull request #2114 from devimc/topic/backport-1.8/fixCRIO+FC

stable-1.8: backport fixes for CRIO+firecracker

virtcontainers/fc.go

@@ -593,19 +593,18 @@ func (fc *firecracker) fcAddBlockDrive(drive config.BlockDrive) error {
 }
 
 // Firecracker supports replacing the host drive used once the VM has booted up
-func (fc *firecracker) fcUpdateBlockDrive(drive config.BlockDrive) error {
+func (fc *firecracker) fcUpdateBlockDrive(path, id string) error {
 	span, _ := fc.trace("fcUpdateBlockDrive")
 	defer span.Finish()
 
 	// Use the global block index as an index into the pool of the devices
 	// created for firecracker.
-	driveID := fcDriveIndexToID(drive.Index)
 	driveParams := ops.NewPatchGuestDriveByIDParams()
-	driveParams.SetDriveID(driveID)
+	driveParams.SetDriveID(id)
 
 	driveFc := &models.PartialDrive{
-		DriveID:    &driveID,
-		PathOnHost: &drive.File, //This is the only property that can be modified
+		DriveID:    &id,
+		PathOnHost: &path, //This is the only property that can be modified
 	}
 	driveParams.SetBody(driveFc)
 	_, err := fc.client().Operations.PatchGuestDriveByID(driveParams)
@@ -619,11 +618,10 @@ func (fc *firecracker) fcUpdateBlockDrive(drive config.BlockDrive) error {
 		actionType := "BlockDeviceRescan"
 		actionInfo := &models.InstanceActionInfo{
 			ActionType: &actionType,
-			Payload:    driveID,
+			Payload:    id,
 		}
 		actionParams.SetInfo(actionInfo)
-		_, err = fc.client().Operations.CreateSyncAction(actionParams)
-		if err != nil {
+		if _, err := fc.client().Operations.CreateSyncAction(actionParams); err != nil {
 			return err
 		}
 	}
@@ -667,26 +665,64 @@ func (fc *firecracker) addDevice(devInfo interface{}, devType deviceType) error
 	return nil
 }
 
+// hotplugBlockDevice supported in Firecracker VMM
+// hot add or remove a block device.
+func (fc *firecracker) hotplugBlockDevice(drive config.BlockDrive, op operation) (interface{}, error) {
+	var path string
+	driveID := fcDriveIndexToID(drive.Index)
+
+	if op == addDevice {
+		path = drive.File
+	} else {
+		// use previous raw file created at createDiskPool, that way
+		// the resource is released by firecracker and it can be destroyed in the host
+		hostURL, err := fc.store.Raw("")
+		if err != nil {
+			return nil, err
+		}
+
+		// We get a full URL from Raw(), we need to parse it.
+		u, err := url.Parse(hostURL)
+		if err != nil {
+			return nil, err
+		}
+
+		path = u.Path
+	}
+
+	return nil, fc.fcUpdateBlockDrive(path, driveID)
+}
+
 // hotplugAddDevice supported in Firecracker VMM
 func (fc *firecracker) hotplugAddDevice(devInfo interface{}, devType deviceType) (interface{}, error) {
 	span, _ := fc.trace("hotplugAddDevice")
 	defer span.Finish()
 
 	switch devType {
 	case blockDev:
-		//The drive placeholder has to exist prior to Update
-		return nil, fc.fcUpdateBlockDrive(*devInfo.(*config.BlockDrive))
+		return fc.hotplugBlockDevice(*devInfo.(*config.BlockDrive), addDevice)
 	default:
 		fc.Logger().WithFields(logrus.Fields{"devInfo": devInfo,
 			"deviceType": devType}).Warn("hotplugAddDevice: unsupported device")
-		return nil, fmt.Errorf("hotplugAddDevice: unsupported device: devInfo:%v, deviceType%v",
+		return nil, fmt.Errorf("Could not hot add device: unsupported device: %v, type: %v",
 			devInfo, devType)
 	}
 }
 
-// hotplugRemoveDevice supported in Firecracker VMM, but no-op
+// hotplugRemoveDevice supported in Firecracker VMM
 func (fc *firecracker) hotplugRemoveDevice(devInfo interface{}, devType deviceType) (interface{}, error) {
-	return nil, nil
+	span, _ := fc.trace("hotplugRemoveDevice")
+	defer span.Finish()
+
+	switch devType {
+	case blockDev:
+		return fc.hotplugBlockDevice(*devInfo.(*config.BlockDrive), removeDevice)
+	default:
+		fc.Logger().WithFields(logrus.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)
+	}
 }
 
 // getSandboxConsole builds the path of the console where we can read

virtcontainers/hypervisor.go

@@ -24,6 +24,11 @@ type HypervisorType string
 
 type operation int
 
+const (
+	addDevice operation = iota
+	removeDevice
+)
+
 const (
 	// FirecrackerHypervisor is the FC hypervisor.
 	FirecrackerHypervisor HypervisorType = "firecracker"

virtcontainers/qemu.go

@@ -117,11 +117,6 @@ var defaultKernelParameters = []Param{
 	{"panic", "1"},
 }
 
-const (
-	addDevice operation = iota
-	removeDevice
-)
-
 type qmpLogger struct {
 	logger *logrus.Entry
 }