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

[Oguz Bektas <o.bektas@proxmox.com>]

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
> > 
> > 
> >