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

Commit d88d468

Browse files
dgibsondgibson
committed
device: Index all devices in spec before updating them
The agent needs to update device entries in the OCI spec so that it has the correct major:minor numbers for the guest, which may differ from the host. Entries in the main device list are looked up by device path, but entries in the device resources list are looked up by (host) major:minor. This is done one device at a time, updating as we go in updateSpecDeviceList(). But since the host and guest have different namespaces, one device might have the same major:minor as a different device on the host. In that case we could update one resource entry to the correct guest values, then mistakenly update it again because it now matches a different host device. To avoid this, rather than looking up and updating one by one, we make all the lookups in advance, creating a map from (host) device path to the indices in the spec where the device and resource entries can be found. Fixes: #834 Signed-off-by: David Gibson <[email protected]>
1 parent e82e2f7 commit d88d468

File tree

2 files changed

+205
-64
lines changed

2 files changed

+205
-64
lines changed

‎device.go‎

Lines changed: 74 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,15 @@ var (
7171
scsiHostPath = filepath.Join(sysClassPrefix, "scsi_host")
7272
)
7373

74-
type deviceHandler func(ctx context.Context, device pb.Device, spec *pb.Spec, s *sandbox) error
74+
// Stores a mapping of device names (in host / outer container naming)
75+
// to the device and resources slots in a container spec
76+
type devIndexEntry struct {
77+
idx int
78+
resourceIdx []int
79+
}
80+
type devIndex map[string]devIndexEntry
81+
82+
type deviceHandler func(ctx context.Context, device pb.Device, spec *pb.Spec, s *sandbox, devIdx devIndex) error
7583

7684
var deviceHandlerList = map[string]deviceHandler{
7785
driverMmioBlkType: virtioMmioBlkDeviceHandler,
@@ -192,15 +200,15 @@ func getPCIDeviceNameImpl(s *sandbox, pciID string) (string, error) {
192200

193201
// device.Id should be the predicted device name (vda, vdb, ...)
194202
// device.VmPath already provides a way to send it in
195-
func virtioMmioBlkDeviceHandler(_ context.Context, device pb.Device, spec *pb.Spec, s *sandbox) error {
203+
func virtioMmioBlkDeviceHandler(_ context.Context, device pb.Device, spec *pb.Spec, s *sandbox, devIdx devIndex) error {
196204
if device.VmPath == "" {
197205
return fmt.Errorf("Invalid path for virtioMmioBlkDevice")
198206
}
199207

200-
return updateSpecDeviceList(device, spec)
208+
return updateSpecDeviceList(device, spec, devIdx)
201209
}
202210

203-
func virtioBlkCCWDeviceHandler(ctx context.Context, device pb.Device, spec *pb.Spec, s *sandbox) error {
211+
func virtioBlkCCWDeviceHandler(ctx context.Context, device pb.Device, spec *pb.Spec, s *sandbox, devIdx devIndex) error {
204212
devPath, err := getBlkCCWDevPath(s, device.Id)
205213
if err != nil {
206214
return err
@@ -212,13 +220,13 @@ func virtioBlkCCWDeviceHandler(ctx context.Context, device pb.Device, spec *pb.S
212220
}
213221

214222
device.VmPath = devPath
215-
return updateSpecDeviceList(device, spec)
223+
return updateSpecDeviceList(device, spec, devIdx)
216224
}
217225

218226
// device.Id should be the PCI address in the format "bridgeAddr/deviceAddr".
219227
// Here, bridgeAddr is the address at which the brige is attached on the root bus,
220228
// while deviceAddr is the address at which the device is attached on the bridge.
221-
func virtioBlkDeviceHandler(_ context.Context, device pb.Device, spec *pb.Spec, s *sandbox) error {
229+
func virtioBlkDeviceHandler(_ context.Context, device pb.Device, spec *pb.Spec, s *sandbox, devIdx devIndex) error {
222230
// When "Id (PCIAddr)" is not set, we allow to use the predicted "VmPath" passed from kata-runtime
223231
if device.Id != "" {
224232
// Get the device node path based on the PCI device address
@@ -229,23 +237,23 @@ func virtioBlkDeviceHandler(_ context.Context, device pb.Device, spec *pb.Spec,
229237
device.VmPath = devPath
230238
}
231239

232-
return updateSpecDeviceList(device, spec)
240+
return updateSpecDeviceList(device, spec, devIdx)
233241
}
234242

235243
// device.Id should be the SCSI address of the disk in the format "scsiID:lunID"
236-
func virtioSCSIDeviceHandler(ctx context.Context, device pb.Device, spec *pb.Spec, s *sandbox) error {
244+
func virtioSCSIDeviceHandler(ctx context.Context, device pb.Device, spec *pb.Spec, s *sandbox, devIdx devIndex) error {
237245
// Retrieve the device path from SCSI address.
238246
devPath, err := getSCSIDevPath(s, device.Id)
239247
if err != nil {
240248
return err
241249
}
242250
device.VmPath = devPath
243251

244-
return updateSpecDeviceList(device, spec)
252+
return updateSpecDeviceList(device, spec, devIdx)
245253
}
246254

247-
func nvdimmDeviceHandler(_ context.Context, device pb.Device, spec *pb.Spec, s *sandbox) error {
248-
return updateSpecDeviceList(device, spec)
255+
func nvdimmDeviceHandler(_ context.Context, device pb.Device, spec *pb.Spec, s *sandbox, devIdx devIndex) error {
256+
return updateSpecDeviceList(device, spec, devIdx)
249257
}
250258

251259
// updateSpecDeviceList takes a device description provided by the caller,
@@ -254,7 +262,7 @@ func nvdimmDeviceHandler(_ context.Context, device pb.Device, spec *pb.Spec, s *
254262
// the same device in the list of devices provided through the OCI spec.
255263
// This is needed to update information about minor/major numbers that cannot
256264
// be predicted from the caller.
257-
func updateSpecDeviceList(device pb.Device, spec *pb.Spec) error {
265+
func updateSpecDeviceList(device pb.Device, spec *pb.Spec, devIdx devIndex) error {
258266
// If no ContainerPath is provided, we won't be able to match and
259267
// update the device in the OCI spec device list. This is an error.
260268
if device.ContainerPath == "" {
@@ -284,42 +292,32 @@ func updateSpecDeviceList(device pb.Device, spec *pb.Spec) error {
284292
}).Info("handling block device")
285293

286294
// Update the spec
287-
for idx, d := range spec.Linux.Devices {
288-
if d.Path == device.ContainerPath {
289-
hostMajor := spec.Linux.Devices[idx].Major
290-
hostMinor := spec.Linux.Devices[idx].Minor
291-
agentLog.WithFields(logrus.Fields{
292-
"device-path": device.VmPath,
293-
"host-device-major": hostMajor,
294-
"host-device-minor": hostMinor,
295-
"guest-device-major": major,
296-
"guest-device-minor": minor,
297-
}).Info("updating block device major/minor into the spec")
298-
299-
spec.Linux.Devices[idx].Major = major
300-
spec.Linux.Devices[idx].Minor = minor
301-
302-
// there is no resource to update
303-
if spec.Linux == nil || spec.Linux.Resources == nil {
304-
return nil
305-
}
295+
idxData, ok := devIdx[device.ContainerPath]
296+
if !ok {
297+
return grpcStatus.Errorf(codes.Internal,
298+
"Should have found a matching device %s in the spec",
299+
device.ContainerPath)
300+
}
306301

307-
// Resources must be updated since they are used to identify the
308-
// device in the devices cgroup.
309-
for idxRsrc, dRsrc := range spec.Linux.Resources.Devices {
310-
if dRsrc.Major == hostMajor && dRsrc.Minor == hostMinor {
311-
spec.Linux.Resources.Devices[idxRsrc].Major = major
312-
spec.Linux.Resources.Devices[idxRsrc].Minor = minor
313-
}
314-
}
302+
agentLog.WithFields(logrus.Fields{
303+
"device-path": device.VmPath,
304+
"host-device-major": spec.Linux.Devices[idxData.idx].Major,
305+
"host-device-minor": spec.Linux.Devices[idxData.idx].Minor,
306+
"guest-device-major": major,
307+
"guest-device-minor": minor,
308+
}).Info("updating block device major/minor into the spec")
315309

316-
return nil
317-
}
310+
spec.Linux.Devices[idxData.idx].Major = major
311+
spec.Linux.Devices[idxData.idx].Minor = minor
312+
313+
// Resources must be updated since they are used to identify the
314+
// device in the devices cgroup.
315+
for _, idxRsrc := range idxData.resourceIdx {
316+
spec.Linux.Resources.Devices[idxRsrc].Major = major
317+
spec.Linux.Resources.Devices[idxRsrc].Minor = minor
318318
}
319319

320-
return grpcStatus.Errorf(codes.Internal,
321-
"Should have found a matching device %s in the spec",
322-
device.VmPath)
320+
return nil
323321
}
324322

325323
// scanSCSIBus scans SCSI bus for the given SCSI address(SCSI-Id and LUN)
@@ -419,12 +417,14 @@ func getBlkCCWDevPath(s *sandbox, bus string) (string, error) {
419417
}
420418

421419
func addDevices(ctx context.Context, devices []*pb.Device, spec *pb.Spec, s *sandbox) error {
420+
devIdx := makeDevIndex(spec)
421+
422422
for _, device := range devices {
423423
if device == nil {
424424
continue
425425
}
426426

427-
err := addDevice(ctx, device, spec, s)
427+
err := addDevice(ctx, device, spec, s, devIdx)
428428
if err != nil {
429429
return err
430430
}
@@ -434,7 +434,34 @@ func addDevices(ctx context.Context, devices []*pb.Device, spec *pb.Spec, s *san
434434
return nil
435435
}
436436

437-
func addDevice(ctx context.Context, device *pb.Device, spec *pb.Spec, s *sandbox) error {
437+
func makeDevIndex(spec *pb.Spec) devIndex {
438+
devIdx := make(devIndex)
439+
440+
if spec == nil || spec.Linux == nil || spec.Linux.Devices == nil {
441+
return devIdx
442+
}
443+
444+
for i, d := range spec.Linux.Devices {
445+
rIdx := make([]int, 0)
446+
447+
if spec.Linux.Resources != nil && spec.Linux.Resources.Devices != nil {
448+
for j, r := range spec.Linux.Resources.Devices {
449+
if r.Major == d.Major && r.Minor == d.Minor {
450+
rIdx = append(rIdx, j)
451+
}
452+
}
453+
}
454+
455+
devIdx[d.Path] = devIndexEntry{
456+
idx: i,
457+
resourceIdx: rIdx,
458+
}
459+
}
460+
461+
return devIdx
462+
}
463+
464+
func addDevice(ctx context.Context, device *pb.Device, spec *pb.Spec, s *sandbox, devIdx devIndex) error {
438465
if device == nil {
439466
return grpcStatus.Error(codes.InvalidArgument, "invalid device")
440467
}
@@ -474,7 +501,7 @@ func addDevice(ctx context.Context, device *pb.Device, spec *pb.Spec, s *sandbox
474501
"Unknown device type %q", device.Type)
475502
}
476503

477-
return devHandler(ctx, *device, spec, s)
504+
return devHandler(ctx, *device, spec, s, devIdx)
478505
}
479506

480507
// updateDeviceCgroupForGuestRootfs updates the device cgroup for container

0 commit comments

Comments
 (0)