Re: [pve-devel] [PATCH qemu-server] apply pending changes at vm stop only if the section is not empty

[Fabian Ebner <f.ebner@proxmox.com>]

Am 01.12.21 um 13:00 schrieb Oguz Bektas:
> we also have the same skipping logic when starting vms. this way we
> avoid calling 'write_config' when there are no present changes to
> commit.
> 
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
>   PVE/QemuServer.pm | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 45b704d..1379f33 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -4998,6 +4998,8 @@ sub vmconfig_delete_or_detach_drive {
>   sub vmconfig_apply_pending {
>       my ($vmid, $conf, $storecfg, $errors) = @_;
>   
> +    return if !scalar(keys %{$conf->{pending}});

Isn't this part of the patch enough? For the caller while starting a VM 
you mentioned, there is an additional load_config() that is skipped if 
there are no pending changes, but for the two callers below, there is no 
such benefit. They just indicate that any pending changes should be 
applied, which is fine even if there are none.

> +
>       my $add_apply_error = sub {
>   	my ($opt, $msg) = @_;
>   	my $err_msg = "unable to apply pending change $opt : $msg";
> @@ -5889,7 +5891,7 @@ sub _do_vm_stop {
>   		die "VM quit/powerdown failed - got timeout\n";
>   	    }
>   	} else {
> -	    vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 1) if $conf;
> +	    vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, scalar(keys %{$conf->{pending}})) if $conf;

Style nit: >100 characters

>   	    return;
>   	}
>       } else {
> @@ -5920,7 +5922,7 @@ sub _do_vm_stop {
>   	sleep 1;
>       }
>   
> -    vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 1) if $conf;
> +    vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, scalar(keys %{$conf->{pending}})) if $conf;
>   }
>   
>   # Note: use $nocheck to skip tests if VM configuration file exists.
>