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 2630CA0618 for ; Tue, 13 Jun 2023 13:26:23 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 04D282ED48 for ; Tue, 13 Jun 2023 13:26:23 +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 ; Tue, 13 Jun 2023 13:26:21 +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 21A8044D9A for ; Tue, 13 Jun 2023 13:26:21 +0200 (CEST) Date: Tue, 13 Jun 2023 13:26:18 +0200 From: Wolfgang Bumiller To: Dominik Csapak Cc: pve-devel@lists.proxmox.com Message-ID: References: <20230606135222.984747-1-d.csapak@proxmox.com> <20230606135222.984747-11-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20230606135222.984747-11-d.csapak@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL -0.024 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 POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes 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 v5 02/15] api: add resource map api endpoints for PCI and USB 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: Tue, 13 Jun 2023 11:26:23 -0000 On Tue, Jun 06, 2023 at 03:52:09PM +0200, Dominik Csapak wrote: > this adds the typical section config crud API calls for > USB and PCI resource mapping to /cluster/resource/{TYPE} > > the only special thing that this series does is the list call > for both has a special 'check-node' parameter that uses the > 'proxyto_callback' to reroute the api call to the given node > so that it can check the validity of the mapping for that node > > in the future when we e.g. broadcast the lspci output via pmxcfs Having this cached somewhere might definitely be desirable... $ time lspci >/dev/null real 0m0.400s almost half a second if there's thunderbolt involved ;-) > we drop the proxyto_callback and directly use the info from > pmxcfs (or we drop the parameter and always check all nodes) > > Signed-off-by: Dominik Csapak > --- > changes from v4: > * rename s/resource/mapping/i > * drop checking for arrayref on parameter map > (we don't need it since the last version of the api array support > patch) > > PVE/API2/Cluster.pm | 8 + > PVE/API2/Cluster/Makefile | 5 + > PVE/API2/Cluster/Mapping.pm | 53 ++++++ > PVE/API2/Cluster/Mapping/Makefile | 18 ++ > PVE/API2/Cluster/Mapping/PCI.pm | 298 ++++++++++++++++++++++++++++++ > PVE/API2/Cluster/Mapping/USB.pm | 293 +++++++++++++++++++++++++++++ > PVE/API2/Hardware.pm | 1 - > PVE/API2/Nodes.pm | 1 + > 8 files changed, 676 insertions(+), 1 deletion(-) > create mode 100644 PVE/API2/Cluster/Mapping.pm > create mode 100644 PVE/API2/Cluster/Mapping/Makefile > create mode 100644 PVE/API2/Cluster/Mapping/PCI.pm > create mode 100644 PVE/API2/Cluster/Mapping/USB.pm > > diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm > index 2e942368..2a9da90e 100644 > --- a/PVE/API2/Cluster.pm > +++ b/PVE/API2/Cluster.pm > @@ -26,6 +26,7 @@ use PVE::API2::ACMEPlugin; > use PVE::API2::Backup; > use PVE::API2::Cluster::BackupInfo; > use PVE::API2::Cluster::Ceph; > +use PVE::API2::Cluster::Mapping; > use PVE::API2::Cluster::Jobs; > use PVE::API2::Cluster::MetricServer; > use PVE::API2::ClusterConfig; > @@ -90,6 +91,12 @@ __PACKAGE__->register_method ({ > subclass => "PVE::API2::Cluster::Jobs", > path => 'jobs', > }); > + > +__PACKAGE__->register_method ({ > + subclass => "PVE::API2::Cluster::Mapping", > + path => 'mapping', > +}); > + > if ($have_sdn) { > __PACKAGE__->register_method ({ > subclass => "PVE::API2::Network::SDN", > @@ -145,6 +152,7 @@ __PACKAGE__->register_method ({ > { name => 'options' }, > { name => 'replication' }, > { name => 'resources' }, > + { name => 'resource' }, ^ should be 'mapping' > { name => 'status' }, > { name => 'tasks' }, > ]; > diff --git a/PVE/API2/Cluster/Makefile b/PVE/API2/Cluster/Makefile > index 5c92e4f6..0c52a241 100644 > --- a/PVE/API2/Cluster/Makefile > +++ b/PVE/API2/Cluster/Makefile > @@ -1,10 +1,13 @@ > include ../../../defines.mk > > +SUBDIRS=Mapping > + > # for node independent, cluster-wide applicable, API endpoints > # ensure we do not conflict with files shipped by pve-cluster!! > PERLSOURCE= \ > BackupInfo.pm \ > MetricServer.pm \ > + Mapping.pm \ > Jobs.pm \ > Ceph.pm > > @@ -13,8 +16,10 @@ all: > .PHONY: clean > clean: > rm -rf *~ > + set -e && for i in ${SUBDIRS}; do ${MAKE} -C $$i $@; done > > .PHONY: install > install: $(PERLSOURCE) > install -d $(PERLLIBDIR)/PVE/API2/Cluster > install -m 0644 $(PERLSOURCE) $(PERLLIBDIR)/PVE/API2/Cluster > + set -e && for i in $(SUBDIRS); do $(MAKE) -C $$i $@; done > diff --git a/PVE/API2/Cluster/Mapping.pm b/PVE/API2/Cluster/Mapping.pm > new file mode 100644 > index 00000000..01fa986b > --- /dev/null > +++ b/PVE/API2/Cluster/Mapping.pm > @@ -0,0 +1,53 @@ > +package PVE::API2::Cluster::Mapping; > + > +use strict; > +use warnings; > + > +use PVE::RESTHandler; > + > +use PVE::API2::Cluster::Mapping::PCI; > +use PVE::API2::Cluster::Mapping::USB; > + > +use base qw(PVE::RESTHandler); > + > +__PACKAGE__->register_method ({ > + subclass => "PVE::API2::Cluster::Mapping::PCI", > + path => 'pci', > +}); > + > +__PACKAGE__->register_method ({ > + subclass => "PVE::API2::Cluster::Mapping::USB", > + path => 'usb', > +}); > + > +__PACKAGE__->register_method ({ > + name => 'index', > + path => '', > + method => 'GET', > + description => "List resource types.", > + permissions => { > + user => 'all', > + }, > + parameters => { > + additionalProperties => 0, > + properties => {}, > + }, > + returns => { > + type => 'array', > + items => { > + type => "object", > + }, > + links => [ { rel => 'child', href => "{name}" } ], > + }, > + code => sub { > + my ($param) = @_; > + > + my $result = [ > + { name => 'pci' }, > + { name => 'usb' }, > + ]; > + > + return $result; > + }}); > + > +1; > diff --git a/PVE/API2/Cluster/Mapping/Makefile b/PVE/API2/Cluster/Mapping/Makefile > new file mode 100644 > index 00000000..e7345ab4 > --- /dev/null > +++ b/PVE/API2/Cluster/Mapping/Makefile > @@ -0,0 +1,18 @@ > +include ../../../../defines.mk > + > +# for node independent, cluster-wide applicable, API endpoints > +# ensure we do not conflict with files shipped by pve-cluster!! > +PERLSOURCE= \ > + PCI.pm \ > + USB.pm > + > +all: > + > +.PHONY: clean > +clean: > + rm -rf *~ > + > +.PHONY: install > +install: ${PERLSOURCE} > + install -d ${PERLLIBDIR}/PVE/API2/Cluster/Mapping > + install -m 0644 ${PERLSOURCE} ${PERLLIBDIR}/PVE/API2/Cluster/Mapping > diff --git a/PVE/API2/Cluster/Mapping/PCI.pm b/PVE/API2/Cluster/Mapping/PCI.pm > new file mode 100644 > index 00000000..b0d3066d > --- /dev/null > +++ b/PVE/API2/Cluster/Mapping/PCI.pm > @@ -0,0 +1,298 @@ > +package PVE::API2::Cluster::Mapping::PCI; > + > +use strict; > +use warnings; > + > +use Storable qw(dclone); > + > +use PVE::Cluster qw(cfs_lock_file); > +use PVE::Mapping::PCI; > +use PVE::JSONSchema qw(get_standard_option); > +use PVE::Tools qw(extract_param); > + > +use PVE::RESTHandler; > + > +use base qw(PVE::RESTHandler); > + > +__PACKAGE__->register_method ({ > + name => 'index', > + path => '', > + method => 'GET', > + # only proxy if we give the 'check-node' parameter > + proxyto_callback => sub { > + my ($rpcenv, $proxyto, $param) = @_; > + return $param->{'check-node'} // 'localhost'; > + }, > + description => "List PCI Hardware Mapping", > + permissions => { > + description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or". > + " 'Mapping.Audit' permissions on '/mapping/pci/'.", > + user => 'all', > + }, > + parameters => { > + additionalProperties => 0, > + properties => { > + 'check-node' => get_standard_option('pve-node', { > + description => "If given, checks the configurations on the given node for ". > + "correctness, and adds relevant errors to the devices.", > + optional => 1, > + }), > + }, > + }, > + returns => { > + type => 'array', > + items => { > + type => "object", > + properties => { > + id => { > + type => 'string', > + description => "The logical ID of the mapping." > + }, > + map => { > + type => 'array', > + description => "The entries of the mapping.", > + items => { > + type => 'string', > + description => "A mapping for a node.", > + }, > + }, > + description => { > + type => 'string', > + description => "A description of the logical mapping.", > + }, > + error => { > + description => "A list of errors when 'check_node' is given.", > + items => { > + type => 'object', > + properties => { > + severity => { > + type => "string", > + description => "The severity of the error", > + }, > + message => { > + type => "string", > + description => "The message of the error", > + }, > + }, > + } > + }, > + }, > + }, > + links => [ { rel => 'child', href => "{name}" } ], > + }, > + code => sub { > + my ($param) = @_; > + > + my $rpcenv = PVE::RPCEnvironment::get(); > + my $authuser = $rpcenv->get_user(); > + my $node = $param->{'check-node'}; > + > + die "Wrong node to check\n" if defined($node) && $node ne PVE::INotify::nodename(); ((might we want explicit 'localhost' suport here? not sure if this has been discussed in any previous versions)) > + > + my $cfg = PVE::Mapping::PCI::config(); > + > + my $res = []; > + > + my $privs = ['Mapping.Modify', 'Mapping.Use', 'Mapping.Audit']; > + > + for my $id (keys $cfg->{ids}->%*) { > + next if !$rpcenv->check_full($authuser, "/mapping/pci/$id", $privs, 1, 1); > + next if !$cfg->{ids}->{$id}; > + > + my $entry = dclone($cfg->{ids}->{$id}); > + $entry->{id} = $id; > + $entry->{digest} = $cfg->{digest}; > + > + if (defined($node)) { > + $entry->{errors} = []; > + if (my $mappings = PVE::Mapping::PCI::get_node_mapping($cfg, $id, $node)) { > + if (!scalar($mappings->@*)) { > + push $entry->{errors}->@*, { > + severity => 'warning', > + message => "No mapping for node $node.", > + }; > + } > + for my $mapping ($mappings->@*) { > + eval { > + PVE::Mapping::PCI::assert_valid($id, $mapping); > + }; > + if (my $err = $@) { > + push $entry->{errors}->@*, { > + severity => 'error', > + message => "Invalid configuration: $err", > + }; > + } > + } > + } > + } > + > + push @$res, $entry; > + } > + > + return $res; > + }, > +}); > + > +__PACKAGE__->register_method ({ > + name => 'get', > + protected => 1, > + path => '{id}', > + method => 'GET', > + description => "GET PCI Mapping.", I think we don't need to capitalize 'Get' in the _description_ ;-) > + permissions => { > + check =>['or', > + ['perm', '/mapping/pci/{name}', ['Mapping.Use']], > + ['perm', '/mapping/pci/{name}', ['Mapping.Modify']], > + ], > + }, > + parameters => { > + additionalProperties => 0, > + properties => { > + id => { > + type => 'string', > + format => 'pve-configid', > + }, > + } > + }, > + returns => { type => 'object' }, > + code => sub { > + my ($param) = @_; > + > + my $cfg = PVE::Mapping::PCI::config(); > + my $name = $param->{id}; > + > + die "mapping '$param->{name}' not found\n" if !defined($cfg->{ids}->{$name}); > + > + my $data = dclone($cfg->{ids}->{$name}); A bunch of these hash walks could be deduplicated around the patch IMO. dclone($cfg->…-> // die "mapping … not found") > + > + $data->{digest} = $cfg->{digest}; > + > + return $data; > + }}); > + > +__PACKAGE__->register_method ({ > + name => 'create', > + protected => 1, > + path => '', > + method => 'POST', > + description => "Create a new hardware mapping.", > + permissions => { > + check => ['perm', '/mapping/pci/{name}', ['Mapping.Modify']], > + }, > + # todo parameters > + parameters => PVE::Mapping::PCI->createSchema(1), > + returns => { > + type => 'null', > + }, > + code => sub { > + my ($param) = @_; > + > + my $id = extract_param($param, 'id'); > + > + my $plugin = PVE::Mapping::PCI->lookup('pci'); > + my $opts = $plugin->check_config($id, $param, 1, 1); > + > + PVE::Mapping::PCI::lock_pci_config(sub { > + my $cfg = PVE::Mapping::PCI::config(); > + > + die "pci ID '$id' already defined\n" if defined($cfg->{ids}->{$id}); > + > + $cfg->{ids}->{$id} = $opts; > + > + PVE::Mapping::PCI::write_pci_config($cfg); > + > + }, "create hardware mapping failed"); > + > + return; > + }, > +}); > + > +__PACKAGE__->register_method ({ > + name => 'update', > + protected => 1, > + path => '{id}', > + method => 'PUT', > + description => "Update a hardware mapping.", > + permissions => { > + check => ['perm', '/mapping/pci/{id}', ['Mapping.Modify']], > + }, > + parameters => PVE::Mapping::PCI->updateSchema(), > + returns => { > + type => 'null', > + }, > + code => sub { > + my ($param) = @_; > + > + my $digest = extract_param($param, 'digest'); > + my $delete = extract_param($param, 'delete'); > + my $id = extract_param($param, 'id'); > + > + if ($delete) { > + $delete = [ PVE::Tools::split_list($delete) ]; > + } > + > + PVE::Mapping::PCI::lock_pci_config(sub { > + my $cfg = PVE::Mapping::PCI::config(); > + > + PVE::Tools::assert_if_modified($cfg->{digest}, $digest) if defined($digest); > + > + die "pci ID '$id' does not exist\n" if !defined($cfg->{ids}->{$id}); > + > + my $plugin = PVE::Mapping::PCI->lookup('pci'); > + my $opts = $plugin->check_config($id, $param, 1, 1); > + > + my $data = $cfg->{ids}->{$id}; > + > + my $options = $plugin->private()->{options}->{pci}; > + PVE::SectionConfig::delete_from_config($data, $options, $opts, $delete); > + > + $data->{$_} = $opts->{$_} for keys $opts->%*; > + > + PVE::Mapping::PCI::write_pci_config($cfg); > + > + }, "update hardware mapping failed"); > + > + return; > + }, > +}); > + > +__PACKAGE__->register_method ({ > + name => 'delete', > + protected => 1, > + path => '{id}', > + method => 'DELETE', > + description => "Remove Hardware Mapping.", > + permissions => { > + check => [ 'perm', '/mapping/pci/{id}', ['Mapping.Modify']], > + }, > + parameters => { > + additionalProperties => 0, > + properties => { > + id => { > + type => 'string', > + format => 'pve-configid', > + }, > + } > + }, > + returns => { type => 'null' }, > + code => sub { > + my ($param) = @_; > + > + my $id = $param->{id}; > + > + PVE::Mapping::PCI::lock_pci_config(sub { > + my $cfg = PVE::Mapping::PCI::config(); > + > + if ($cfg->{ids}->{$id}) { > + delete $cfg->{ids}->{$id}; > + } > + > + PVE::Mapping::PCI::write_pci_config($cfg); > + > + }, "delete pci mapping failed"); > + > + return; > + } > +}); > + > +1; > diff --git a/PVE/API2/Cluster/Mapping/USB.pm b/PVE/API2/Cluster/Mapping/USB.pm > new file mode 100644 > index 00000000..fced96eb > --- /dev/null > +++ b/PVE/API2/Cluster/Mapping/USB.pm > @@ -0,0 +1,293 @@ (...) > + > +__PACKAGE__->register_method ({ > + name => 'get', > + protected => 1, > + path => '{id}', > + method => 'GET', > + description => "GET USB Mapping.", I think we don't need to capitalize 'Get' in the _description_ ;-) > + permissions => { > + check =>['or', > + ['perm', '/mapping/usb/{id}', ['Mapping.Use']], > + ['perm', '/mapping/usb/{id}', ['Mapping.Modify']], > + ], > + }, > + parameters => { > + additionalProperties => 0, > + properties => { > + id => { > + type => 'string', > + format => 'pve-configid', > + }, > + } > + }, > + returns => { type => 'object' }, > + code => sub { > + my ($param) = @_; > + > + my $cfg = PVE::Mapping::USB::config(); > + my $id = $param->{id}; > + > + die "mapping '$param->{id}' not found\n" if !defined($cfg->{ids}->{$id}); > + > + my $data = dclone($cfg->{ids}->{$id}); > + > + $data->{digest} = $cfg->{digest}; > + > + return $data; > + }}); > + > +__PACKAGE__->register_method ({ > + name => 'create', > + protected => 1, > + path => '', > + method => 'POST', > + description => "Create a new hardware mapping.", > + permissions => { > + check => ['perm', '/mapping/usb/{name}', ['Mapping.Modify']], > + }, > + # todo parameters > + parameters => PVE::Mapping::USB->createSchema(1), > + returns => { > + type => 'null', > + }, > + code => sub { > + my ($param) = @_; > + > + my $id = extract_param($param, 'id'); > + > + my $plugin = PVE::Mapping::USB->lookup('usb'); > + my $opts = $plugin->check_config($id, $param, 1, 1); > + > + PVE::Mapping::USB::lock_usb_config(sub { > + my $cfg = PVE::Mapping::USB::config(); > + > + die "usb ID '$id' already defined\n" if defined($cfg->{ids}->{$id}); > + > + $cfg->{ids}->{$id} = $opts; > + > + PVE::Mapping::USB::write_usb_config($cfg); > + > + }, "create hardware mapping failed"); > + > + return; > + }, > +}); > + > +__PACKAGE__->register_method ({ > + name => 'update', > + protected => 1, > + path => '{id}', > + method => 'PUT', > + description => "Update a hardware mapping.", > + permissions => { > + check => ['perm', '/mapping/usb/{id}', ['Mapping.Modify']], > + }, > + parameters => PVE::Mapping::USB->updateSchema(), > + returns => { > + type => 'null', > + }, > + code => sub { > + my ($param) = @_; > + > + my $digest = extract_param($param, 'digest'); > + my $delete = extract_param($param, 'delete'); > + my $id = extract_param($param, 'id'); > + > + if ($delete) { > + $delete = [ PVE::Tools::split_list($delete) ]; > + } > + > + PVE::Mapping::USB::lock_usb_config(sub { > + my $cfg = PVE::Mapping::USB::config(); > + > + PVE::Tools::assert_if_modified($cfg->{digest}, $digest) if defined($digest); > + > + die "usb ID '$id' does not exist\n" if !defined($cfg->{ids}->{$id}); > + > + my $plugin = PVE::Mapping::USB->lookup('usb'); > + my $opts = $plugin->check_config($id, $param, 1, 1); > + > + my $data = $cfg->{ids}->{$id}; > + > + my $options = $plugin->private()->{options}->{usb}; > + PVE::SectionConfig::delete_from_config($data, $options, $opts, $delete); > + > + $data->{$_} = $opts->{$_} for keys $opts->%*; > + > + PVE::Mapping::USB::write_usb_config($cfg); > + > + }, "update hardware mapping failed"); > + > + return; > + }, > +}); > + > +__PACKAGE__->register_method ({ > + name => 'delete', > + protected => 1, > + path => '{id}', > + method => 'DELETE', > + description => "Remove Hardware Mapping.", > + permissions => { > + check => [ 'perm', '/mapping/usb/{id}', ['Mapping.Modify']], > + }, > + parameters => { > + additionalProperties => 0, > + properties => { > + id => { > + type => 'string', > + format => 'pve-configid', > + }, > + } > + }, > + returns => { type => 'null' }, > + code => sub { > + my ($param) = @_; > + > + my $id = $param->{id}; > + > + PVE::Mapping::USB::lock_usb_config(sub { > + my $cfg = PVE::Mapping::USB::config(); > + > + if ($cfg->{ids}->{$id}) { > + delete $cfg->{ids}->{$id}; > + } > + > + PVE::Mapping::USB::write_usb_config($cfg); > + > + }, "delete usb mapping failed"); > + > + return; > + } > +}); > + > +1; > diff --git a/PVE/API2/Hardware.pm b/PVE/API2/Hardware.pm > index f59bfbe0..1c6fd8f5 100644 > --- a/PVE/API2/Hardware.pm > +++ b/PVE/API2/Hardware.pm > @@ -21,7 +21,6 @@ __PACKAGE__->register_method ({ > path => 'usb', > }); > > - stray cleanup hunk? > __PACKAGE__->register_method ({ > name => 'index', > path => '', > diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm > index bfe5c40a..bf498bed 100644 > --- a/PVE/API2/Nodes.pm > +++ b/PVE/API2/Nodes.pm > @@ -278,6 +278,7 @@ __PACKAGE__->register_method ({ > { name => 'query-url-metadata' }, > { name => 'replication' }, > { name => 'report' }, > + { name => 'resource-map' }, is this even still used? > { name => 'rrd' }, # fixme: remove? > { name => 'rrddata' },# fixme: remove? > { name => 'scan' }, > -- > 2.30.2