[pve-devel] [PATCH v2 guest-common 02/18] refactor method used by config GET calls into AbstractConfig

[Thomas Lamprecht]

your commit subject is so general that it is completely of no use..
after reading it one has not idea at all about which function with
was use gets refactored out..

How about:
> abstract config: add general load_current_config implementation

On 9/30/19 2:44 PM, Oguz Bektas wrote:
> since this method will be both used by qemu and lxc config GET calls, it

how about:
> this code is already used by qemu-servers GET config API call, once
> container can also do pending changes the will use the same code. So add
> a general implementation here which uses some $class helpers allowing to
> abstract away the difference in child classes.

Besides that, looks OK

> makes sense to move it into AbstractConfig. only difference is that qemu
> also hides the cipassword when it's set. this can be handled by having
> qemu overwrite the method and add the cipassword code.
> 
> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> ---
>  PVE/AbstractConfig.pm | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/PVE/AbstractConfig.pm b/PVE/AbstractConfig.pm
> index 910ca86..6d3f169 100644
> --- a/PVE/AbstractConfig.pm
> +++ b/PVE/AbstractConfig.pm
> @@ -136,6 +136,41 @@ sub cleanup_pending {
>      return $changes;
>  }
>  
> +sub load_current_config {
> +    my ($class, $vmid, $snapname, $current) = @_;
> +
> +    my $conf = $class->load_config($vmid);
> +
> +    if ($snapname) {
> +	my $snapshot = $conf->{snapshots}->{$snapname};
> +	die "snapshot '$snapname' does not exist\n" if !defined($snapshot);
> +
> +	# we need the digest of the file
> +	$snapshot->{digest} = $conf->{digest};
> +	$conf = $snapshot;
> +    }
> +
> +    # take pending changes in
> +    if (!$current) {
> +	foreach my $opt (keys %{$conf->{pending}}) {
> +	    next if $opt eq 'delete';
> +	    my $value = $conf->{pending}->{$opt};
> +	    next if ref($value); # just to be sure
> +	    $conf->{$opt} = $value;
> +	}
> +	my $pending_delete_hash = $class->parse_pending_delete($conf->{pending}->{delete});
> +	foreach my $opt (keys %$pending_delete_hash) {
> +	    delete $conf->{$opt} if $conf->{$opt};
> +	}
> +    }
> +
> +    delete $conf->{snapshots};
> +    delete $conf->{pending};
> +
> +    return $conf;
> +}
> +
> +
>  # Lock config file using flock, run $code with @param, unlock config file.
>  # $timeout is the maximum time to aquire the flock
>  sub lock_config_full {
>