[pve-devel] [PATCH v2] access permissions cleanup fix

[Dietmar Maurer]

applied, thanks.

but have some comments (inline)

On 08/13/2015 01:41 PM, Alen Grizonic wrote:
> for removed vms and pools
>
> Signed-off-by: Alen Grizonic <a.grizonic at proxmox.com>
> ---
>   PVE/AccessControl.pm | 26 ++++++++++++++++++++++----
>   1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/PVE/AccessControl.pm b/PVE/AccessControl.pm
> index bfee816..54577aa 100644
> --- a/PVE/AccessControl.pm
> +++ b/PVE/AccessControl.pm
> @@ -478,10 +478,7 @@ sub delete_pool_acl {
>   
>       my $path = "/pool/$pool";
>   
> -    foreach my $aclpath (keys %{$usercfg->{acl}}) {
> -	delete ($usercfg->{acl}->{$aclpath})
> -	    if $usercfg->{acl}->{$aclpath} eq 'path';
> -    }
> +    delete ($usercfg->{acl}->{$path})
>   }
This looks like a separate/independent bug fix. I would prefer to have 
an extra patch for
things like this.
>   
>   # we automatically create some predefined roles by splitting privs
> @@ -1096,6 +1093,27 @@ sub check_permissions {
>       return 1;
>   }
>   
> +sub remove_vm_access {
> +    my ($vmid) = @_;
> +    my $delVMaccessFn = sub {
> +        my $usercfg = cfs_read_file("user.cfg");
> +
> +        if (my $acl = $usercfg->{acl}->{'/vms/'.$vmid}) {
> +            delete $usercfg->{acl}->{'/vms/'.$vmid};
> +            cfs_write_file("user.cfg", $usercfg);
first call to cfs_write_file
> +        }
> +        if (my $pool = $usercfg->{vms}->{$vmid}) {
> +            if (my $data = $usercfg->{pools}->{$pool}) {
> +                delete $data->{vms}->{$vmid};
> +                delete $usercfg->{vms}->{$vmid};
> +                cfs_write_file("user.cfg", $usercfg);
second call. I added a cleanup to avoid double write.
> +            }
> +        }
> +    };
> +
> +    lock_user_config($delVMaccessFn, "access permissions cleanup for VM $vmid failed");
> +}
> +
>   sub add_vm_to_pool {
>       my ($vmid, $pool) = @_;
>