From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 3978461BE3 for ; Thu, 10 Feb 2022 12:52:38 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2D5901C78A for ; Thu, 10 Feb 2022 12:52:38 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 4BE671C781 for ; Thu, 10 Feb 2022 12:52:37 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 1EA1546BEE for ; Thu, 10 Feb 2022 12:52:31 +0100 (CET) Message-ID: <32b79391-62b6-50a6-7238-9e2f8a630dcb@proxmox.com> Date: Thu, 10 Feb 2022 12:52:25 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Content-Language: en-US To: pve-devel@lists.proxmox.com, =?UTF-8?Q?Fabian_Gr=c3=bcnbichler?= References: <20220209130750.902245-1-f.gruenbichler@proxmox.com> <20220209130750.902245-3-f.gruenbichler@proxmox.com> From: Fabian Ebner In-Reply-To: <20220209130750.902245-3-f.gruenbichler@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.136 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [migrate.pm, qemuserver.pm, lxc.pm] Subject: Re: [pve-devel] [PATCH v5 container 1/3] fix #1532: add target-storage support to migration X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Feb 2022 11:52:38 -0000 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 > --- > > 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 ;) Also, the parameter is named 'targetstorage' for VMs. I do agree this one is nicer, but it might be confusing. > 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. > + 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. > @@ -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. > + }; > + > + 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;