From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 8247E1FF16F for ; Thu, 19 Dec 2024 17:01:17 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 776EBBE1F; Thu, 19 Dec 2024 17:01:17 +0100 (CET) From: Daniel Kral To: f.gruenbichler@proxmox.com Date: Thu, 19 Dec 2024 17:01:07 +0100 Message-Id: <20241219160107.183014-1-d.kral@proxmox.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20240416122054.733817-2-f.gruenbichler@proxmox.com> References: <20240416122054.733817-2-f.gruenbichler@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.004 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 Subject: Re: [pve-devel] [PATCH v2 access-control 1/1] pools: define resource limits 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: , Reply-To: Proxmox VE development discussion Cc: pve-devel@lists.proxmox.com Content-Type: multipart/mixed; boundary="===============4287630379095564719==" Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" --===============4287630379095564719== Content-Transfer-Encoding: quoted-printable On 16/04/2024 14:20, Fabian Gr=C3=BCnbichler wrote:=0D > and handle them when parsing/writing user.cfg=0D > =0D > Signed-off-by: Fabian Gr=C3=BCnbichler =0D > ---=0D > =0D > Notes:=0D > - make limit schema public for pve-guest-common usage=0D > =0D > src/PVE/AccessControl.pm | 42 +++++++++++++++++++++++++++++++++++++--=0D > src/test/parser_writer.pl | 14 ++++++-------=0D > 2 files changed, 47 insertions(+), 9 deletions(-)=0D > =0D > diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm=0D > index 21f93ff..f1863c8 100644=0D > --- a/src/PVE/AccessControl.pm=0D > +++ b/src/PVE/AccessControl.pm=0D > @@ -72,6 +72,36 @@ sub pve_verify_realm {=0D > PVE::Auth::Plugin::pve_verify_realm(@_);=0D > }=0D > =0D > +our $pool_limits_desc =3D {=0D > + "mem-config" =3D> {=0D > + type =3D> 'integer',=0D > + description =3D> "Sum of memory (in MB) guests in this pools can be con= figured with.",=0D =0D I think this should be in MiB.=0D =0D Also, I think it's a little bit more readable if we'd rephase it to use=0D either "maximum amount of" or "upper limit for", e.g.:=0D =0D "The maximum amount of memory (in MiB), which can be configured for all=0D guests in this pool."=0D =0D > + optional =3D> 1,=0D > + },=0D > + "mem-run" =3D> {=0D > + type =3D> 'integer',=0D > + description =3D> "Sum of memory (in MB) guests in this pools can be sta= rted with.",=0D =0D I think this should be in MiB.=0D =0D Similar changes as to 'mem-config', e.g.:=0D =0D "The maximum amount of memory (in MiB), which can be configured for=0D running guests in this pool at the same time."=0D =0D And maybe append something like:=0D =0D "This amount must be lower than 'mem-config'."=0D =0D I thought about using "allocated to" instead of "configured for", but=0D this would likely cause readers to believe that it's the actual=0D allocated amount of memory.=0D =0D > + optional =3D> 1,=0D > + },=0D > + "cpu-config" =3D> {=0D > + type =3D> 'integer',=0D > + description =3D> "Sum of (virtual) cores guests in this pools can be co= nfigured with.",=0D =0D Similar to 'mem-config':=0D =0D "The maximum amount of virtual CPU cores, which can be configured for=0D all guests in this pool."=0D =0D > + optional =3D> 1,=0D > + },=0D > + "cpu-run" =3D> {=0D > + type =3D> 'integer',=0D > + description =3D> "Sum of (virtual) cores guests in this pools can be st= arted with.",=0D =0D Similar to 'mem-run':=0D =0D "The maximum amount of virtual CPU cores, which can be configured for=0D running guests in this pool at the same time. This amount must be lower=0D than 'cpu-config'."=0D =0D > + optional =3D> 1,=0D > + },=0D > +};=0D > +=0D > +PVE::JSONSchema::register_format('pve-pool-limits', $pool_limits_desc);= =0D > +PVE::JSONSchema::register_standard_option('pve-pool-limits', {=0D > + type =3D> 'string',=0D > + format =3D> $pool_limits_desc,=0D > + optional =3D> 1,=0D > +});=0D > +=0D > # Locking both config files together is only ever allowed in one order:= =0D > # 1) tfa config=0D > # 2) user config=0D > @@ -1524,7 +1554,7 @@ sub parse_user_config {=0D > warn "user config - ignore invalid path in acl '$pathtxt'\n";=0D > }=0D > } elsif ($et eq 'pool') {=0D > - my ($pool, $comment, $vmlist, $storelist) =3D @data;=0D > + my ($pool, $comment, $vmlist, $storelist, $limits) =3D @data;=0D > =0D > if (!verify_poolname($pool, 1)) {=0D > warn "user config - ignore pool '$pool' - invalid characters in pool n= ame\n";=0D > @@ -1575,6 +1605,13 @@ sub parse_user_config {=0D > }=0D > $cfg->{pools}->{$pool}->{storage}->{$storeid} =3D 1;=0D > }=0D > +=0D > + if ($limits) {=0D > + my $parsed_limits =3D eval { PVE::JSONSchema::parse_property_string($p= ool_limits_desc, $limits) };=0D > + warn "Failed to parse pool limits for '$pool' - $@\n" if $@;=0D > +=0D > + $cfg->{pools}->{$pool}->{limits} =3D $parsed_limits;=0D > + }=0D > } elsif ($et eq 'token') {=0D > my ($tokenid, $expire, $privsep, $comment) =3D @data;=0D > =0D > @@ -1656,7 +1693,8 @@ sub write_user_config {=0D > my $vmlist =3D join (',', sort keys %{$d->{vms}});=0D > my $storelist =3D join (',', sort keys %{$d->{storage}});=0D > my $comment =3D $d->{comment} ? PVE::Tools::encode_text($d->{comment}) = : '';=0D > - $data .=3D "pool:$pool:$comment:$vmlist:$storelist:\n";=0D > + my $limits =3D $d->{limits} ? PVE::JSONSchema::print_property_string($d= ->{limits}, $pool_limits_desc) : '';=0D > + $data .=3D "pool:$pool:$comment:$vmlist:$storelist:$limits:\n";=0D > }=0D > =0D > $data .=3D "\n";=0D > diff --git a/src/test/parser_writer.pl b/src/test/parser_writer.pl=0D > index 80c346b..2e6eb61 100755=0D > --- a/src/test/parser_writer.pl=0D > +++ b/src/test/parser_writer.pl=0D > @@ -431,12 +431,12 @@ my $default_raw =3D {=0D > 'test_role_privs_invalid' =3D> 'role:testrole:VM.Invalid,Datastore.Audi= t,VM.Allocate:',=0D > },=0D > pools =3D> {=0D > - 'test_pool_empty' =3D> 'pool:testpool::::',=0D > - 'test_pool_invalid' =3D> 'pool:testpool::non-numeric:inval!d:',=0D > - 'test_pool_members' =3D> 'pool:testpool::123,1234:local,local-zfs:',=0D > - 'test_pool_duplicate_vms' =3D> 'pool:test_duplicate_vms::123,1234::',=0D > - 'test_pool_duplicate_vms_expected' =3D> 'pool:test_duplicate_vms::::',= =0D > - 'test_pool_duplicate_storages' =3D> 'pool:test_duplicate_storages:::loc= al,local-zfs:',=0D > + 'test_pool_empty' =3D> 'pool:testpool:::::',=0D > + 'test_pool_invalid' =3D> 'pool:testpool::non-numeric:inval!d::',=0D > + 'test_pool_members' =3D> 'pool:testpool::123,1234:local,local-zfs::',=0D > + 'test_pool_duplicate_vms' =3D> 'pool:test_duplicate_vms::123,1234:::',= =0D > + 'test_pool_duplicate_vms_expected' =3D> 'pool:test_duplicate_vms:::::',= =0D > + 'test_pool_duplicate_storages' =3D> 'pool:test_duplicate_storages:::loc= al,local-zfs::',=0D > },=0D > acl =3D> {=0D > 'acl_simple_user' =3D> 'acl:1:/:test@pam:PVEVMAdmin:',=0D > @@ -1018,7 +1018,7 @@ my $tests =3D [=0D > 'user:test@pam:0:0::::::'."\n".=0D > 'token:test@pam!test:0:0::'."\n\n".=0D > 'group:testgroup:::'."\n\n".=0D > - 'pool:testpool::::'."\n\n".=0D > + 'pool:testpool:::::'."\n\n".=0D > 'role:testrole::'."\n\n",=0D > },=0D > ];=0D > -- =0D > 2.39.2=0D --===============4287630379095564719== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel --===============4287630379095564719==--