On 9/23/19 2:19 PM, Thomas Lamprecht wrote:
On 9/23/19 9:56 AM, Fabian Ebner wrote:
Thanks to Stefan and Thomas for the suggestions.

Changes from v1:
* update parameter description
* warn instead of die
This information is better to go a bit below, namely...

Signed-off-by: Fabian Ebner <f.eb...@proxmox.com>
---
.. here, after the "---" and before the diff-stat below.

This way it's visible in for all mail/patch readers but not included
in the commit once applied to git, which is OK as it's "patch-historical
meta" information.


Ok, I'll do that next time.


  PVE/API2/Qemu.pm | 11 +++++++----
  1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 8be0b7b..b355051 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -3257,7 +3257,7 @@ __PACKAGE__->register_method({
              }),
            online => {
                type => 'boolean',
-               description => "Use online/live migration.",
+               description => "Use online/live migration if VM is running. Ignored 
if VM is stopped.",
                optional => 1,
            },
            force => {
@@ -3318,9 +3318,6 @@ __PACKAGE__->register_method({
my $vmid = extract_param($param, 'vmid'); - raise_param_exc({ targetstorage => "Live storage migration can only be done online." })
-           if !$param->{online} && $param->{targetstorage};
-
        raise_param_exc({ force => "Only root may use this option." })
            if $param->{force} && $authuser ne 'root@pam';
@@ -3341,8 +3338,14 @@ __PACKAGE__->register_method({
        if (PVE::QemuServer::check_running($vmid)) {
            die "cant migrate running VM without --online\n"
                if !$param->{online};
+       } else {
+           warn "VM isn't running. Doing offline migration instead." if 
$param->{online};
With perl "warn" and "die" have a bit of an special behavior depending of the
string ending. If it ends with a new line the warning or "exception" gets
outputted as is. If it does not end with a \n the perl module and line gets 
added
by perl, compare:

# perl -we 'warn "foo\n"'
# perl -we 'warn "foo"'

Normally, we only omit the newline on purpose on internal "safe-guard" errors,
those which normally should never get seen by an user. (I fixed this up for you
in a follow-up patch).

Got it.


+           $param->{online} = 0;
        }
+ raise_param_exc({ targetstorage => "Live storage migration can only be done online." })
+           if !$param->{online} && $param->{targetstorage};
could it make sense to add a modus (or change this into one) where we start the 
VM up
paused and do our normal storage migrations (over QEMU) then? or rather improve 
storage
migration code to be able to migrate those storages to other ones?

Yes, I'll try to work on a solution where for disks we migrate with storage_migrate() the VM config is updated with the new location and where new disk names are used in case of collisions. Currently that only happens with disks migrated via QEMU. This should make it possible to specify --targetstorage for offline migration as well and is also needed for online migration with unused disks.


+
        my $storecfg = PVE::Storage::config();
if( $param->{targetstorage}) {


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

Reply via email to