Re: [pve-devel] [PATCH common 1/1] add PVE/HardwareMap

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On July 19, 2022 1:46 pm, Dominik Csapak wrote:
> this adds functionality for the hardwaremap config (as json)
> the format of the config is like this:
> 
> {
>     usb => {
> 	name => {
> 	    nodename1 => { /* mapping object */ },
> 	    nodename2 => { /* mapping object */ }
> 	}
>     },
>     pci => {
> 	/* same as above */
>     },
>     digest => "<DIGEST-STRING>"
> }

kind of missing an argument for why
- this is a json file instead of a regular section config (the two 
  stored types share most of their structure?)
- this is a cluster-wide file containing a map per node instead of 
  being one config file per node?

from what I can piece together from the rest of the series ;)
- cannot be a cluster-wide section config since values differ 
  (potentially) on each node (so a simple `nodes` filter is not enough)
- we don't want ID foo to be type USB on one node and type PCI on 
  another
- we want to check all nodes for migration precondition checks

the first one is a given obviously. the type mismatch wouldn't actually 
cause any problems although it could be confusing possibly. the 
migration check could just check all (relevant) nodes.

not saying I'm opposed to the "one json file" approach per se, but it 
would be nice to weigh the pros and cons before deviating from our usual 
approach to config files.

> 
> it also adds some helpers for the api schema & asserting that the
> device mappings are valid (by checking the saved properties
> against the ones found on the current available devices)
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/Makefile           |   1 +
>  src/PVE/HardwareMap.pm | 363 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 364 insertions(+)
>  create mode 100644 src/PVE/HardwareMap.pm
> 
> diff --git a/src/Makefile b/src/Makefile
> index 13de6c6..8527704 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -17,6 +17,7 @@ LIB_SOURCES = \
>  	Daemon.pm \
>  	Exception.pm \
>  	Format.pm \
> +	HardwareMap.pm \
>  	INotify.pm \
>  	JSONSchema.pm \
>  	LDAP.pm \
> diff --git a/src/PVE/HardwareMap.pm b/src/PVE/HardwareMap.pm
> new file mode 100644
> index 0000000..1b94abc
> --- /dev/null
> +++ b/src/PVE/HardwareMap.pm
> @@ -0,0 +1,363 @@
> +package PVE::HardwareMap;
> +
> +use strict;
> +use warnings;
> +
> +use Digest::SHA;
> +use JSON;
> +use Storable qw(dclone);
> +
> +use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_write_file cfs_lock_file);
> +use PVE::INotify;
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::SysFSTools;
> +
> +use base qw(Exporter);
> +
> +our @EXPORT_OK = qw(find_device_on_current_node);
> +
> +my $FILENAME = "nodes/hardware-map.conf";
> +cfs_register_file($FILENAME, \&read_hardware_map, \&write_hardware_map);
> +
> +# a mapping format per type
> +my $format = {
> +    usb => {
> +	vendor => {
> +	    description => "The vendor ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	},
> +	device => {
> +	    description => "The device ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	},
> +	'subsystem-vendor' => {
> +	    description => "The subsystem vendor ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	    optional => 1,
> +	},
> +	'subsystem-device' => {
> +	    description => "The subsystem device ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	    optional => 1,
> +	},
> +	path => {
> +	    description => "The path to the usb device.",
> +	    type => 'string',
> +	    optional => 1,
> +	    pattern => qr/^(\d+)\-(\d+(\.\d+)*)$/,
> +	},
> +	comment => {
> +	    description => "Description.",
> +	    type => 'string',
> +	    optional => 1,
> +	    maxLength => 4096,
> +	},
> +    },
> +    pci => {
> +	vendor => {
> +	    description => "The vendor ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	},
> +	device => {
> +	    description => "The device ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	},
> +	'subsystem-vendor' => {
> +	    description => "The subsystem vendor ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	    optional => 1,
> +	},
> +	'subsystem-device' => {
> +	    description => "The subsystem device ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	    optional => 1,
> +	},
> +	path => {
> +	    description => "The path to the device. If the function is omitted, the whole device is mapped. In that case use the attrubes of the first device.",
> +	    type => 'string',
> +	    pattern => "(?:[a-f0-9]{4,}:)?[a-f0-9]{2}:[a-f0-9]{2}(?:\.[a-f0-9])?",
> +	},
> +	mdev => {
> +	    description => "The Device supports mediated devices.",
> +	    type => 'boolean',
> +	    optional => 1,
> +	    default => 0,
> +	},
> +	iommugroup => {
> +	    type => 'integer',
> +	    description => "The IOMMU group in which the device is in.",
> +	    optional => 1,
> +	},
> +	comment => {
> +	    description => "Description.",
> +	    type => 'string',
> +	    optional => 1,
> +	    maxLength => 4096,
> +	},
> +    },
> +};
> +
> +my $name_format = {
> +    description => "The custom name for the device",
> +    type => 'string',
> +    format => 'pve-configid',
> +};
> +
> +sub find_device_on_current_node {
> +    my ($type, $id) = @_;
> +
> +    my $cfg = config();
> +    my $node = PVE::INotify::nodename();
> +
> +    return undef if !defined($cfg->{$type}->{$id}) || !defined($cfg->{$type}->{$id}->{$node});
> +    return $cfg->{$type}->{$id}->{$node};
> +}
> +
> +sub config {
> +    return cfs_read_file($FILENAME);
> +}
> +
> +sub lock_config {
> +    my ($code, $errmsg) = @_;
> +
> +    cfs_lock_file($FILENAME, undef, $code);
> +    if (my $err = $@) {
> +	$errmsg ? die "$errmsg: $err" : die $err;
> +    }
> +}
> +
> +sub write_config {
> +    my ($cfg) = @_;
> +
> +    cfs_write_file($FILENAME, $cfg);
> +}
> +
> +sub check_prop {
> +    my ($schema, $key, $value) = @_;
> +    my $errors = {};
> +    PVE::JSONSchema::check_prop($value, $schema, '', $errors);
> +    if (scalar(keys %$errors)) {
> +	die "$errors->{$key}\n" if $errors->{$key};
> +	die "$errors->{_root}\n" if $errors->{_root};
> +	die "unknown error\n";
> +    }
> +}
> +
> +sub check_config {
> +    my ($cfg) = @_;
> +
> +    for my $type (keys %$format) {
> +	my $type_cfg = $cfg->{$type};
> +	my $type_format = $format->{$type};
> +
> +	for my $name (keys %$type_cfg) {
> +	    check_prop($name_format, 'name', $name);
> +
> +	    for my $node (keys $type_cfg->{$name}->%*) {
> +		check_prop(get_standard_option('pve-node'), 'node', $node);
> +		my $entry = $type_cfg->{$name}->{$node};
> +
> +		# check required props
> +		for my $prop  (keys %$type_format) {
> +		    next if $type_format->{$prop}->{optional};
> +		    die "missing property '$prop' for $type entry '$name'\n"
> +			if !defined($entry->{$prop});
> +		}
> +
> +		for my $prop (keys %$entry) {
> +		    check_prop($type_format->{$prop}, $prop, $entry->{$prop});
> +		}
> +	    }
> +	}
> +    }
> +}
> +
> +sub read_hardware_map {
> +    my ($filename, $raw)  = @_;
> +
> +    my $digest = Digest::SHA::sha1_hex($raw);
> +
> +    if (!defined($raw) || $raw eq '') {
> +	return {
> +	    digest => $digest,
> +	};
> +    }
> +
> +    my $cfg = from_json($raw);
> +    check_config($cfg);
> +    $cfg->{digest} = $digest;
> +
> +    return $cfg;
> +}
> +
> +sub write_hardware_map {
> +    my ($filename, $cfg) = @_;
> +
> +    check_config($cfg);
> +
> +    return to_json($cfg);
> +}
> +
> +my $pci_valid = sub {
> +    my ($cfg) = @_;
> +
> +    my $path = $cfg->{path} // '';
> +
> +    if ($path !~ m/\.[a-f0-9]/i) {
> +	# whole device, add .0 (must exist)
> +	$path = "$path.0";
> +    }
> +
> +    my $info = PVE::SysFSTools::pci_device_info($path, 1);
> +    die "pci device '$path' not found\n" if !defined($info);
> +
> +    my $correct_props = {
> +	vendor => $info->{vendor},
> +	device => $info->{device},
> +	'subsystem-vendor' => $info->{'subsystem_vendor'},
> +	'subsystem-device' => $info->{'subsystem_device'},
> +	mdev => $info->{mdev},
> +	iommugroup => $info->{iommugroup},
> +    };
> +
> +    for my $prop (sort keys %$correct_props) {
> +	next if !defined($correct_props->{$prop}) && !defined($cfg->{$prop});
> +	die "no '$prop' for device '$path'\n"
> +	    if defined($correct_props->{$prop}) && !defined($cfg->{$prop});
> +	die "'$prop' configured but should not be\n"
> +	    if !defined($correct_props->{$prop}) && defined($cfg->{$prop});
> +
> +	my $correct_prop = $correct_props->{$prop};
> +	$correct_prop =~ s/^0x//;
> +	my $configured_prop = $cfg->{$prop};
> +	$configured_prop =~ s/^0x//;
> +
> +	die "'$prop' does not match for '$cfg->{name}' ($correct_prop != $configured_prop)\n"
> +	    if $correct_prop ne $configured_prop;
> +    }
> +
> +    return 1;
> +};
> +
> +my $usb_valid = sub {
> +    my ($cfg) = @_;
> +
> +    my $name = $cfg->{name};
> +    my $vendor = $cfg->{vendor};
> +    my $device = $cfg->{device};
> +
> +    my $usb_list = PVE::SysFSTools::scan_usb();
> +
> +    my $info;
> +    if (my $path = $cfg->{path}) {
> +	for my $dev (@$usb_list) {
> +	    next if !$dev->{usbpath} || !$dev->{busnum};
> +	    my $usbpath = "$dev->{busnum}-$dev->{usbpath}";
> +	    next if $usbpath ne $path;
> +	    $info = $dev;
> +	}
> +	die "usb device '$path' not found\n" if !defined($info);
> +
> +	die "'vendor' does not match for '$name'\n"
> +	    if $info->{vendid} ne $cfg->{vendor};
> +	die "'device' does not match for '$name'\n"
> +	    if $info->{prodid} ne $cfg->{device};
> +    } else {
> +	for my $dev (@$usb_list) {
> +	    next if $dev->{vendid} ne $vendor;
> +	    next if $dev->{prodid} ne $device;
> +	    $info = $dev;
> +	}
> +	die "usb device '$vendor:$device' not found\n" if !defined($info);
> +    }
> +
> +    return 1;
> +};
> +
> +sub assert_device_valid {
> +    my ($type, $cfg) = @_;
> +
> +    if ($type eq 'usb') {
> +	return $usb_valid->($cfg);
> +    } elsif ($type eq 'pci') {
> +	return $pci_valid->($cfg);
> +    }
> +
> +    die "invalid type $type\n";
> +}
> +
> +sub createSchema {
> +    my ($type) = @_;
> +
> +    my $schema = {};
> +
> +    $schema->{name} = $name_format;
> +    $schema->{node} = get_standard_option('pve-node');
> +
> +    for my $opt (sort keys $format->{$type}->%*) {
> +	$schema->{$opt} = $format->{$type}->{$opt};
> +    }
> +
> +    return {
> +	additionalProperties => 0,
> +	properties => $schema,
> +    };
> +
> +}
> +
> +sub updateSchema {
> +    my ($type) = @_;
> +
> +    my $schema = {};
> +
> +    $schema->{name} = $name_format;
> +    $schema->{node} = get_standard_option('pve-node');
> +
> +    my $deletable = [];
> +
> +    for my $opt (sort keys $format->{$type}->%*) {
> +	$schema->{$opt} = dclone($format->{$type}->{$opt});
> +	$schema->{$opt}->{optional} = 1;
> +	if ($format->{$type}->{$opt}->{optional}) {
> +	    push @$deletable, $opt;
> +	}
> +    }
> +
> +    my $deletable_pattern = join('|', @$deletable);
> +
> +    $schema->{delete} = {
> +	type => 'string', format => 'pve-configid-list',
> +	description => "A list of settings you want to delete.",
> +	maxLength => 4096,
> +	optional => 1,
> +    };
> +
> +    $schema->{digest} = get_standard_option('pve-config-digest');
> +
> +    return {
> +	additionalProperties => 0,
> +	properties => $schema,
> +    };
> +}
> +
> +sub options {
> +    my ($type) = @_;
> +
> +    my $opts = {};
> +
> +    for my $opt (sort keys $format->{$type}->%*) {
> +	$opts->{$opt}->{optional} = $format->{$type}->{$opt}->{optional};
> +    }
> +
> +    return $opts;
> +}
> +
> +1;
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>