[pve-devel] [PATCH container 1/9] add pending section to lxc config parser/writer

[Thomas Lamprecht]

On 11.09.19 09:39, Fabian Grünbichler wrote:
> NAK, this breaks existing configs with a snapshot called pending (which 
> is an allowed snapshot name, and not such an uncommon word that we can 
> confidently say that noone would be affected). we could do _PENDING_ or 

naming it pending for VMs was possible and broke things in obvious ways,
nevertheless we never had a single report about it, AFAICT, even after
pending being released ~4.5 years ago. So while yes, the chance is there
I'd guess that's highly unlikely - not that that really helps us.

> __PENDING__ (similar to what we do with __base__ and __replicate__ and 
> __migration__ storage snapshots).

confuses things more, IMO, the other two are special snapshots, this is
completely something different, so mixing those two into having a similar
syntax may make people think that "__PENDING__" is a pending snapshot.

IMO, the snapshots should have been prefixed with a marker from the
beginning, but as the time machine isn't there yet and retro actively
re-writing snapshot sections as "[snap:<name>]" or the like isn't a to
easy task, I'd maybe rather add a prefix for non-snapshot sections with
a snapshot incompatible syntax. [_special:PENDING] or the like.

Another possible way could be to really to the 
[snap:<name>] for snapshots, rewriting old ones once the config is written
out for other changes anyway, and for names where we are not sure like
"pending" check if's there's a "snaptime" property in the section?
As else it cannot be a valid snapshot made through the API.

> 
> in general, this would align the VM and CT config parser/writer pairs 
> more closely, so it's a step in the right direction.
> 
> On September 5, 2019 4:11 pm, Oguz Bektas wrote:
>> allow parsing and writing of the pending changes section in CTID.conf
>> files.
>>
>> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
>> ---
>>  src/PVE/LXC/Config.pm | 35 ++++++++++++++++++++++++++++++-----
>>  1 file changed, 30 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
>> index 9790345..08b958f 100644
>> --- a/src/PVE/LXC/Config.pm
>> +++ b/src/PVE/LXC/Config.pm
>> @@ -751,6 +751,7 @@ sub parse_pct_config {
>>      my $res = {
>>  	digest => Digest::SHA::sha1_hex($raw),
>>  	snapshots => {},
>> +	pending => {},
>>      };
>>  
>>      $filename =~ m|/lxc/(\d+).conf$|
>> @@ -766,7 +767,13 @@ sub parse_pct_config {
>>      foreach my $line (@lines) {
>>  	next if $line =~ m/^\s*$/;
>>  
>> -	if ($line =~ m/^\[([a-z][a-z0-9_\-]+)\]\s*$/i) {
>> +	if ($line =~ m/^\[PENDING\]\s*$/i) {
>> +	    $section = 'pending';
>> +	    $conf->{description} = $descr if $descr;
>> +	    $descr = '';
>> +	    $conf = $res->{$section} = {};
>> +	    next;
>> +	} elsif ($line =~ m/^\[([a-z][a-z0-9_\-]+)\]\s*$/i) {
>>  	    $section = $1;
>>  	    $conf->{description} = $descr if $descr;
>>  	    $descr = '';
>> @@ -794,6 +801,13 @@ sub parse_pct_config {
>>  	    $descr .= PVE::Tools::decode_text($2);
>>  	} elsif ($line =~ m/snapstate:\s*(prepare|delete)\s*$/) {
>>  	    $conf->{snapstate} = $1;
>> +	} elsif ($line =~ m/^delete:\s*(.*\S)\s*$/) {
>> +	    my $value = $1;
>> +	    if ($section eq 'pending') {
>> +		$conf->{delete} = $value;
>> +	    } else {
>> +		warn "vm $vmid - property 'delete' is only allowed in [PENDING]\n";
> 
> you copied this without the typo, but did not fix it in the original ;)
> 
>> +	    }
>>  	} elsif ($line =~ m/^([a-z][a-z_]*\d*):\s*(\S.*)\s*$/) {
>>  	    my $key = $1;
>>  	    my $value = $2;
>> @@ -832,14 +846,19 @@ sub write_pct_config {
>>      }
>>  
>>      my $generate_raw_config = sub {
>> -	my ($conf) = @_;
>> +	my ($conf, $pending) = @_;
>>  
>>  	my $raw = '';
>>  
>>  	# add description as comment to top of file
>> -	my $descr = $conf->{description} || '';
>> -	foreach my $cl (split(/\n/, $descr)) {
>> -	    $raw .= '#' .  PVE::Tools::encode_text($cl) . "\n";
>> +	if (defined(my $descr = $conf->{description})) {
>> +	    if ($descr) {
>> +		foreach my $cl (split(/\n/, $descr)) {
>> +		    $raw .= '#' .  PVE::Tools::encode_text($cl) . "\n";
>> +		}
>> +	    } else {
>> +		$raw .= "#\n" if $pending;
>> +	    }
>>  	}
>>  
>>  	foreach my $key (sort keys %$conf) {
>> @@ -864,7 +883,13 @@ sub write_pct_config {
>>  
>>      my $raw = &$generate_raw_config($conf);
>>  
>> +    if (scalar(keys %{$conf->{pending}})){
>> +	$raw .= "\n[PENDING]\n";
>> +	$raw .= &$generate_raw_config($conf->{pending}, 1);
>> +    }
>> +
>>      foreach my $snapname (sort keys %{$conf->{snapshots}}) {
>> +	die "internal error" if $snapname eq 'pending';
>>  	$raw .= "\n[$snapname]\n";
>>  	$raw .= &$generate_raw_config($conf->{snapshots}->{$snapname});
>>      }
>> -- 
>> 2.20.1
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel at pve.proxmox.com
>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>