From 03fb9c50c180d3359178c30e06f1122df312ae76 Mon Sep 17 00:00:00 2001 From: Bo Chen Date: Wed, 15 Jul 2020 19:03:36 -0700 Subject: [PATCH 1/2] clh: Set 'Id' explicitly while hotplugging block device To support unplug block device, we need to set the 'Id' explicitly while hotplugging devices with cloud-hypervisor HTTP API. Fixes: #2832 Signed-off-by: Bo Chen --- virtcontainers/clh.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/virtcontainers/clh.go b/virtcontainers/clh.go index cc6e4dfcab..1668bd3e30 100644 --- a/virtcontainers/clh.go +++ b/virtcontainers/clh.go @@ -402,7 +402,11 @@ func (clh *cloudHypervisor) getThreadIDs() (vcpuThreadIDs, error) { return vcpuInfo, nil } -func (clh *cloudHypervisor) hotplugBlockDevice(drive *config.BlockDrive) error { +func clhDriveIndexToID(i int) string { + return "clh_drive_" + strconv.Itoa(i) +} + +func (clh *cloudHypervisor) hotplugAddBlockDevice(drive *config.BlockDrive) error { if clh.config.BlockDeviceDriver != config.VirtioBlock { return fmt.Errorf("incorrect hypervisor configuration on 'block_device_driver':"+ " using '%v' but only support '%v'", clh.config.BlockDeviceDriver, config.VirtioBlock) @@ -417,6 +421,8 @@ func (clh *cloudHypervisor) hotplugBlockDevice(drive *config.BlockDrive) error { return openAPIClientError(err) } + driveID := clhDriveIndexToID(drive.Index) + //Explicitly set PCIAddr to NULL, so that VirtPath can be used drive.PCIAddr = "" @@ -427,6 +433,7 @@ func (clh *cloudHypervisor) hotplugBlockDevice(drive *config.BlockDrive) error { Path: drive.File, Readonly: drive.ReadOnly, VhostUser: false, + Id: driveID, } _, _, err = cl.VmAddDiskPut(ctx, blkDevice) } @@ -461,7 +468,7 @@ func (clh *cloudHypervisor) hotplugAddDevice(devInfo interface{}, devType device switch devType { case blockDev: drive := devInfo.(*config.BlockDrive) - return nil, clh.hotplugBlockDevice(drive) + return nil, clh.hotplugAddBlockDevice(drive) case vfioDev: device := devInfo.(*config.VFIODev) return nil, clh.hotPlugVFIODevice(*device) From 44b58e4151d1fc7debed41274b65c37233a437e3 Mon Sep 17 00:00:00 2001 From: Bo Chen Date: Thu, 13 Aug 2020 16:46:27 -0700 Subject: [PATCH 2/2] clh: Add support to unplug block devices This patch enables kata+clh to unplug block devices, which is required to pass cri-o integration tests. Fixes: #2832 Signed-off-by: Bo Chen --- virtcontainers/clh.go | 36 ++++++++++++++++++++++++++++++++++-- virtcontainers/clh_test.go | 31 +++++++++++++++++++++++++++---- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/virtcontainers/clh.go b/virtcontainers/clh.go index 1668bd3e30..fa44feac1a 100644 --- a/virtcontainers/clh.go +++ b/virtcontainers/clh.go @@ -89,6 +89,8 @@ type clhClient interface { VmAddDevicePut(ctx context.Context, vmAddDevice chclient.VmAddDevice) (chclient.PciDeviceInfo, *http.Response, error) // Add a new disk device to the VM VmAddDiskPut(ctx context.Context, diskConfig chclient.DiskConfig) (chclient.PciDeviceInfo, *http.Response, error) + // Remove a device from the VM + VmRemoveDevicePut(ctx context.Context, vmRemoveDevice chclient.VmRemoveDevice) (*http.Response, error) } type CloudHypervisorVersion struct { @@ -478,9 +480,39 @@ 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) hotplugRemoveDevice(devInfo interface{}, devType deviceType) (interface{}, error) { - clh.Logger().WithField("function", "hotplugRemoveDevice").Warn("hotplug remove device not supported") - return nil, nil + span, _ := clh.trace("hotplugRemoveDevice") + defer span.Finish() + + switch devType { + case blockDev: + return nil, clh.hotplugRemoveBlockDevice(devInfo.(*config.BlockDrive)) + 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) + } } func (clh *cloudHypervisor) hypervisorConfig() HypervisorConfig { diff --git a/virtcontainers/clh_test.go b/virtcontainers/clh_test.go index 9b76884fb5..5be13e6d27 100644 --- a/virtcontainers/clh_test.go +++ b/virtcontainers/clh_test.go @@ -104,6 +104,11 @@ func (c *clhClientMock) VmAddDiskPut(ctx context.Context, diskConfig chclient.Di return chclient.PciDeviceInfo{}, nil, nil } +//nolint:golint +func (c *clhClientMock) VmRemoveDevicePut(ctx context.Context, vmRemoveDevice chclient.VmRemoveDevice) (*http.Response, error) { + return nil, nil +} + func TestCloudHypervisorAddVSock(t *testing.T) { assert := assert.New(t) clh := cloudHypervisor{} @@ -363,7 +368,7 @@ func TestCheckVersion(t *testing.T) { } } -func TestCloudHypervisorHotplugBlockDevice(t *testing.T) { +func TestCloudHypervisorHotplugAddBlockDevice(t *testing.T) { assert := assert.New(t) clhConfig, err := newClhConfig() @@ -374,13 +379,31 @@ func TestCloudHypervisorHotplugBlockDevice(t *testing.T) { clh.APIClient = &clhClientMock{} clh.config.BlockDeviceDriver = config.VirtioBlock - err = clh.hotplugBlockDevice(&config.BlockDrive{Pmem: false}) + err = clh.hotplugAddBlockDevice(&config.BlockDrive{Pmem: false}) assert.NoError(err, "Hotplug disk block device expected no error") - err = clh.hotplugBlockDevice(&config.BlockDrive{Pmem: true}) + err = clh.hotplugAddBlockDevice(&config.BlockDrive{Pmem: true}) assert.Error(err, "Hotplug pmem block device expected error") clh.config.BlockDeviceDriver = config.VirtioSCSI - err = clh.hotplugBlockDevice(&config.BlockDrive{Pmem: false}) + err = clh.hotplugAddBlockDevice(&config.BlockDrive{Pmem: false}) assert.Error(err, "Hotplug block device not using 'virtio-blk' expected error") } + +func TestCloudHypervisorHotplugRemoveBlockDevice(t *testing.T) { + assert := assert.New(t) + + clhConfig, err := newClhConfig() + assert.NoError(err) + + clh := &cloudHypervisor{} + 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.hotplugRemoveBlockDevice(&config.BlockDrive{Pmem: true}) + assert.Error(err, "Hotplug remove pmem block device expected error") +}