[pve-devel] [PATCH v3 container 1/2] Fix #1924: add snapshot parameter

[Thomas Lamprecht]

On 1/21/19 3:46 PM, Rhonda D'Vine wrote:> The config CLI command offer the config and showcmd functions. The
> output of that may vary with respect to a given snapshot. This adds a
> switch that shows the corresponding snapshot's config.
> 
> This patch needs the one in pve-guest-common applied first, and have the
> versioned dependency in debian/rules adjusted.

s/rules/control/ + this happened already and you actually bump the version
dependency yourself here, so please remove that as its a bit confusing.

> 
> Signed-off-by: Rhonda D'Vine <rhonda at proxmox.com>
> ---
>  debian/control             |  2 +-
>  src/PVE/API2/LXC/Config.pm | 19 +++++++++++++++++++
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/debian/control b/debian/control
> index 15f42ad..42a07df 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -4,7 +4,7 @@ Priority: extra
>  Maintainer: Proxmox Support Team <support at proxmox.com>
>  Build-Depends: debhelper (>= 7.0.50~),
>                 libpve-common-perl,
> -               libpve-guest-common-perl | libpve-common-perl (<= 4.0-89),
> +               libpve-guest-common-perl (>> 2.0-18) | libpve-common-perl (<= 4.0-89),
>                 libpve-storage-perl,
>                 libtest-mockmodule-perl,
>                 lxc (>= 3.0.2-1) | lxc-pve (>= 3.0.1+pve1-1),
> diff --git a/src/PVE/API2/LXC/Config.pm b/src/PVE/API2/LXC/Config.pm
> index 1ba0ffd..3ea3111 100644
> --- a/src/PVE/API2/LXC/Config.pm
> +++ b/src/PVE/API2/LXC/Config.pm
> @@ -35,6 +35,14 @@ __PACKAGE__->register_method({
>  	properties => {
>  	    node => get_standard_option('pve-node'),
>  	    vmid => get_standard_option('pve-vmid', { completion => \&PVE::LXC::complete_ctid }),
> +	    snapshot => get_standard_option('pve-snapshot-name', {
> +		description => "Fetch config values from given snapshot.",
> +		optional => 1,
> +		completion => sub {
> +		    my ($cmd, $pname, $cur, $args) = @_;
> +		    PVE::LXC::Config->snapshot_list($cur->[0]);

Did you test this? $cur is the current argument, thus a string, not an array
reference. Thus I _always_ get:
> Can't use string ("") as an ARRAY ref while "strict refs" in use at /usr/share/perl5/PVE/API2/Qemu.pm line 840.
when trying a completion (independent if I have already written parts of a
snapshot name or not).
I guess you meant to write $args->[0], as you used $_[3][0] in your v2, but the
element from @_ with index 3 is $args not $cur..

Please address this (here and in the qemu-server one), use a $vmid intermediate
variable (as in v2) so a read quickly sees what gets passed to the helper and
test it. Quick, untested (because trivial), changes are haunted to have >90%
chance to inflict chaos and doom...

> +		},
> +	    }),
>  	},
>      },
>      returns => {
> @@ -57,6 +65,17 @@ __PACKAGE__->register_method({
>  
>  	my $conf = PVE::LXC::Config->load_config($param->{vmid});
>  
> +	my $snapname = $param->{snapshot};
> +	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;
> +	}
> +
>  	delete $conf->{snapshots};
>  
>  	return $conf;
>