[pve-devel] [PATCH qemu-server] api2: fix vmconfig_apply_pending errors handling

[pve-devel] [PATCH qemu-server] api2: fix vmconfig_apply_pending errors handling

[Alexandre Derumier]

commit
https://git.proxmox.com/?p=qemu-server.git;a=commit;h=eb5e482ded9ae6aeb6575de9441b79b90a5de531

have introduced error handling for offline pending apply,

-               PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $running);
+               PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $running, $errors);

 sub vmconfig_apply_pending {
-    my ($vmid, $conf, $storecfg) = @_;
+    my ($vmid, $conf, $storecfg, $errors) = @_;

but they was wrong nonused $running param, so currently $errors are not correctly handled

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 PVE/API2/Qemu.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 1e540f5..f2557e3 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1413,7 +1413,7 @@ my $update_vm_api  = sub {
 	    if ($running) {
 		PVE::QemuServer::vmconfig_hotplug_pending($vmid, $conf, $storecfg, $modified, $errors);
 	    } else {
-		PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $running, $errors);
+		PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $errors);
 	    }
 	    raise_param_exc($errors) if scalar(keys %$errors);
 
-- 
2.20.1

Re: [pve-devel] [PATCH qemu-server] api2: fix vmconfig_apply_pending errors handling

[Fabian Grünbichler]

On July 6, 2021 12:02 am, Alexandre Derumier wrote:
> commit
> https://git.proxmox.com/?p=qemu-server.git;a=commit;h=eb5e482ded9ae6aeb6575de9441b79b90a5de531
> 
> have introduced error handling for offline pending apply,
> 
> -               PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $running);
> +               PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $running, $errors);
> 
>  sub vmconfig_apply_pending {
> -    my ($vmid, $conf, $storecfg) = @_;
> +    my ($vmid, $conf, $storecfg, $errors) = @_;
> 
> but they was wrong nonused $running param, so currently $errors are not correctly handled

$running was indeed not used since the introduction of hotplug 
functionality in 2015 - but this also means that half of that commit was 
not actually tested (@Oguz - please take another look and confirm it 
works as expected WITH this patch here).

errors should still be handled "correctly" without this patch (as in, 
mostly not ;)), the main difference is whether changing the config 
returns a proper error or not when applying pending changes fails.

> 
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> ---
>  PVE/API2/Qemu.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 1e540f5..f2557e3 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1413,7 +1413,7 @@ my $update_vm_api  = sub {
>  	    if ($running) {
>  		PVE::QemuServer::vmconfig_hotplug_pending($vmid, $conf, $storecfg, $modified, $errors);
>  	    } else {
> -		PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $running, $errors);
> +		PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $errors);
>  	    }
>  	    raise_param_exc($errors) if scalar(keys %$errors);
>  
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] [PATCH qemu-server] api2: fix vmconfig_apply_pending errors handling

[Oguz Bektas]

hi,

true, it seems that parameter was leftover! thanks for noticing that.

now i tested alexandre's patch.

when i have a pending change that cannot be applied, the appropriate
error message is returned (unable to apply pending change: foo) and the
it stays in [PENDING] section of config.

Tested-by: Oguz Bektas <o.bektas@proxmox.com>


On Tue, Jul 06, 2021 at 10:10:23AM +0200, Fabian Grünbichler wrote:
> On July 6, 2021 12:02 am, Alexandre Derumier wrote:
> > commit
> > https://git.proxmox.com/?p=qemu-server.git;a=commit;h=eb5e482ded9ae6aeb6575de9441b79b90a5de531
> > 
> > have introduced error handling for offline pending apply,
> > 
> > -               PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $running);
> > +               PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $running, $errors);
> > 
> >  sub vmconfig_apply_pending {
> > -    my ($vmid, $conf, $storecfg) = @_;
> > +    my ($vmid, $conf, $storecfg, $errors) = @_;
> > 
> > but they was wrong nonused $running param, so currently $errors are not correctly handled
> 
> $running was indeed not used since the introduction of hotplug 
> functionality in 2015 - but this also means that half of that commit was 
> not actually tested (@Oguz - please take another look and confirm it 
> works as expected WITH this patch here).
> 
> errors should still be handled "correctly" without this patch (as in, 
> mostly not ;)), the main difference is whether changing the config 
> returns a proper error or not when applying pending changes fails.
> 
> > 
> > Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> > ---
> >  PVE/API2/Qemu.pm | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> > index 1e540f5..f2557e3 100644
> > --- a/PVE/API2/Qemu.pm
> > +++ b/PVE/API2/Qemu.pm
> > @@ -1413,7 +1413,7 @@ my $update_vm_api  = sub {
> >  	    if ($running) {
> >  		PVE::QemuServer::vmconfig_hotplug_pending($vmid, $conf, $storecfg, $modified, $errors);
> >  	    } else {
> > -		PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $running, $errors);
> > +		PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $errors);
> >  	    }
> >  	    raise_param_exc($errors) if scalar(keys %$errors);
> >  
> > -- 
> > 2.20.1
> > 
> > 
> > _______________________________________________
> > pve-devel mailing list
> > pve-devel@lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> > 
> > 
> > 

[pve-devel] applied: [PATCH qemu-server] api2: fix vmconfig_apply_pending errors handling

[Thomas Lamprecht]

On 06.07.21 00:02, Alexandre Derumier wrote:
> commit
> https://git.proxmox.com/?p=qemu-server.git;a=commit;h=eb5e482ded9ae6aeb6575de9441b79b90a5de531
> 
> have introduced error handling for offline pending apply,
> 
> -               PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $running);
> +               PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $running, $errors);
> 
>  sub vmconfig_apply_pending {
> -    my ($vmid, $conf, $storecfg) = @_;
> +    my ($vmid, $conf, $storecfg, $errors) = @_;
> 
> but they was wrong nonused $running param, so currently $errors are not correctly handled
> 
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> ---
>  PVE/API2/Qemu.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, with Oguz T-b, thanks!