[pve-devel] [PATCH common 1/1] SysFSTools: do not assume pci domain 0000

[Dominik Csapak]

On 11/12/19 2:55 PM, Thomas Lamprecht wrote:
> On 11/12/19 2:23 PM, Dominik Csapak wrote:
>> but prepend '0000' to ids where no domain is given, to keep the ability
>> to use the shorthand syntax (e.g. 00:01.0 instead of 0000:00:01.0)
>>
>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
>> ---
>>   src/PVE/SysFSTools.pm | 20 +++++++++++++-------
>>   1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/PVE/SysFSTools.pm b/src/PVE/SysFSTools.pm
>> index 2da3a38..a8d9a7f 100644
>> --- a/src/PVE/SysFSTools.pm
>> +++ b/src/PVE/SysFSTools.pm
>> @@ -73,9 +73,9 @@ sub lspci {
>>   
>>       dir_glob_foreach("$pcisysfs/devices", $pciregex, sub {
>>   	my ($fullid, $domain, $bus, $slot, $function) = @_;
>> -	my $id = "$bus:$slot.$function";
>> +	my $id = "$domain:$bus:$slot.$function";
>>   
>> -	if (defined($filter) && !ref($filter) && $id !~ m/^\Q$filter\E/) {
>> +	if (defined($filter) && !ref($filter) && $id !~ m/^(0000:)?\Q$filter\E/) {
>>   	    return; # filter ids early
>>   	}
>>   
>> @@ -279,13 +279,15 @@ sub pci_dev_group_bind_to_vfio {
>>       }
>>       die "Cannot find vfio-pci module!\n" if !-d $vfio_basedir;
>>   
>> +    $pciid = "0000:$pciid" if $pciid !~ m/^[0-9a-f]{4}:/;
>> +
>>       # get IOMMU group devices
>> -    opendir(my $D, "$pcisysfs/devices/0000:$pciid/iommu_group/devices/") || die "Cannot open iommu_group: $!\n";
>> -      my @devs = grep /^0000:/, readdir($D);
>> +    opendir(my $D, "$pcisysfs/devices/$pciid/iommu_group/devices/") || die "Cannot open iommu_group: $!\n";
>> +      my @devs = grep /^[0-9a-f]{4}:/, readdir($D);
>>       closedir($D);
>>   
>>       foreach my $pciid (@devs) {
>> -	$pciid =~ m/^([:\.\da-f]+)$/ or die "PCI ID $pciid not valid!\n";
>> +	$pciid =~ m/^([:\.0-9a-f]+)$/ or die "PCI ID $pciid not valid!\n";
>>   
>>           # pci bridges, switches or root ports are not supported
>>           # they have a pci_bus subdirectory so skip them
>> @@ -301,7 +303,9 @@ sub pci_dev_group_bind_to_vfio {
>>   sub pci_create_mdev_device {
>>       my ($pciid, $uuid, $type) = @_;
>>   
>> -    my $basedir = "$pcisysfs/devices/0000:$pciid";
>> +    $pciid = "0000:$pciid" if $pciid !~ m/^[0-9a-f]{4}:/;
>> +
>> +    my $basedir = "$pcisysfs/devices/$pciid";
>>       my $mdev_dir = "$basedir/mdev_supported_types";
>>   
>>       die "pci device '$pciid' does not support mediated devices \n"
>> @@ -336,7 +340,9 @@ sub pci_create_mdev_device {
>>   sub pci_cleanup_mdev_device {
>>       my ($pciid, $uuid) = @_;
>>   
>> -    my $basedir = "$pcisysfs/devices/0000:$pciid/$uuid";
>> +    $pciid = "0000:$pciid" if $pciid !~ m/^[0-9a-f]{4}:/;
> 
> maybe above could be moved in a helper?
> could either return the "full" ID or a tuple like:
> my ($full, $domain, $bus, $slot) = ...
> 
> not to sure though, and can also be OK without this for the
> few call places..

i thought about this too, but we only use it 3 times, and it did
not seem worth it for a single line of code

and the only place where we use the domain/bus/slot split is in
the 'pci_device_info' sub...

> 
>> +
>> +    my $basedir = "$pcisysfs/devices/$pciid/$uuid";
>>   
>>       if (! -e $basedir) {
>>   	return 1; # no cleanup necessary if it does not exist
>>