>> - enum => ['secure', 'insecure'], >> + enum => ['secure', 'insecure', 'external'], >>unnecessary? migration_type 'external' is not used anywhere in the code
yes, indeed. it was for the V1, I forgot to remove it. > + >>maybe add an additional check if migration_type is set that it is set to >>'secure' as external migration should always be secure? I don't known if we give user choice or not? (like for classic migration). ----- Mail original ----- De: "David Limbeck" <d.limb...@proxmox.com> À: "pve-devel" <pve-devel@pve.proxmox.com> Envoyé: Mercredi 19 Décembre 2018 10:35:46 Objet: Re: [pve-devel] [PATCH v3 qemu-server 4/7] migrate : phase2 : migrate external comments inline On 11/27/18 4:38 PM, Alexandre Derumier wrote: > --- > PVE/API2/Qemu.pm | 18 +++++++++++++++--- > PVE/QemuMigrate.pm | 21 ++++++++++++++------- > PVE/QemuServer.pm | 20 ++++++++++++++++---- > 3 files changed, 45 insertions(+), 14 deletions(-) > > diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm > index b23db56..b85fd6d 100644 > --- a/PVE/API2/Qemu.pm > +++ b/PVE/API2/Qemu.pm > @@ -1932,7 +1932,7 @@ __PACKAGE__->register_method({ > migratedfrom => get_standard_option('pve-node',{ optional => 1 }), > migration_type => { > type => 'string', > - enum => ['secure', 'insecure'], > + enum => ['secure', 'insecure', 'external'], unnecessary? migration_type 'external' is not used anywhere in the code > description => "Migration traffic is encrypted using an SSH " . > "tunnel by default. On secure, completely private networks " . > "this can be disabled to increase performance.", > @@ -1948,7 +1948,12 @@ __PACKAGE__->register_method({ > description => "Target storage for the migration. (Can be '1' to use the same > storage id as on the source node.)", > type => 'string', > optional => 1 > - } > + }, > + external_migration => { > + description => "Enable external migration.", > + type => 'boolean', > + optional => 1, > + }, > }, > }, > returns => { > @@ -1994,6 +1999,13 @@ __PACKAGE__->register_method({ > raise_param_exc({ targetstorage => "targetstorage can only by used with > migratedfrom." }) > if $targetstorage && !$migratedfrom; > > + my $external_migration = extract_param($param, 'external_migration'); > + raise_param_exc({ external_migration => "Only root may use this option." }) > + if $external_migration && $authuser ne 'root@pam'; > + > + raise_param_exc({ external_migration => "targetstorage can't be used with > external_migration." }) > + if ($targetstorage && $external_migration); > + maybe add an additional check if migration_type is set that it is set to 'secure' as external migration should always be secure? > # read spice ticket from STDIN > my $spice_ticket; > if ($stateuri && ($stateuri eq 'tcp') && $migratedfrom && ($rpcenv->{type} eq > 'cli')) { > @@ -2034,7 +2046,7 @@ __PACKAGE__->register_method({ > syslog('info', "start VM $vmid: $upid\n"); > > PVE::QemuServer::vm_start($storecfg, $vmid, $stateuri, $skiplock, > $migratedfrom, undef, > - $machine, $spice_ticket, $migration_network, $migration_type, > $targetstorage); > + $machine, $spice_ticket, $migration_network, $migration_type, > $targetstorage, $external_migration); > > return; > }; > diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm > index 1dea286..b4dc8f7 100644 > --- a/PVE/QemuMigrate.pm > +++ b/PVE/QemuMigrate.pm > @@ -597,7 +597,9 @@ sub phase2 { > > my $conf = $self->{vmconf}; > > - $self->log('info', "starting VM $vmid on remote node '$self->{node}'"); > + my $targetvmid = $self->{opts}->{targetvmid} ? $self->{opts}->{targetvmid} > : $vmid; > + > + $self->log('info', "starting VM $targetvmid on remote node > '$self->{node}'"); > > my $raddr; > my $rport; > @@ -613,10 +615,14 @@ sub phase2 { > $spice_ticket = $res->{ticket}; > } > > - push @$cmd , 'qm', 'start', $vmid, '--skiplock', '--migratedfrom', > $nodename; > - > my $migration_type = $self->{opts}->{migration_type}; > > + push @$cmd , 'qm', 'start', $targetvmid, '--skiplock'; > + > + push @$cmd, '--migratedfrom', $nodename if > !$self->{opts}->{migration_external}; > + > + push @$cmd, '--external_migration' if $self->{opts}->{migration_external}; > + > push @$cmd, '--migration_type', $migration_type; > > push @$cmd, '--migration_network', $self->{opts}->{migration_network} > @@ -633,7 +639,7 @@ sub phase2 { > } > > if ($self->{opts}->{targetstorage}) { > - push @$cmd, '--targetstorage', $self->{opts}->{targetstorage}; > + push @$cmd, '--targetstorage', $self->{opts}->{targetstorage} if > !$self->{opts}->{migration_external}; > } > > my $spice_port; > @@ -650,7 +656,7 @@ sub phase2 { > } > elsif ($line =~ m!^migration listens on > unix:(/run/qemu-server/(\d+)\.migrate)$!) { > $raddr = $1; > - die "Destination UNIX sockets VMID does not match source VMID" if $vmid ne > $2; > + die "Destination UNIX sockets VMID does not match source VMID" if > $targetvmid ne $2; > $ruri = "unix:$raddr"; > } > elsif ($line =~ m/^migration listens on port (\d+)$/) { > @@ -720,13 +726,14 @@ sub phase2 { > > my $start = time(); > > - if ($self->{opts}->{targetstorage} && > defined($self->{online_local_volumes})) { > + if (($self->{opts}->{targetstorage} && > defined($self->{online_local_volumes})) || > $self->{opts}->{migration_external}) { > $self->{storage_migration} = 1; > $self->{storage_migration_jobs} = {}; > $self->log('info', "starting storage migration"); > > die "The number of local disks does not match between the source and the > destination.\n" > - if (scalar(keys %{$self->{target_drive}}) != scalar > @{$self->{online_local_volumes}}); > + if !$self->{opts}->{migration_external} && (scalar(keys > %{$self->{target_drive}}) != scalar @{$self->{online_local_volumes}}); > + > foreach my $drive (keys %{$self->{target_drive}}){ > my $nbd_uri = $self->{target_drive}->{$drive}->{nbd_uri}; > $self->log('info', "$drive: start migration to $nbd_uri"); > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm > index 8023150..0b6c857 100644 > --- a/PVE/QemuServer.pm > +++ b/PVE/QemuServer.pm > @@ -5006,7 +5006,7 @@ sub vmconfig_update_disk { > > sub vm_start { > my ($storecfg, $vmid, $statefile, $skiplock, $migratedfrom, $paused, > - $forcemachine, $spice_ticket, $migration_network, $migration_type, > $targetstorage) = @_; > + $forcemachine, $spice_ticket, $migration_network, $migration_type, > $targetstorage, $external_migration) = @_; > > PVE::QemuConfig->lock_config($vmid, sub { > my $conf = PVE::QemuConfig->load_config($vmid, $migratedfrom); > @@ -5031,7 +5031,19 @@ sub vm_start { > > my $local_volumes = {}; > > - if ($targetstorage) { > + if ($external_migration) { > + foreach_drive($conf, sub { > + my ($ds, $drive) = @_; > + > + return if drive_is_cdrom($drive); > + > + my $volid = $drive->{file}; > + > + return if !$volid; > + > + $local_volumes->{$ds} = $volid; > + }); > + } elsif ($targetstorage) { > foreach_drive($conf, sub { > my ($ds, $drive) = @_; > > @@ -5219,7 +5231,7 @@ sub vm_start { > } > > #start nbd server for storage migration > - if ($targetstorage) { > + if ($targetstorage || $external_migration) { > my $nodename = PVE::INotify::nodename(); > my $migrate_network_addr = > PVE::Cluster::get_local_migration_ip($migration_network); > my $localip = $migrate_network_addr ? $migrate_network_addr : > PVE::Cluster::remote_node_ip($nodename, 1); > @@ -5238,7 +5250,7 @@ sub vm_start { > } > } > > - if ($migratedfrom) { > + if ($migratedfrom || $external_migration) { > eval { > set_migration_caps($vmid); > }; _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel