<div dir="ltr"><div style="font-family:arial,sans-serif;font-size:12.666666984558105px"><div><div><div>Exported Variable:<br><br></div>The particular variable referenced is the canonical list of Valid Privileges, necessary for assessing whether any given Privilege is, in fact on the list of valid and assignable Privileges. The change made allows the variable to referenced outside the scope of the module that contains it; so that it can be re-used in other modules (specifically in PVE::API2::Cluster). I would like to highlight that it is NOT really a Global Variable. It is a lexically scoped, exported variable, available to other modules within the PVE package by reference to PVE:AccessControl::valid_privs (eg, it does not collide with other declarations of valid_privs in other modules). If a different approach is preferred, please advise what the desired strategy is - if it is strictly necessary, then for example a utility function could be added to PVE::AccessControl to export the values contained in the valid_privs variable instead. If the Proxmox has some specific design goals in mind the patch can be altered to accommodate.<br>
<br><br></div>check_any call for valid_privs:<br><br></div>This check is assessing whether the user has any permissions assigned to the pool and therefore, by extension, whether they should have visibility of the Pool at all, or whether the Pool should be omitted from their view of the list of Pools. In other words, if the user has no rights assigned to the Pool, then their console/screen should not be cluttered with it in this view (as they can not perform any actions on it). This check achieves that goal.<br>
<br><br></div><span style="font-family:arial,sans-serif;font-size:12.666666984558105px">!important tag on image url:</span><br style="font-family:arial,sans-serif;font-size:12.666666984558105px"><div style="font-family:arial,sans-serif;font-size:12.666666984558105px">
<div><br></div>This was included in the original cut of the patch to cover a specific browser incompatibility scenario; where the author observed that the image was not showing in its desired situation. This is a common problem with older versions of Internet Explorer and occasionally I have observed it in Chrome as well (where a less-specific CSS selector overrides a more-specific CSS selector). </div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jul 21, 2014 at 5:40 PM, Dietmar Maurer <span dir="ltr"><<a href="mailto:dietmar@proxmox.com" target="_blank">dietmar@proxmox.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">here are a few comments and question (inline):<br>
<div class=""><br>
> diff -Nuarp usr/share/perl5/PVE/AccessControl.pm<br>
> usr/share/perl5/PVE/AccessControl.pm<br>
> --- usr/share/perl5/PVE/AccessControl.pm 2014-06-27 16:23:34.305587119<br>
> +1200<br>
> +++ usr/share/perl5/PVE/AccessControl.pm 2014-06-27 16:23:44.077692909<br>
> +1200<br>
> @@ -492,7 +492,7 @@ my $privgroups = {<br>
> },<br>
> };<br>
><br>
> -my $valid_privs = {};<br>
> +our $valid_privs = {};<br>
<br>
</div>using global vars is bad design!<br>
<div><div class="h5"><br>
> my $special_roles = {<br>
> 'NoAccess' => {}, # no priviledges<br>
> diff -Nuarp usr/share/perl5/PVE/API2/Cluster.pm<br>
> usr/share/perl5/PVE/API2/Cluster.pm<br>
> --- usr/share/perl5/PVE/API2/Cluster.pm 2014-06-27 16:22:37.302970017<br>
> +1200<br>
> +++ usr/share/perl5/PVE/API2/Cluster.pm 2014-06-27 16:26:30.939493106<br>
> +1200<br>
> @@ -19,6 +19,8 @@ use PVE::RESTHandler;<br>
> use PVE::RPCEnvironment;<br>
> use PVE::JSONSchema qw(get_standard_option);<br>
><br>
> +use PVE::AccessControl qw(valid_privs);<br>
> +<br>
> use base qw(PVE::RESTHandler);<br>
><br>
> __PACKAGE__->register_method ({<br>
> @@ -160,12 +162,20 @@ __PACKAGE__->register_method({<br>
> my $vmlist = PVE::Cluster::get_vmlist() || {};<br>
> my $idlist = $vmlist->{ids} || {};<br>
><br>
> + my $storage_pool = {};<br>
> +<br>
> + foreach my $pool (keys(%{$usercfg->{pools}})) {<br>
> + foreach my $storage (keys(%{$usercfg->{pools}->{$pool}-<br>
> >{storage}})) {<br>
> + $storage_pool->{$storage}=$pool;<br>
> + }<br>
> + }<br>
> +<br>
> my $pooldata = {};<br>
> if (!$param->{type} || $param->{type} eq 'pool') {<br>
> foreach my $pool (keys %{$usercfg->{pools}}) {<br>
> my $d = $usercfg->{pools}->{$pool};<br>
><br>
> - next if !$rpcenv->check($authuser, "/pool/$pool", [<br>
> 'Pool.Allocate' ], 1);<br>
> + next if !$rpcenv->check_any($authuser, "/pool/$pool", [keys<br>
> $PVE::AccessControl::valid_privs], 1);<br>
<br>
</div></div>Please can you explain that change?<br>
<div><div class="h5"><br>
><br>
> my $entry = {<br>
> id => "/pool/$pool",<br>
> @@ -185,9 +195,9 @@ __PACKAGE__->register_method({<br>
><br>
> my $data = $idlist->{$vmid};<br>
> my $entry = PVE::API2Tools::extract_vm_stats($vmid, $data,<br>
> $rrd);<br>
> - if ($entry->{uptime}) {<br>
> - if (my $pool = $usercfg->{vms}->{$vmid}) {<br>
> - if (my $pe = $pooldata->{$pool}) {<br>
> + if (my $pool = $usercfg->{vms}->{$vmid}) {<br>
> + if (my $pe = $pooldata->{$pool}) {<br>
> + if ($entry->{uptime}) {<br>
> $pe->{uptime} = $entry->{uptime} if !$pe-<br>
> >{uptime} || $entry->{uptime} > $pe->{uptime};<br>
> $pe->{mem} = 0 if !$pe->{mem};<br>
> $pe->{mem} += $entry->{mem};<br>
> @@ -198,6 +208,7 @@ __PACKAGE__->register_method({<br>
> $pe->{maxcpu} = 0 if !$pe->{maxcpu};<br>
> $pe->{maxcpu} += $entry->{maxcpu};<br>
> }<br>
> + $entry->{pool} = $pe->{pool};<br>
> }<br>
> }<br>
><br>
> @@ -228,6 +239,9 @@ __PACKAGE__->register_method({<br>
> next if !PVE::Storage::storage_check_enabled($cfg,<br>
> $storeid, $node, 1);<br>
><br>
> my $entry =<br>
> PVE::API2Tools::extract_storage_stats($storeid, $scfg, $node, $rrd);<br>
> + if (exists $storage_pool->{$entry->{storage}}) {<br>
> + $entry->{"pool"}=$storage_pool->{$entry-<br>
> >{storage}};<br>
> + }<br>
> push @$res, $entry;<br>
> }<br>
> }<br>
> diff -Nuarp usr/share/pve-manager/css/ext-pve.css usr/share/pve-<br>
> manager/css/ext-pve.css<br>
> --- usr/share/pve-manager/css/ext-pve.css 2014-06-27 16:33:46.291799762<br>
> +1200<br>
> +++ usr/share/pve-manager/css/ext-pve.css 2014-06-27 16:34:56.083492567<br>
> +1200<br>
> @@ -108,7 +108,7 @@<br>
> .x-tree-node-pool,<br>
> .x-grid-tree-pool-expanded .x-tree-node-pool<br>
> {<br>
> - background-image:url(../images/connect_established.png);<br>
> + background-image:url(../images/connect_established.png) !important;<br>
<br>
</div></div>why is this required?<br>
<br>
<br>
</blockquote></div><br></div>