github/kata-containers
1
0
Fork 1
mirror of https://github.com/kata-containers/kata-containers.git synced 2025-07-21 10:51:05 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #4197 from fidencio/topic/workaround-race-condition-on-removing-and-adding-device-with-clh

Browse Source 
clh: Rely on Cloud Hypervisor for generating the device ID
This commit is contained in:
Fabiano Fidêncio 2022-05-04 11:50:14 +02:00 committed by GitHub
commit ec250c10e9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 9 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/runtime/virtcontainers/clh.go
View File

@ -165,6 +165,7 @@ type cloudHypervisor struct {
APIClient clhClient APIClient clhClient
ctx context.Context ctx context.Context
id string id string
devicesIds map[string]string
vmconfig chclient.VmConfig vmconfig chclient.VmConfig
state CloudHypervisorState state CloudHypervisorState
config HypervisorConfig config HypervisorConfig
@ -358,6 +359,7 @@ func (clh *cloudHypervisor) CreateVM(ctx context.Context, id string, network Net
clh.id = id clh.id = id
clh.state.state = clhNotReady clh.state.state = clhNotReady
clh.devicesIds = make(map[string]string)
clh.Logger().WithField("function", "CreateVM").Info("creating Sandbox") clh.Logger().WithField("function", "CreateVM").Info("creating Sandbox")
@ -670,7 +672,6 @@ func (clh *cloudHypervisor) hotplugAddBlockDevice(drive *config.BlockDrive) erro
clhDisk := *chclient.NewDiskConfig(drive.File) clhDisk := *chclient.NewDiskConfig(drive.File)
clhDisk.Readonly = &drive.ReadOnly clhDisk.Readonly = &drive.ReadOnly
clhDisk.VhostUser = func(b bool) *bool { return &b }(false) clhDisk.VhostUser = func(b bool) *bool { return &b }(false)
clhDisk.Id = &driveID
diskRateLimiterConfig := clh.getDiskRateLimiterConfig() diskRateLimiterConfig := clh.getDiskRateLimiterConfig()
if diskRateLimiterConfig != nil { if diskRateLimiterConfig != nil {
@ -683,6 +684,7 @@ func (clh *cloudHypervisor) hotplugAddBlockDevice(drive *config.BlockDrive) erro
return fmt.Errorf("failed to hotplug block device %+v %s", drive, openAPIClientError(err)) return fmt.Errorf("failed to hotplug block device %+v %s", drive, openAPIClientError(err))
} }
clh.devicesIds[driveID] = pciInfo.GetId()
drive.PCIPath, err = clhPciInfoToPath(pciInfo) drive.PCIPath, err = clhPciInfoToPath(pciInfo)
return err return err
@ -696,11 +698,11 @@ func (clh *cloudHypervisor) hotPlugVFIODevice(device *config.VFIODev) error {
// Create the clh device config via the constructor to ensure default values are properly assigned // Create the clh device config via the constructor to ensure default values are properly assigned
clhDevice := *chclient.NewVmAddDevice() clhDevice := *chclient.NewVmAddDevice()
clhDevice.Path = &device.SysfsDev clhDevice.Path = &device.SysfsDev
clhDevice.Id = &device.ID
pciInfo, _, err := cl.VmAddDevicePut(ctx, clhDevice) pciInfo, _, err := cl.VmAddDevicePut(ctx, clhDevice)
if err != nil { if err != nil {
return fmt.Errorf("Failed to hotplug device %+v %s", device, openAPIClientError(err)) return fmt.Errorf("Failed to hotplug device %+v %s", device, openAPIClientError(err))
} }
clh.devicesIds[device.ID] = pciInfo.GetId()
// clh doesn't use bridges, so the PCI path is simply the slot // clh doesn't use bridges, so the PCI path is simply the slot
// number of the device. This will break if clh starts using // number of the device. This will break if clh starts using
@ -761,13 +763,15 @@ func (clh *cloudHypervisor) HotplugRemoveDevice(ctx context.Context, devInfo int
ctx, cancel := context.WithTimeout(context.Background(), clhHotPlugAPITimeout*time.Second) ctx, cancel := context.WithTimeout(context.Background(), clhHotPlugAPITimeout*time.Second)
defer cancel() defer cancel()
originalDeviceID := clh.devicesIds[deviceID]
remove := *chclient.NewVmRemoveDevice() remove := *chclient.NewVmRemoveDevice()
remove.Id = &deviceID remove.Id = &originalDeviceID
_, err := cl.VmRemoveDevicePut(ctx, remove) _, err := cl.VmRemoveDevicePut(ctx, remove)
if err != nil { if err != nil {
err = fmt.Errorf("failed to hotplug remove (unplug) device %+v: %s", devInfo, openAPIClientError(err)) err = fmt.Errorf("failed to hotplug remove (unplug) device %+v: %s", devInfo, openAPIClientError(err))
} }
delete(clh.devicesIds, deviceID)
return nil, err return nil, err
} }

2
src/runtime/virtcontainers/clh_test.go
View File

@ -563,6 +563,7 @@ func TestCloudHypervisorHotplugAddBlockDevice(t *testing.T) {
clh := &cloudHypervisor{} clh := &cloudHypervisor{}
clh.config = clhConfig clh.config = clhConfig
clh.APIClient = &clhClientMock{} clh.APIClient = &clhClientMock{}
clh.devicesIds = make(map[string]string)
clh.config.BlockDeviceDriver = config.VirtioBlock clh.config.BlockDeviceDriver = config.VirtioBlock
err = clh.hotplugAddBlockDevice(&config.BlockDrive{Pmem: false}) err = clh.hotplugAddBlockDevice(&config.BlockDrive{Pmem: false})
@ -585,6 +586,7 @@ func TestCloudHypervisorHotplugRemoveDevice(t *testing.T) {
clh := &cloudHypervisor{} clh := &cloudHypervisor{}
clh.config = clhConfig clh.config = clhConfig
clh.APIClient = &clhClientMock{} clh.APIClient = &clhClientMock{}
clh.devicesIds = make(map[string]string)
_, err = clh.HotplugRemoveDevice(context.Background(), &config.BlockDrive{}, BlockDev) _, err = clh.HotplugRemoveDevice(context.Background(), &config.BlockDrive{}, BlockDev)
assert.NoError(err, "Hotplug remove block device expected no error") assert.NoError(err, "Hotplug remove block device expected no error")