Re: [pve-devel] [PATCH guest-common v3 2/4] mapping: pci: check the mdev configuration on the device too

2024-05-31 Thread Fiona Ebner
Am 19.04.24 um 14:45 schrieb Dominik Csapak:
> but that lives int he 'global' part of the mapping config, not in a
> specific mapping. To check that, add it to the $configured_props from
> there.
> 
> this requires all call sites to be adapted otherwise the check will
> always fail for devices that are capable of mediated devices
> 
> Signed-off-by: Dominik Csapak 
> ---
> changes from v2:
> * adapt to changes of previous patch
>  src/PVE/Mapping/PCI.pm | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Mapping/PCI.pm b/src/PVE/Mapping/PCI.pm
> index ef1bd8d..b412c4d 100644
> --- a/src/PVE/Mapping/PCI.pm
> +++ b/src/PVE/Mapping/PCI.pm
> @@ -131,7 +131,7 @@ sub options {
>  
>  # checks if the given config is valid for the current node
>  sub assert_valid {
> -my ($name, $mapping) = @_;
> +my ($name, $mapping, $cfg) = @_;
>  
>  my @paths = split(';', $mapping->{path} // '');
>  
> @@ -151,6 +151,7 @@ sub assert_valid {
>   my $expected_props = {
>   id => "$info->{vendor}:$info->{device}",
>   iommugroup => $info->{iommugroup},
> + mdev => $info->{mdev},

The value here returned by SysFSTools is undef when not supported...

>   };
>  
>   if (defined($info->{'subsystem_vendor'}) && 
> defined($info->{'subsystem_device'})) {
> @@ -158,6 +159,8 @@ sub assert_valid {
>   }
>  
>   my $configured_props = { $mapping->%{qw(id iommugroup subsystem-id)} };
> + # check mdev from globabl mapping config
> + $configured_props->{mdev} = $cfg->{mdev};
>  

...while the value in the config can be an explicit '0', which will trip
up the check below. (It can also be undef in the config and will be via UI).

Also leads to the error

unexpected property 'mdev' configured for device ':00:1b.0'

when SysFSTools returns undef and the property is present in the in the
config, even though it's not per-se unexpected. If we convert to
explicit "0" and "1" values, we would also get a better error message
that points out the value mismatch.


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH guest-common v3 2/4] mapping: pci: check the mdev configuration on the device too

2024-04-19 Thread Dominik Csapak
but that lives int he 'global' part of the mapping config, not in a
specific mapping. To check that, add it to the $configured_props from
there.

this requires all call sites to be adapted otherwise the check will
always fail for devices that are capable of mediated devices

Signed-off-by: Dominik Csapak 
---
changes from v2:
* adapt to changes of previous patch
 src/PVE/Mapping/PCI.pm | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Mapping/PCI.pm b/src/PVE/Mapping/PCI.pm
index ef1bd8d..b412c4d 100644
--- a/src/PVE/Mapping/PCI.pm
+++ b/src/PVE/Mapping/PCI.pm
@@ -131,7 +131,7 @@ sub options {
 
 # checks if the given config is valid for the current node
 sub assert_valid {
-my ($name, $mapping) = @_;
+my ($name, $mapping, $cfg) = @_;
 
 my @paths = split(';', $mapping->{path} // '');
 
@@ -151,6 +151,7 @@ sub assert_valid {
my $expected_props = {
id => "$info->{vendor}:$info->{device}",
iommugroup => $info->{iommugroup},
+   mdev => $info->{mdev},
};
 
if (defined($info->{'subsystem_vendor'}) && 
defined($info->{'subsystem_device'})) {
@@ -158,6 +159,8 @@ sub assert_valid {
}
 
my $configured_props = { $mapping->%{qw(id iommugroup subsystem-id)} };
+   # check mdev from globabl mapping config
+   $configured_props->{mdev} = $cfg->{mdev};
 
my $merged = { %$expected_props, %$configured_props }; # just for the 
keys
for my $prop (sort keys %$merged) {
-- 
2.39.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel