[pve-devel] [PATCH v3 container 4/5] config: is_volume_in_use: ability to search only snapshots

[Fabian Grünbichler]

On Tue, Sep 26, 2017 at 02:43:03PM +0200, Wolfgang Bumiller wrote:
> ---
> This patch is new.
> 
>  src/PVE/LXC/Config.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
> index c45ce7e..0330d5e 100644
> --- a/src/PVE/LXC/Config.pm
> +++ b/src/PVE/LXC/Config.pm
> @@ -1176,14 +1176,14 @@ sub classify_mountpoint {
>  }
>  
>  sub is_volume_in_use {
> -    my ($class, $config, $volid, $include_snapshots) = @_;
> +    my ($class, $config, $volid, $include_snapshots, $only_snapshots) = @_;

that is a bit of a stange interface, since only_snapshots implies
include_snapshots ;) maybe we could factor the snapshot check into its
own sub and use it below and expose it as well?

>      my $used = 0;
>  
>      $class->foreach_mountpoint($config, sub {
>  	my ($ms, $mountpoint) = @_;
>  	return if $used;
>  	$used = $mountpoint->{type} eq 'volume' && $mountpoint->{volume} eq $volid;
> -    });
> +    }) if !$only_snapshots;

could we move the foreach into a private sub, and replace the recursion
and this instance with calls to the private sub? would improve
readability IMHO..

>  
>      my $snapshots = $config->{snapshots};
>      if ($include_snapshots && $snapshots) {
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel