[pve-devel] [PATCH v2 container 10/18] adapt CT config parser for pending changes

[Fabian Grünbichler]

On October 2, 2019 12:22 pm, Thomas Lamprecht wrote:
> On 9/30/19 2:44 PM, Oguz Bektas wrote:
>> config parser can now read/write [pve:pending] section. this was named
>> such, instead of [PENDING], after on- and offline discussion regarding
>> namespacing the pending section and snapshots.
>> 
>> this also adds an optional non-capturing regex group into the parser for
>> [snap: snapname] entries which can be supported in PVE 7.0
> 
> 1. completely nothing to do with pending changes itself -> separate patch
> 
> 2. they could be supported in 6.x already? PVE cluster host need to be on the
> same version of software, we guarantee only that old -> new and new <-> new
> works, so doing this now is no issue... Else we never could add any new
> feature in a stable release..
> 
> Also this is missing from qemu-server?

the problem is that this is not just a case of new -> old does not work, 
but it's a case of new -> old silently drops parts (/most) of your guest 
config.. since it's just nice to have, but not blocking anything 
important, we should postpone it to 7.0 IMHO (where we can assume that 
the 'old' 6.x parser already supports it, so the fallout is basically 
non-existent at that point).

> 
>> 
>> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
>> ---
>>  src/PVE/LXC/Config.pm | 37 ++++++++++++++++++++++++++++++++-----
>>  1 file changed, 32 insertions(+), 5 deletions(-)
>> 
>> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
>> index 9790345..47bd4bb 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,14 @@ 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/^\[pve:pending\]\s*$/i) {
> 
> why not add the new stuff to the end of the elsif block? less churn..

for qemu-server, we need to add it up-front to parse it earlier then a 
snapshot called pending, this would keep the two parsers (that we 
hopefully unify some day) closer together. OTOH, we'd drop all of this 
if we unify it anyway, so..

> 
>> +	    $section = 'pending';
>> +	    $conf->{description} = $descr if $descr;
>> +	    $descr = '';
>> +	    $conf = $res->{$section} = {};
>> +	    next;
>> +	} elsif ($line =~ m/^\[(?:snap:)?([a-z][a-z0-9_\-]+)\]\s*$/i) {
>> +	    # extended regex for namespacing snapshots in PVE 7.0
>>  	    $section = $1;
>>  	    $conf->{description} = $descr if $descr;
>>  	    $descr = '';
>> @@ -794,6 +802,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 [pve:pending]\n";
>> +	    }
>>  	} elsif ($line =~ m/^([a-z][a-z_]*\d*):\s*(\S.*)\s*$/) {
>>  	    my $key = $1;
>>  	    my $value = $2;
>> @@ -832,14 +847,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 +884,14 @@ sub write_pct_config {
>>  
>>      my $raw = &$generate_raw_config($conf);
>>  
>> +    if (scalar(keys %{$conf->{pending}})){
>> +	$raw .= "\n[pve:pending]\n";
>> +	$raw .= &$generate_raw_config($conf->{pending}, 1);
>> +    }
>> +
>>      foreach my $snapname (sort keys %{$conf->{snapshots}}) {
>> +	# TODO: namespace snapshots for PVE 7.0
>> +	#$raw .= "\n[snap:$snapname]\n";
>>  	$raw .= "\n[$snapname]\n";
>>  	$raw .= &$generate_raw_config($conf->{snapshots}->{$snapname});
>>      }
>> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>