[pve-devel] [PATCH] Check if VM is running when migrating with --online 1

[Thomas Lamprecht]

On 19.09.19 15:05, Thomas Lamprecht wrote:
> On 16.09.19 10:50, Stefan Reiter wrote:
>> On 9/16/19 9:46 AM, Fabian Ebner wrote:
>>> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
>>> ---
>>>   PVE/API2/Qemu.pm | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
>>> index b30931d..ab5912c 100644
>>> --- a/PVE/API2/Qemu.pm
>>> +++ b/PVE/API2/Qemu.pm
>>> @@ -3341,6 +3341,9 @@ __PACKAGE__->register_method({
>>>       if (PVE::QemuServer::check_running($vmid)) {
>>>           die "cant migrate running VM without --online\n"
>>>           if !$param->{online};
>>> +    } else {
>>> +        die "cant migrate stopped VM with --online\n"
>>> +        if $param->{online};
>>
>> Do we actually want to forbid this, or maybe just ignore (potentially with a warning)?
>>
>> I'm imagining a use-case where someone runs a migration from a script, they'd now have to check if the VM is running to find the correct "qm migrate" command. Maybe something like "--online auto"?
> 
> IMO doing nothing is best, the "online" flag should be rather seen
> as "allow live migration" not "enforce live migration, else die".
> 
> Maybe we could just extend the description of the "online" parameter
> definition a bit to underline that?
> 
> Something like:
> "Use online/live migration if VM is running. Ignored if VM is stopped."
> 
> (just from top of my head, may be possibly improved wording wise)


OK, now after reading some other mail I've found the reason why you want
to enforce this[0]. 

1. It would be nice if a commit message also included this reason from [0].
2. Maybe we could set the online $param to false if it's not running to "fix"
   the issue from [0] without dying here.

[0]: https://pve.proxmox.com/pipermail/pve-devel/2019-September/039023.html