all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH qemu-server 1/1] create_vm: assume HA state 'started' when live-restoring guests
@ 2025-11-20 16:34 Michael Köppl
  2025-11-21  9:45 ` Fiona Ebner
  2025-11-27 15:42 ` Michael Köppl
  0 siblings, 2 replies; 9+ messages in thread
From: Michael Köppl @ 2025-11-20 16:34 UTC (permalink / raw)
  To: pve-devel

To avoid shutting down the VM when performing a live-restore, consider
live-restore=1 to translate to a HA state of 'started', similar to
start=1.

Reported-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
Used defined-or since in pve-manager, we only ever set one of the two
values and in any other case I'd give precedence to the 'start'
parameter if it is explicitly set.

 src/PVE/API2/Qemu.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index c580bf63..33427ca7 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -1237,7 +1237,8 @@ __PACKAGE__->register_method({
         my $bwlimit = extract_param($param, 'bwlimit');
         my $force = extract_param($param, 'force');
         my $pool = extract_param($param, 'pool');
-        my $start_after_create = extract_param($param, 'start');
+        my $start_after_create = extract_param($param, 'start')
+            // extract_param($param, 'live-restore');
         my $ha_managed = extract_param($param, 'ha-managed');
         my $storage = extract_param($param, 'storage');
         my $unique = extract_param($param, 'unique');
-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pve-devel] [PATCH qemu-server 1/1] create_vm: assume HA state 'started' when live-restoring guests
  2025-11-20 16:34 [pve-devel] [PATCH qemu-server 1/1] create_vm: assume HA state 'started' when live-restoring guests Michael Köppl
@ 2025-11-21  9:45 ` Fiona Ebner
  2025-11-21 10:05   ` Thomas Lamprecht
  2025-11-27 15:42 ` Michael Köppl
  1 sibling, 1 reply; 9+ messages in thread
From: Fiona Ebner @ 2025-11-21  9:45 UTC (permalink / raw)
  To: Michael Köppl, pve-devel

Am 20.11.25 um 5:34 PM schrieb Michael Köppl:
> To avoid shutting down the VM when performing a live-restore, consider
> live-restore=1 to translate to a HA state of 'started', similar to
> start=1.
> 
> Reported-by: Fiona Ebner <f.ebner@proxmox.com>
> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
> ---
> Used defined-or since in pve-manager, we only ever set one of the two
> values and in any other case I'd give precedence to the 'start'
> parameter if it is explicitly set.

This could be part of the commit message. But from looking at the code,
it seems like the actual behavior if start=0 and live-restore=1 is still
doing a live restore, so the HA state should still be started in that case.

> 
>  src/PVE/API2/Qemu.pm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
> index c580bf63..33427ca7 100644
> --- a/src/PVE/API2/Qemu.pm
> +++ b/src/PVE/API2/Qemu.pm
> @@ -1237,7 +1237,8 @@ __PACKAGE__->register_method({
>          my $bwlimit = extract_param($param, 'bwlimit');
>          my $force = extract_param($param, 'force');
>          my $pool = extract_param($param, 'pool');
> -        my $start_after_create = extract_param($param, 'start');
> +        my $start_after_create = extract_param($param, 'start')
> +            // extract_param($param, 'live-restore');

It's already started during create, so using the $start_after_create
variable seems like a slight misfit. Why not just also check for
$live_restore when setting the HA state when the resource is added?

>          my $ha_managed = extract_param($param, 'ha-managed');
>          my $storage = extract_param($param, 'storage');
>          my $unique = extract_param($param, 'unique');



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pve-devel] [PATCH qemu-server 1/1] create_vm: assume HA state 'started' when live-restoring guests
  2025-11-21  9:45 ` Fiona Ebner
@ 2025-11-21 10:05   ` Thomas Lamprecht
  2025-11-21 10:15     ` Fiona Ebner
  2025-11-25 13:44     ` Michael Köppl
  0 siblings, 2 replies; 9+ messages in thread
From: Thomas Lamprecht @ 2025-11-21 10:05 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fiona Ebner, Michael Köppl

Am 21.11.25 um 10:45 schrieb Fiona Ebner:
> Am 20.11.25 um 5:34 PM schrieb Michael Köppl:
>> To avoid shutting down the VM when performing a live-restore, consider
>> live-restore=1 to translate to a HA state of 'started', similar to
>> start=1.
>>
>> Reported-by: Fiona Ebner <f.ebner@proxmox.com>
>> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
>> ---
>> Used defined-or since in pve-manager, we only ever set one of the two
>> values and in any other case I'd give precedence to the 'start'
>> parameter if it is explicitly set.
> 
> This could be part of the commit message. But from looking at the code,
> it seems like the actual behavior if start=0 and live-restore=1 is still
> doing a live restore, so the HA state should still be started in that case.
> 
>>
>>  src/PVE/API2/Qemu.pm | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
>> index c580bf63..33427ca7 100644
>> --- a/src/PVE/API2/Qemu.pm
>> +++ b/src/PVE/API2/Qemu.pm
>> @@ -1237,7 +1237,8 @@ __PACKAGE__->register_method({
>>          my $bwlimit = extract_param($param, 'bwlimit');
>>          my $force = extract_param($param, 'force');
>>          my $pool = extract_param($param, 'pool');
>> -        my $start_after_create = extract_param($param, 'start');
>> +        my $start_after_create = extract_param($param, 'start')
>> +            // extract_param($param, 'live-restore');
> 
> It's already started during create, so using the $start_after_create
> variable seems like a slight misfit. Why not just also check for
> $live_restore when setting the HA state when the resource is added?
> 

We lock the config on restore, or?
If so, couldn't we handle this in the HA stack and do not shutdown if a
restore lock is present in the config?


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pve-devel] [PATCH qemu-server 1/1] create_vm: assume HA state 'started' when live-restoring guests
  2025-11-21 10:05   ` Thomas Lamprecht
@ 2025-11-21 10:15     ` Fiona Ebner
  2025-11-21 10:24       ` Thomas Lamprecht
  2025-11-25 13:44     ` Michael Köppl
  1 sibling, 1 reply; 9+ messages in thread
From: Fiona Ebner @ 2025-11-21 10:15 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox VE development discussion, Michael Köppl

Am 21.11.25 um 11:04 AM schrieb Thomas Lamprecht:
> Am 21.11.25 um 10:45 schrieb Fiona Ebner:
>> Am 20.11.25 um 5:34 PM schrieb Michael Köppl:
>>> To avoid shutting down the VM when performing a live-restore, consider
>>> live-restore=1 to translate to a HA state of 'started', similar to
>>> start=1.
>>>
>>> Reported-by: Fiona Ebner <f.ebner@proxmox.com>
>>> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
>>> ---
>>> Used defined-or since in pve-manager, we only ever set one of the two
>>> values and in any other case I'd give precedence to the 'start'
>>> parameter if it is explicitly set.
>>
>> This could be part of the commit message. But from looking at the code,
>> it seems like the actual behavior if start=0 and live-restore=1 is still
>> doing a live restore, so the HA state should still be started in that case.
>>
>>>
>>>  src/PVE/API2/Qemu.pm | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
>>> index c580bf63..33427ca7 100644
>>> --- a/src/PVE/API2/Qemu.pm
>>> +++ b/src/PVE/API2/Qemu.pm
>>> @@ -1237,7 +1237,8 @@ __PACKAGE__->register_method({
>>>          my $bwlimit = extract_param($param, 'bwlimit');
>>>          my $force = extract_param($param, 'force');
>>>          my $pool = extract_param($param, 'pool');
>>> -        my $start_after_create = extract_param($param, 'start');
>>> +        my $start_after_create = extract_param($param, 'start')
>>> +            // extract_param($param, 'live-restore');
>>
>> It's already started during create, so using the $start_after_create
>> variable seems like a slight misfit. Why not just also check for
>> $live_restore when setting the HA state when the resource is added?
>>
> 
> We lock the config on restore, or?
> If so, couldn't we handle this in the HA stack and do not shutdown if a
> restore lock is present in the config?

But that'd be more coupling? Further below in the endpoint we have (two
instances of):

>             if ($ha_managed) {
>                 print "Add as HA resource\n";
>                 my $state = $start_after_create ? 'started' : 'stopped';
>                 my $cmd = ['ha-manager', 'add', "vm:$vmid", '--state', $state];
>                 eval { PVE::Tools::run_command($cmd); };
>                 warn $@ if $@;
>             }

So we could just put $start_after_create || $live_restore there


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pve-devel] [PATCH qemu-server 1/1] create_vm: assume HA state 'started' when live-restoring guests
  2025-11-21 10:15     ` Fiona Ebner
@ 2025-11-21 10:24       ` Thomas Lamprecht
  2025-11-21 10:32         ` Fiona Ebner
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Lamprecht @ 2025-11-21 10:24 UTC (permalink / raw)
  To: Fiona Ebner, Proxmox VE development discussion, Michael Köppl

Am 21.11.25 um 11:15 schrieb Fiona Ebner:
> Am 21.11.25 um 11:04 AM schrieb Thomas Lamprecht:
>> We lock the config on restore, or?
>> If so, couldn't we handle this in the HA stack and do not shutdown if a
>> restore lock is present in the config?
> 
> But that'd be more coupling? Further below in the endpoint we have (two
> instances of):

Making the HA actually aware of how the resources it manages work, so that
it can make better decisions, is definitively not more coupling.

> 
>>             if ($ha_managed) {
>>                 print "Add as HA resource\n";
>>                 my $state = $start_after_create ? 'started' : 'stopped';
>>                 my $cmd = ['ha-manager', 'add', "vm:$vmid", '--state', $state];
>>                 eval { PVE::Tools::run_command($cmd); };
>>                 warn $@ if $@;
>>             }
> 
> So we could just put $start_after_create || $live_restore there

Does not solves the case for when restores over a existing VM that is a HA
resource already, FWICT.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pve-devel] [PATCH qemu-server 1/1] create_vm: assume HA state 'started' when live-restoring guests
  2025-11-21 10:24       ` Thomas Lamprecht
@ 2025-11-21 10:32         ` Fiona Ebner
  2025-11-21 11:13           ` Thomas Lamprecht
  0 siblings, 1 reply; 9+ messages in thread
From: Fiona Ebner @ 2025-11-21 10:32 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox VE development discussion, Michael Köppl

Am 21.11.25 um 11:23 AM schrieb Thomas Lamprecht:
> Am 21.11.25 um 11:15 schrieb Fiona Ebner:
>> Am 21.11.25 um 11:04 AM schrieb Thomas Lamprecht:
>>> We lock the config on restore, or?
>>> If so, couldn't we handle this in the HA stack and do not shutdown if a
>>> restore lock is present in the config?
>>
>> But that'd be more coupling? Further below in the endpoint we have (two
>> instances of):
> 
> Making the HA actually aware of how the resources it manages work, so that
> it can make better decisions, is definitively not more coupling.

Fair point.

>>
>>>             if ($ha_managed) {
>>>                 print "Add as HA resource\n";
>>>                 my $state = $start_after_create ? 'started' : 'stopped';
>>>                 my $cmd = ['ha-manager', 'add', "vm:$vmid", '--state', $state];
>>>                 eval { PVE::Tools::run_command($cmd); };
>>>                 warn $@ if $@;
>>>             }
>>
>> So we could just put $start_after_create || $live_restore there
> 
> Does not solves the case for when restores over a existing VM that is a HA
> resource already, FWICT.

The code block above still needs fixing regardless? Since it sets the
state for a newly added resource.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pve-devel] [PATCH qemu-server 1/1] create_vm: assume HA state 'started' when live-restoring guests
  2025-11-21 10:32         ` Fiona Ebner
@ 2025-11-21 11:13           ` Thomas Lamprecht
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Lamprecht @ 2025-11-21 11:13 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fiona Ebner, Michael Köppl

Am 21.11.25 um 11:32 schrieb Fiona Ebner:
> Am 21.11.25 um 11:23 AM schrieb Thomas Lamprecht:
>> Am 21.11.25 um 11:15 schrieb Fiona Ebner:
>>> Am 21.11.25 um 11:04 AM schrieb Thomas Lamprecht:
>>>> We lock the config on restore, or?
>>>> If so, couldn't we handle this in the HA stack and do not shutdown if a
>>>> restore lock is present in the config?
>>>
>>> But that'd be more coupling? Further below in the endpoint we have (two
>>> instances of):
>>
>> Making the HA actually aware of how the resources it manages work, so that
>> it can make better decisions, is definitively not more coupling.
> 
> Fair point.

To also be fair, it is coupling, but this is warranted one (at least for the
current design); not all coupling is inherently bad, it always should be
rather explicit and not hidden though.

>>>>             if ($ha_managed) {
>>>>                 print "Add as HA resource\n";
>>>>                 my $state = $start_after_create ? 'started' : 'stopped';
>>>>                 my $cmd = ['ha-manager', 'add', "vm:$vmid", '--state', $state];
>>>>                 eval { PVE::Tools::run_command($cmd); };
>>>>                 warn $@ if $@;
>>>>             }
>>>
>>> So we could just put $start_after_create || $live_restore there
>>
>> Does not solves the case for when restores over a existing VM that is a HA
>> resource already, FWICT.
> 
> The code block above still needs fixing regardless? Since it sets the
> state for a newly added resource.

Yes, I think so. One is for not stopping during restore and the other is
for not stopping them after the restore, and live-restore already *does*
imply start-after-create (well, restore here, but same same).


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pve-devel] [PATCH qemu-server 1/1] create_vm: assume HA state 'started' when live-restoring guests
  2025-11-21 10:05   ` Thomas Lamprecht
  2025-11-21 10:15     ` Fiona Ebner
@ 2025-11-25 13:44     ` Michael Köppl
  1 sibling, 0 replies; 9+ messages in thread
From: Michael Köppl @ 2025-11-25 13:44 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox VE development discussion, Fiona Ebner,
	Michael Köppl

On Fri Nov 21, 2025 at 11:05 AM CET, Thomas Lamprecht wrote:
> Am 21.11.25 um 10:45 schrieb Fiona Ebner:
>> Am 20.11.25 um 5:34 PM schrieb Michael Köppl:
>>> To avoid shutting down the VM when performing a live-restore, consider
>>> live-restore=1 to translate to a HA state of 'started', similar to
>>> start=1.
>>>
>>> Reported-by: Fiona Ebner <f.ebner@proxmox.com>
>>> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
>>> ---
>>> Used defined-or since in pve-manager, we only ever set one of the two
>>> values and in any other case I'd give precedence to the 'start'
>>> parameter if it is explicitly set.
>> 
>> This could be part of the commit message. But from looking at the code,
>> it seems like the actual behavior if start=0 and live-restore=1 is still
>> doing a live restore, so the HA state should still be started in that case.
>> 
>>>
>>>  src/PVE/API2/Qemu.pm | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
>>> index c580bf63..33427ca7 100644
>>> --- a/src/PVE/API2/Qemu.pm
>>> +++ b/src/PVE/API2/Qemu.pm
>>> @@ -1237,7 +1237,8 @@ __PACKAGE__->register_method({
>>>          my $bwlimit = extract_param($param, 'bwlimit');
>>>          my $force = extract_param($param, 'force');
>>>          my $pool = extract_param($param, 'pool');
>>> -        my $start_after_create = extract_param($param, 'start');
>>> +        my $start_after_create = extract_param($param, 'start')
>>> +            // extract_param($param, 'live-restore');
>> 
>> It's already started during create, so using the $start_after_create
>> variable seems like a slight misfit. Why not just also check for
>> $live_restore when setting the HA state when the resource is added?
>> 
>
> We lock the config on restore, or?
> If so, couldn't we handle this in the HA stack and do not shutdown if a
> restore lock is present in the config?

Thought a bit about this for my approach for v2. I implemented v2 like
@Fiona suggested ($start_after_create || $live_restore), but also added
a check that only sets the status of an existing resource instead of
trying to add one when restoring over an existing VM that already is a
HA resource. I'm not sure I understand why this would instead be handled
in the HA stack depending on the existence of the lock, though. Setting
the state correctly should already be enough, no? Or am I missing
something?



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pve-devel] [PATCH qemu-server 1/1] create_vm: assume HA state 'started' when live-restoring guests
  2025-11-20 16:34 [pve-devel] [PATCH qemu-server 1/1] create_vm: assume HA state 'started' when live-restoring guests Michael Köppl
  2025-11-21  9:45 ` Fiona Ebner
@ 2025-11-27 15:42 ` Michael Köppl
  1 sibling, 0 replies; 9+ messages in thread
From: Michael Köppl @ 2025-11-27 15:42 UTC (permalink / raw)
  To: Michael Köppl, pve-devel

Superseded-by: https://lore.proxmox.com/pve-devel/20251127154041.194423-1-m.koeppl@proxmox.com

On Thu Nov 20, 2025 at 5:34 PM CET, Michael Köppl wrote:
> To avoid shutting down the VM when performing a live-restore, consider
> live-restore=1 to translate to a HA state of 'started', similar to
> start=1.
>
> Reported-by: Fiona Ebner <f.ebner@proxmox.com>
> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
> ---
> Used defined-or since in pve-manager, we only ever set one of the two
> values and in any other case I'd give precedence to the 'start'
> parameter if it is explicitly set.
>
>  src/PVE/API2/Qemu.pm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
> index c580bf63..33427ca7 100644
> --- a/src/PVE/API2/Qemu.pm
> +++ b/src/PVE/API2/Qemu.pm
> @@ -1237,7 +1237,8 @@ __PACKAGE__->register_method({
>          my $bwlimit = extract_param($param, 'bwlimit');
>          my $force = extract_param($param, 'force');
>          my $pool = extract_param($param, 'pool');
> -        my $start_after_create = extract_param($param, 'start');
> +        my $start_after_create = extract_param($param, 'start')
> +            // extract_param($param, 'live-restore');
>          my $ha_managed = extract_param($param, 'ha-managed');
>          my $storage = extract_param($param, 'storage');
>          my $unique = extract_param($param, 'unique');



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-11-27 15:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-20 16:34 [pve-devel] [PATCH qemu-server 1/1] create_vm: assume HA state 'started' when live-restoring guests Michael Köppl
2025-11-21  9:45 ` Fiona Ebner
2025-11-21 10:05   ` Thomas Lamprecht
2025-11-21 10:15     ` Fiona Ebner
2025-11-21 10:24       ` Thomas Lamprecht
2025-11-21 10:32         ` Fiona Ebner
2025-11-21 11:13           ` Thomas Lamprecht
2025-11-25 13:44     ` Michael Köppl
2025-11-27 15:42 ` Michael Köppl

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal