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