* [pve-devel] [PATCH qemu-server] api2: fix vmconfig_apply_pending errors handling
@ 2021-07-05 22:02 Alexandre Derumier
2021-07-06 8:10 ` Fabian Grünbichler
2021-07-06 10:44 ` [pve-devel] applied: " Thomas Lamprecht
0 siblings, 2 replies; 4+ messages in thread
From: Alexandre Derumier @ 2021-07-05 22:02 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH qemu-server] api2: fix vmconfig_apply_pending errors handling
2021-07-05 22:02 [pve-devel] [PATCH qemu-server] api2: fix vmconfig_apply_pending errors handling Alexandre Derumier
@ 2021-07-06 8:10 ` Fabian Grünbichler
2021-07-06 9:44 ` Oguz Bektas
2021-07-06 10:44 ` [pve-devel] applied: " Thomas Lamprecht
1 sibling, 1 reply; 4+ messages in thread
From: Fabian Grünbichler @ 2021-07-06 8:10 UTC (permalink / raw)
To: Proxmox VE development discussion
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
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH qemu-server] api2: fix vmconfig_apply_pending errors handling
2021-07-06 8:10 ` Fabian Grünbichler
@ 2021-07-06 9:44 ` Oguz Bektas
0 siblings, 0 replies; 4+ messages in thread
From: Oguz Bektas @ 2021-07-06 9:44 UTC (permalink / raw)
To: Fabian Grünbichler; +Cc: Proxmox VE development discussion
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
> >
> >
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pve-devel] applied: [PATCH qemu-server] api2: fix vmconfig_apply_pending errors handling
2021-07-05 22:02 [pve-devel] [PATCH qemu-server] api2: fix vmconfig_apply_pending errors handling Alexandre Derumier
2021-07-06 8:10 ` Fabian Grünbichler
@ 2021-07-06 10:44 ` Thomas Lamprecht
1 sibling, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2021-07-06 10:44 UTC (permalink / raw)
To: Proxmox VE development discussion, Alexandre Derumier
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!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-06 10:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-05 22:02 [pve-devel] [PATCH qemu-server] api2: fix vmconfig_apply_pending errors handling Alexandre Derumier
2021-07-06 8:10 ` Fabian Grünbichler
2021-07-06 9:44 ` Oguz Bektas
2021-07-06 10:44 ` [pve-devel] applied: " Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox