From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 0540A8A045 for ; Mon, 1 Aug 2022 14:59:29 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E78582FEAC for ; Mon, 1 Aug 2022 14:58:58 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 1 Aug 2022 14:58:56 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id C666D42365 for ; Mon, 1 Aug 2022 14:58:55 +0200 (CEST) Date: Mon, 01 Aug 2022 14:58:47 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20220719114639.3035048-1-d.csapak@proxmox.com> <20220719114639.3035048-5-d.csapak@proxmox.com> In-Reply-To: <<20220719114639.3035048-5-d.csapak@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1659355657.jpyy24eapt.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.161 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH common 1/1] add PVE/HardwareMap X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Aug 2022 12:59:29 -0000 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: >=20 > { > usb =3D> { > name =3D> { > nodename1 =3D> { /* mapping object */ }, > nodename2 =3D> { /* mapping object */ } > } > }, > pci =3D> { > /* same as above */ > }, > digest =3D> "" > } kind of missing an argument for why - this is a json file instead of a regular section config (the two=20 stored types share most of their structure?) - this is a cluster-wide file containing a map per node instead of=20 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=20 (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=20 another - we want to check all nodes for migration precondition checks the first one is a given obviously. the type mismatch wouldn't actually=20 cause any problems although it could be confusing possibly. the=20 migration check could just check all (relevant) nodes. not saying I'm opposed to the "one json file" approach per se, but it=20 would be nice to weigh the pros and cons before deviating from our usual=20 approach to config files. >=20 > 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) >=20 > Signed-off-by: Dominik Csapak > --- > src/Makefile | 1 + > src/PVE/HardwareMap.pm | 363 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 364 insertions(+) > create mode 100644 src/PVE/HardwareMap.pm >=20 > diff --git a/src/Makefile b/src/Makefile > index 13de6c6..8527704 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -17,6 +17,7 @@ LIB_SOURCES =3D \ > 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_l= ock_file); > +use PVE::INotify; > +use PVE::JSONSchema qw(get_standard_option); > +use PVE::SysFSTools; > + > +use base qw(Exporter); > + > +our @EXPORT_OK =3D qw(find_device_on_current_node); > + > +my $FILENAME =3D "nodes/hardware-map.conf"; > +cfs_register_file($FILENAME, \&read_hardware_map, \&write_hardware_map); > + > +# a mapping format per type > +my $format =3D { > + usb =3D> { > + vendor =3D> { > + description =3D> "The vendor ID", > + type =3D> 'string', > + pattern =3D> qr/^(:?0x)?[0-9A-Fa-f]{4}$/, > + }, > + device =3D> { > + description =3D> "The device ID", > + type =3D> 'string', > + pattern =3D> qr/^(:?0x)?[0-9A-Fa-f]{4}$/, > + }, > + 'subsystem-vendor' =3D> { > + description =3D> "The subsystem vendor ID", > + type =3D> 'string', > + pattern =3D> qr/^(:?0x)?[0-9A-Fa-f]{4}$/, > + optional =3D> 1, > + }, > + 'subsystem-device' =3D> { > + description =3D> "The subsystem device ID", > + type =3D> 'string', > + pattern =3D> qr/^(:?0x)?[0-9A-Fa-f]{4}$/, > + optional =3D> 1, > + }, > + path =3D> { > + description =3D> "The path to the usb device.", > + type =3D> 'string', > + optional =3D> 1, > + pattern =3D> qr/^(\d+)\-(\d+(\.\d+)*)$/, > + }, > + comment =3D> { > + description =3D> "Description.", > + type =3D> 'string', > + optional =3D> 1, > + maxLength =3D> 4096, > + }, > + }, > + pci =3D> { > + vendor =3D> { > + description =3D> "The vendor ID", > + type =3D> 'string', > + pattern =3D> qr/^(:?0x)?[0-9A-Fa-f]{4}$/, > + }, > + device =3D> { > + description =3D> "The device ID", > + type =3D> 'string', > + pattern =3D> qr/^(:?0x)?[0-9A-Fa-f]{4}$/, > + }, > + 'subsystem-vendor' =3D> { > + description =3D> "The subsystem vendor ID", > + type =3D> 'string', > + pattern =3D> qr/^(:?0x)?[0-9A-Fa-f]{4}$/, > + optional =3D> 1, > + }, > + 'subsystem-device' =3D> { > + description =3D> "The subsystem device ID", > + type =3D> 'string', > + pattern =3D> qr/^(:?0x)?[0-9A-Fa-f]{4}$/, > + optional =3D> 1, > + }, > + path =3D> { > + description =3D> "The path to the device. If the function is omitte= d, the whole device is mapped. In that case use the attrubes of the first d= evice.", > + type =3D> 'string', > + pattern =3D> "(?:[a-f0-9]{4,}:)?[a-f0-9]{2}:[a-f0-9]{2}(?:\.[a-f0-9= ])?", > + }, > + mdev =3D> { > + description =3D> "The Device supports mediated devices.", > + type =3D> 'boolean', > + optional =3D> 1, > + default =3D> 0, > + }, > + iommugroup =3D> { > + type =3D> 'integer', > + description =3D> "The IOMMU group in which the device is in.", > + optional =3D> 1, > + }, > + comment =3D> { > + description =3D> "Description.", > + type =3D> 'string', > + optional =3D> 1, > + maxLength =3D> 4096, > + }, > + }, > +}; > + > +my $name_format =3D { > + description =3D> "The custom name for the device", > + type =3D> 'string', > + format =3D> 'pve-configid', > +}; > + > +sub find_device_on_current_node { > + my ($type, $id) =3D @_; > + > + my $cfg =3D config(); > + my $node =3D PVE::INotify::nodename(); > + > + return undef if !defined($cfg->{$type}->{$id}) || !defined($cfg->{$t= ype}->{$id}->{$node}); > + return $cfg->{$type}->{$id}->{$node}; > +} > + > +sub config { > + return cfs_read_file($FILENAME); > +} > + > +sub lock_config { > + my ($code, $errmsg) =3D @_; > + > + cfs_lock_file($FILENAME, undef, $code); > + if (my $err =3D $@) { > + $errmsg ? die "$errmsg: $err" : die $err; > + } > +} > + > +sub write_config { > + my ($cfg) =3D @_; > + > + cfs_write_file($FILENAME, $cfg); > +} > + > +sub check_prop { > + my ($schema, $key, $value) =3D @_; > + my $errors =3D {}; > + 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) =3D @_; > + > + for my $type (keys %$format) { > + my $type_cfg =3D $cfg->{$type}; > + my $type_format =3D $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 =3D $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) =3D @_; > + > + my $digest =3D Digest::SHA::sha1_hex($raw); > + > + if (!defined($raw) || $raw eq '') { > + return { > + digest =3D> $digest, > + }; > + } > + > + my $cfg =3D from_json($raw); > + check_config($cfg); > + $cfg->{digest} =3D $digest; > + > + return $cfg; > +} > + > +sub write_hardware_map { > + my ($filename, $cfg) =3D @_; > + > + check_config($cfg); > + > + return to_json($cfg); > +} > + > +my $pci_valid =3D sub { > + my ($cfg) =3D @_; > + > + my $path =3D $cfg->{path} // ''; > + > + if ($path !~ m/\.[a-f0-9]/i) { > + # whole device, add .0 (must exist) > + $path =3D "$path.0"; > + } > + > + my $info =3D PVE::SysFSTools::pci_device_info($path, 1); > + die "pci device '$path' not found\n" if !defined($info); > + > + my $correct_props =3D { > + vendor =3D> $info->{vendor}, > + device =3D> $info->{device}, > + 'subsystem-vendor' =3D> $info->{'subsystem_vendor'}, > + 'subsystem-device' =3D> $info->{'subsystem_device'}, > + mdev =3D> $info->{mdev}, > + iommugroup =3D> $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 =3D $correct_props->{$prop}; > + $correct_prop =3D~ s/^0x//; > + my $configured_prop =3D $cfg->{$prop}; > + $configured_prop =3D~ s/^0x//; > + > + die "'$prop' does not match for '$cfg->{name}' ($correct_prop !=3D $con= figured_prop)\n" > + if $correct_prop ne $configured_prop; > + } > + > + return 1; > +}; > + > +my $usb_valid =3D sub { > + my ($cfg) =3D @_; > + > + my $name =3D $cfg->{name}; > + my $vendor =3D $cfg->{vendor}; > + my $device =3D $cfg->{device}; > + > + my $usb_list =3D PVE::SysFSTools::scan_usb(); > + > + my $info; > + if (my $path =3D $cfg->{path}) { > + for my $dev (@$usb_list) { > + next if !$dev->{usbpath} || !$dev->{busnum}; > + my $usbpath =3D "$dev->{busnum}-$dev->{usbpath}"; > + next if $usbpath ne $path; > + $info =3D $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 =3D $dev; > + } > + die "usb device '$vendor:$device' not found\n" if !defined($info); > + } > + > + return 1; > +}; > + > +sub assert_device_valid { > + my ($type, $cfg) =3D @_; > + > + 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) =3D @_; > + > + my $schema =3D {}; > + > + $schema->{name} =3D $name_format; > + $schema->{node} =3D get_standard_option('pve-node'); > + > + for my $opt (sort keys $format->{$type}->%*) { > + $schema->{$opt} =3D $format->{$type}->{$opt}; > + } > + > + return { > + additionalProperties =3D> 0, > + properties =3D> $schema, > + }; > + > +} > + > +sub updateSchema { > + my ($type) =3D @_; > + > + my $schema =3D {}; > + > + $schema->{name} =3D $name_format; > + $schema->{node} =3D get_standard_option('pve-node'); > + > + my $deletable =3D []; > + > + for my $opt (sort keys $format->{$type}->%*) { > + $schema->{$opt} =3D dclone($format->{$type}->{$opt}); > + $schema->{$opt}->{optional} =3D 1; > + if ($format->{$type}->{$opt}->{optional}) { > + push @$deletable, $opt; > + } > + } > + > + my $deletable_pattern =3D join('|', @$deletable); > + > + $schema->{delete} =3D { > + type =3D> 'string', format =3D> 'pve-configid-list', > + description =3D> "A list of settings you want to delete.", > + maxLength =3D> 4096, > + optional =3D> 1, > + }; > + > + $schema->{digest} =3D get_standard_option('pve-config-digest'); > + > + return { > + additionalProperties =3D> 0, > + properties =3D> $schema, > + }; > +} > + > +sub options { > + my ($type) =3D @_; > + > + my $opts =3D {}; > + > + for my $opt (sort keys $format->{$type}->%*) { > + $opts->{$opt}->{optional} =3D $format->{$type}->{$opt}->{optional}; > + } > + > + return $opts; > +} > + > +1; > --=20 > 2.30.2 >=20 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20