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 C46AE9200A for ; Wed, 31 Jan 2024 16:56:39 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A561E3D818 for ; Wed, 31 Jan 2024 16:56:39 +0100 (CET) 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 ; Wed, 31 Jan 2024 16:56:38 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 5C6F04051E for ; Wed, 31 Jan 2024 16:56:38 +0100 (CET) Message-ID: <65e8c2fb-0cea-45d7-8e87-0fcb2044ecd7@proxmox.com> Date: Wed, 31 Jan 2024 16:56:36 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Proxmox VE development discussion , Markus Frank References: <20231108085254.53574-1-m.frank@proxmox.com> <20231108085254.53574-8-m.frank@proxmox.com> From: Fiona Ebner In-Reply-To: <20231108085254.53574-8-m.frank@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.223 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 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [pci.pm, usb.pm, dir.pm, defines.mk, mapping.pm] Subject: Re: [pve-devel] [PATCH manager v8 7/7] api: add resource map api endpoints for directories 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: Wed, 31 Jan 2024 15:56:39 -0000 Since you require the invariant that there is no duplicate entry for a single node in qemu-server, you could check when adding or updating the mapping that no such duplicate is created. For example, pvesh create /cluster/mapping/dir --id foo --map node=pve8a1,path=/root/bar --map node=pve8a1,path=/root/baz currently does not fail, but it would be nicer if it did. Am 08.11.23 um 09:52 schrieb Markus Frank: > Signed-off-by: Markus Frank > --- > PVE/API2/Cluster/Mapping.pm | 7 + > PVE/API2/Cluster/Mapping/Dir.pm | 309 ++++++++++++++++++++++++++++++ > PVE/API2/Cluster/Mapping/Makefile | 3 +- > 3 files changed, 318 insertions(+), 1 deletion(-) > create mode 100644 PVE/API2/Cluster/Mapping/Dir.pm > > diff --git a/PVE/API2/Cluster/Mapping.pm b/PVE/API2/Cluster/Mapping.pm > index 40386579..c5993208 100644 > --- a/PVE/API2/Cluster/Mapping.pm > +++ b/PVE/API2/Cluster/Mapping.pm > @@ -5,6 +5,7 @@ use warnings; > > use PVE::API2::Cluster::Mapping::PCI; > use PVE::API2::Cluster::Mapping::USB; > +use PVE::API2::Cluster::Mapping::Dir; > > use base qw(PVE::RESTHandler); > > @@ -18,6 +19,11 @@ __PACKAGE__->register_method ({ > path => 'usb', > }); > > +__PACKAGE__->register_method ({ > + subclass => "PVE::API2::Cluster::Mapping::Dir", > + path => 'dir', > +}); Nit: not sorted alphabetically > + > __PACKAGE__->register_method ({ > name => 'index', > path => '', > @@ -43,6 +49,7 @@ __PACKAGE__->register_method ({ > my $result = [ > { name => 'pci' }, > { name => 'usb' }, > + { name => 'dir' }, Nit: not sorted alphabetically > ]; > > return $result; > diff --git a/PVE/API2/Cluster/Mapping/Dir.pm b/PVE/API2/Cluster/Mapping/Dir.pm > new file mode 100644 > index 00000000..f6e8f26f > --- /dev/null > +++ b/PVE/API2/Cluster/Mapping/Dir.pm > @@ -0,0 +1,309 @@ > +package PVE::API2::Cluster::Mapping::Dir; > + > +use strict; > +use warnings; > + > +use Storable qw(dclone); > + > +use PVE::Mapping::Dir (); > +use PVE::JSONSchema qw(get_standard_option); > +use PVE::Tools qw(extract_param); > + > +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 Dir Hardware Mapping", s/Dir/directory not sure it should include "hardware" and no need for title case > + permissions => { > + description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or". > + " 'Mapping.Audit' permissions on '/mapping/dir/'.", > + 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 diagnostics for the devices to the response.", s/devices/directories > + 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.", > + }, > + xattr => { > + type => 'boolean', > + description => "Enable support for extended attributes.", > + optional => 1, > + }, > + acl => { > + type => 'boolean', > + description => "Enable support for posix ACLs (implies --xattr).", > + optional => 1, > + }, > + checks => { > + type => "array", > + optional => 1, > + description => "A list of checks, only present if 'check_node' is set.", It's "check-node" > + items => { > + type => 'object', > + properties => { > + severity => { > + type => "string", > + enum => ['warning', 'error'], > + description => "The severity of the error", > + }, > + message => { > + type => "string", > + description => "The message of the error", > + }, > + }, > + } > + }, > + }, > + }, > + links => [ { rel => 'child', href => "{id}" } ], > + }, > + code => sub { > + my ($param) = @_; > + > + my $rpcenv = PVE::RPCEnvironment::get(); missing use statement for PVE::RPCEnvironment at the start of the file > + my $authuser = $rpcenv->get_user(); > + > + my $check_node = $param->{'check-node'}; > + my $local_node = PVE::INotify::nodename(); missing use statement for PVE::INotify at the start of the file > + > + die "wrong node to check - $check_node != $local_node\n" > + if defined($check_node) && $check_node ne 'localhost' && $check_node ne $local_node; > + > + my $cfg = PVE::Mapping::Dir::config(); > + > + my $can_see_mapping_privs = ['Mapping.Modify', 'Mapping.Use', 'Mapping.Audit']; > + > + my $res = []; > + for my $id (keys $cfg->{ids}->%*) { > + next if !$rpcenv->check_any($authuser, "/mapping/dir/$id", $can_see_mapping_privs, 1); > + next if !$cfg->{ids}->{$id}; > + > + my $entry = dclone($cfg->{ids}->{$id}); > + $entry->{id} = $id; > + $entry->{digest} = $cfg->{digest}; > + > + if (defined($check_node)) { > + $entry->{checks} = []; > + if (my $mappings = PVE::Mapping::Dir::get_node_mapping($cfg, $id, $check_node)) { > + if (!scalar($mappings->@*)) { > + push $entry->{checks}->@*, { > + severity => 'warning', > + message => "No mapping for node $check_node.", > + }; > + } > + for my $mapping ($mappings->@*) { > + eval { PVE::Mapping::Dir::assert_valid($id, $mapping) }; assert_valid() only takes one parameter, i.e. the config > + if (my $err = $@) { > + push $entry->{checks}->@*, { > + severity => 'error', > + message => "Invalid configuration: $err", > + }; > + } > + } > + } > + } > + > + push @$res, $entry; > + } > + > + return $res; > + }, > +}); > + > +__PACKAGE__->register_method ({ > + name => 'get', > + protected => 1, > + path => '{id}', > + method => 'GET', > + description => "Get Dir Mapping.", s/Dir/directory no need for title case ---snip--- > +__PACKAGE__->register_method ({ > + name => 'create', > + protected => 1, > + path => '', > + method => 'POST', > + description => "Create a new hardware mapping.", s/hardware/directory/ > + permissions => { > + check => ['perm', '/mapping/dir', ['Mapping.Modify']], > + }, > + parameters => PVE::Mapping::Dir->createSchema(1), > + returns => { > + type => 'null', > + }, > + code => sub { > + my ($param) = @_; > + > + my $id = extract_param($param, 'id'); > + > + my $plugin = PVE::Mapping::Dir->lookup('dir'); > + my $opts = $plugin->check_config($id, $param, 1, 1); > + > + PVE::Mapping::Dir::lock_dir_config(sub { > + my $cfg = PVE::Mapping::Dir::config(); > + > + die "dir ID '$id' already defined\n" if defined($cfg->{ids}->{$id}); > + > + $cfg->{ids}->{$id} = $opts; > + > + PVE::Mapping::Dir::write_dir_config($cfg); > + > + }, "create hardware mapping failed"); s/hardware/directory/ > + > + return; > + }, > +}); > + > +__PACKAGE__->register_method ({ > + name => 'update', > + protected => 1, > + path => '{id}', > + method => 'PUT', > + description => "Update a hardware mapping.", s/hardware/directory/ > + permissions => { > + check => ['perm', '/mapping/dir/{id}', ['Mapping.Modify']], > + }, > + parameters => PVE::Mapping::Dir->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::Dir::lock_dir_config(sub { > + my $cfg = PVE::Mapping::Dir::config(); > + > + PVE::Tools::assert_if_modified($cfg->{digest}, $digest) if defined($digest); > + > + die "dir ID '$id' does not exist\n" if !defined($cfg->{ids}->{$id}); > + > + my $plugin = PVE::Mapping::Dir->lookup('dir'); > + my $opts = $plugin->check_config($id, $param, 1, 1); > + > + my $data = $cfg->{ids}->{$id}; > + > + my $options = $plugin->private()->{options}->{dir}; > + PVE::SectionConfig::delete_from_config($data, $options, $opts, $delete); > + > + $data->{$_} = $opts->{$_} for keys $opts->%*; > + > + PVE::Mapping::Dir::write_dir_config($cfg); > + > + }, "update hardware mapping failed"); s/hardware/directory/ > + > + return; > + }, > +}); > + > +__PACKAGE__->register_method ({ > + name => 'delete', > + protected => 1, > + path => '{id}', > + method => 'DELETE', > + description => "Remove Hardware Mapping.", s/hardware/directory/ no need for title case > + permissions => { > + check => [ 'perm', '/mapping/dir', ['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::Dir::lock_dir_config(sub { > + my $cfg = PVE::Mapping::Dir::config(); > + > + if ($cfg->{ids}->{$id}) { > + delete $cfg->{ids}->{$id}; > + } > + > + PVE::Mapping::Dir::write_dir_config($cfg); > + > + }, "delete dir mapping failed"); s/dir/directory/ > + > + return; > + } > +}); > + > +1; > diff --git a/PVE/API2/Cluster/Mapping/Makefile b/PVE/API2/Cluster/Mapping/Makefile > index e7345ab4..a80c8ab3 100644 > --- a/PVE/API2/Cluster/Mapping/Makefile > +++ b/PVE/API2/Cluster/Mapping/Makefile > @@ -4,7 +4,8 @@ include ../../../../defines.mk > # ensure we do not conflict with files shipped by pve-cluster!! > PERLSOURCE= \ > PCI.pm \ > - USB.pm > + USB.pm \ > + Dir.pm Nit: not sorted alphabetically