On February 5, 2020 9:29 am, Dominik Csapak wrote:
> when setting a netX option that is semantically the same as the
> one already set but in a different order, e.g.:
>
> in config:
> net0: name=eth0,bridge=vmbr0,hwaddr=AA:AA:AA:AA:AA:AA,type=veth
> setting via api:
> net0: bridge=vmbr0,name=eth0,hwaddr=AA:AA:AA:AA:AA:AA,type=veth
>
> the code tries to 'hot-apply' the change (which is no change really)
> where the api line then gets parsed and printed which results
> in the same string already in the config
>
> then we do a 'cleanup_pending' which removes it from pending, since
> the config already contains the exact same options, but
> then we overwrite the config from pending (which is empty)
> resulting in an invalid config line:
> --8<--
> net0:
> -->8--
>
> to prevent this, we only overwrite the config here when
> there is still an option in in $conf->{pending}->{$opt}, meaning
> there was a meaningful change
>
> Signed-off-by: Dominik Csapak <d.csa...@proxmox.com>
> ---
> i am not really sure if this is the correct place to fix this, because
> i think we never should trigger apply_pending when the requested
> config is semantically identical to what is already in the config,
> but i did not really see when or how to do that in a good and generic way
> (should be parse/print all property strings at the beginning?)
> so sending it as an rfc
I think the right way to do it is like in QemuServer.pm - don't cleanup
pending while iterating over it, but before/after iteration.
> src/PVE/LXC/Config.pm | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
> index 310aba6..49b9f70 100644
> --- a/src/PVE/LXC/Config.pm
> +++ b/src/PVE/LXC/Config.pm
> @@ -1305,7 +1305,10 @@ sub vmconfig_apply_pending {
> $add_apply_error->($opt, $err);
> } else {
> $class->cleanup_pending($conf);
> - $conf->{$opt} = delete $conf->{pending}->{$opt};
> + # $conf->{pending}->{$opt} is now empty if we have the same
> + # value already in config, so do not overwrite the value in config
> + $conf->{$opt} = delete $conf->{pending}->{$opt}
> + if defined($conf->{pending}->{$opt});
> }
> }
>
> --
> 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
