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 6E4E09B847 for ; Thu, 25 May 2023 12:41:16 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4829529A88 for ; Thu, 25 May 2023 12:40:46 +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 ; Thu, 25 May 2023 12:40:44 +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 A2DA4470CB for ; Thu, 25 May 2023 12:40:44 +0200 (CEST) Message-ID: Date: Thu, 25 May 2023 12:40:43 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: pve-devel@lists.proxmox.com References: <20230525101753.2078811-1-d.csapak@proxmox.com> <20230525101753.2078811-4-d.csapak@proxmox.com> From: Dominik Csapak In-Reply-To: <20230525101753.2078811-4-d.csapak@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.015 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy 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 T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pve-devel] [PATCH guest-common v4 1/1] add PCI/USB Resource configs 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: Thu, 25 May 2023 10:41:16 -0000 On 5/25/23 12:17, Dominik Csapak wrote: > adds a config file for each type of resource (usb/pci) by using a 'map' > array propertystring for each node mapping > > in each mapping we save the path(s) and some other information to detect > hardware changes (if possible) like the vendor/device id > > both configs have custom header parser/formatter to omit the type (since > we only want one type per config here) > > also each config has some helpers like find_on_current_node > > the resulting config (e.g. for pci) would look like this: > > --- > some-pci-device > description some device > map node=node1,path=0000:00:01.0,id=1234:1234,iommugroup=4 > map node=node2,path=0000:01:01.0,id=1234:1234,iommugroup=5 > map node=node3,path=0000:02:01.0,id=1234:1234,iommugroup=6 > --- > > some special notes: > * mdev is a per config entry, since it does not make sense to mix mdev > and non-mdev devices > * there can be multiple pci paths given per node mapping, this is > intended to have a different semantic than in the qemu config, namely > it will select the first available instead of passing all through as a > multifunction device just noticed that commit message is slightly outdated, a single mapping has the same semantic as in the qemu config, and when you want multiple mappings per node there must be multiple entries > > Signed-off-by: Dominik Csapak > --- > src/Makefile | 3 + > src/PVE/Resource/PCI.pm | 226 ++++++++++++++++++++++++++++++++++++++++ > src/PVE/Resource/USB.pm | 183 ++++++++++++++++++++++++++++++++ > 3 files changed, 412 insertions(+) > create mode 100644 src/PVE/Resource/PCI.pm > create mode 100644 src/PVE/Resource/USB.pm > > diff --git a/src/Makefile b/src/Makefile > index 57a360b..d92b441 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -14,6 +14,9 @@ install: PVE > install -m 0644 PVE/Replication.pm ${PERL5DIR}/PVE/ > install -m 0644 PVE/StorageTunnel.pm ${PERL5DIR}/PVE/ > install -m 0644 PVE/Tunnel.pm ${PERL5DIR}/PVE/ > + install -d ${PERL5DIR}/PVE/Resource > + install -m 0644 PVE/Resource/PCI.pm ${PERL5DIR}/PVE/Resource/ > + install -m 0644 PVE/Resource/USB.pm ${PERL5DIR}/PVE/Resource/ > install -d ${PERL5DIR}/PVE/VZDump > install -m 0644 PVE/VZDump/Plugin.pm ${PERL5DIR}/PVE/VZDump/ > install -m 0644 PVE/VZDump/Common.pm ${PERL5DIR}/PVE/VZDump/ > diff --git a/src/PVE/Resource/PCI.pm b/src/PVE/Resource/PCI.pm > new file mode 100644 > index 0000000..d4d9c44 > --- /dev/null > +++ b/src/PVE/Resource/PCI.pm > @@ -0,0 +1,226 @@ > +package PVE::Resource::PCI; > + > +use strict; > +use warnings; > + > +use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_lock_file cfs_write_file); > +use PVE::INotify; > +use PVE::JSONSchema qw(get_standard_option parse_property_string); > +use PVE::SectionConfig; > +use PVE::SysFSTools; > + > +use base qw(PVE::SectionConfig); > + > +my $FILENAME = 'resource/pci.cfg'; > + > +cfs_register_file($FILENAME, > + sub { __PACKAGE__->parse_config(@_); }, > + sub { __PACKAGE__->write_config(@_); }); > + > + > +# so we don't have to repeat the type every time > +sub parse_section_header { > + my ($class, $line) = @_; > + > + if ($line =~ m/^(\S+)\s*$/) { > + my $id = $1; > + my $errmsg = undef; # set if you want to skip whole section > + eval { PVE::JSONSchema::pve_verify_configid($id) }; > + $errmsg = $@ if $@; > + my $config = {}; # to return additional attributes > + return ('pci', $id, $errmsg, $config); > + } > + return undef; > +} > + > +sub format_section_header { > + my ($class, $type, $sectionId, $scfg, $done_hash) = @_; > + > + return "$sectionId\n"; > +} > + > +sub type { > + return 'pci'; > +} > + > +my $PCI_RE = "[a-f0-9]{4,}:[a-f0-9]{2}:[a-f0-9]{2}(?:\.[a-f0-9])?"; > + > +my $map_fmt = { > + node => get_standard_option('pve-node'), > + id =>{ > + description => "The vendor and device ID that is expected. Used for". > + " detecting hardware changes", > + type => 'string', > + pattern => qr/^[0-9A-Fa-f]{4}:[0-9A-Fa-f]{4}$/, > + }, > + 'subsystem-id' => { > + description => "The subsystem vendor and device ID that is expected. Used". > + " for detecting hardware changes.", > + type => 'string', > + pattern => qr/^[0-9A-Fa-f]{4}:[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 attributes of the first device. You can give" > + ." multiple paths as a semicolon seperated list, the first available will then" > + ." be chosen on guest start.", > + type => 'string', > + pattern => "(?:${PCI_RE};)*${PCI_RE}", > + }, > + iommugroup => { > + type => 'integer', > + description => "The IOMMU group in which the device is to be expected in.". > + "Used for detecting hardware changes.", > + optional => 1, > + }, > + description => { > + description => "Description of the node specific device.", > + type => 'string', > + optional => 1, > + maxLength => 4096, > + }, > +}; > + > +my $defaultData = { > + propertyList => { > + id => { > + type => 'string', > + description => "The ID of the logical PCI resource.", > + format => 'pve-configid', > + }, > + description => { > + description => "Description of the logical PCI device.", > + type => 'string', > + optional => 1, > + maxLength => 4096, > + }, > + mdev => { > + type => 'boolean', > + optional => 1, > + }, > + map => { > + type => 'array', > + description => 'A list of maps for the cluster nodes.', > + optional => 1, > + items => { > + type => 'string', > + format => $map_fmt, > + }, > + }, > + }, > +}; > + > +sub private { > + return $defaultData; > +} > + > +sub options { > + return { > + description => { optional => 1 }, > + mdev => { optional => 1 }, > + map => {}, > + }; > +} > + > +# checks if the given config is valid for the current node > +sub assert_valid { > + my ($name, $cfg) = @_; > + > + my @paths = split(';', $cfg->{path} // ''); > + > + my $idx = 0; > + for my $path (@paths) { > + > + my $multifunction = 0; > + if ($path !~ m/\.[a-f0-9]/i) { > + # whole device, add .0 (must exist) > + $path = "$path.0"; > + $multifunction = 1; > + } > + > + my $info = PVE::SysFSTools::pci_device_info($path, 1); > + die "pci device '$path' not found\n" if !defined($info); > + > + my $correct_props = { > + id => "$info->{vendor}:$info->{device}", > + iommugroup => $info->{iommugroup}, > + }; > + > + if (defined($info->{'subsystem_vendor'}) && defined($info->{'subsystem_device'})) { > + $correct_props->{'subsystem-id'} = "$info->{'subsystem_vendor'}:$info->{'subsystem_device'}"; > + } > + > + for my $prop (sort keys %$correct_props) { > + next if $prop eq 'iommugroup' && $idx > 0; # check iommu only on the first device > + > + 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//g; > + my $configured_prop = $cfg->{$prop}; > + $configured_prop =~ s/0x//g; > + > + die "'$prop' does not match for '$name' ($correct_prop != $configured_prop)\n" > + if $correct_prop ne $configured_prop; > + } > + $idx++; > + } > + > + return 1; > +}; > + > +sub config { > + return cfs_read_file($FILENAME); > +} > + > +sub lock_pci_config { > + my ($code, $errmsg) = @_; > + > + cfs_lock_file($FILENAME, undef, $code); > + if (my $err = $@) { > + $errmsg ? die "$errmsg: $err" : die $err; > + } > +} > + > +sub write_pci_config { > + my ($cfg) = @_; > + > + cfs_write_file($FILENAME, $cfg); > +} > + > +sub find_on_current_node { > + my ($id) = @_; > + > + my $cfg = PVE::Resource::PCI::config(); > + my $node = PVE::INotify::nodename(); > + > + # ignore errors > + return get_node_mapping($cfg, $id, $node); > +} > + > +sub get_node_mapping { > + my ($cfg, $id, $nodename) = @_; > + > + return undef if !defined($cfg->{ids}->{$id}); > + > + my $res = []; > + for my $map ($cfg->{ids}->{$id}->{map}->@*) { > + my $entry = eval { parse_property_string($map_fmt, $map) }; > + warn $@ if $@; > + if ($entry && $entry->{node} eq $nodename) { > + push $res->@*, $entry; > + } > + } > + > + return $res; > +} > + > +PVE::Resource::PCI->register(); > +PVE::Resource::PCI->init(); > + > +1; > diff --git a/src/PVE/Resource/USB.pm b/src/PVE/Resource/USB.pm > new file mode 100644 > index 0000000..05ea789 > --- /dev/null > +++ b/src/PVE/Resource/USB.pm > @@ -0,0 +1,183 @@ > +package PVE::Resource::USB; > + > +use strict; > +use warnings; > + > +use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_lock_file cfs_write_file); > +use PVE::INotify; > +use PVE::JSONSchema qw(get_standard_option parse_property_string); > +use PVE::SectionConfig; > +use PVE::SysFSTools; > + > +use base qw(PVE::SectionConfig); > + > +my $FILENAME = 'resource/usb.cfg'; > + > +cfs_register_file($FILENAME, > + sub { __PACKAGE__->parse_config(@_); }, > + sub { __PACKAGE__->write_config(@_); }); > + > + > +# so we don't have to repeat the type every time > +sub parse_section_header { > + my ($class, $line) = @_; > + > + if ($line =~ m/^(\S+)\s*$/) { > + my $id = $1; > + my $errmsg = undef; # set if you want to skip whole section > + eval { PVE::JSONSchema::pve_verify_configid($id) }; > + $errmsg = $@ if $@; > + my $config = {}; # to return additional attributes > + return ('usb', $id, $errmsg, $config); > + } > + return undef; > +} > + > +sub format_section_header { > + my ($class, $type, $sectionId, $scfg, $done_hash) = @_; > + > + return "$sectionId\n"; > +} > + > +sub type { > + return 'usb'; > +} > + > +my $map_fmt = { > + node => get_standard_option('pve-node'), > + 'id' => { > + description => "The vendor and device ID that is expected. If a USB path". > + " is given, it is only used for detecting hardware changes", > + type => 'string', > + pattern => qr/^[0-9A-Fa-f]{4}:[0-9A-Fa-f]{4}$/, > + }, > + path => { > + description => "The path to the usb device.", > + type => 'string', > + optional => 1, > + pattern => qr/^(\d+)\-(\d+(\.\d+)*)$/, > + }, > + description => { > + description => "Description of the node specific device.", > + type => 'string', > + optional => 1, > + maxLength => 4096, > + }, > +}; > + > +my $defaultData = { > + propertyList => { > + id => { > + type => 'string', > + description => "The ID of the logical PCI resource.", > + format => 'pve-configid', > + }, > + description => { > + description => "Description of the logical PCI device.", > + type => 'string', > + optional => 1, > + maxLength => 4096, > + }, > + map => { > + type => 'array', > + description => 'A list of maps for the cluster nodes.', > + items => { > + type => 'string', > + format => $map_fmt, > + }, > + }, > + }, > +}; > +sub private { > + return $defaultData; > +} > + > +sub options { > + return { > + description => { optional => 1 }, > + map => {}, > + }; > +} > + > +# checks if the given device is valid for the current node > +sub assert_valid { > + my ($name, $cfg) = @_; > + > + my $id = $cfg->{id}; > + > + 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); > + > + my $realId = "$info->{vendid}:$info->{prodid}"; > + die "'id' does not match for '$name' ($realId != $id)\n" > + if $realId ne $id; > + } else { > + for my $dev (@$usb_list) { > + my $realId = "$dev->{vendid}:$dev->{prodid}"; > + next if $realId ne $id; > + $info = $dev; > + } > + die "usb device '$id' not found\n" if !defined($info); > + } > + > + return 1; > +}; > + > +sub config { > + return cfs_read_file($FILENAME); > +} > + > +sub lock_usb_config { > + my ($code, $errmsg) = @_; > + > + cfs_lock_file($FILENAME, undef, $code); > + if (my $err = $@) { > + $errmsg ? die "$errmsg: $err" : die $err; > + } > +} > + > +sub write_usb_config { > + my ($cfg) = @_; > + > + cfs_write_file($FILENAME, $cfg); > +} > + > +sub find_on_current_node { > + my ($id) = @_; > + > + my $cfg = config(); > + my $node = PVE::INotify::nodename(); > + > + return get_node_mapping($cfg, $id, $node); > +} > + > +sub get_node_mapping { > + my ($cfg, $id, $nodename) = @_; > + > + return undef if !defined($cfg->{ids}->{$id}); > + > + my $res = []; > + for my $map ($cfg->{ids}->{$id}->{map}->@*) { > + my $entry = eval { parse_property_string($map_fmt, $map) }; > + warn $@ if $@; > + if ($entry && $entry->{node} eq $nodename) { > + push $res->@*, $entry; > + } > + } > + > + return $res; > +} > + > +PVE::Resource::USB->register(); > +PVE::Resource::USB->init(); > + > +1;