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.bek...@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@pve.proxmox.com > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel