[pve-devel] [RFC PATCH container] fix setting same netX value several times

[Fabian Grünbichler]

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.csapak at 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 at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>