* [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
0 siblings, 1 reply; 8+ 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] 8+ 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
0 siblings, 1 reply; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ messages in thread
end of thread, other threads:[~2025-11-25 13:44 UTC | newest]
Thread overview: 8+ 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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox