On February 10, 2022 12:52 pm, Fabian Ebner wrote: > Am 09.02.22 um 14:07 schrieb Fabian Grünbichler: >> re-using helpers that already exist for qemu-server. this is a >> pre-requisite for extending remote migration support to containers. >> >> Signed-off-by: Fabian Grünbichler <f.gruenbich...@proxmox.com> >> --- >> >> Notes: >> new in v5, no GUI yet until possible wrinkles are ironed out >> >> requires pve-common/pve-guest-common with changes from this series >> >> src/PVE/API2/LXC.pm | 32 ++++++++++++++++++++++ >> src/PVE/LXC/Migrate.pm | 62 ++++++++++++++++++++++++++++++++---------- >> 2 files changed, 79 insertions(+), 15 deletions(-) >> >> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm >> index 7573814..61eaaf7 100644 >> --- a/src/PVE/API2/LXC.pm >> +++ b/src/PVE/API2/LXC.pm >> @@ -36,6 +36,18 @@ BEGIN { >> } >> } >> >> +my $check_storage_access_migrate = sub { >> + my ($rpcenv, $authuser, $storecfg, $storage, $node) = @_; >> + >> + PVE::Storage::storage_check_enabled($storecfg, $storage, $node); >> + >> + $rpcenv->check($authuser, "/storage/$storage", >> ['Datastore.AllocateSpace']); >> + >> + my $scfg = PVE::Storage::storage_config($storecfg, $storage); >> + die "storage '$storage' does not support CT rootdirs\n" >> + if !$scfg->{content}->{rootdir}; >> +}; >> + >> __PACKAGE__->register_method ({ >> subclass => "PVE::API2::LXC::Config", >> path => '{vmid}/config', >> @@ -1091,6 +1103,7 @@ __PACKAGE__->register_method({ >> description => "Target node.", >> completion => \&PVE::Cluster::complete_migration_target, >> }), >> + 'target-storage' => get_standard_option('pve-targetstorage'), > > This option is currently registered in PVE/QemuServer.pm and I don't > think we want to depend on that ;)
ugh, yes. > Also, the parameter is named 'targetstorage' for VMs. I do agree this > one is nicer, but it might be confusing. I opted for 'target-storage' since it's how we'd name the parameter now (same for qemu-server's remote migration API with its multiple target-foo parameters ;)), and would like to phase out the 'targetstorage' in the regular VM migration via an alias.. > >> online => { >> type => 'boolean', >> description => "Use online/live migration.", >> @@ -1149,6 +1162,25 @@ __PACKAGE__->register_method({ >> if !$param->{online} && !$param->{restart}; >> } >> >> + if (my $targetstorage = delete $param->{'target-storage'}) { >> + my $storecfg = PVE::Storage::config(); >> + my $storagemap = eval { >> PVE::JSONSchema::parse_idmap($targetstorage, 'pve-storage-id') }; >> + raise_param_exc({ targetstorage => "failed to parse storage map: >> $@" }) > > Should be 'target-storage' if we go with that. ack. > >> + if $@; >> + >> + $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.Disk']) >> + if !defined($storagemap->{identity}); >> + >> + foreach my $target_sid (values %{$storagemap->{entries}}) { >> + $check_storage_access_migrate->($rpcenv, $authuser, $storecfg, >> $target_sid, $target); >> + } >> + >> + $check_storage_access_migrate->($rpcenv, $authuser, $storecfg, >> $storagemap->{default}, $target) >> + if $storagemap->{default}; >> + >> + $param->{storagemap} = $storagemap; >> + } >> + >> if (PVE::HA::Config::vm_is_ha_managed($vmid) && $rpcenv->{type} ne >> 'ha') { >> >> my $hacmd = sub { >> diff --git a/src/PVE/LXC/Migrate.pm b/src/PVE/LXC/Migrate.pm >> index 95562e4..c85a09c 100644 >> --- a/src/PVE/LXC/Migrate.pm >> +++ b/src/PVE/LXC/Migrate.pm > > ----8<---- > >> @@ -194,7 +210,8 @@ sub phase1 { >> next if @{$dl->{$storeid}} == 0; >> >> # check if storage is available on target node >> - PVE::Storage::storage_check_enabled($self->{storecfg}, $storeid, >> $self->{node}); >> + my $targetsid = PVE::JSONSchema::map_id($self->{opts}->{storagemap}, >> $storeid); >> + PVE::Storage::storage_check_enabled($self->{storecfg}, $targetsid, >> $self->{node}); >> >> die "content type 'rootdir' is not available on storage '$storeid'\n" >> if !$scfg->{content}->{rootdir}; > > Should use target's scfg. ack - had it in prepare (for the config-referenced volumes), but missed the second one here. > >> @@ -275,25 +292,38 @@ sub phase1 { >> next if $rep_volumes->{$volid}; >> my ($sid, $volname) = PVE::Storage::parse_volume_id($volid); >> push @{$self->{volumes}}, $volid; >> - my $bwlimit = PVE::Storage::get_bandwidth_limit('migration', [$sid], >> $opts->{bwlimit}); >> + >> # JSONSchema and get_bandwidth_limit use kbps - storage_migrate bps >> + my $bwlimit = $volhash->{$volid}->{bwlimit}; >> $bwlimit = $bwlimit * 1024 if defined($bwlimit); >> >> - my $storage_migrate_opts = { >> - 'ratelimit_bps' => $bwlimit, >> - 'insecure' => $opts->{migration_type} eq 'insecure', >> - 'with_snapshots' => $volhash->{$volid}->{snapshots}, >> + my $targetsid = $volhash->{$volid}->{targetsid}; >> + >> + my $new_volid = eval { >> + my $storage_migrate_opts = { >> + 'ratelimit_bps' => $bwlimit, >> + 'insecure' => $opts->{migration_type} eq 'insecure', >> + 'with_snapshots' => $volhash->{$volid}->{snapshots}, > > Since we update the config below, I think we can now also enable > allow_rename here. Otherwise, it's rather easy to run into conflicts > when mapping two storages on the source side to the same one on the target. true! > >> + }; >> + >> + my $logfunc = sub { $self->log('info', $_[0]); }; >> + return PVE::Storage::storage_migrate( >> + $self->{storecfg}, >> + $volid, >> + $self->{ssh_info}, >> + $targetsid, >> + $storage_migrate_opts, >> + $logfunc, >> + ); >> }; >> >> - my $logfunc = sub { $self->log('info', $_[0]); }; >> - eval { >> - PVE::Storage::storage_migrate($self->{storecfg}, $volid, >> $self->{ssh_info}, >> - $sid, $storage_migrate_opts, >> $logfunc); >> - }; >> if (my $err = $@) { >> - die "storage migration for '$volid' to storage '$sid' failed - >> $err\n"; >> + die "storage migration for '$volid' to storage '$targetsid' failed >> - $err\n"; >> } >> >> + $self->{volume_map}->{$volid} = $new_volid; >> + $self->log('info', "volume '$volid' is '$new_volid' on the target\n"); >> + >> eval { PVE::Storage::deactivate_volumes($self->{storecfg}, [$volid]); }; >> if (my $err = $@) { >> $self->log('warn', $err); >> @@ -316,6 +346,8 @@ sub phase1 { >> >> # transfer replication state before moving config >> $self->transfer_replication_state() if $rep_volumes; >> + PVE::LXC::Config->update_volume_ids($conf, $self->{volume_map}); >> + PVE::LXC::Config->write_config($vmid, $conf); >> PVE::LXC::Config->move_config_to_node($vmid, $self->{node}); >> $self->{conf_migrated} = 1; >> $self->switch_replication_job_target() if $rep_volumes; > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel