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

Reply via email to