This repository has been archived by the owner on May 12, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 114
Use PCI Addresses to determine the device names for virtio-blk devices #227
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
185831c
uevent: Capture device name in uevents
amshinde d10734b
uevent: Add a global listener for uevents
amshinde 3b565ad
block: Use PCI address to determine block device name
amshinde d29bf53
block: Get rid of device prediction for Storage as well
amshinde File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,14 @@ const ( | |
driverSCSIType = "scsi" | ||
) | ||
|
||
const rootBusPath = "/devices/pci0000:00" | ||
|
||
var ( | ||
sysBusPrefix = "/sys/bus/pci/devices" | ||
pciBusPathFormat = "%s/%s/pci_bus/" | ||
systemDevPath = "/dev" | ||
) | ||
|
||
// SCSI variables | ||
var ( | ||
// Here in "0:0", the first number is the SCSI host number because | ||
|
@@ -42,31 +50,123 @@ var ( | |
scsiHostPath = filepath.Join(sysClassPrefix, "scsi_host") | ||
) | ||
|
||
type deviceHandler func(device pb.Device, spec *pb.Spec) error | ||
type deviceHandler func(device pb.Device, spec *pb.Spec, s *sandbox) error | ||
|
||
var deviceHandlerList = map[string]deviceHandler{ | ||
driverBlkType: virtioBlkDeviceHandler, | ||
driverSCSIType: virtioSCSIDeviceHandler, | ||
} | ||
|
||
func virtioBlkDeviceHandler(device pb.Device, spec *pb.Spec) error { | ||
// First need to make sure the expected device shows up properly, | ||
// and then we need to retrieve its device info (such as major and | ||
// minor numbers), useful to update the device provided | ||
// through the OCI specification. | ||
devName := strings.TrimPrefix(device.VmPath, devPrefix) | ||
checkUevent := func(uEv *uevent.Uevent) bool { | ||
return (uEv.Action == "add" && | ||
filepath.Base(uEv.DevPath) == devName) | ||
// getDevicePCIAddress fetches the complete PCI address in sysfs, based on the PCI | ||
// identifier provided. This should be in the format: "bridgeAddr/deviceAddr". | ||
// Here, bridgeAddr is the address at which the brige is attached on the root bus, | ||
// while deviceAddr is the address at which the device is attached on the bridge. | ||
func getDevicePCIAddress(pciID string) (string, error) { | ||
tokens := strings.Split(pciID, "/") | ||
|
||
if len(tokens) != 2 { | ||
return "", fmt.Errorf("PCI Identifier for device should be of format [bridgeAddr/deviceAddr], got %s", pciID) | ||
} | ||
|
||
bridgeID := tokens[0] | ||
deviceID := tokens[1] | ||
|
||
// Deduce the complete bridge address based on the bridge address identifier passed | ||
// and the fact that bridges are attached on the main bus with function 0. | ||
pciBridgeAddr := fmt.Sprintf("0000:00:%s.0", bridgeID) | ||
|
||
// Find out the bus exposed by bridge | ||
bridgeBusPath := fmt.Sprintf(pciBusPathFormat, sysBusPrefix, pciBridgeAddr) | ||
|
||
files, err := ioutil.ReadDir(bridgeBusPath) | ||
if err != nil { | ||
return "", fmt.Errorf("Error with getting bridge pci bus : %s", err) | ||
} | ||
|
||
busNum := len(files) | ||
if busNum != 1 { | ||
return "", fmt.Errorf("Expected an entry for bus in %s, got %d entries instead", bridgeBusPath, busNum) | ||
} | ||
if err := waitForDevice(device.VmPath, devName, checkUevent); err != nil { | ||
|
||
bus := files[0].Name() | ||
|
||
// Device address is based on the bus of the bridge to which it is attached. | ||
// We do not pass devices as multifunction, hence the trailing 0 in the address. | ||
pciDeviceAddr := fmt.Sprintf("%s:%s.0", bus, deviceID) | ||
|
||
bridgeDevicePCIAddr := fmt.Sprintf("%s/%s", pciBridgeAddr, pciDeviceAddr) | ||
agentLog.WithField("completePCIAddr", bridgeDevicePCIAddr).Info("Fetched PCI address for device") | ||
|
||
return bridgeDevicePCIAddr, nil | ||
} | ||
|
||
func getBlockDeviceNodeName(s *sandbox, pciID string) (string, error) { | ||
pciAddr, err := getDevicePCIAddress(pciID) | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
var devName string | ||
var notifyChan chan string | ||
|
||
fieldLogger := agentLog.WithField("pciID", pciID) | ||
|
||
// Check if the PCI identifier is in PCI device map. | ||
s.Lock() | ||
for key, value := range s.pciDeviceMap { | ||
if strings.Contains(key, pciAddr) { | ||
devName = value | ||
fieldLogger.Info("Device found in pci device map") | ||
break | ||
} | ||
} | ||
|
||
// If device is not found in the device map, hotplug event has not | ||
// been received yet, create and add channel to the watchers map. | ||
// The key of the watchers map is the device we are interested in. | ||
// Note this is done inside the lock, not to miss any events from the | ||
// global udev listener. | ||
if devName == "" { | ||
notifyChan := make(chan string, 1) | ||
s.deviceWatchers[pciAddr] = notifyChan | ||
} | ||
s.Unlock() | ||
|
||
if devName == "" { | ||
fieldLogger.Info("Waiting on channel for device notification") | ||
select { | ||
case devName = <-notifyChan: | ||
case <-time.After(time.Duration(timeoutHotplug) * time.Second): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove the watcher and close its notification channel on timeout. Otherwise the channel may exist forever if the device never shows up. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
s.Lock() | ||
delete(s.deviceWatchers, pciAddr) | ||
close(notifyChan) | ||
s.Unlock() | ||
|
||
return "", grpcStatus.Errorf(codes.DeadlineExceeded, | ||
"Timeout reached after %ds waiting for device %s", | ||
timeoutHotplug, pciAddr) | ||
} | ||
} | ||
|
||
return filepath.Join(systemDevPath, devName), nil | ||
} | ||
|
||
// device.Id should be the PCI address in the format "bridgeAddr/deviceAddr". | ||
// Here, bridgeAddr is the address at which the brige is attached on the root bus, | ||
// while deviceAddr is the address at which the device is attached on the bridge. | ||
func virtioBlkDeviceHandler(device pb.Device, spec *pb.Spec, s *sandbox) error { | ||
// Get the device node path based on the PCI device address | ||
devPath, err := getBlockDeviceNodeName(s, device.Id) | ||
if err != nil { | ||
return err | ||
} | ||
device.VmPath = devPath | ||
|
||
return updateSpecDeviceList(device, spec) | ||
} | ||
|
||
func virtioSCSIDeviceHandler(device pb.Device, spec *pb.Spec) error { | ||
// device.Id should be the SCSI address of the disk in the format "scsiID:lunID" | ||
func virtioSCSIDeviceHandler(device pb.Device, spec *pb.Spec, s *sandbox) error { | ||
// Retrieve the device path from SCSI address. | ||
devPath, err := getSCSIDevPath(device.Id) | ||
if err != nil { | ||
|
@@ -270,13 +370,13 @@ func getSCSIDevPath(scsiAddr string) (string, error) { | |
return filepath.Join(devPrefix, scsiDiskName), nil | ||
} | ||
|
||
func addDevices(devices []*pb.Device, spec *pb.Spec) error { | ||
func addDevices(devices []*pb.Device, spec *pb.Spec, s *sandbox) error { | ||
for _, device := range devices { | ||
if device == nil { | ||
continue | ||
} | ||
|
||
err := addDevice(device, spec) | ||
err := addDevice(device, spec, s) | ||
if err != nil { | ||
return err | ||
} | ||
|
@@ -286,7 +386,7 @@ func addDevices(devices []*pb.Device, spec *pb.Spec) error { | |
return nil | ||
} | ||
|
||
func addDevice(device *pb.Device, spec *pb.Spec) error { | ||
func addDevice(device *pb.Device, spec *pb.Spec, s *sandbox) error { | ||
if device == nil { | ||
return grpcStatus.Error(codes.InvalidArgument, "invalid device") | ||
} | ||
|
@@ -326,5 +426,5 @@ func addDevice(device *pb.Device, spec *pb.Spec) error { | |
"Unknown device type %q", device.Type) | ||
} | ||
|
||
return devHandler(*device, spec) | ||
return devHandler(*device, spec, s) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please let s.wg.Wait() also wait for the udev listener goroutine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The udev listener routine does not quit for any reason, thats why I have not added the wait for it.
It should quit when the main thread quits.