[pve-devel] [PATCH v4 qemu-server 2/2] vmconfig_apply_pending: remove redundant write/load config calls

[Thomas Lamprecht]

On 1/7/20 4:55 PM, Oguz Bektas wrote:
> since we handle errors gracefully now, we don't need to write & save
> config every time we change a setting.
> 
> note: this results in a change of behavior in the API. since errors are
> handled gracefully instead of "die"ing, when there is a pending change
> which cannot be applied for some reason, it will get logged in the
> tasklog but the vm will continue booting regardless. the non-applied
> change will stay in the pending section of the configuration.
> 
> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> ---
> 
> v3 -> v4:
> * add explanation in commit message about behavior change in API
> 
>  PVE/QemuServer.pm | 21 +++++----------------
>  1 file changed, 5 insertions(+), 16 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 2de8376..9c86bee 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -4993,19 +4993,11 @@ sub vmconfig_apply_pending {
>  	my $force = $pending_delete_hash->{$opt}->{force};
>  	eval {
>  	    die "internal error" if $opt =~ m/^unused/;
> -	    $conf = PVE::QemuConfig->load_config($vmid); # update/reload
>  	    if (!defined($conf->{$opt})) {
>  		PVE::QemuConfig->remove_from_pending_delete($conf, $opt);
why keep above, it's handled below...

> -		PVE::QemuConfig->write_config($vmid, $conf);
>  	    } elsif (is_valid_drivename($opt)) {
>  		vmconfig_delete_or_detach_drive($vmid, $storecfg, $conf, $opt, $force);
>  		PVE::QemuConfig->remove_from_pending_delete($conf, $opt);

why keep above, it's handled below...

> -		delete $conf->{$opt};
> -		PVE::QemuConfig->write_config($vmid, $conf);
> -	    } else {
> -		PVE::QemuConfig->remove_from_pending_delete($conf, $opt);
> -		delete $conf->{$opt};
> -		PVE::QemuConfig->write_config($vmid, $conf);
>  	    }
>  	};
>  	if (my $err = $@) {
> @@ -5013,24 +5005,20 @@ sub vmconfig_apply_pending {
>  	} else {
>  	    PVE::QemuConfig->remove_from_pending_delete($conf, $opt);

... here.

>  	    delete $conf->{$opt};
> -	    PVE::QemuConfig->write_config($vmid, $conf);
>  	}
> +
>      }
>  
>      $conf = PVE::QemuConfig->load_config($vmid); # update/reload

why keep that one?

>  
>      foreach my $opt (keys %{$conf->{pending}}) { # add/change
> -	$conf = PVE::QemuConfig->load_config($vmid); # update/reload
> -
> +	next if $opt eq 'delete'; # just to be sure
>  	eval {
>  	    if (defined($conf->{$opt}) && ($conf->{$opt} eq $conf->{pending}->{$opt})) {
>  		# skip if nothing changed
>  	    } elsif (is_valid_drivename($opt)) {
>  		vmconfig_register_unused_drive($storecfg, $vmid, $conf, parse_drive($opt, $conf->{$opt}))
>  		    if defined($conf->{$opt});
> -		$conf->{$opt} = $conf->{pending}->{$opt};
> -	    } else {
> -		$conf->{$opt} = $conf->{pending}->{$opt};
>  	    }
>  	};
>  	if (my $err = $@) {
> @@ -5039,9 +5027,10 @@ sub vmconfig_apply_pending {
>  	    $conf->{$opt} = delete $conf->{pending}->{$opt};
>  	    PVE::QemuConfig->cleanup_pending($conf);
>  	}
> -
> -	PVE::QemuConfig->write_config($vmid, $conf);
>      }
> +
> +    # write all changes at once to avoid unnecessary i/o
> +    PVE::QemuConfig->write_config($vmid, $conf);
>  }
>  
>  my $safe_num_ne = sub {
>